Benjamin Kaduk <[email protected]> writes:
> On Fri, Aug 23, 2019 at 12:30:27AM +0000, Acee Lindem (acee) wrote:
>> Hi Ben,
>>
>> On 8/22/19, 1:46 AM, "Benjamin Kaduk via Datatracker" <[email protected]>
>> wrote:
>>
>> Benjamin Kaduk has entered the following ballot position for
>> draft-ietf-ospf-yang-26: Discuss
>>
>> When responding, please keep the subject line intact and reply to all
>> email addresses included in the To and CC lines. (Feel free to cut this
>> introductory paragraph, however.)
>>
>>
>> Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
>> for more information about IESG DISCUSS and COMMENT positions.
>>
>>
>> The document, along with other ballot positions, can be found here:
>> https://datatracker.ietf.org/doc/draft-ietf-ospf-yang/
>>
>>
>>
>> ----------------------------------------------------------------------
>> DISCUSS:
>> ----------------------------------------------------------------------
>>
>> (1) Can we check whether it's okay to use the yang "string" type for raw
>> cryptographic keys (e.g., ospfv2-key, ospfv3-key)? My understanding was
>> that yang strings were limited to human-readable, but that the crypto
>> keys could be raw binary values.
>>
>> It does only include human-readable keys. Here is the RFC 7950 YANG ABNF
>> definition.
>>
>>
>> ;; any Unicode or ISO/IEC 10646 character, including tab, carriage
>> ;; return, and line feed but excluding the other C0 control
>> ;; characters, the surrogate blocks, and the noncharacters
>> yang-char = %x09 / %x0A / %x0D / %x20-D7FF /
>> ; exclude surrogate blocks %xD800-DFFF
>> %xE000-FDCF / ; exclude noncharacters %xFDD0-FDEF
>> %xFDF0-FFFD / ; exclude noncharacters %xFFFE-FFFF
>> %x10000-1FFFD / ; exclude noncharacters %x1FFFE-1FFFF
>> %x20000-2FFFD / ; exclude noncharacters %x2FFFE-2FFFF
>> %x30000-3FFFD / ; exclude noncharacters %x3FFFE-3FFFF
>> %x40000-4FFFD / ; exclude noncharacters %x4FFFE-4FFFF
>> %x50000-5FFFD / ; exclude noncharacters %x5FFFE-5FFFF
>> %x60000-6FFFD / ; exclude noncharacters %x6FFFE-6FFFF
>> %x70000-7FFFD / ; exclude noncharacters %x7FFFE-7FFFF
>> %x80000-8FFFD / ; exclude noncharacters %x8FFFE-8FFFF
>> %x90000-9FFFD / ; exclude noncharacters %x9FFFE-9FFFF
>> %xA0000-AFFFD / ; exclude noncharacters %xAFFFE-AFFFF
>> %xB0000-BFFFD / ; exclude noncharacters %xBFFFE-BFFFF
>> %xC0000-CFFFD / ; exclude noncharacters %xCFFFE-CFFFF
>> %xD0000-DFFFD / ; exclude noncharacters %xDFFFE-DFFFF
>> %xE0000-EFFFD / ; exclude noncharacters %xEFFFE-EFFFF
>> %xF0000-FFFFD / ; exclude noncharacters %xFFFFE-FFFFF
>> %x100000-10FFFD ; exclude noncharacters %x10FFFE-10FFFF
>>
>> However, this is the intent as this is support for implementations that
>> don't support key-chains (RFC 8177). The "Security Considerations" recommend
>> key chains. We'll add the hexadecimal specification as another reason for
>> preferring key-chains.
>
> Okay, that's enough to resolve the discuss.
>
>>
>>
>> (2) Do we need to say anything about how to indicate when there are
>> discontinuities for the various "counter" types?
>>
>> I've never heard any mention of counter discontinuities in the context of
>> OSPF control plane counters.
>
> Oh, I think this is more of a YANG thing than an OSPF thing (and I'm hardly
> a YANG expert). That is, the yang:counter32 and similar types are
> constrained to only ever increment ("count events that happened", roughly),
> as opposed to a gauge32 that measures the current level of something and
> can go up or down, or a generic uint32. Of course, in the real world
> software crashes or restarts, so a YANG consumer might in practice see the
> value of a "counter" type go backwards, even though that's forbidden by the
> spec. In some cases (and I don't know exactly when it becomes most
In fact, such occassions are taken into account in the definition of the
yang:counter32 type [RFC6991]. The ietf-ospf also properly describes, for each
use of that type, the situations when a discontinuity may occur.
> relevant), the YANG model includes a "discontinuity time" (e.g., router
> restart time) to indicate when the counters were last reset to zero, in
> order to give consumers a sense for how long it took the counter to get
> that big. It's entirely possible that this doesn't make sense for the OSPF
> counters, and if you tell me that I'm happy to clear.
Another means for learning about discontinuities may be notifications - for
example, a discontinuity in nbr-event-count can probably be inferred from the
nbr-state-change notification.
Lada
>
> Searching for "RFC YANG discontinuity" brings up several representative
> results, such as RFC 8343, which says:
>
> leaf discontinuity-time {
> type yang:date-and-time;
> mandatory true;
> description
> "The time on the most recent occasion at which any one or
> more of this interface's counters suffered a
> discontinuity. If no such discontinuities have occurred
> since the last re-initialization of the local management
> subsystem, then this node contains the time the local
> management subsystem re-initialized itself.";
>
>
>>
>> ----------------------------------------------------------------------
>> COMMENT:
>> ----------------------------------------------------------------------
>>
>> I'm happy to see the discussion around Roman's Discuss.
>>
>> Section 1
>>
>> YANG [RFC6020][RFC7950] is a data definition language used to define
>> the contents of a conceptual data store that allows networked devices
>> to be managed using NETCONF [RFC6241]. YANG is proving relevant
>> beyond its initial confines, as bindings to other interfaces (e.g.,
>> ReST) and encodings other than XML (e.g., JSON) are being defined.
>>
>> This text feels a bit stale at this point.
>>
>> Updated in -27.
>>
>> Section 2
>>
>> model varies among router vendors. Differences are observed in terms
>> of how the protocol instance is tied to the routing domain, how
>> multiple protocol instances are be instantiated among others.
>>
>> nit: the grammar here is a bit odd, with the comma suggesting the start
>> of a list but no "and" present.
>>
>> Fixed in -27.
>>
>> Section 2.2
>>
>> The ospf module is intended to match to the vendor specific OSPF
>> configuration construct that is identified by the local identifier
>> 'name'.
>>
>> I don't really understand what this is intended to mean.
>>
>> I removed this paragraph and augmented the next one.
>
> Thanks; it makes a lot more sense to me now.
>
>> Section 2.7
>>
>> hello-timer in the module claims to be a rt-types:timer-value-seconds16
>> but shows up in the tree(s) as a uint32.
>> Similarly, the wait-itmer is a rt-types:timer-value-seconds32, which
>> also shows up in the tree(s) as a uint32, which is perhaps more
>> reasonable but perhaps not entirely accurate.
>> Other 'timer' leafs seem to have similar issues.
>>
>> Fixed.
>>
>> Section 3
>>
>> feature ospfv2-authentication-trailer {
>> description
>> "Use OSPFv2 authentication trailer for OSPFv2
>> authentication.";
>>
>> Is the feature for "use" or "support for"?
>> (Similarly for the ospfv3 authentication features.)
>>
>> The latter - fixed.
>>
>> identity ospfv2-lsa-option {
>> description
>> "Baes idenity for OSPFv2 LSA option flags.";
>>
>> nit: "Base"
>>
>> Fixed.
>>
>> identity v2-p-bit {
>> base ospfv2-lsa-option;
>> description
>> "Only used in type-7 LSA. When set, an NSSA
>> border router should translate the type-7 LSA
>> to a type-5 LSA.";
>>
>> There seem to be a few "identity <foo>-bit" stanzas whose description do
>> not mention the named bit specifically (but many that do). Do we want
>> to be consistent about doing so? (Likewise for <foo>-flag.)
>>
>> Made these consistent. This was added very late when we realized that YANG
>> type bit is not extendable.
>
> The YANG type bit is not extendable?! I'll have to remebmer that for the
> future...
>
>> grouping tlv {
>> description
>> "Type-Length-Value (TLV)";
>> leaf type {
>> type uint16;
>> description "TLV type.";
>> }
>> leaf length {
>> type uint16;
>> description "TLV length (octets).";
>> }
>> leaf value {
>> type yang:hex-string;
>> description "TLV value.";
>>
>> Is there a way to apply a constraint so the 'length' matches the
>> hext-string's length?
>>
>> Since this is returned operational data, it doesn't make a lot of sense to
>> put constraints on it. It is basically what the router returns. If it is
>> wrong, it wouldn't have been parsed correctly in the first place.
>
> Ah, good point.
>
>> grouping router-capabilities-tlv {
>>
>> The various descriptions hereunder could perhaps benefit from section
>> references, as, e.g., two nodes named "informational-capabilities" may
>> otherwise require some effort to distinguish. Well, aside from the fact
>> that one is currently listed as "capabilitiess" with two esses, which
>> seems unlikely to be intended.
>>
>> I fixed the description on these to differential between the list uint32
>> flags and the identities.
>
> Thanks (BTW please double-check the description for "leaf functional-flag",
> that currenty looks like "Individual informational capability flag.")
>
>> grouping ospf-router-lsa-bits {
>> container rputer-bits {
>>
>> s/rputer/router/?
>>
>> Fixed.
>>
>> container te-opaque {
>> [...]
>> container link-tlv {
>> description "Describes a singel link, and it is constructed
>> of a set of Sub-TLVs.";
>>
>> s/singel/single/
>>
>> Fixed.
>>
>> grouping ospfv3-lsa-external {
>> [...]
>> leaf referenced-link-state-id {
>> type yang:dotted-quad;
>>
>> RFC 5340 section 2.2 implies that the Link State ID is going to be a
>> 32-bit identifier that need not be represented as dotted-quad, as it
>> does not have addressing semantics. (dotted-quad seems to be used for
>> Link-State-ID-shaped things elsewhere, too, though the preferred form
>> may be the union of dotted-quad and uint32.)
>>
>> Good catch. This should be unit32 for OSPFv3. There were two instances of
>> this.
>>
>> grouping lsa-common {
>> description
>> "Common fields for OSPF LSA representation.";
>> leaf decode-completed {
>> type boolean;
>> description
>> "The OSPF LSA body was successfully decoded other than
>> unknown TLVs. Unknown LSAs types and OSPFv2 unknown
>> opaque LSA types are not decoded. Additionally,
>> malformed LSAs are generally not accepted and are
>> not be in the Link State Database.";
>>
>> nit: "are not be" is not grammatical.
>>
>> Fixed - "will not be".
>>
>> grouping lsa-key {
>> description
>> "OSPF LSA key.";
>>
>> This could maybe benefit from a more descriptive description; is this a
>> sort or lookup key, for example?
>>
>> Yes - I've improved.
>>
>> container database {
>> description "Container for per AS-scope LSA statistics.";
>> list as-scope-lsa-type {
>> [...]
>> leaf lsa-cksum-sum {
>> type uint32;
>> description
>> "The sum of the LSA checksums of the LSA type.";
>>
>> [It's not entirely clear to me why this sum-of-checksums is a useful
>> thing to track, but it may not be this document's role to do so.
>> Though, perhaps we do need to say if the sum is computed as integers
>> modulo 2**32.]
>>
>> This is from the OSPF MIB (RFC 4750). It is not used functionally as one
>> cannot guarantee differing LSDBs will not yield the same checksum. However,
>> if the checksums are different, you can assure the databases are different.
>
> Thanks for the extra text -- it helps confirm my guess as to what's going
> on.
>
>> leaf transmit-delay {
>> type uint16;
>> units seconds;
>> description
>> "Estimated time needed to transmit Link State Update
>> (LSU) packets on the interface (seconds). LSAs have
>> their age incremented by this amount on advertised
>> on the interface. A sample value would be 1 second.";
>>
>> nit: "on advertised on" does not seem grammatical.
>>
>> Fixed - "when advertised on".
>>
>> leaf lls {
>> [...]
>> container ttl-security {
>>
>> Should these have a default value?
>>
>> I think 1 would be appropriate since only virtual-links and sham-links
>> require more.
>>
>> case ospfv3-auth-ipsec {
>> when "derived-from-or-self(../../../../../../rt:type, "
>> + "'ospfv3')" {
>> description "Applied to OSPFv3 only.";
>> }
>> if-feature ospfv3-authentication-ipsec;
>> leaf sa {
>> type string;
>>
>> I don't see RFC 4552 talking about names for SAs; where would this be
>> discussed (and, are they guaranteed to be human-readable)?
>>
>> If it is not human readable, how would it be configured?
>
> I wasn't sure if this was config or operational state, but you make a good
> point.
>
>> leaf poll-interval {
>> type uint16;
>> units seconds;
>> description
>> "Neighbor poll interval (seconds) for sending OSPF
>> hello packets to discover the neighbor on NBMA
>> networks. This interval dictates the granularity for
>> discovery of new neighbors. A sample would be 2 minutes
>> for a legacy Packet Data Network (PDN) X.25 network.";
>>
>> Maybe "2 minutes (120 seconds)" since the units are seconds?
>>
>> Fixed.
>>
>> container preference {
>> description
>> "Route preference configuration In many
>> implementations, preference is referred to as
>> administrative distance.";
>>
>> nit: missing sentence break?
>>
>> Fixed.
>>
>> For the spf- and lsa-logs, do we require that the 'id' is assigned in
>> any particular order, or do we just rely on the included timestamp(s)
>> for any time-ordering required by the consumer?
>>
>> I added ordering to the description of the spf-log and lsa-log. However, the
>> id is a purely internal value to provide a uniqueness key for the YANG list.
>
> Understood, and thanks.
>
>> grouping notification-neighbor {
>> [...]
>> leaf neighbor-ip-addr {
>> type yang:dotted-quad;
>> description "Neighbor address.";
>>
>> neighbors can only be identified by IPv4 addresses?
>>
>> Changed to IP address type.
>>
>> leaf packet-source {
>> type yang:dotted-quad;
>>
>> packet sources, too? (multiple times)
>>
>> Changed to IP address type.
>>
>> description
>> "This notification is sent when aa neighbor
>> state change is detected.";
>>
>> nit: s/aa/a/
>>
>> Fixed.
>>
>> Section 4
>>
>> Additionally, local specificationn of OSPF authentication keys and
>> the associated authentication algorithm is supported for legacy
>> implementations that do not support key-chains [RFC8177] for legacy
>> implementations that do not support key-chains. It is RECOMMENDED
>> that implementations migrate to key-chains due the seamless support
>> of key and algorithm rollover, as well as, the encryption of key
>> using the Advanced Encryption Standard (AES) Key Wrap Padding
>> Algorithm [RFC5649].
>>
>> (Roman caught the nits, so I won't duplicate that.)
>> I expected to see something about keeping the actual key material
>> secret, as well.
>>
>> Can you suggest some text?
>
> How's this?
>
> The actual authentication key data (whether locally specified or part of a
> key-chain) is sensitive and needs to be kept secret from unauthorized
> parties; compromise of the key data would allow an attacker to forge OSPF
> traffic that would be accepted as authentic, potentially compromising the
> entirey OSPF domain.
>
> Thanks,
>
> Ben
>
> _______________________________________________
> Lsr mailing list
> [email protected]
> https://www.ietf.org/mailman/listinfo/lsr
--
Ladislav Lhotka
Head, CZ.NIC Labs
PGP Key ID: 0xB8F92B08A9F76C67
_______________________________________________
Lsr mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/lsr
