Subject: Re: [Ntp] Last Call: <draft-ietf-ntp-yang-data-model-10.txt> (A YANG Data Model for NTP) to Proposed Standard

From: tom petch <[email-masked]>
Date: Mon, 8 Feb 2021 11:21:51 +0000

I have two main problems and a lot of lesser ones with this I-D; given
the number, about 50, I am not optimistic that a single cycle of
revision will see them addressed.

I see a potential loophole in the security which I will post separately
since the audience is likely to be different.

References are missing or not specific enough so when I try to compare
values in the I-D with those of the protocol, either I cannot find them
or they seem to be different. Giving values to enumerations is unusual
in YANG, since NETCONF does not transmit them, and their presence
suggests that they are protocol values, in which case I want to see what
the protocol says.  A reference to a 110 page I-D, with two updating
I-D, is inadequate IMO - section numbers are needed in every case.

Introduction
should mention support, or lack thereof, for NMDA

1.4.  Prefixes in Data Node Names
            | ianach    | iana-crypt-hash          | [RFC7317] |
the reference is wrong; this is an IANA- maintained module so the
reference must be to the IANA website

1.5.  Refrences in the Model
/Refrences/References/
/refrenced in /referenced in /

2.  NTP data model

I do not see the value of a condensed model followed immediately by a
full model. Perhaps the full model should be an Appendix although at
less than three pages, this is quite small and would be ok on its own
IMHO.

4.  Relationship with RFC 7317
/supports per-interface configurations /
support per-interface configuration/

5.  Access Rules
/refer access-mode) and attach different acl-rule/
see access-mode) and attach a different acl-rule/

6.  Key Management
/32-bits unsigned /32-bit unsigned/

/this YANG modules/this YANG module/

NTP association (for example unicast-server),
/specefied/specified/

7.  NTP YANG Module

     import iana-crypt-hash {
      reference         "RFC 7317: A YANG Data Model for System
Management";
wrong reference - this module is IANA-maintained so the reference must
be to the IANA website

     contact
        WG List:  <mailto: [email-masked]
this is not the address I see on the datatracker

the I-D has five editors but there are only two here

     typedef access-mode {
I cannot find this in RFC5905

     typedef association-mode {
this I can find but it ranges from 0 to 7 whereas the I-D has 0 to 4 - is
this intended?

     typedef ntp-sync-state {
this I cannot find; a search for 'spike' yields a value of 2 in the
RFC, 5 here - is this intended?

              effect in XXX seconds.";
for what value of XXX?

       leaf packet-sent {
       leaf packet-received {
       leaf packet-dropped {
            discontinuities in the value of sysUpTime.";
those who have been involved with network management for ten years or
less will likely not recognise this object.  You could add a reference
but I suggest you replace it with a YANG-based approach; see for example
how draft-ietf-ospf-yang handles discontinuities

           leaf access-mode {
/defination/definition/

           leaf clock-refid {
...                         reference clock of the peer to
                which clock is synchronized.";

I do not understand this.  Presumably this corresponds to
               type string {
                 length "4";
from the three type union but what object is this?

           leaf clock-offset {
examples could do with units to make it clear that it is '1.232mS' and
not '1.232s'

         leaf address {
           type inet:host;
this includes the domain name, which I see no mention of in the RFC

       list associations {
            /and isconfigured is required/and isconfigured are required/

         leaf address {
           type inet:host;
as above, the description seems to ignore the option of the domain name

         leaf refid {
same union as for leaf clock-refid, but a completely different
description, neither of which I understand.

                '20.1.1.1'
this address would appear to be assigned to Microsoft, not an
affiliation I see among the authors.  Is the company ok with this?

         leaf reach {
           type uint8;
is this the 8-bit p.reach shift register? reference needed (again:-)

         leaf unreach {
ditto

         leaf poll {
           type uint8;
           units "seconds";
           description
             "The polling interval for current association";
is there a useful default?  2s appears in the RFC in places

         leaf offset {
as above, the example values would be clearer with units

         leaf transmit-time {
           type yang:date-and-time;
           description
             "This is the local time, in timestamp format,
              at which the NTP packet departed the peer(T3).
              If the peer becomes unreachable the value is set to zero.";
I think, but am not sure, that a yang:date-and-time can never be set to
zero, the syntax does not allow it; the usual approach with YANG is a
union with another type which can indicate a special condition - int,
boolean, etc

         leaf input-time {
           type yang:date-and-time;
ditto

             leaf ttl {
               type uint8;
               description
                 "Specifies the time to live (TTL)
TTL does not exist in IPv6

             uses common-attributes {
               description
/attribute like/attributes such as/

             leaf ttl {
               type uint8;
               description
                 "Specifies the maximum time to live (TTL) for
TTL does not exist in IPv6

             uses common-attributes {
/attributes like/attributes such as/

             leaf beacon {
what are the units and is there a default? Is there a maximum of 15?  As
ever, a reference could tell me.

8.  Usage Example
lots of examples but none for IPv6 or JSON

8.1
      <sys:ntp xmlns:sys="urn:ietf:params:xml:ns:yang:ietf-ntp">
sys: is a defined prefix and must not be re-used

          <refid>20.1.1.1</refid>
as above, is Microsoft ok with this?

8.2
        <sys:ntp xmlns:sys="urn:ietf:params:xml:ns:yang:ietf-ntp">
sys: is a defined prefix and must not be re-used

8.3
        <sys:ntp xmlns:sys="urn:ietf:params:xml:ns:yang:ietf-ntp">
sys: is a defined prefix and must not be re-used

8.4
        <sys:ntp xmlns:sys="urn:ietf:params:xml:ns:yang:ietf-ntp">
sys: is a defined prefix and must not be re-used

8.5
  "224.1.1.1"
would appear to be a reserved address.  Other RFC used 224.0.1.1

        <sys:ntp xmlns:sys="urn:ietf:params:xml:ns:yang:ietf-ntp">
and again, twice

              <address>224.1.1.1</address>
as above

8.6
  "224.1.1.1"
as above

         <sys:ntp xmlns:sys="urn:ietf:params:xml:ns:yang:ietf-ntp">
as above, twice

8.7
      <ntp xmlns="urn:ietf:params:xml:ns:yang:ietf-ntp">
as above

8.8
      <ntp xmlns="urn:ietf:params:xml:ns:yang:ietf-ntp">
as above

          <refid>20.1.1.1</refid>
as above

8.9
      <ntp xmlns="urn:ietf:params:xml:ns:yang:ietf-ntp">
as above

12.2
    [RFC7317]  Bierman, A. and M. Bjorklund, "A YANG Data Model
wrong reference in the wrong place
this is an IANA-maintained module and so the reference must be to the
IANA website; and since the module is imported, the reference must be
Normative.

Tom Petch

----- Original Message -----
From: "The IESG" <[email-masked]>
To: "IETF-Announce" <[email-masked]>
Cc: <[email-masked]>; <[email-masked]>; <[email-masked]>;
<[email-masked]>; <[email-masked]>
Sent: Friday, January 29, 2021 10:39 PM
Subject: Last Call: <draft-ietf-ntp-yang-data-model-10.txt> (A YANG Data
Model for NTP) to Proposed Standard

---

From: tom petch <[email-masked]>
Date: Mon, 8 Feb 2021 11:37:09 +0000

This is my second response to this Last Call, about a possible security 
issue.

RFC8573 seems clear that MD5 must not be used to effect security for NTP 
but this I-D imports iana-crypt-hash which allows MD5 without any 
restriction, so is MD5 allowed or not?

There are features defined which allow the hash in iana-crypt-hash to be 
restricted but this I-D does not use them.

Probably iana-crypt-hash should be updated - I will raise that on the 
NETMOD WG list.

The I-D also uses MD5 in a way that would appear not to be security 
related, to hash an IPv6 address.

In passing, this I-D has three references to RFC7317.  This is wrong - 
the module is IANA-maintained and so the references should be to the 
IANA website.

The secdir reviewer might be interested in my thoughts.

Tom Petch

On 29/01/2021 22:39, The IESG wrote:
>
> The IESG has received a request from the Network Time Protocol WG (ntp) to
> consider the following document: - 'A YANG Data Model for NTP'
>    <draft-ietf-ntp-yang-data-model-10.txt> as Proposed Standard
>
> The IESG plans to make a decision in the next few weeks, and solicits final
> comments on this action. Please send substantive comments to the
> [email-masked] mailing lists by 2021-02-12. Exceptionally, comments may
> be sent to [email-masked] instead. In either case, please retain the beginning
> of the Subject line to allow automated sorting.
>
> Abstract
>
>
>     This document defines a YANG data model for Network Time Protocol
>     (NTP) implementations.  The data model includes configuration data
>     and state data.
>
>

---

From: Dhruv Dhody <[email-masked]>
Date: Mon, 8 Feb 2021 22:35:08 +0530

Hi Tom,

Thanks for your detailed review. Lets discuss the security first -

> restriction, so is MD5 allowed or not?

Good question. While it is easy to restrict the use of MD5 by adding a
must statement, I want to check if it is a good idea. The YANG model
is written in such a way that it supports older versions of NTP as
well. Would barring MD5 configuration be an issue if there are older
implementations in the network still? I think perhaps adding a warning
in the description is a good idea. I did a quick search and dont see
other YANG models doing a check either. Would be good to get some
guidance on this.

> restricted but this I-D does not use them.

I didn't see any reason to use them in the NTP Yang. Can you?

> related, to hash an IPv6 address.

This is as per RFC 5905 -

   If using the IPv4 address family, the identifier is the four-
   octet IPv4 address.  If using the IPv6 address family, it is the
   first four octets of the MD5 hash of the IPv6 address.

> the module is IANA-maintained and so the references should be to the

But even the iana-crypt-hash YANG model put RFC 7317 as a reference -

     revision 2014-08-06 {
       description
         "Initial revision.";
       reference
         "RFC 7317: A YANG Data Model for System Management";
     }

I will start working on your other comments and prepare a new version.

Thanks!
Dhruv

---

From: Harlan Stenn <[email-masked]>
Date: Mon, 8 Feb 2021 10:42:46 -0800

> other YANG models doing a check either. Would be good to get some

We're still seeing LOTS of ntpv3 traffic, from systems that have been
out there for an amusingly long time.  I expect these systems to
continue to stick around, and MD5 will be a part of that.

This is separate from the question of "will anybody want to monitor
these with the YANG-NTP stuff?"

-- 
Harlan Stenn <[email-masked]>
http://networktimefoundation.org - be a member!

---

From: "Ulrich Windl" <[email-masked]>
Date: Tue, 09 Feb 2021 08:39:15 +0100

>>> Dhruv Dhody <[email-masked]> schrieb am 08.02.2021 um 18:05 in
Nachricht
<CAB75xn7tXa1BCHd=KFR9DC=[email-masked]>:
> Hi Tom,
> 
> Thanks for your detailed review. Lets discuss the security first ‑
> 
>> but this I‑D imports iana‑crypt‑hash which allows MD5 without any

I just checked the docs of a recent ntpd: The docs say DES was replaced with
MD5, but they do in no way say that MD5 is obsolete.
For example the docs (man ntp.conf) say:
FILES
       /etc/ntp.conf  the default name of the configuration file
       ntp.keys       private MD5 keys

Regards,
Ulrich

> 
>> There are features defined which allow the hash in iana‑crypt‑hash to be
>> restricted but this I‑D does not use them.
>>
> 
>> Probably iana‑crypt‑hash should be updated ‑ I will raise that on the
>> NETMOD WG list.
>>
>> The I‑D also uses MD5 in a way that would appear not to be security
> This is as per RFC 5905 ‑
> 
>    If using the IPv4 address family, the identifier is the four‑
>> In passing, this I‑D has three references to RFC7317.  This is wrong ‑
>> the module is IANA‑maintained and so the references should be to the
>> IANA website.
>>
> 
> But even the iana‑crypt‑hash YANG model put RFC 7317 as a reference ‑
> 
>      revision 2014‑08‑06 {
>        description
>          "Initial revision.";
>        reference
>> > consider the following document: ‑ 'A YANG Data Model for NTP'
>> >    <draft‑ietf‑ntp‑yang‑data‑model‑10.txt> as Proposed Standard
>> >
>> > last‑[email-masked] mailing lists by 2021‑02‑12. Exceptionally, comments
may
> _______________________________________________
> ntp mailing list
> [email-masked] 
> https://www.ietf.org/mailman/listinfo/ntp

---

From: Harlan Stenn <[email-masked]>
Date: Mon, 8 Feb 2021 23:58:57 -0800

>        ntp.keys       private MD5 keys

That's a bug in the documentation.  ntp.keys holds private keys of
possibly many types.

---

From: Hal Murray <[email-masked]>
Date: Tue, 09 Feb 2021 01:34:46 -0800

[email-masked] said:
> RFC8573 seems clear that MD5 must not be used to effect security for NTP  but
> this I-D imports iana-crypt-hash which allows MD5 without any  restriction,
> so is MD5 allowed or not? 

"Allowed" is the key word.  Just because somebody published an RFC doesn't 
mean that all the gear out in the field will get updated.  As Harlan pointed 
out, there is a very very long tail on NTP deployments.

I think it makes sense for iana-crypt-hash to include slots for historic 
items.  If nothing else, it is a good place to say "historic" or "deprecated" 
and give references to the details.

If you think a Yang model should discourage using MD5, then I suggest adding 
words to say that.  Better would be to phrase things so that it also includes 
other algorithms that get kicked out of the club after the RFC is published.  
I don't know of any place that publishes an up-to-date list of crypto-hashing 
algorithms and their status.

----------

I'm looking at [email-masked]

It says:
         id | hash function | feature
         ---+---------------+-------------------
          1 | MD5           | crypt-hash-md5
          5 | SHA-256       | crypt-hash-sha-256
          6 | SHA-512       | crypt-hash-sha-512

If NTP is the only use, then I'd suggest adding a deprecated note.  But I 
assume that is used by other than NTP so that may not be appropriate.  But 
maybe if MD5 is deprecated for NTP it should be deprecated for other uses too. 
 ???

What happened to slots 2, 3, and 4?

Existing NTP code also supports SHA-1

RFC 8573 that deprecated using MD5 with NTP suggests using AES-CMAC.  Note 
that is CMAC rather than HMAC and that NTP uses it's own scheme rather than 
HMAC as described in RFC 6151.

The NTPsec code supports any hash (or CMAC) algorithm that the underlying 
library from OpenSSL supports.

-- 
These are my opinions.  I hate spam.

---

From: Dhruv Dhody <[email-masked]>
Date: Tue, 9 Feb 2021 15:56:56 +0530

Hi Hal,

Good point, I propose making this change -

OLD:
|     +--rw key?         ianach:crypt-hash
NEW:
|     +--rw key
|     |  +--rw (key-string-style)?
|     |     +--:(keystring)
|     |     |  +--rw keystring?            string
|     |     +--:(hexadecimal) {hex-key-string}?
|     |        +--rw hexadecimal-string?   yang:hex-string
END

The algorithm field already uses the identity
key-chain:crypto-algorithm from RFC 8177 which supports hmac-sha-1-12,
aes-cmac-prf-128, md5, sha-1, hmac-sha-1, hmac-sha-256, hmac-sha-384,
hmac-sha-512 and the ability to add more identities. The above change
would align with RFC 8177.

Thanks,
Dhruv

---

From: Miroslav Lichvar <[email-masked]>
Date: Tue, 9 Feb 2021 11:29:48 +0100

> library from OpenSSL supports.

There is also NTS. The model doesn't seem to have an option to enable
that.

This brings a question how can be the YANG model extended with new,
possibly implementation-specific, options, or new values for some
options. For example, the model says minpoll and maxpoll have a range
of 4..17, but no implementation that I know follows that.

-- 
Miroslav Lichvar

---

From: Dhruv Dhody <[email-masked]>
Date: Tue, 9 Feb 2021 16:12:34 +0530

Hi Miroslav,

> There is also NTS. The model doesn't seem to have an option to enable

Using the usual YANG augmentation technique
[https://tools.ietf.org/html/rfc7950#section-4.2.8], one can augment
this YANG model to add NTS details. Another I-D would need to be
published based on the interest in the WG.

> of 4..17, but no implementation that I know follows that.

The YANG augment allows for vendors to add vendor-specific parameters
to standard data models in an interoperable way. That said I will
reconfirm with my co-authors the range for minpoll and maxpoll and if
that needs to change.

Thanks!
Dhruv

---

From: tom petch <[email-masked]>
Date: Tue, 9 Feb 2021 10:10:30 +0000

A separate thought to my previous two.

Section 4 is very keen that this I-D and the system module which 
configures ntp SHOULD NOT coexist but this is not enforced by the YANG.

It could be.  Import the system module and make the presence container 
in this I-D dependent on the absence of the presence container in 
/system/ntp.

Tom Petch

---

From: tom petch <[email-masked]>
Date: Tue, 9 Feb 2021 10:23:57 +0000

Dhruv

After many years, Security (AD, secdir, advisor) still have the power to 
surprise me but I would still be surprised if Security were happy with 
an I-D which places no constraint on MD5 when the IETF has published RFC 
deprecating its use and NTP has RFC8573 which specifically deprecates it.

Yet Security may not realise this from reading this I-D since the 
unrestricted use of MD5 is not immediately apparent so my post was aimed 
at bringing this to the attention of Security.  As to whether this needs 
a note in Security Considerations or enforcing by YANG or both I am less 
clear on - that is up to Security.  If the YANG is to deprecate it, then 
the features in ianach make that possible.

Whether or not MD5 is widely used in the field is irrelevant.  The IETF 
consensus it to deprecate its use and I am sure that the IESG will want 
this I-D to do just that.

Tom Petch

---

From: tom petch <[email-masked]>
Date: Tue, 9 Feb 2021 10:33:57 +0000

>         ntp.keys       private MD5 keys

Ulrich
The issue here is what the IETF is willing to give consensus for, as 
opposed to what is out in the field.  For some years now, the IETF has 
deprecated the use of MD5 (even though there are contexts in which it is 
still strong enough) so the point of my post was to flag to those in the 
IETF who are concerned with security, such as Security ADs, that this 
I-D places no constraint in the YANG on the use of MD5.  I think that 
that will not gain consensus and that changes will be needed, either in 
the Security Considerations or the YANG or both.

Tom Petch

> .

---

From: tom petch <[email-masked]>
Date: Tue, 9 Feb 2021 11:07:49 +0000

> algorithms and their status.

IANA TLS HashAlgorithm

It lacks a Recommended column which other TLS registries often have.

This is the sort of service that the Security Area could provide for the 
rest of the IETF, but then, this is the IETF:-)

iana-crypt-hash belongs to the NETMOD WG which is why I said that I 
would raise the issue there.  That WG IMHO lacks the expertise to 
specify a status so that would have to come from SAAG, a Security AD or 
some such.

That module is IANA-maintained and is Expert Review so I think that the 
consensus of the NETMOD WG to make MD5 status deprecated would be 
feasible.  (It would not be possible, or sensible, to remove MD5 - that 
would be a new module and an RFC).

Tom Petch

---

From: "Salz, Rich" <[email-masked]>
Date: Tue, 9 Feb 2021 13:29:02 +0000

>    It lacks a Recommended column which other TLS registries often have.

The recommended column has specific meanings, part of which is that it is recommended *for use in TLS.*  For example, SHA1 as the TLS handshake digest is okay, but as a general digitial signature mechanism it is not. (The other part is that recommended means that a TLS RFC using it has been published, basically.)

I'm one of the designated experts for the TLS registries.

---

From: Dhruv Dhody <[email-masked]>
Date: Thu, 11 Feb 2021 15:09:21 +0530

Hi Tom,

Thanks for your review. I have posted an updated version of the YANG.

https://datatracker.ietf.org/doc/draft-ietf-ntp-yang-data-model/
https://www.ietf.org/rfcdiff?url1=draft-ietf-ntp-yang-data-model-10&url2=draft-ietf-ntp-yang-data-model-12

Please find my consolidated response below -

Email 1:
========

> I-D, is inadequate IMO - section numbers are needed in every case.

I have added section numbers in most of the cases.
I have changed enums to identity.

> should mention support, or lack thereof, for NMDA

Its present in Section 1.1 already, I have added keyword NMDA as well now.

> reference must be to the IANA website

No longer using ianach in this update.

> /refrenced in /referenced in /

Updated

> less than three pages, this is quite small and would be ok on its own

Updated

> support per-interface configuration/

Updated

> see access-mode) and attach a different acl-rule/

Updated

> /this YANG modules/this YANG module/

Updated

> NTP association (for example unicast-server),

Updated

> wrong reference - this module is IANA-maintained so the reference must

Not using iana-crypt-hash anymore!

> the I-D has five editors but there are only two here

There are 2 co-authors marked as editors. RFC 8407 says the document
author or editor contact information needs to be present.

> I cannot find this in RFC5905

Section 9.2 and Appendix A.5.4. I have added section number in reference.

> this I can find but it ranges from 0 to 7 whereas the I-D has 0 to 4 - is

Changed to identity.

> RFC, 5 here - is this intended?

Instead of enum and values, changed to identity.

> for what value of XXX?

Removed.

> how draft-ietf-ospf-yang handles discontinuities

Updated as suggested.

> /defination/definition/

Updated

> from the three type union but what object is this?

I created a typedef and cleaned it up. This is as described in Section
7.3 of RFC5905 under Reference ID.

> examples could do with units to make it clear that it is '1.232mS' and

Updated

> this includes the domain name, which I see no mention of in the RFC

Changed to inet:ip-address

> description, neither of which I understand.

Updated (all of the above).

Updated to 203.0.113.1

> is this the 8-bit p.reach shift register? reference needed (again:-)

Added.

> is there a useful default?  2s appears in the RFC in places

No default in this case.

> as above, the example values would be clearer with units

Updated

> union with another type which can indicate a special condition - int,

Thanks, updated as per your suggestion.

> TTL does not exist in IPv6

This TTL is not related to IP. See section 3.1 for manycast and
updated with reference.

> /attribute like/attributes such as/

Updated

> TTL does not exist in IPv6

As above.

> /attributes like/attributes such as/

Updated

> ever, a reference could tell me.

Added reference

> lots of examples but none for IPv6 or JSON

Added one IPv6. I dont think we need add one for JSON as well.

> sys: is a defined prefix and must not be re-used

Oops, updated here and all other instance!

> as above, is Microsoft ok with this?

Updated to 203.0.113.1

> sys: is a defined prefix and must not be re-used

Updated

>               <address>224.1.1.1</address>

Updated in all cases.

> IANA website; and since the module is imported, the reference must be

No longer using iana-crypt-hash.

> Model for NTP) to Proposed Standard

Email 2:
========

> Probably iana-crypt-hash should be updated - I will raise that on the

No longer using iana-crypt-hash, instead made this change to allow all
sort of key formats -

OLD:

The comment related to use of MD5 still applies. I have not yet added
a check for the algorithm. I have updated the description and security
consideration to highligh that MD5 is depricated as per RFC 8573. Lets
see what the IESG suggests...

> related, to hash an IPv6 address.

This is as per RFC 5905 - "If using the IPv6 address family, it is the
first four octets of the MD5 hash of the IPv6 address."

> the module is IANA-maintained and so the references should be to the

No longer using iana-crypt-hash.

> The secdir reviewer might be interested in my thoughts.

Email 3:
========

> in this I-D dependent on the absence of the presence container in

Updated as per your suggestion!

====

Thanks again for your detailed review. It has improved the YANG model
significantly!

Thanks!
Dhruv & Ankit

On Sat, Jan 30, 2021 at 4:09 AM The IESG <[email-masked]> wrote:
> The file can be obtained via
> https://datatracker.ietf.org/doc/draft-ietf-ntp-yang-data-model/
>
>
>
> No IPR declarations have been submitted directly on this I-D.
>
>
>
>
>

---

From: tom petch <[email-masked]>
Date: Fri, 12 Feb 2021 12:01:22 +0000

> https://www.ietf.org/rfcdiff?url1=draft-ietf-ntp-yang-data-model-10&url2=draft-ietf-ntp-yang-data-model-12

That was quick!  I have done a first pass but have yet to chase down the 
references and one at least looks odd to me. I expect to complete the 
process on Monday am GMT.

Meanwhile,

s.6
/For this reason, this YANG module allow/
For this reason, this YANG module allows/

s.7
This has lost its two character margin, needs insetting

typedef refid I like, much clearer, but perhaps could do with a 
reference for MD5, RFC1321

/Discountinuities in the value/Discontinuities in the value/
in three places

when 'false() = boolean(/sys:system/sys:ntp)' {
I am sure that this is valid but is not something I am used to seeing. 
An aspect of YANG that I have never understood is when you have to spell 
things out and when they are implicit and I think that all you need here is
when not <presence container>
but would want to verify that with a YANG doctor

The definition of each possible values:/
The definition of each possible value:/

       leaf poll {
         type uint8;
This seems wrong to me.  Looking at s.7.3
    Poll: 8-bit signed integer representing the maximum interval between
    successive messages, in log2 seconds.  Suggested default limits for
    minimum and maximum poll intervals are 6 and 10, respectively.
ie signed integer and not uint

TTL good to have the reference - I was barking up the wrong tree

I note
#define TTLMAX          8       /* max ttl manycast */
I assume that this is a matter of choice - I see 255 in examples which 
is at the other end of the spectrum

           leaf beacon {
             type uint8;
I cannot find a definition of this in the RFC (which I think a defect in 
the RFC).  Reverse engineering the code I see beacon compared to 
unreach, which is defined, as
    unreach: integer representing the number of seconds the server has
    been unreachable
which suggests that beacon is a number of seconds, and not a log base 2 
value or a counter; is this right? (The RFC should say IMHO)

s.8
/for illustration purporses./for illustration purposes./

Back on Monday, all being well

I got a bounce message last time from Yi saying on holiday until 22nd 
February.

Tom Petch.

---

From: Harlan Stenn <[email-masked]>
Date: Fri, 12 Feb 2021 12:48:05 -0800

On 2/12/2021 4:01 AM, tom petch wrote:
>> ...

Why are 6 and 10 useful as suggested limits for minimum and maximum poll
values?

---

From: Benjamin Kaduk <[email-masked]>
Date: Sun, 14 Feb 2021 17:11:27 -0800

Hi Tom,

> this I-D to do just that.

Thanks for flagging the issue; it's good to have many eyes looking out for
these things.

That said, I think recent practice has been to not take a strict hard line
that MD5 cannot be used ever, and that non-cryptographic uses for legacy
compatibility can be retained, when accompanied by a disclaimer that the
use of MD5 is not for cryptographic purposes and that MD5 is not a secure
cryptographic hash function.

There is an example of this buried in my remarks at
https://datatracker.ietf.org/doc/draft-ietf-extra-imap4rev2/ballot/ that
resulted in the text now found at
https://tools.ietf.org/html/draft-ietf-extra-imap4rev2-29#section-11.6 .

-Ben

---

From: tom petch <[email-masked]>
Date: Mon, 15 Feb 2021 11:55:10 +0000

> process on Monday am GMT.

Ian

I have run out of time but have made a second pass.  Some aspects still 
confuse me, where I cannot be sure of the correlation between YANG and 
RFC, notable time and mode; I want the words to be identical and they 
are not!

   identity access-mode {
I do not understand. The RFC has protocol, association and packet modes 
but not access mode.  This is perhaps section 3, but I am uncertain, the 
terminology is not quite the same, which is perhaps ok for an NTP 
expert, which I am not, but not in a technical specification IMHO.

Thus you derive
   identity query-access-mode {
but searching the RFC only yields a match in Security.  I want the terms 
to be identical!

In contrast, association mode is in the RFC and so I can see
   identity broadcast-client {
which lacks a value for mode - perhaps 6.  But I would still like a 
reference to section 3 for these.

   identity ntp-sync-state {
is again troublesome.  The RFC has
#define NSET     0       /* clock never set */
#define FSET     1       /* frequency set from file */
#define SPIK     2       /* spike detected */
#define FREQ     3       /* frequency mode */
#define SYNC     4       /* clock synchronized */
whereas the I-D has
   identity clock-not-set {
   identity freq-set-by-cfg {
   identity clock-set {
   identity freq-not-determined {
   identity clock-synchronized {
   identity spike {
which is close but I want them to be identical or a note saying how they 
are different.

Then what is the difference between this and
   identity clock-state {
which has
   identity synchronized {
   identity unsynchronized {
Why have both?

Other thoughts

I wonder about the role of
   identity aes-cmac {
     base key-chain:crypto-algorithm;
What does it add that a direct reference to AES-CMAC does not have? Why 
import key-chain.  I am not saying it has no value but am unclear of 
that value.  And
     leaf algorithm {
could do with a reference for AES-CMAC.

Some authors include the prefix on the module in the list of prefix; 
seems logical.

    typedef refid
MD5 could do with a reference.

         leaf clock-stratum {
What is syspeer? I do not see it in the RFC:-)

         leaf clock-precision {
should be signed not unsigned I think

leaf poll
is indeed poll in section 7.3 but I wonder why  13.1 uses hpoll? 
Probably a comment on the RFC and not on the I-D.

With ones from my previous post, below, that is it!

Tom Petch

---

From: tom petch <[email-masked]>
Date: Mon, 15 Feb 2021 12:15:34 +0000

On 15/02/2021 03:23, Hal Murray wrote:
>> compatibility can be retained, when accompanied by a disclaimer that the use
>> of MD5 is not for cryptographic purposes and that MD5 is not a secure
> I'm missing the big picture.  What is Yang supposed to do?
>
> I'd expect it would be describing existing practices.  I don't expect it to be
> trying to add MUSTs to other RFCs.
>
> I'd be happy with notes that a use case has been deprecated, especially if
> there is an RFC to point to.
>
> But unless I'm missing something, Yang is not the place to be trying to
> enforce good crypto practices.  Most people working on NTP won't pay any
> attention to Yang if they even know it exists.
>
>
> NTP uses MD5 in two places.  One is hashing IPv6 addresses to make something
> that fits into a slot that only has room for IPv4 addresses.  I don't think
> there are any crypto/security considerations.
>
> The other is for authenticating packets.  RFC 8573 deprecates that usage.  A
> note in a Yang document saying  "using MD5 for authenticating NTP has been
> deprecated by RFC 8573" seems like a good idea.  I think anything stronger
> will be inappropriate.  But maybe I don't understand what Yang is all about.

Hal

YANG provides configuration and management for IETF (and other 
protocols).  I am not sure which MUST you have in mind but RFC5905 is 
updated by RFC8573 and I see nothing in this I-D as it stands that goes 
beyond those RFC.

It is not the job of YANG to enforce good practice but it is the job of 
an author to produce something that gets through the IESG and, in some 
areas, the IESG is ahead of common practice, the use of IPv6 and 
Security being two where I regularly see the IESG wanting more than I 
see in the world at large.  (Thus the IETF has just produced an I-D 
updating 100 or so RFC to deprecate the use of older versions of TLS; 
this does not surprise me but I do not seeing increasing the security of 
the Internet, perhaps the opposite, but then this is security!)

So MD5 may be in widespread use for security in lots of protocols but 
that does not mean that an I-D (implicitly) recommending its use will 
pass the IESG!  I do not know the answer to that which is why I posted a 
separate e-mail on the topic intending to catch the eye of a Security 
I-D:-).  The I-D has passed a secdir review but that may not have 
realised the implication buried in the YANG.  Perhaps a note in the 
Security Considerations deprecating MD5 is enough but that is not my call.

Tom Petch

---

From: Hal Murray <[email-masked]>
Date: Mon, 15 Feb 2021 06:34:10 -0800

[email-masked] said:
> leaf poll is indeed poll in section 7.3 but I wonder why 13.1 uses hpoll?

There  is a slot in the header of NTP packets called poll.

There are 2 "poll" slots in the peer struct that a host uses to keep track of 
the info associated with a server it is talking to.
  hpoll is this hosts polling interval when querying that server.
  ppoll is that servers polling interval (as reported in the last packet from 
that server)

Section 7.3 is Packet Header Variables
Section 13.1 is Poll Process Variables (aka the stuff in the struct)

---

From: tom petch <[email-masked]>
Date: Mon, 15 Feb 2021 17:36:46 +0000

> https://tools.ietf.org/html/draft-ietf-extra-imap4rev2-29#section-11.6 .

Ben

Thank you for noticing:-)
The -10 that I reviewed had
- MD5 is used to turn an IPv6 address into a 32-bit identifier
- MD5 can be used for authentication without constraint
- AES-CMAC cannot be used for authentication.
I do not have a view on the first but see the second and third as 
contradicting RFC8573 and so in need of change.  Allowing AES-CMAC I see 
as not controversial but do not have a view as to what to do with MD5 
used for authentication e.g.
- allow without constraint
- allow, but deprecated, in the YANG
- allow with a note in Security COnsiderations
- do not allow in the YANG
etc.
I am still sitting on the fence on that issue, lacking the secuirty 
insight as to what would be acceptable, to you and the IESG.

Tom Petch

---

From: Benjamin Kaduk <[email-masked]>
Date: Mon, 15 Feb 2021 15:48:48 -0800

> insight as to what would be acceptable, to you and the IESG.

I would mostly have expected the MD5 authentication option to be split off
into a separate YANG module that aguments the base one, or maybe hidden
behind a YANG feature, in order to give implementations the ability to "do
the right thing" (that is, expose only the modern crypto) while remaining
compliant with the primary YANG module.  As far as I know, either of those
options would still make it pretty trivial to also expose the MD5
functionality for legacy compatibility (which is in some different sense
also "the right thing to do", at least for now), but would make it clear
that it's not endorsed by the IETF to the same level.

-Ben

---

From: tom petch <[email-masked]>
Date: Tue, 16 Feb 2021 16:52:17 +0000

Ben

Thank you for the suggestions.  I will pursue it with Dhruv and see what 
he wants to do.  YANG feature would seem the simpler option to me but 
either sound fine.

Tom Petch

---

From: Harlan Stenn <[email-masked]>
Date: Tue, 16 Feb 2021 18:31:34 -0800

> he wants to do.  YANG feature would seem the simpler option to me but

For whatever it's worth, I'm in favor of simple, clear, honest information.

---

From: Hal Murray <[email-masked]>
Date: Tue, 16 Feb 2021 18:50:40 -0800

> - AES-CMAC cannot be used for authentication.

SHA-1 is also in use.

> - do not allow in the YANG

I would suggest adding a short note at the point of use indicating that it has 
been deprecated with a pointer to the Security Considerations section and a 
paragraph there summarizing RFC 8573.

---

From: Dhruv Dhody <[email-masked]>
Date: Wed, 17 Feb 2021 17:04:43 +0530

Hi Tom,

We have posted another update.

Updated: https://datatracker.ietf.org/doc/draft-ietf-ntp-yang-data-model/
Diff: https://www.ietf.org/rfcdiff?url2=draft-ietf-ntp-yang-data-model-13

As suggested by Ben, added a YANG feature for MD5.

Further details -

Updated

> This has lost its two character margin, needs insetting

Not sure where. I did run "pyang -f yang --keep-comments --yang-line-length 69"

> reference for MD5, RFC1321

I added the reference around the new MD5 feature (as suggested by Ben)
and not at refid.

> /Discountinuities in the value/Discontinuities in the value/

Updated.

> but would want to verify that with a YANG doctor

I have used when presence before but not sure what is the best way for
"when not presence" and didn't see it before :)

> The definition of each possible value:/

Updated.

> ie signed integer and not uint

int8 is better, updated.

> is at the other end of the spectrum

Updated example.

> value or a counter; is this right? (The RFC should say IMHO)

Yes, it is not log2. I have added more text in the description.

> /for illustration purporses./for illustration purposes./

Updated.

> expert, which I am not, but not in a technical specification IMHO.

The access-mode is related to the ACL. It is not related to
association and packet modes. The RFC does not go into detail but
mentions its use in Section 9.2.

> but searching the RFC only yields a match in Security.  I want the terms

This is not mentioned, but the granularity comes from the existing ACL
configurations with NTP.

> reference to section 3 for these.

I have made an update and aligned it as per the RFC

Updated

>    identity unsynchronized {

I have added the description to say one is a higher-level state and
another a granular one.

> could do with a reference for AES-CMAC.

We are no longer using key-chain to make sure we could use the YANG
feature for MD5.

> What is syspeer? I do not see it in the RFC:-)

Updated.

> should be signed not unsigned I think

Updated.

> Probably a comment on the RFC and not on the I-D.

Hal provided the details.

Thanks!
Dhruv & Ankit

---

From: Harlan Stenn <[email-masked]>
Date: Wed, 17 Feb 2021 03:39:11 -0800

>> ie signed integer and not uint

Why "default limits" and not "default values"?

---

From: Dhruv Dhody <[email-masked]>
Date: Wed, 17 Feb 2021 17:22:31 +0530

Hi Harlan,

> Why "default limits" and not "default values"?

The text with the term "default limits" is from RFC 5905 (section 7.3).

In the YANG model, we have the leaves minpoll and maxpoll with default
values as 6 and 10 the leaf poll is the current polling interval.

Thanks!
Dhruv

---

From: Harlan Stenn <[email-masked]>
Date: Wed, 17 Feb 2021 04:01:24 -0800

Hi Dhruv,

> The text with the term "default limits" is from RFC 5905 (section 7.3).

Different context.

I just re-read -13 and it does not use "limits", which is good.

I am wondering who will be confused if we see 6/10 used, and then
somebody notices that the iburst flag is in play so the initiating side
polls every 2 seconds (poll=2) up to 8 times even though minpoll is 6.

> values as 6 and 10 the leaf poll is the current polling interval.

Yes, thanks.

---

From: Harlan Stenn <[email-masked]>
Date: Wed, 17 Feb 2021 04:03:34 -0800

> polls every 2 seconds (poll=2) up to 8 times even though minpoll is 6.

Perhaps changing:

      description
        "The minimum poll interval used in this association.";

to:

      description
        "The minimum poll interval normally used in this association.";

---

From: Hal Murray <[email-masked]>
Date: Sun, 14 Feb 2021 19:23:39 -0800

---

From: "Ulrich Windl" <[email-masked]>
Date: Mon, 15 Feb 2021 09:50:28 +0100

>>> Harlan Stenn <[email-masked]> schrieb am 12.02.2021 um 21:48 in Nachricht
> Why are 6 and 10 useful as suggested limits for minimum and maximum poll

Maybe because of this (man ntp.conf):
"The maximum poll interval  defaults  to 10 (1,024 s), but can be increased by the
maxpoll option to an upper limit of 17 (36.4 h).  The minimum  poll interval
defaults to 6 (64 s), but can be decreased by the minpoll option to a lower limit
of 4 (16 s)."

Also (RFC): "Typical secondary servers and clients on fast LANs are within a few
hundred microseconds with poll intervals up to 1024 seconds, which was the maximum
with NTPv3.  With NTPv4, servers and clients are precise within a few tens of
milliseconds with poll intervals up to 36 hours."
"A client sends messages to each server with a poll interval of 2^tau seconds,
as determined by the poll exponent tau.  In NTPv4, tau ranges from 4 (16 s) to
17 (36 h)."

"Suggested default limits for minimum and maximum poll intervals are 6 and 10, respectively."

Regards,
Ulrich

---

From: Harlan Stenn <[email-masked]>
Date: Mon, 15 Feb 2021 04:20:00 -0800

> "Suggested default limits for minimum and maximum poll intervals are 6 and 10, respectively."

Then they are "Suggested default >values< for ...", not >limits<.

---

From: tom petch <[email-masked]>
Date: Wed, 17 Feb 2021 12:32:57 +0000

> Diff: https://www.ietf.org/rfcdiff?url2=draft-ietf-ntp-yang-data-model-13

Indeed and I have downloaded it.  I will look at Friday or perhaps 
Thursday.  I am conscious that the Last Call period has expired and that 
sometimes ADs press the 'Go' button while I am still thinking:-(  Ben 
did provide a link to text he had crafted about non-cryptographic use of 
MD5 which I rather liked - up to you whether or not you want it include it.

Tom Petch

---

From: tom petch <[email-masked]>
Date: Fri, 19 Feb 2021 10:30:28 +0000

-13 looks better

Perhaps a comment for the AD.  You have added a reference for MD5 which 
I think necessary but as Informative which I find debatable given its 
use to generate a 32-bit identifier from a IPv6 address, ie its use is 
going to go up and up, not fade away.  I am aware that the RFC is 
Informational not Standards Track so a Normative Reference would be a 
downref which needs calling out in a Last Call which means a new Last 
Call.  (Given the number of changes made since -10, perhaps that is not 
such a left field idea:-)  The reference in RFC5905 is Normative.  Like 
I say, one for the AD.

Hash algorilthms - a minefield.  You have lots of choice but that 
increases the probability of incompatability between client and server. 
  One is good, obsolescent and now recommended is good, but a choice of 
eight without even including SHA3?  I note too that the Netconf WG 
regard SHA1 as obsolete.

Access mode I think deserves more text, a sub-section in s.5, not just a 
mention.  Authors vary as to how much text there is in a YANG I-D but I 
think that the better ones have more rather than less, bridging the gap 
from protocol specification to YANG module, and, as you have seen, I 
went looking in the RFC for an explanation, which was a waste of time:-) 
  And I really do not understand the text.
'can be performed on the local NTP service'
I find ambiguous.  Does it mean that the entity in which this YANG 
module is will send such requests? Or does it mean that the entity will 
respond to such requests?  And how does that vary with server, client, 
peer?  I cannot tell from the I-D.

identity broadcast-client could do with mode 6 to bring it in line with 
other identity

s.8.5
You slip in a version 3 which I guarantee will attract the attention of 
an AD!.  You mentioned earlier that you intended the module to work with 
V3 which I did not pick up from reading the I-D.  If that is your 
intention, then I think that this needs calling out, perhaps in the 
Introduction as well as the text preceding s.8.5

Tom Petch

---

From: "Ulrich Windl" <[email-masked]>
Date: Fri, 19 Feb 2021 12:52:45 +0100

>>> tom petch <[email-masked]> schrieb am 19.02.2021 um 11:30 in
Nachricht
<[email-masked]>:
> ‑13 looks better
> use to generate a 32‑bit identifier from a IPv6 address, ie its use is 

Well, first using IPv6->MD5 is a 128bit->128 bit transformation, and then
using 32 bits of that Is not worse than doing 128->160 bits (like SHA-1) and
then take 32 bits of it. After all, the purpose was NOT to hide the IPv6
address, but to "stir the bits" to reduce the probability of conflicts. Things
are as they are, but as I proposed, a better approach would be to use some
random identifier like UUID for the server identity... Others thin a server
should not have a unique ID at all. It's all not related to using MD5,
however.

Regards,
Ulrich

> Call.  (Given the number of changes made since ‑10, perhaps that is not 
> such a left field idea:‑)  The reference in RFC5905 is Normative.  Like 
> I say, one for the AD.
> 
> Hash algorilthms ‑ a minefield.  You have lots of choice but that 
> Access mode I think deserves more text, a sub‑section in s.5, not just a 
> mention.  Authors vary as to how much text there is in a YANG I‑D but I 
> went looking in the RFC for an explanation, which was a waste of time:‑) 
> peer?  I cannot tell from the I‑D.
> 
> identity broadcast‑client could do with mode 6 to bring it in line with 
> other identity
> 
> s.8.5
> V3 which I did not pick up from reading the I‑D.  If that is your 
>>> Updated: https://datatracker.ietf.org/doc/draft‑ietf‑ntp‑yang‑data‑model/

>>> Diff: https://www.ietf.org/rfcdiff?url2=draft‑ietf‑ntp‑yang‑data‑model‑13

>> sometimes ADs press the 'Go' button while I am still thinking:‑(  Ben
>> did provide a link to text he had crafted about non‑cryptographic use of
>> MD5 which I rather liked ‑ up to you whether or not you want it include
it.
>>> Further details ‑
>>> Not sure where. I did run "pyang ‑f yang ‑‑keep‑comments
>>> ‑‑yang‑line‑length 69"
>>>
>>>>      Poll: 8‑bit signed integer representing the maximum interval
>>>> between
>>>> TTL good to have the reference ‑ I was barking up the wrong tree
>>>>
>>>> I note
>>>> I assume that this is a matter of choice ‑ I see 255 in examples which
>>>>     identity access‑mode {
>>> The access‑mode is related to the ACL. It is not related to
>>>>     identity query‑access‑mode {
>>>>     identity broadcast‑client {
>>>> which lacks a value for mode ‑ perhaps 6.  But I would still like a
>>>>     identity ntp‑sync‑state {
>>>> whereas the I‑D has
>>>>     identity clock‑not‑set {
>>>>     identity freq‑set‑by‑cfg {
>>>>     identity clock‑set {
>>>>     identity freq‑not‑determined {
>>>>     identity clock‑synchronized {
>>>>     identity spike {
>>>>     identity clock‑state {
>>> I have added the description to say one is a higher‑level state and
>>> another a granular one.
>>>
>>>> Other thoughts
>>>>
>>>>     identity aes‑cmac {
>>>>       base key‑chain:crypto‑algorithm;
>>>> What does it add that a direct reference to AES‑CMAC does not have? Why
>>>> import key‑chain.  I am not saying it has no value but am unclear of
>>>> that value.  And
>>>>       leaf algorithm {
>>>> could do with a reference for AES‑CMAC.
>>>>
>>>
>>> We are no longer using key‑chain to make sure we could use the YANG
>>> feature for MD5.
>>>
>>>>           leaf clock‑stratum {
>>>> What is syspeer? I do not see it in the RFC:‑)
>>>>
>>>
>>> Updated.
>>>
>>>>           leaf clock‑precision {
>>>> Probably a comment on the RFC and not on the I‑D.
>>>>
>>>
>>>>>> https://datatracker.ietf.org/doc/draft‑ietf‑ntp‑yang‑data‑model/ 
>>>>>> 
>
https://www.ietf.org/rfcdiff?url1=draft‑ietf‑ntp‑yang‑data‑model‑10&url2=draft‑ietf‑nt

> p‑yang‑data‑model‑12
>>>>     identity access‑mode {
>>>>     identity query‑access‑mode {
>>>>     identity broadcast‑client {
>>>> which lacks a value for mode ‑ perhaps 6.  But I would still like a
>>>>     identity ntp‑sync‑state {
>>>> whereas the I‑D has
>>>>     identity clock‑not‑set {
>>>>     identity freq‑set‑by‑cfg {
>>>>     identity clock‑set {
>>>>     identity freq‑not‑determined {
>>>>     identity clock‑synchronized {
>>>>     identity spike {
>>>>     identity clock‑state {
>>>>     identity aes‑cmac {
>>>>       base key‑chain:crypto‑algorithm;
>>>> What does it add that a direct reference to AES‑CMAC does not have? Why
>>>> import key‑chain.  I am not saying it has no value but am unclear of
>>>> that value.  And
>>>>       leaf algorithm {
>>>> could do with a reference for AES‑CMAC.
>>>>
>>>>           leaf clock‑stratum {
>>>> What is syspeer? I do not see it in the RFC:‑)
>>>>
>>>>           leaf clock‑precision {
>>>> Probably a comment on the RFC and not on the I‑D.
>>>>
>>>>>      Poll: 8‑bit signed integer representing the maximum interval
>>>>> between
>>>>> TTL good to have the reference ‑ I was barking up the wrong tree
>>>>>
>>>>> I note
>>>>> I assume that this is a matter of choice ‑ I see 255 in examples which
>>>>>> Please find my consolidated response below ‑
>>>>>>
>>>>>> Email 1:
>>>>>> ========
>>>>>>
>>>>>>> I have two main problems and a lot of lesser ones with this I‑D;
>>>>>>> given
>>>>>>> values in the I‑D with those of the protocol, either I cannot find
>>>>>>> them
>>>>>>> the protocol says.  A reference to a 110 page I‑D, with two updating
>>>>>>> I‑D, is inadequate IMO ‑ section numbers are needed in every case.
>>>>>>>
>>>>>>
>>>>>>>               | ianach    | iana‑crypt‑hash          | [RFC7317] |
>>>>>>> the reference is wrong; this is an IANA‑ maintained module so the
>>>>>>> /supports per‑interface configurations /
>>>>>>> support per‑interface configuration/
>>>>>>>
>>>>>>
>>>>>> Updated
>>>>>>
>>>>>>> 5.  Access Rules
>>>>>>> /refer access‑mode) and attach different acl‑rule/
>>>>>>> see access‑mode) and attach a different acl‑rule/
>>>>>>>
>>>>>>
>>>>>> Updated
>>>>>>
>>>>>>> 6.  Key Management
>>>>>>> /32‑bits unsigned /32‑bit unsigned/
>>>>>>>
>>>>>>> NTP association (for example unicast‑server),
>>>>>>> /specefied/specified/
>>>>>>>
>>>>>>
>>>>>> Updated
>>>>>>
>>>>>>> 7.  NTP YANG Module
>>>>>>>
>>>>>>>        import iana‑crypt‑hash {
>>>>>>> wrong reference ‑ this module is IANA‑maintained so the reference
>>>>>>> must
>>>>>>> be to the IANA website
>>>>>>>
>>>>>>
>>>>>> Not using iana‑crypt‑hash anymore!
>>>>>>
>>>>>>>        contact
>>>>>>> the I‑D has five editors but there are only two here
>>>>>>>
>>>>>>
>>>>>> There are 2 co‑authors marked as editors. RFC 8407 says the document
>>>>>>>        typedef access‑mode {
>>>>>>>        typedef association‑mode {
>>>>>>> this I can find but it ranges from 0 to 7 whereas the I‑D has 0 to 4
>>>>>>> ‑ is
>>>>>>> this intended?
>>>>>>>
>>>>>>
>>>>>> Changed to identity.
>>>>>>
>>>>>>>        typedef ntp‑sync‑state {
>>>>>>> RFC, 5 here ‑ is this intended?
>>>>>>>
>>>>>>
>>>>>>>          leaf packet‑sent {
>>>>>>>          leaf packet‑received {
>>>>>>>          leaf packet‑dropped {
>>>>>>> but I suggest you replace it with a YANG‑based approach; see for
>>>>>>> example
>>>>>>> how draft‑ietf‑ospf‑yang handles discontinuities
>>>>>>>
>>>>>>
>>>>>> Updated as suggested.
>>>>>>
>>>>>>
>>>>>>>              leaf access‑mode {
>>>>>>> /defination/definition/
>>>>>>>
>>>>>>
>>>>>> Updated
>>>>>>
>>>>>>>              leaf clock‑refid {
>>>>>>>              leaf clock‑offset {
>>>>>> Changed to inet:ip‑address
>>>>>>
>>>>>>>          list associations {
>>>>>>> same union as for leaf clock‑refid, but a completely different
>>>>>>> is this the 8‑bit p.reach shift register? reference needed (again:‑)
>>>>>>>
>>>>>>>            leaf unreach {
>>>>>>> ditto
>>>>>>>
>>>>>>
>>>>>> Added.
>>>>>>
>>>>>>>            leaf poll {
>>>>>>>              type uint8;
>>>>>>>              units "seconds";
>>>>>>>              description
>>>>>>>            leaf transmit‑time {
>>>>>>>              type yang:date‑and‑time;
>>>>>>> I think, but am not sure, that a yang:date‑and‑time can never be
>>>>>>> set to
>>>>>>> union with another type which can indicate a special condition ‑ int,
>>>>>>> boolean, etc
>>>>>>>
>>>>>>>            leaf input‑time {
>>>>>>>              type yang:date‑and‑time;
>>>>>>> ditto
>>>>>>>
>>>>>>
>>>>>>>                uses common‑attributes {
>>>>>>>                uses common‑attributes {
>>>>>>>         <sys:ntp xmlns:sys="urn:ietf:params:xml:ns:yang:ietf‑ntp">
>>>>>>> sys: is a defined prefix and must not be re‑used
>>>>>>>
>>>>>>
>>>>>>>           <sys:ntp xmlns:sys="urn:ietf:params:xml:ns:yang:ietf‑ntp">
>>>>>>> sys: is a defined prefix and must not be re‑used
>>>>>>>
>>>>>>> 8.3
>>>>>>>           <sys:ntp xmlns:sys="urn:ietf:params:xml:ns:yang:ietf‑ntp">
>>>>>>> sys: is a defined prefix and must not be re‑used
>>>>>>>
>>>>>>> 8.4
>>>>>>>           <sys:ntp xmlns:sys="urn:ietf:params:xml:ns:yang:ietf‑ntp">
>>>>>>> sys: is a defined prefix and must not be re‑used
>>>>>>>
>>>>>>> 8.5
>>>>>>>     "224.1.1.1"
>>>>>>>           <sys:ntp xmlns:sys="urn:ietf:params:xml:ns:yang:ietf‑ntp">
>>>>>>> and again, twice
>>>>>>>
>>>>>>>            <sys:ntp xmlns:sys="urn:ietf:params:xml:ns:yang:ietf‑ntp">
>>>>>>> as above, twice
>>>>>>>
>>>>>>> 8.7
>>>>>>>         <ntp xmlns="urn:ietf:params:xml:ns:yang:ietf‑ntp">
>>>>>>> as above
>>>>>>>
>>>>>>> 8.8
>>>>>>>         <ntp xmlns="urn:ietf:params:xml:ns:yang:ietf‑ntp">
>>>>>>> as above
>>>>>>>
>>>>>>>             <refid>20.1.1.1</refid>
>>>>>>> as above
>>>>>>>
>>>>>>> 8.9
>>>>>>>         <ntp xmlns="urn:ietf:params:xml:ns:yang:ietf‑ntp">
>>>>>>> as above
>>>>>>>
>>>>>>
>>>>>> Updated in all cases.
>>>>>>
>>>>>>> 12.2
>>>>>>> this is an IANA‑maintained module and so the reference must be to the
>>>>>> No longer using iana‑crypt‑hash.
>>>>>>
>>>>>>> Tom Petch
>>>>>>>
>>>>>>>
>>>>>>> ‑‑‑‑‑ Original Message ‑‑‑‑‑
>>>>>>> From: "The IESG" <iesg‑[email-masked]>
>>>>>>> To: "IETF‑Announce" <ietf‑[email-masked]>
>>>>>>> Cc: <[email-masked]>; <ntp‑[email-masked]>; <[email-masked]>;
>>>>>>> <[email-masked]>; <draft‑ietf‑ntp‑yang‑data‑[email-masked]>
>>>>>>> Subject: Last Call: <draft‑ietf‑ntp‑yang‑data‑model‑10.txt> (A
>>>>>>> YANG Data
>>>>>> No longer using iana‑crypt‑hash, instead made this change to allow all
>>>>>> sort of key formats ‑
>>>>>>
>>>>>> OLD:
>>>>>> |     +‑‑rw key?         ianach:crypt‑hash
>>>>>> NEW:
>>>>>> |     +‑‑rw key
>>>>>> |     |  +‑‑rw (key‑string‑style)?
>>>>>> |     |     +‑‑:(keystring)
>>>>>> |     |     |  +‑‑rw keystring?            string
>>>>>> |     |     +‑‑:(hexadecimal) {hex‑key‑string}?
>>>>>> |     |        +‑‑rw hexadecimal‑string?   yang:hex‑string
>>>>>> END
>>>>>>
>>>>>> This is as per RFC 5905 ‑ "If using the IPv6 address family, it is the
>>>>>> No longer using iana‑crypt‑hash.
>>>>>>
>>>>>>> Section 4 is very keen that this I‑D and the system module which
>>>>>>> in this I‑D dependent on the absence of the presence container in
>>>>>>> /system/ntp.
>>>>>>>
>>>>>>
>>>>>> On Sat, Jan 30, 2021 at 4:09 AM The IESG <iesg‑[email-masked]>
>>>>>> wrote:
>>>>>>>
>>>>>>
>>> .
>>>
>

---

From: Danny Mayer <[email-masked]>
Date: Fri, 19 Feb 2021 09:16:00 -0500

Maybe I can help clarify the issue with MD5 hashing of an IPv6 address. 
The usage is for the Reference ID in the packet and the Reference ID is 
designed to prevent timing loops. For non-stratum-1 server references 
this was the IPv4 address of the chosen parent clock as chosen by the 
receiving client. IPv6 addresses don't fit into a 32-bit field so this 
hashing scheme was invented. (See section 7.3 of RFC5905). Unfortunately 
this has been totally misunderstood and does not deal with outgoing 
packets from the same server using different IP addresses. There is no 
proper way to fix this in the Ntpv4 packets, it will need to wait for a 
V5 version of the protocol. V5 would need to have each ntp server 
generate a random number that will then be including in all outgoing 
packets regardless of interfaces and IP addresses that the receiving 
server can use for it's referenceID in its own outgoing packets.

None of this is related to the security issues of MD5 hashes.

I hope this helps,

Danny

On 2/19/21 5:30 AM, tom petch wrote:
> -13 looks better
>
> Perhaps a comment for the AD.  You have added a reference for MD5 
> which I think necessary but as Informative which I find debatable 
> given its use to generate a 32-bit identifier from a IPv6 address, ie 
> its use is going to go up and up, not fade away.  I am aware that the 
> RFC is Informational not Standards Track so a Normative Reference 
> would be a downref which needs calling out in a Last Call which means 
> a new Last Call.  (Given the number of changes made since -10, perhaps 
> that is not such a left field idea:-)  The reference in RFC5905 is 
> Normative.  Like I say, one for the AD.
>
> increases the probability of incompatability between client and 
> server.  One is good, obsolescent and now recommended is good, but a 
> choice of eight without even including SHA3?  I note too that the 
> Netconf WG regard SHA1 as obsolete.
>
> Access mode I think deserves more text, a sub-section in s.5, not just 
> a mention.  Authors vary as to how much text there is in a YANG I-D 
> but I think that the better ones have more rather than less, bridging 
> the gap from protocol specification to YANG module, and, as you have 
> seen, I went looking in the RFC for an explanation, which was a waste 
> of time:-)  And I really do not understand the text.
> module is will send such requests? Or does it mean that the entity 
> will respond to such requests?  And how does that vary with server, 
> client, peer?  I cannot tell from the I-D.
>
> identity broadcast-client could do with mode 6 to bring it in line 
> with other identity
>
> s.8.5
> You slip in a version 3 which I guarantee will attract the attention 
> of an AD!.  You mentioned earlier that you intended the module to work 
> with V3 which I did not pick up from reading the I-D.  If that is your

---

From: "Salz, Rich" <[email-masked]>
Date: Fri, 19 Feb 2021 15:05:55 +0000

I thought a Yang model was supposed to be an on-the-wire representation of what the server did. Am I wrong? If I'm right, then the issue around MD5 is with the server, not this doc.

---

From: James Browning <[email-masked]>
Date: Fri, 19 Feb 2021 08:13:41 -0800

On Fri, Feb 19, 2021, at 7:06 AM Salz, Rich <rsalz=
[email-masked]> wrote:

> I thought a Yang model was supposed to be an on-the-wire representation of
> what the server did. Am I wrong? If I'm right, then the issue around MD5 is
> with the server, not this doc.
>

My (mis)understanding is that a server that has an IPv6 server peer will
send the four octets extracted from the MD5 hash of the peers' address in
the refid field of its responses.

---

From: tom petch <[email-masked]>
Date: Fri, 19 Feb 2021 16:34:45 +0000

> I thought a Yang model was supposed to be an on-the-wire representation of what the server did. Am I wrong? If I'm right, then the issue around MD5 is with the server, not this doc.

Rich

There are two issues with MD5 and NTP.

One is security, where a crypto-hash is used to authenticate NTPv4 
packets, and the hash specified in the NTPv4 base spec was MD5 but this 
was updated by RFC8573 so that MD5 is now deprecated.  I picked up on 
this in my first review, that the YANG model used MD5 and made no 
mention of its deprecation.  I knew that RFC8573 should be included but 
was unclear whether or not it would be acceptable to still include MD5. 
  Ben, Security AD, said yes, we should, and that is what we now have 
(along with a number of other hash).  My recent comment was that the 
Netconf WG label SHA1 as obsolete so should we include it? and what 
about such as SHA3?  The more options the greater a risk of mismatch but 
that is not an issue I am equipped to resolve, likely one for the IESG 
(much as I hate generating work for them).

The other MD5 usage is generating a 32-bit identifier with a good 
probability of being unique, for entities with IPv6 address, and that I 
see no problem with, as Ben confirmed.  That means that the I-D will 
reference the MD5 RFC, which is Informational and so potentially a 
downref.  Again, one for the AD to resolve (which is why I put it first 
on my previous post).

I shall follow the progress of this I-D through the IESG and will learn 
therefrom.

Tom Petch

---

From: Christian Huitema <[email-masked]>
Date: Fri, 19 Feb 2021 08:54:50 -0800

On 2/19/2021 8:34 AM, tom petch wrote:
>> I thought a Yang model was supposed to be an on-the-wire 
>> representation of what the server did. Am I wrong? If I'm right, then 
>> the issue around MD5 is with the server, not this doc.
>
> Rich
>
> packets, and the hash specified in the NTPv4 base spec was MD5 but 
> this was updated by RFC8573 so that MD5 is now deprecated.  I picked 
> up on this in my first review, that the YANG model used MD5 and made 
> no mention of its deprecation.  I knew that RFC8573 should be included 
> but was unclear whether or not it would be acceptable to still include 
> MD5.  Ben, Security AD, said yes, we should, and that is what we now 
> have (along with a number of other hash).  My recent comment was that 
> the Netconf WG label SHA1 as obsolete so should we include it? and 
> what about such as SHA3? The more options the greater a risk of 
> mismatch but that is not an issue I am equipped to resolve, likely one 
> for the IESG (much as I hate generating work for them).
>
> probability of being unique, for entities with IPv6 address, and that 
> I see no problem with, as Ben confirmed.  That means that the I-D will 
> downref.  Again, one for the AD to resolve (which is why I put it 
> first on my previous post).

There is another problem. Because MD5 is deprecated, some software 
development organizations have checks that prevent use of MD5. The test 
suites will include such checks, and the software will normally not ship 
if the checks detect presence of MD5 in the product. Of course there are 
ways around, but they are not quite as simple as replacing MD5 by 
SHA256, even for those "non crypto" cases.

-- Christian Huitema

---

From: "Salz, Rich" <[email-masked]>
Date: Fri, 19 Feb 2021 18:13:39 +0000

Thanks for those comments, but I don't think it addressed my question:  does the yang model JUST describe what the NTP server does?

If so, it seems all these issues are for the NTP server, not the yang model.

---

From: Martin Burnicki <[email-masked]>
Date: Mon, 22 Feb 2021 09:39:44 +0100

> SHA256, even for those "non crypto" cases.

Imaging SHA-1 wasn't supported anymore at all, how would you be able to 
track the history of commit hashes in a git repository, which are based 
on SHA-1?

So isn't it a really silly approach to stop supporting certain features 
in general just because they aren't appropriate anymore for certain tasks?

Martin
-- 
Martin Burnicki

Senior Software Engineer

MEINBERG Funkuhren GmbH & Co. KG
Email: [email-masked]
Phone: +49 5281 9309-414
Linkedin: https://www.linkedin.com/in/martinburnicki/

Lange Wand 9, 31812 Bad Pyrmont, Germany
Amtsgericht Hannover 17HRA 100322
Geschäftsführer/Managing Directors: Günter Meinberg, Werner Meinberg, 
Andre Hartmann, Heiko Gerstung
Websites: https://www.meinberg.de  https://www.meinbergglobal.com
Training: https://www.meinberg.academy

---

From: "Ulrich Windl" <[email-masked]>
Date: Tue, 23 Feb 2021 09:21:23 +0100

>>> Martin Burnicki <martin.burnicki=[email-masked]> schrieb am
22.02.2021 um 09:39 in Nachricht
> in general just because they aren't appropriate anymore for certain tasks?

I agree: There are even valid use cases for CRC-32.

---

From: tom petch <[email-masked]>
Date: Tue, 23 Feb 2021 11:26:27 +0000

> We have posted another update.

And yet another update (along with 5000 others, or so it seems, as the 
submission window closes:-)

I have been through it and it looks good; I think that I now understand 
access.

I note a 'synchornize' - perhaps 'synchronize'.  That apart, I think 
that I will not now come up with anything new; I got to the examples 
before I realised that the section numbers had changed and that I had 
not noticed that a new section had been inserted early on.  My eyes are 
seeing what they want to see and not what is there:-(

Tom Petch