[tcpm] Comments for draft-scharf-tcpm-yang-tcp-06

[Juhamatti Kuusisaari <[e-mail]>]

Hello,

I read through draft-scharf-tcpm-yang-tcp-06 and overall it looks fine to me.

Nevertheless, there are a couple of items that may need
clarifications/improvements.

(1) I believe "leaf include-tcp-options" should be "leaf
ignore-tcp-options" with a false default as the options are included
by default in the RFC 5925. In my opinion, this would better emphasize
the fact that options really should be included by default and not
including them should be a special case. Change suggestion in detail
below:

      leaf include-tcp-options {
        type boolean;
        must "../enable-ao = 'true'";
        description
          "Include TCP options in HMAC calculation.";
      }
=>
      leaf ignore-tcp-options {
        type boolean;
        default "false";
        must "../enable-ao = 'true'";
        description
          "Ignore TCP options in MAC calculation.";
      }

Please also note the "HMAC"->"MAC" change suggestion. And yes, I do
realize that a default could be added to the original "include" leaf.
After pondering about this, I do think "ignore" leaf would be a better
end result for the reasons I mentioned above.

(2) There is now a leaf that says:

      leaf accept-ao-mismatch {
        type boolean;
        must "../enable-ao = 'true'";
        description
          "Accept packets with HMAC mismatch.";
      }

It is true that RFC 5925 allows non-existing MKT connections that
should be accepted. Then again, the above configuration and its
description looks to me that any mismatch would be accepted. So, maybe
a configuration setting better reflecting RFC 5925 would be something
on the lines of

      leaf accept-key-mismatch {
        type boolean;
        must "../enable-ao = 'true'";
        description
          "Accept TCP segments with a Master Key Tuple (MKT) that is
not configured.";
      }

As this configuration option does not have such a strong default as
the former one, I do not see a need to change its logic otherwise nor
add a default. I would assume that most security aware users would
have "false" there as a setting - especially those users that would
use a YANG model to do the configuration.

Best regards,
--
 Juhamatti