Subject: [tcpm] Comments for draft-ietf-tcpm-yang-tcp-00

From: Juhamatti Kuusisaari <[email-masked]>
Date: Sun, 1 Nov 2020 10:06:34 +0200

Hello,

My comments included below apply also to draft-ietf-tcpm-yang-tcp-00.

In brief:
 * include-tcp-options -> ignore-tcp-options with default false
 * accept-ao-mismatch -> accept-key-mismatch

BR,
--
 Juhamatti

---------- Forwarded message ---------
From: Juhamatti Kuusisaari <[email-masked]>
Date: Fri, 18 Sep 2020 at 11:53
Subject: [tcpm] Comments for draft-scharf-tcpm-yang-tcp-06
To: tcpm IETF list <[email-masked]>, Scharf, Michael
<[email-masked]>

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

---

From: "Scharf, Michael" <[email-masked]>
Date: Sun, 1 Nov 2020 08:14:44 +0000

Hi Juhamatti,

Addressing your comments is on my TODO list. Please stay tuned.

The version draft-ietf-tcpm-yang-tcp-00 is just a copy of the previous version in order to simplify future change tracking. Version -01 will address your feedback as well as other comments.

Thanks, and sorry for the delayed response

Michael

> -----Original Message-----
> Sent: Sunday, November 1, 2020 9:07 AM
> To: tcpm IETF list <[email-masked]>; Scharf, Michael <Michael.Scharf@hs-
> esslingen.de>
> Subject: [tcpm] Comments for draft-ietf-tcpm-yang-tcp-00
> 
> Hello,
>

---

From: Mahesh Jethanandani <[email-masked]>
Date: Tue, 3 Nov 2020 09:11:47 -0800

Hi Juhamatti,

> On Nov 1, 2020, at 1:06 AM, Juhamatti Kuusisaari <[email-masked]> wrote:
> 
> Hello,
> 

Would it not be simple to add a ‘default true’ to ‘include-tcp-options’ to achieve the same result? Somehow my head does not comprehend a double negative very well :-).

> use a YANG model to do the configuration.

I am fine with making this change.

Thanks

> 
> Best regards,
> --
> Juhamatti
> 
> _______________________________________________
> tcpm mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/tcpm

Mahesh Jethanandani
[email-masked]

---

From: Juhamatti Kuusisaari <[email-masked]>
Date: Wed, 4 Nov 2020 07:11:24 +0200

Hello Mahesh,

> Would it not be simple to add a ‘default true’ to ‘include-tcp-options’ to achieve the same result? Somehow my head does not comprehend a double negative very well :-).

Yes, ‘default true’ to ‘include-tcp-options’ would achieve the same
end result. However, I think it would guide implementers to have a
special "include"-mode for options in the TCP AO implementation,
especially as TCP MD5 does not have options included. There should be
nothing special about including options, the special part is to ignore
them. Thus, I think 'ignore-tcp-options' is the way to go. Then again,
having 'default true' with ‘include-tcp-options’ is certainly an
improvement and another valid choice.

--
 Juhamatti

---

From: Joseph Touch <[email-masked]>
Date: Wed, 4 Nov 2020 13:36:04 -0800

Is there a convention in YANG for such situations?

I personally would not expect all values to have “default=true” or “on" everywhere. Sometimes you’re actively turning on something that is typically disabled (default=false or off).

I think the “include-tcp-options” matches the way it’s described in the doc (TCP option flag, where true implies includdd) and the default=true would match better.

Joe

> On Nov 3, 2020, at 9:11 PM, Juhamatti Kuusisaari <[email-masked]> wrote:
> 
> Hello Mahesh,
>

---

From: tom petch <[email-masked]>
Date: Thu, 5 Nov 2020 11:04:09 +0000

From: tcpm <[email-masked]> on behalf of Joseph Touch <[email-masked]>
Sent: 04 November 2020 21:36
Is there a convention in YANG for such situations?

<tp>
>From a YANG, rather than TCPM, perspective, I am not clear what you are asking.  In this case RFC5925 - which the leaf would benefit from as a reference to - seems clear to me
'by default including options'
in which case I would expect the YANG to default true.
I think it good practice to be explicit in any YANG description clause of a boolean as to what true means or false or ideally both).
Also, I think the choice of 'include' in the leaf identifier unfortunate given that 'include' is a YANG technical term that appears in every YANG module.  I know that it used in RFC5925 but still see it as unfortunate.

Tom Petch

Joe

---

From: Joseph Touch <[email-masked]>
Date: Thu, 5 Nov 2020 07:30:48 -0800

> On Nov 5, 2020, at 3:04 AM, tom petch <[email-masked]> wrote:
> 
> From a YANG, rather than TCPM, perspective, I am not clear what you are asking.

The question is whether toggles are typically true by default or false by default. It doesn’t appear so.

>  In this case RFC5925 - which the leaf would benefit from as a reference to - seems clear to me
> Also, I think the choice of 'include' in the leaf identifier unfortunate given that 'include' is a YANG technical term that appears in every YANG module.

It doesn’t need to be; the term could be “options-covered-by-MAC”, default=true

>  I know that it used in RFC5925 but still see it as unfortunate.

The YANG model and TCP-AO had overlapping development periods. It’s impossible to expect either to have taken the other completely into account.

Joe

---

From: tom petch <[email-masked]>
Date: Thu, 5 Nov 2020 17:12:56 +0000

From: Joseph Touch <[email-masked]>
Sent: 05 November 2020 15:30

<tp>
Ah yes, clear now.  I agree that there is no such convention in YANG.  Simplifying, if you want a default, then the default must be explicitly specified in the model.  If there is no default and the leaf is not set to a value e.g. by Netconf, then the leaf does not exist in the YANG data tree, for Booleans as for anything else.  If you want the leaf always to be present, then YANG constraints allow you to enforce that.  As I said, given the wording in the RFC I would see default true as appropriate here.
As an aside the issue of defaults caused much discussion in Netconf as to whether or not they should be returned in a get and this led to three different processing options for retrieval but the YANG side of things has stayed the same.

Tom Petch

Joe