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

[tom petch <[e-mail]>]

From: tcpm <[e-mail]> on behalf of Joseph Touch <[e-mail]>
Sent: 04 November 2020 21:36
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.

<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

> On Nov 3, 2020, at 9:11 PM, Juhamatti Kuusisaari <[e-mail]> wrote:
>
> 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
>
> On Tue, 3 Nov 2020 at 19:11, Mahesh Jethanandani
> <[e-mail]> wrote:
>>
>> Hi Juhamatti,
>>
>> On Nov 1, 2020, at 1:06 AM, Juhamatti Kuusisaari <[e-mail]> wrote:
>>
>> 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 <[e-mail]>
>> Date: Fri, 18 Sep 2020 at 11:53
>> Subject: [tcpm] Comments for draft-scharf-tcpm-yang-tcp-06
>> To: tcpm IETF list <[e-mail]>, Scharf, Michael
>> <[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.";
>>     }
>>
>>
>> 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 :-).
>>
>> 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.
>>
>>
>> I am fine with making this change.
>>
>> Thanks
>>
>>
>> Best regards,
>> --
>> Juhamatti
>>
>> _______________________________________________
>> tcpm mailing list
>> [e-mail]
>> https://www.ietf.org/mailman/listinfo/tcpm
>>
>>
>> Mahesh Jethanandani
>> [e-mail]
>>
>>
>>
>>
>>
>
> _______________________________________________
> tcpm mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/tcpm

_______________________________________________
tcpm mailing list
[e-mail]
https://www.ietf.org/mailman/listinfo/tcpm