Hi, I agree with Juergen. The protocol has a "too-big" error is the server will not accept a big string. There should not be a false choice between an arbitrary maximum and "terabytes".
You cannot use a range-stmt to specify the minimum required length that must be supported. length "3..max" does not allow strings of length 0 - 2. The description-stmt can have "A server MUST accept a string up to 64 characters in length" which lets the client choose a length from 0 to 64, and this will work on all server implementations. Andy On Wed, Jan 9, 2019 at 5:10 AM Juergen Schoenwaelder < j.schoenwael...@jacobs-university.de> wrote: > Hi, > > please see my other email about the distinction I make between a hard > length restriction and the minimum length expected to be supported. I > wonder how you can sensibly pick a limit for things like non-best-reason: > > leaf non-best-reason { > type string; > description > "Information field to describe why the alternate > is not best."; > } > > You are simply creating an arbitrary restriction. And humble server is > not likely to send you an jpg image (and a bogus server will do so > anyway). (There are other similar objects.) > > Since I am searching for 'type string', I wonder whether these are > clear enough definitions. > > leaf prefix { > type string; > description > "Protected prefix."; > } > leaf alternate { > type string; > description > "Alternate nexthop for the prefix."; > } > > What is the (canonical) format of the allowed values? (There are more of > these.) > > /js > > On Wed, Jan 09, 2019 at 12:52:07PM +0000, stephane.litkow...@orange.com > wrote: > > Hi Tom, > > > > If you agree, I will set a length restriction on each string (ops and > cfg). It looks clearer for me rather than setting it in the description. > > > > For the references, I'm working on it. > > > > Brgds, > > > > > > -----Original Message----- > > From: tom petch [mailto:ie...@btconnect.com] > > Sent: Wednesday, January 09, 2019 12:38 > > To: LITKOWSKI Stephane OBS/OINIS; Ebben Aries; yang-doct...@ietf.org > > Cc: draft-ietf-isis-yang-isis-cfg....@ietf.org; lsr@ietf.org > > Subject: Re: [Lsr] Yangdoctors last call review of > draft-ietf-isis-yang-is-is-cfg-29 > > > > Stephane > > > > Thanks for persisting. > > > > On string lengths, I take your point about no user input to Operational > > State; there, my concern is more about giving guidance to implementors > > as to what they should cater for - as I said, this has been a topic of > > lively discussion in other WG. Even before that, whenever I see a > > string, I think is there an implicit length restriction and if not, > > should there be an explicit one which, as Juergen suggested, could be in > > the description clause. My experience is that those working with > > networks think about size, about length; those coming from applications > > tend to think 'What is a few terabytes between friends?' and are unaware > > that sizes that may be commonplace in servers and associated storage can > > create difficulties over a network. Whatever, I leave this one up to > > you. > > > > On references, I would like a change; you say this information is in the > > base ISO spec. Well, yes, to me that means that it should be a > > Normative Reference. I could not understand the description of e.g. > > 'i/e' and needed to look it up but seemingly cannot do so with the > > listed references of the I-D. Note that RFC such as RFC5305 and RFC6119 > > do reference International Standard 10589 and I think that this one > > should too, perhaps in s.2.7 and s.5. > > > > Tom Petch > > > > ----- Original Message ----- > > From: <stephane.litkow...@orange.com> > > Sent: Wednesday, January 09, 2019 9:18 AM > > > > Hi Tom, > > > > Please find inline answers. > > > > > > -----Original Message----- > > From: tom petch [mailto:ie...@btconnect.com] > > Sent: Tuesday, January 08, 2019 18:45 > > > > Ok. Top-posting the ones that are not 'ok': > > > > I said that I thought that LSP did not need expanding on first use and > > then checked the RFC Editor list to find that it is not one they regard > > as well-known and that LSR protocols use it differently to others, so I > > suggest expanding LSP on first use. > > > > [SLI] Already done for the next version. > > > > On lengths of text messages, perhaps I am too sensitive to buffer > > overrun attacks and the like and so want a maximum on many things (and > > then people attach a friendly, 20Mbyte photo to their e-mail at > > Christmas and > > wonder why their ESP rejects the message so I do not congratulate them > > on the latest addition to the family:-). The IDR WG had a lively > > discussion about maximum message lengths in 2017 and that has also > > stayed in my mind. I have seen the comments on this by Juergen and > > Lada; perhaps as Juergen intimates, something in the Description would > > help; and while the server may not be rogue, it may not have a perfect > > implementation. > > > > [SLI] What I need to understand from your comment on string length > > enforcement is if it applies to operational state or just config states > > ? I do not see any issue of not enforcing the operational state as there > > is no input from the user there and so no attack vector, this is purely > > internal to the implementation. For config statements, it makes sense as > > there is an input from the user that can be anything. > > > > > > On the length of password, I saw a Security AD wanting clarification on > > this not too long ago so you may see this comment again from one such . > > Likewise, MD5 tends to be a red flag although I see it appears in bgp > > yang. > > > > I like the sort of detail in ippm-twamp-yang, on algorithms, entropy and > > such like (although I have not seen a review by Security AD/directorate > > of that). > > > > But I am left confused as to exactly what the cited object is doing. > > Yes, TLV10 provides authentication for any PDU, but what are the fields > > in the YANG module doing? Is > > leaf authentication-type { > > the first octet of TLV10? Is > > leaf authentication-key { > > the rest of TLV10? And where is this 'presented' as the YANG module > > says? Are you thinking of a YANG client presenting the field to a user > > at a terminal, one router presenting it to another, or what? > > > > I am using RFC5310 as my source for TLV10 and wondering why that is not > > a Normative Reference for this I-D > > > > [SLI] TLV 10 is defined in the base ISO spec of IS-IS. RFC5310 just adds > > the crypto auth as new types. > > The authentication-type is the first byte of the TLV (called > > Authentication Type as well). > > The authentication-key cannot be mapped directly to the > > authentication-value field. This is the case for ClearText password > > authtype but not crypto which adds a keyID in front of the > > authentication data. > > > > > > On the I/E bit, the question is, which standard? I have 30 Normative > > references to choose from. I found up/down in RFC5305, but only by > > accident, and I have not found i/e yet so a reference would be good. > > > > [SLI] I/E bit is part of the base ISO spec of IS-IS and relevant for > > "legacy advertisements" of prefix and links. By the way, after checking, > > the position of the i-e leaf is currently wrong and needs to be within > > the metric (default, delay...). I will fix this. > > > > > > Tom Petch > > > > ----- Original Message ----- > > From: stephane.litkow...@orange.com > > Sent: Monday, January 07, 2019 9:22 AM > > > > Hi Tom, > > > > Thanks for your comments. > > I wish you an happy new year ! > > > > Please find inline comments. > > > > Brgds, > > > > -----Original Message----- > > From: tom petch [mailto:ie...@btconnect.com] > > Sent: Wednesday, January 02, 2019 12:17 > > > > Here are the rest of my comments on -29 with a slight tweak to the > > subject line. I would regard most of these (but not the first two) as > > non-discussable, ie I won't complain if you disagree:-) > > > > RFC1195 is in the YANG module but not the references of the I-D > > [SLI] Will fix it > > > > RFC5029 is in the YANG module but not the references of the I-D > > [SLI] Will fix it > > > > PSNPs, CSNPs > > [SLI] Will fix it > > > > expand on first use - LSP I think ok > > > > leaf best { type boolean; > > what is true and what false? I can guess from the English semantics of > > the name but would rather not guess. > > [SLI] Will fix it > > To replace the current description which is : ""Indicates if the > > alternate is the preferred."", do you prefer: "Set to true when the > > alternate is preferred, set to false otherwise" ? > > > > > > leaf non-best-reason { type string; > > suggest a maximum length, perhaps 127 or 255 ( unless you expect > > screenshots or packet traces to be attached). As it stands, you could > > validly receive > > a length of 18446744073709551615. > > [SLI] Agree, will fix it > > > > You have a mixture of > > System-id system-id System id System ID System Id system id system ID > > suggest consistency; system-id wfm > > [SLI] Will fix it > > > > You have a mixture of > > lsp-id LSPID LSP ID > > here, perhaps lsp-id for the names and LSP ID in the text > > [SLI] Will fix it > > > > case password { leaf key { type string; > > perhaps better with a minimum length > > [SLI] I agree that it could make sense but is it really something that > > we should impose ? > > > > > > leaf i-e { type boolean; > > what is true and what false? here I am reluctant even to guess > > [SLI] This is coming from the standard, is it really worth repeating it > > ? Same for up/down bit. > > > > > > /"Authentication keyto/ "Authentication key to/ > > > > " the authentication key MUST NOT be presented in" > > RFC2119 language means that RFC2119 boilerplate should be in the YANG > > module (but without the [..] ie the reference must be plain text not an > > anchor). > > > > [SLI] You are right, it is missing. > > > > > > It is recommended to use an MD5 > > hash to present the authentication-key."; > > Mmm I think that this may be a red flag to security AD or directorate as > > being too vague as well as MD5 too weak; and I think this should be > > explicitly called out in Security Considerations. > > > > [SLI] I agree that there is a point to discuss here. The fact is that we > > must not retrieve passwords in clear text. Maybe it is something with a > > wider scope than IS-IS. How do the other models deal with passwords > > retrieved through "get" or "get-config" ? > > > > > > list level-db { key level; leaf level { > > A common convention is for a list of leaf thing to be named things i.e. > > list levels { key level; leaf level { > > > > [SLI] ack > > > > rpc clear-adjacency { > > "Name of the IS-IS protocol instance whose IS-IS > > information is being queried. > > queried or cleared? > > [SLI] "cleared" > > > > Tom Petch > > > > > > ----- Original Message ----- > > From: "tom petch" <ie...@btconnect.com> > > Sent: Monday, December 31, 2018 6:21 PM > > > > > > > Stephane > > > > > > A new and different comment. > > > > > > grouping neighbor-gmpls-extensions { > > > > > > has a text reference to RFC5307 which does not appear in the > > references > > > for the I-D. However, before adding it, I would like to know why it > > is > > > a good reference for switching capabilities (which is part of this > > > grouping). I think that the reference for switching capabilities > > should > > > be RFC7074 (which this I-D does not currently reference and should > > IMO). > > > > > > And that begs the question, why is switching-capability an > > unrestricted > > > uint8 when only 12 values are valid and three are deprecated? > > > > > > So why not use > > > > > > draft-ietf-teas-yang-te-types? > > > > > > I have a number of additional comments on cfg-29 but this is the one > > > that may take some discussion. > > > > > > Tom Petch > > > > > > > > > ----- Original Message ----- > > > From: <stephane.litkow...@orange.com> > > > > > > Hi Tom, > > > > > > Thanks for your comments. I will fix them asap. > > > Regarding: > > > " Line length is within the RFC limit but the effect is to spread many > > > of the description clauses over multiple lines with indentation of 56 > > > characters, not user friendly e.g. > > > description > > > "List of max LSP > > > bandwidths for different > > > priorities."; > > > " > > > What's your suggestion on this one ? > > > > > > Brgds, > > > > > > -----Original Message----- > > > From: tom petch [mailto:ie...@btconnect.com] > > > Sent: Tuesday, November 27, 2018 12:11 > > > Subject: Re: [Lsr] Yangdoctors last call review of > > > draft-ietf-isis-yang-isis-cfg-24 > > > > > > Some quirks in-25 > > > > > > I see lots of YANG reference statements - good - but no mention of > > them > > > in the I-D references - not so good. My list is > > > > > > 5130 > > > 5305 > > > 5306 > > > 5880 > > > 5881 > > > 6119 > > > 6232 > > > 7794 > > > 7810 > > > 7917 > > > 8405 > > > > > > Also perhaps > > > OLD > > > reference "RFC XXXX - YANG Data Model for Bidirectional > > > Forwarding Detection (BFD).Please replace YYYY with > > > published RFC > > > number for draft-ietf-bfd-yang."; > > > > > > NEW > > > reference "RFC YYYY - YANG Data Model for Bidirectional > > > Forwarding Detection (BFD). > > > > > > -- Note to RFC Editor Please replace YYYY with published RFC > > > number for draft-ietf-bfd-yang."; > > > > > > OLD > > > reference "draft-ietf-bfd-yang-xx.txt: > > > YANG Data Model for Bidirectional Forwarding > > > Detection (BFD)"; > > > NEW > > > reference "RFC YYYY - YANG Data Model for Bidirectional > > > Forwarding Detection (BFD). > > > > > > -- Note to RFC Editor Please replace YYYY with published RFC > > > number for draft-ietf-bfd-yang."; > > > > > > > > > Line length is within the RFC limit but the effect is to spread many > > of > > > the description clauses over multiple lines with indentation of 56 > > > characters, not user friendly > > > e.g. > > > description > > > "List of max LSP > > > bandwidths for different > > > priorities."; > > > > > > > > > Acknowledgements is TBD. I note that the editor list of the YANG > > module > > > is somewhat longer than the editor list of the I-D. > > > > > > I note that the augmentation of interfaces seems to have no > > conditional > > > and so will augment all interfaces. I think that this is a generic > > issue > > > but do not see it being addressed anywhere. > > > > > > In a similar vein, you are defining MPLS objects and I am unclear > > > whether or not those should be conditional, or part of the MPLS YANG > > > modules or both (copying Tarek for this) > > > > > > Tom Petch > > > > > > ----- Original Message ----- > > > From: "Ebben Aries" <e...@juniper.net> > > > Sent: Tuesday, October 23, 2018 12:45 AM > > > > > > > Reviewer: Ebben Aries > > > > Review result: On the Right Track > > > > > > > > 1 module in this draft: > > > > - ietf-i...@2018-08-09.yang > > > > > > > > No YANG compiler errors or warnings (from pyang 1.7.5 and yanglint > > > 0.16.54) > > > > > > > > "ietf-isis@2018-08-09" module is compatible with the NMDA > > > architecture. > > > > > > > > Module ietf-i...@2018-08-09.yang: > > > > - Both the description and the draft name reference that this module > > > is > > > > specific to configuration but contains operational state nodes in > > > addition > > > > to RPCs and notifications. Any wording suggesting this is only > > > > configuration should be changed > > > > - Module description must contain most recent copyright notice per > > > > > > > > > https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-20#section-3.1 > > > > - Module description reads "common across all of the vendor > > > implementations". > > > > I don't think this needs to be called out as such as that is the > > > overall > > > > intention of *all* IETF models > > > > - This module contains '22' features (and the respective OSPF module > > > currently > > > > contains '26'). While it is understood the purpose of these > > > features in the > > > > module, take precaution as to complexity for clients if they need > > to > > > > understand >= quantity of features per module in use on a > > > > network-element. We are going to end up w/ feature explosion to > > > convey > > > > *all* possible features of each network-element leading to > > > divergence back > > > > towards native models at the end of the day. A large amount of > > > these > > > > feature names could be defined within a more global namespace > > (e.g. > > > nsr) but > > > > this gives us a granular yet cumbersome approach (e.g. feature > > > isis:nsr, > > > > ospf:nsr, etc..) > > > > - RPC 'clear-adjacency' does not have any input leaf that covers > > > clearing a > > > > specific neighbor/adjacency (See comments below as well regarding > > > RPC > > > > alignment w/ the OSPF model) > > > > - RPC 'clear-adjacency' has an input node of 'interface' however > > this > > > is just > > > > a string type. Is there any reason this is not a > > > leafref/if:interface-ref > > > > (much like in the OSPF model) > > > > - Child nodes within a container or list SHOULD NOT replicate the > > > parent > > > > identifier per > > > > > > > > > https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-20#section-4.3. > > > 1. > > > > > > > > A case in point is the list /afs/af that has a leaf of 'af' > > > > <afs> > > > > <af> > > > > <af>ipv4</af> > > > > <enable>true</enable> > > > > </af> > > > > </afs> > > > > > > > > Not only is this replication, but we should likely not abbreviate > > > 'afs' if > > > > we are using the expanded 'address-family' in other IETF models > > such > > > as > > > > ietf-i2rs-rib > > > > > > > > > > > > General comments on the draft + nits: > > > > - Since YANG tree diagrams are used, please include an informative > > > reference > > > > per > > > > > https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-20#section-3.4 > > > > - Section 1.1 does not need to exist since this would be covered by > > > the > > > > reference mentioned above > > > > - Reference to NMDA compliance should be contained within Section 1 > > > (vs. > > > > Section 2) per > > > > > > > > > https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-20#section-3.5 > > > > - Section 2: It seems reference should be given to the location of > > > where the > > > > ietf-routing module is defined (As well as reference to NMDA RFC > > in > > > the > > > > above reference) > > > > - Section 2.1: "Additional modules may be created this to > > support..." > > > needs > > > > slight rewording adjustment > > > > - Section 3: The RPC operations are named 'clear-adjacency' and > > > > 'clear-database' rather w/ reliance off namespacing for > > uniqueness. > > > This > > > > section refers to 'clear-isis-database' and 'clear-isis-adjacency' > > > > - Section 4: Notification name mismatch in this section from actual > > > naming > > > > within the module (e.g. 'adjacency-change' should rather be > > > > 'adjacency-state-change') > > > > - Section 7: Security Considerations will need updating to be > > > patterned after > > > > the latest version of the template at > > > > https://trac.ietf.org/trac/ops/wiki/yang-security-guidelines per > > > > > > > > > https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-20#section-3.7 > > > > - Section 12: All modules imported within this module MUST be > > > referenced > > > > within this section per > > > > > > > > > https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-20#section-3.9. > > > > There are quite a few missing from this section right now > > > > - Appendix A: Some of the XML elements are off in alignment > > > > - Appendix A: Examples must be validated. The example given has the > > > following > > > > issues: > > > > - /routing[name='SLI'] and /routing/description are invalid data > > > nodes and > > > > do not exist. I'm not sure why they are in the XML example here > > > > - The example is meant to reference configuration however > > > > /routing/interfaces is a r/o container > > > > - The control-plane-protocol 'type' needs to be qualified - e.g. > > > > <type > > > xmlns:isis="urn:ietf:params:xml:ns:yang:ietf-isis">isis:isis</type> > > > > - The area-address does not validate against the pattern regex and > > > must end > > > > with a '.' e.g. > > > > <area-address>49.0001.0000.</area-address> > > > > - metric-type/value is set to 'wide' which is invalid. This > > should > > > rather > > > > be 'wide-only' > > > > - isis/afs/af/af is set to 'ipv4-unicast' which is invalid. This > > > should > > > > rather be 'ipv4' per iana-routing-types > > > > - /interfaces/interface/type must be populated and is invalid. > > This > > > should > > > > rather be qualified as such: > > > > <type > > > > > xmlns:ianaift="urn:ietf:params:xml:ns:yang:iana-if-type">ianaift:softwar > > > eLoopback</type> > > > > <type > > > > > xmlns:ianaift="urn:ietf:params:xml:ns:yang:iana-if-type">ianaift:etherne > > > tCsmacd</type> > > > > - /interfaces/interface/link-up-down-trap-enable must have a value > > > > associated as such: > > > > <link-up-down-trap-enable>enabled</link-up-down-trap-enable> > > > > - NP container 'priority' has a must statement checking if an > > > interface-type > > > > is set to 'broadcast' however if you take the XML example from > > > this > > > > section, it will fail to validate even if <priority> is not > > > defined > > > > underneath an interface-type of 'point-to-point'. It seems to > > me > > > that > > > > this logic may need to be readjusted or not exist at all > > (priority > > > can > > > > still be set on implementations on loopback interfaces - which > > > would > > > > default to 'broadcast' in the example here). Could you not > > solve > > > this > > > > with use of 'when' vs. 'must' as such: > > > > > > > > when '../interface-type = "broadcast"' { > > > > description "Priority can only be set for broadcast > > > interfaces."; > > > > } > > > > > > > > > > > > > https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-20#section-4.18 > > > ..2. > > > > > > > > - /interfaces/interface/ipv4/mtu must contain a valid value (and > > > likely not > > > > need to be defined for Loopback0) > > > > - 'isis/mpls-te/ipv4-router-id' is invalid and should rather be > > > > 'isis/mpls/te-rid/ipv4-router-id' > > > > - 'isis/afs/af/enabled' is invalid and should rather be > > > 'isis/afs/af/enable' > > > > - Examples should use IPv6 addresses where appropriate per > > > > > > > > > https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-20#section-3.12 > > > > > > > > ________________________________________________________________________ > > > _________________________________________________ > > > > > > > _________________________________________________________________________________________________________________________ > > > > Ce message et ses pieces jointes peuvent contenir des informations > confidentielles ou privilegiees et ne doivent donc > > pas etre diffuses, exploites ou copies sans autorisation. Si vous avez > recu ce message par erreur, veuillez le signaler > > a l'expediteur et le detruire ainsi que les pieces jointes. Les messages > electroniques etant susceptibles d'alteration, > > Orange decline toute responsabilite si ce message a ete altere, deforme > ou falsifie. Merci. > > > > This message and its attachments may contain confidential or privileged > information that may be protected by law; > > they should not be distributed, used or copied without authorisation. > > If you have received this email in error, please notify the sender and > delete this message and its attachments. > > As emails may be altered, Orange is not liable for messages that have > been modified, changed or falsified. > > Thank you. > > > > _______________________________________________ > > yang-doctors mailing list > > yang-doct...@ietf.org > > https://www.ietf.org/mailman/listinfo/yang-doctors > > -- > Juergen Schoenwaelder Jacobs University Bremen gGmbH > Phone: +49 421 200 3587 Campus Ring 1 | 28759 Bremen | Germany > Fax: +49 421 200 3103 <https://www.jacobs-university.de/> > > _______________________________________________ > yang-doctors mailing list > yang-doct...@ietf.org > https://www.ietf.org/mailman/listinfo/yang-doctors >
_______________________________________________ Lsr mailing list Lsr@ietf.org https://www.ietf.org/mailman/listinfo/lsr