Subject: Re: [dhcwg] Benjamin Kaduk's Discuss on draft-ietf-dhc-dhcpv6-yang-24: (with DISCUSS and COMMENT)

From: [email-masked]
Date: Thu, 3 Mar 2022 14:39:33 +0100

Hi Benjamin,

Thanks for your comments. Please see inline below.

BR,
Ian

> On 25. Feb 2022, at 23:18, Benjamin Kaduk <[email-masked]> wrote:
> 
> Hi Ian,
> 
> As promised in my reply on the DISCUSS thread, a few remaining comments
> interspersed here.  I'll trim heavily since the information density would
> be quite low otherwise.
> 
> On Mon, Jan 17, 2022 at 06:39:56PM +0100, [email-masked] wrote:
>> 
>>> On 15. Dec 2021, at 01:07, Benjamin Kaduk via Datatracker <[email-masked]> wrote:
>>> 
>>> 
>>>              leaf pool-prefix {
>>>                type inet:ipv6-prefix;
>>>                mandatory true;
>>>                description
>>>                  "IPv6 prefix for the pool.";
>>> 
>>> Does this prefix need to be contained within the overall
>>> "network-prefix"?  (Same question for prefix-pools' pool-prefix.)
>> 
>> [if - With the DHCP server implementations that I am familiar with, yes it would
>> need to be contained, but I guess other implementations may not. I’m not aware 
>> of any way of enforcing this in the YANG.]
> 
> I'm not aware of any way to enforce it via YANG either, but perhaps we
> could put a few more words in the description string that covers the
> expected relationship in prose.

[if - I’ve changed the prefix-pool description (for addresses and prefixes) to:
                                         
                "IPv6 prefix for the pool. Should be contained within   
                the network-prefix, if configured.”;
]     

> 
>>> 
>>>                list active-lease {
>>>                  key leased-prefix;
>>>                  description
>>>                    "List of active prefix leases.";
>>>                  leaf leased-prefix {
>>>                    type inet:ipv6-prefix;
>>>                    description
>>>                      "Active leased prefix entry.";
>>> 
>>> Is the prefix length available from some other information in the tree?
>>> If not, should it be listed here?
>> 
>> [if I’m not sure I understand the comment. The prefix pool has the 
>> ‘Client-prefix-length’ leaf which defines the length of prefixes that
>> are offered for the pd-pool.
> 
> I think the "client-prefix-length" leaf is what I was looking for, thanks.
> 
>>> Section 5
>>> 
>>> It's probably worth mentioning the risk of notifications turning into a
>>> DoS vector (absent rate-limiting) here.
>> 
>> [if - This is surely a problem with Netconf notifications overall, rather than specifically
>> For the DHCP modules? I’ve checked through RFC5277 (Netconf event notifications) and
>> There is nothing on rate-limiting messages or in the Security Considerations section
>> on this subject.]
> 
> It would be nice if 5277 had said more on this topic, sure.
> I think what's interesting to me here, specifically, is that there is a
> layer of separation between where the (hypothetical) attacker sits and the
> DoS-via-notifications channel.  That is, because this module exists,
> there's potential for an attacker that can only send DHCPv6 traffic to
> induce new traffic to be generated (as YANG notifications) fromm the DHCPv6
> server to the recipient of the notifications.  In the scenario in question,
> the frequency of notification traffic is controlled by the attacker's
> DHCPv6 traffic rather than by the DHCPv6 server itself.  The new security
> consideration is the leverage for turning DHCPv6 traffic into a different
> kind of traffic.  Does that make sense?  I don't really expect this to be a
> useful DoS vector in practice, but I've been wrong before, and the cost of
> documenting the risk seems pretty low.

[if - I have added the following paragraph to the Security Considerations:

        An attacker sending DHCPv6 messages which cause the server to   
        generate 'invalid-client-detected' and 'decline-received'       
        notifications could be used as a DoS attack. Such an attack     
        could be mitigated by the NETCONF client unsubscribing          
        from the affected notifications.
]

> 
>>> 
>>>  [RFC7824] covers privacy considerations for DHCPv6 and is applicable
>>>  here.
>>> 
>>> I'd suggest mentioning the RFC 7844 privacy profile as a partial
>>> mitigation that is sometimes applicable.
>> 
>> [if - Do you mean that RFC7844 can provide partial mitigation of the server
>> Privacy issues in RFC7824?]
> 
> On closer look, it doesn't look to actually be the case that RFC7844
> provides any means for mitigating server privacy issues, just client
> privacy issues.  But since we provide YANG models for DHCPv6 client
> behavior here, wouldn't even that limited functionality still be relevant?

[if - I’ve added the following text to the Security Considerations:

        [RFC7844] describes anonyimty profiles for      
        DHCP clients. These can be used to prevent client tracking      
        on a visited network. Support for this can be enabled by        
        implementing the 'anon-profile' feature in the client           
        module.
]

> 
> [now in the "NITS" portion]
>>>            case mysql {
>>>              leaf mysql-name {
>>>                type string;
>>>                description
>>>                  "Name of the database.";
>>>              }
>>>              [...]
>>> 
>>> This seems to not have provision for configuring mandatory TLS usage for
>>> connection to the mysql server.
>>> (Same for the postgres case.)
>> 
>> [if - I’ve just found draft-ietf-netconf-tls-client-server-26 which covers this and
>> Would seem to be the right way to do this.
>> 
>> However, importing this module and its dependencies and using the tis-client-grouping for 
>> Mysql and Postgres results in the tree diagram growing from 49 lines to
>> 401 with the TLS configuration, which somewhat detracts from the point of the example.
>> 
>> Given that this is provided as an example module for a theoretical implementation,
>> Would it make more sense to remove the mysql-host choice and add text in
>> The mysql-host configuration description to say that the database must be running
>> On the localhost? This could be noted in the Appendix descriptive text as well.]
> 
> That seems like a reasonable way to keep the example focused (please note
> that as far as I know, the situation for postgres and mysql is equivalent
> in this regard).
> In the general case, even running only on localhost is not always
> sufficient protection such that TLS is not needed, so I'd advise covering
> that in the Appendix descriptive text as well.  Brainstorming (please do
> adjust as appropriate), that might resemble:
> 
> This exmaple aims to illustrate the extension of the server YANG module
> with configuration such as a lease storage database.  For simplicity and to
> maintain focus on that goal, it assumes that the actual database server is
> colocated and can serve traffic securely on localhost without additional
> cryptographic protection.  A production service would likely not be
> colocated and thus use TLS to secure the database connection between DHCPv6
> server and database server; the TLS configuration would require additional
> configuration, as might be modeled using
> [draft-ietf-netconf-tls-client-server].

[if - I’ve removed the mysql-host and Postgresql-host choices from the model and updated
the descriptions to read:

        Name of the MySQL database, running on the localhost.

In the relevant appendix text, I’ve added the following paragraph:

        For simplicity, this example module assumes that the DHCPv6    
        server is colocated with the MySQL or PostgreSQL database        
        server and can serve traffic securely on the localhost without  
        additional cryptographic protection.  In a production           
        deployment, these functions would likely not be colocated       
        and thus use TLS to secure the database connection between      
        the DHCPv6 server and database server. A YANG module for        
        configuring TLS is defined in [draft-ietf-netconf-tls-client-server].  
]

> 
> 
> Many thanks for all the updates that I trimmed -- there was a lot of good
> stuff in there and it's great to see that.
> 
> -Ben

---

From: <[email-masked]>
Date: Mon, 7 Mar 2022 20:13:10 +0000

Hi Benjamin,

I’ve just uploaded a version with my described changes for both your DISCSUSS and other comments. It also addresses the other comments received during the IESG review.

Thanks,
Ian

From: dhcwg <[email-masked]> on behalf of Benjamin Kaduk <[email-masked]>
Date: Friday, 25. February 2022 at 22:54
To: [email-masked] <[email-masked]>
Cc: [email-masked] <[email-masked]>, [email-masked] <[email-masked]>, The IESG <[email-masked]>, [email-masked] <[email-masked]>
Subject: Re: [dhcwg] Benjamin Kaduk's Discuss on draft-ietf-dhc-dhcpv6-yang-24: (with DISCUSS and COMMENT)
Hi Ian,

My apologies for the rather slow response.

The changes below all look good to me -- thank you for the careful
attention and confirming with Tim/Bernie that they properly reflect the
protocol fields in question.

I will also go and reply to a couple topics in the COMMENT section from
your original reply, in a separate message.

Please go ahead and upload a new revision when you get a chance, and I can
update my ballot position accordingly.

Thanks again,

Ben

On Wed, Feb 16, 2022 at 05:01:09PM +0100, [email-masked] wrote:
> Hi Benjamin,
>
> Please see below regarding the changes that have been made to address your DISCUSS.
>
> Thanks,
> Ian
>
> > Benjamin Kaduk has entered the following ballot position for
> > draft-ietf-dhc-dhcpv6-yang-24: 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/blog/handling-iesg-ballot-positions/
> > for more information about how to handle DISCUSS and COMMENT positions.
> >
> >
> > The document, along with other ballot positions, can be found here:
> > https://datatracker.ietf.org/doc/draft-ietf-dhc-dhcpv6-yang/
> >
> >
> >
> > ----------------------------------------------------------------------
> > DISCUSS:
> > ----------------------------------------------------------------------
> >
> > Roman's comment touched on a related point, but I'd like to (hopefully
> > briefly) discuss the way we encode certain opaque protocol fields.
> > There are some places where we clearly intend a hex representation (a
> > string with a pattern that's marked out as pairs of hex digits), but
> > there are others where we just say "type string" with no indication of
> > encoding, and even a "type binary".  If we want the specification to
> > admit interoperable implementation we need to be more clear about what
> > encoding we expect for all of these nodes.  I have noted most (possibly
> > all, but please double check) in the COMMENT section, with a preference
> > for "type binary" where we don't need to apply regex restrictions.  But
> > that's just a personal preference, and choosing to use hex (or base64,
> > or any other well-defined) encoding will suffice to resolve the discuss
> > point.
> >
> [if - In each of the cases that you identified (5 in total), the types have been changed to ‘binary’. I’ve checked through the models and you have identified all of the relevant instances in your review.
>
> Here is a summary of the changes that have been made:
>
>
> Ietf-dhcpv6-common.yang
> ----------------------------------
> Auth-option-group
> This is now modelled with a choice for the protocol type (conf-token or rkap). In both cases the relevant leaves (token-auth-information and auth-info-value respectively) are type binary.
>
> Vendor-specific-information-option-group/
> sub-option-data -> binary
>
> Ietf-dhcpv6-relay.yang
> ———————————————
> Interface-id-option-group
> interface-id -> binary
>
>
> ietf-dhcpv6-client.yang
> ——————————
> User-class-option-group
> -> user-class-data
>
> Vendor-class-option-group
> -> vendor-class-data
>
>
> For reference, here are the associated review comments:
>
>     grouping auth-option-group {
>     [...]
>       container auth-option {
>       [...]
>         leaf auth-information {
>           type string;
>           description
>             "The authentication information, as specified by the
>             protocol and algorithm used in this Authentication
>             option.";
>
> This should probably be binary, not a string.  Or, if it needs to be a
> string, it should specify an encoding (e.g., hex).
>
> ------------------
>
>
>             leaf sub-option-data {
>               type string {
>                 pattern '([0-9a-fA-F]{2}){0,}';
>               }
>               description
>                 "The data area for the sub-option.";
>
> Why does this need to be hex-encoded (vs. YANG binary)?
> And, if it is hex-encoded, we should probably say so explicitly in the
> description.
>
> —————————
>
>
>
>
> Section 4.3
>
>     grouping interface-id-option-group {
>       [...]
>        leaf interface-id {
>           type string;
>           description
>             "An opaque value of arbitrary length generated by the
>             relay agent to identify one of the relay agent's
>             interfaces.";
>
> I think this is better as a YANG "binary" type than a string.  If not,
> we should say something about the encoding.
>
> --------------------
>
>
>     grouping user-class-option-group {
>       [...]
>           leaf user-class-data {
>             type string;
>             description
>               "Opaque field representing a User Class
>               of which the client is a member.";
>
> I think this would be better as YANG binary than string.  If we keep it
> as string, we need to say what encoding is used.
>
> -------------------
>
>       container vendor-class-option {
>         [...]
>         list vendor-class-option-instances {
>         [...]
>           list vendor-class-data-element {
>            [...]
>             leaf vendor-class-data {
>               type string;
>               description
>                 "Opaque field representing a vendor class of which
>                 the client is a member.";
>
> (likewise here)
>
> -----------------
>
>
> >
> > ----------------------------------------------------------------------
> > COMMENT:
> > ----------------------------------------------------------------------
> >
> > Thanks to the shepherd for answering the second part of question (1) in
> > the template, as well as the first; this part of the question is often
> > missed.  (There is, however, a third part as well...)
> >
> > Section 1.2.1
> >
> > Table 1 is described as showing "the DHCPv6 options that are modeled,
> > the element(s) they are sent by, and the relevant YANG module name".  In
> > my interpretation that means that:
> >
> > - the only options that are in the table are ones that are modeled
> > - the contents of the table shows which elements send those options
> >
> > But the contents of the table don't quite seem to match that -- it seems
> > that they only indicate which nodes have YANG modeling for how to send
> > the option, and not indicate nodes that send the option but do not need
> > YANG configuration to control how to populate the option.  (E.g.,
> > OPTION_AUTH and OPTION_USER_CLASS.)  If this is the intent, then perhaps
> > it's clearer to say "which YANG modules they are modeled for" or
> > something similar, rather than "the element(s) they are sent by".
> >
> > Section 3.1
> >
> >   *  address/prefix-pool-utilization-threshold-exceeded: Raised when
> >      the number of leased addresses or prefixes exceeds the configured
> >      usage threshold.
> >
> > The node that seems to be where the "configured usage threshold" is
> > configured is scoped to an address pool or prefix pool.  Should this
> > description say "number of leased [...] in an address pool exceeds the
> > configured usage threshold"?
> >
> > Section 4.1
> >
> >         +------+------+----------+--------------+
> >         | 0001 | 0006 | 28490058 | 00005E005300 |
> >         +------+------+----------+--------------+
> >
> >         This example includes the 2-octet DUID type of 1 (0x01), the
> >         hardware type is 0x06 (IEEE Hardware Types) the creation
> >         time is 0x028490058 (constructed as described above). Finally,
> >         the link-layer address is 0x5E005300 (EUI-48 address
> >         00-00-5E-00-53-00)";
> >
> > Should there be some more zeros in here (e.g., in the artwork before
> > 28490058)?
> >
> >     typedef duid-en {
> >       type duid-base {
> >         pattern '0002'
> >           + '[0-9a-fA-F]{4,}';
> >       }
> >       description
> >         "DUID type 2, assigned by vendor based on Enterprise
> >         Number (DUID-EN). This DUID consists of the 4-octet vendor's
> >         registered Private Enterprise Number as maintained by IANA
> >         followed by a unique identifier assigned by the vendor. For
> >         example:
> >
> > A 4-octet PEN would be 8 hex digits (the YANG allows as few as 4).
> >
> >     typedef duid-unstructured {
> >       type duid-base {
> >         pattern '(000[1-4].*|.*[^0-9a-fA-F].*)' {
> >           modifier invert-match;
> >         }
> >       }
> >
> > My understanding is that the "pattern" modifiers from the base type and
> > the derived type are "and"-ed together, so the "|.*[^0-9a-fA-F].*"
> > clause is redundant with the pattern in the base type and thus not
> > needed.
> >
> >       container status {
> >       [...]
> >         leaf message {
> >           type string;
> >           description
> >             "A UTF-8 encoded text string suitable for display to an
> >             end user. It MUST NOT be null-terminated.";
> >
> > (Related to Roman's comment)
> > This is probably more of a comment on RFC 8415 than this document, but
> > BCP 18 seems pretty clear that a string destined for display to a human
> > should be accompanied by a language indicator.
> > (This comment probably applies to every "leaf message" but I will just
> > mention it this once.)
> >
> >     grouping auth-option-group {
> >     [...]
> >       container auth-option {
> >       [...]
> >         leaf auth-information {
> >           type string;
> >           description
> >             "The authentication information, as specified by the
> >             protocol and algorithm used in this Authentication
> >             option.";
> >
> > This should probably be binary, not a string.  Or, if it needs to be a
> > string, it should specify an encoding (e.g., hex).
> >
> >     grouping status-code-option-group {
> >
> > FWIW, this grouping seems unused by the rest of the document.  (Perhaps
> > a remnant of a decision to move the status information out of the
> > options section and into a higher level of the relevant YANG trees?)
> >
> >             leaf sub-option-data {
> >               type string {
> >                 pattern '([0-9a-fA-F]{2}){0,}';
> >               }
> >               description
> >                 "The data area for the sub-option.";
> >
> > Why does this need to be hex-encoded (vs. YANG binary)?
> > And, if it is hex-encoded, we should probably say so explicitly in the
> > description.
> >
> > Section 4.2
> >
> >       leaf preferred-lifetime {
> >         type dhc6:timer-seconds32;
> >         description
> >           "Preferred lifetime for the Identity Association (IA).";
> >         reference "RFC 8415: Dynamic Host Configuration Protocol for
> >           IPv6 (DHCPv6), Section 6";
> >
> > Is Section 6 really the best section reference for this concept?
> >
> >     grouping message-stats {
> >
> > The relay module has a counter for discarded messages; would a similar
> > counter make sense here?  (Also for the server module, I think.)
> >
> > Also, YANG does have dedicated "counter" types to more precisely
> > indicate semantics than just "uint32".  They are by definition
> > monotonically increasing, though, and are their use is typically
> > accompanied by a "last discontinuity time" node to indicate when they
> > have been reset.
> >
> >     grouping info-refresh-time-option-group {
> >       [...]
> >         leaf info-refresh-time {
> >           type dhc6:timer-seconds32;
> >           description
> >             "Time duration relative to the current time, expressed
> >             in units of seconds.";
> >
> > Is this really "relative to the current time" for purposes of the YANG
> > module?  This is the description that RFC 8415 uses, yes, but that
> > refers to the current time when the protocol message is received, and
> > YANG interactions may be asynchronous to that.
> >
> >           container address-pools {
> >           [...]
> >             list address-pool {
> >               key pool-id;
> >               [...]
> >               leaf pool-id {
> >                 type string;
> >
> > What's the motivation for making the pool-id a string vs the option-set
> > and allocation-range lists that have an integer identifier?  (Also
> > applies to prefix pools.)
> >
> >               leaf pool-prefix {
> >                 type inet:ipv6-prefix;
> >                 mandatory true;
> >                 description
> >               leaf max-address-utilization {
> >                 type dhc6:threshold;
> >                 description
> >                   "Maximum amount of the addresses in the
> >                   pool which can be simultaneously allocated,
> >                   calculated as a percentage of the available
> >                   addresses (end-address minus start-address plus
> >                   one).";
> >
> > Do we want to mention the relationship between this node and the
> > "address-pool-utiliziation-threshold-exceeded" notification?  I see the
> > pointer the other direction, but since we don't use the word "threshold"
> > in the description here I wouldn't mind a more explicit linkage.
> > (Likewise for the prefix-allocation case.)
> >
> >                 list active-lease {
> >                   key leased-prefix;
> >                   description
> >     notification decline-received {
> >       if-feature na-assignment;
> >       description
> >         "Notification sent when the server has received a
> >         Decline (9) message from a client.";
> >
> > Is this a potential DoS vector to the notification recipient (if a
> > malicious client just starts declining everything)?  Should we say
> > anything about rate-limiting?
> >
> >     notification non-success-code-sent {
> >       description
> >         "Notification sent when the server responded
> >         to a client with non-success status code.";
> >
> > (similarly here)
> >
> > Section 4.3
> >
> >     grouping interface-id-option-group {
> >       [...]
> >        leaf interface-id {
> >           type string;
> >           description
> >             "An opaque value of arbitrary length generated by the
> >             relay agent to identify one of the relay agent's
> >             interfaces.";
> >
> > I think this is better as a YANG "binary" type than a string.  If not,
> > we should say something about the encoding.
> >
> > Section 4.4
> >
> > Why do we use "prefix-del" for the feature name in this module but the
> > full "prefix-delegation" in the other modules?
> >
> >     grouping message-statistics {
> >
> > The relay module has a counter for discarded messages; should we?
> >
> >     grouping option-request-option-group {
> >       description
> >         "OPTION_ORO (6) Option Request Option. A client MUST include
> >         an Option Request option in a Solicit, Request, Renew,
> >            Rebind, or Information-request message to inform the server
> >              about options the client wants the server to send to the
> >              client.";
> >
> > If I understand correctly, there are further MUST-level requirements for
> > what options must be present in the ORO.  E.g., INFORMATION_REFRESH_TIME
> > is required in Information-Request.  Do we want to mention that here?
> > Are there any considerations for how that requirement interacts with the
> > values configured by YANG?  E.g., do we expect implementations to
> > forcibly add those required options on top of the configured ones, in
> > scenarios where they are required?
> >
> >     grouping user-class-option-group {
> >       [...]
> >           leaf user-class-data {
> >             type string;
> >             description
> >               "Opaque field representing a User Class
> >               of which the client is a member.";
> >
> > I think this would be better as YANG binary than string.  If we keep it
> > as string, we need to say what encoding is used.
> >
> >       container vendor-class-option {
> >         [...]
> >         list vendor-class-option-instances {
> >         [...]
> >           list vendor-class-data-element {
> >            [...]
> >             leaf vendor-class-data {
> >               type string;
> >               description
> >                 "Opaque field representing a vendor class of which
> >                 the client is a member.";
> >
> > (likewise here)
> >
> >       leaf client-duid {
> >         if-feature "non-temp-addr or prefix-del " +
> >           "or temp-addr and not anon-profile";
> >
> > Does the default YANG operator precedence do what we want with respect
> > to ((non-temp-addr or prefix-del or temp-addr) and not anon-profile) vs
> > (non-temp-addr or prefix-del or (temp-addr and not anon-profile))?
> >> From the context in the rest of the document, it's clear that we want
> > the former, but I'm not familiar enough with YANG minutiae to say if
> > that's what we actually are specifying.
> >
> >     notification invalid-ia-address-detected {
> >       if-feature "non-temp-addr or temp-addr";
> >       [...]
> >       leaf ia-id {
> >         type uint32;
> >         mandatory true;
> >         description
> >           "IA-ID";
> >
> > If I understand correctly, in DHCP the IA-ID is scoped per client DUID.
> > Does this notification convey enough information to disambiguate what
> > scope this IA-ID value belongs to?
> >
> >       [...]
> >       leaf ia-na-t1-timer {
> >         type uint32;
> >         description
> >           "The value of the T1 time field for non-temporary address
> >           allocations (OPTION_IA_NA).";
> >       }
> >       leaf ia-na-t2-timer {
> >         type uint32;
> >         description
> >           "The value of the preferred-lifetime field for non-temporary
> >           address allocations (OPTION_IA_NA).";
> >
> > Should these two be here without a feature conditional?  They seem to be
> > NA-specific but we could send this notification if NA is not
> > implemented.
> >
> >     notification transmission-failed {
> >         [...]
> >       leaf failure-type {
> >         type enumeration {
> >
> > (side note?) I get antsy about not leaving an extension point for other
> > types of (re)transmission failure, but I also don't have any specific
> > scenario in mind that's not already covered, so.
> >
> >     notification unsuccessful-status-code {
> >       description
> >         "Notification sent when the client receives a message that
> >         includes an unsuccessful Status Code option.";
> >
> > As for the other notifications on failure, is this a potential DoS
> > vector?
> >
> > Section 5
> >
> > Can a misconfigured client cause problems for a (honest) server, e.g.,
> > by sending a lot of requests for a lot of things, very quickly?
> >
> >   All data nodes defined in the YANG modules which can be created,
> >   modified, and deleted (i.e., config true, which is the default) are
> >   considered sensitive.  Write operations (e.g., edit-config) to these
> >   data nodes without proper protection can have a negative effect on
> >   network operations.
> >
> > Do we want to go further and give some broad treatment of how the
> > negative effects would come about (e.g., by disrupting address
> > allocation and the routability of addresses/prefixes that clients
> > attempt to use)?
> >
> >   An attacker with read/write access the DHCPv6 relay can undertake
> >   various attacks, such as:
> >
> >   *  Modifying the relay's "destination-address" to send messages to a
> >      rogue DHCPv6 server.
> >
> >   *  Deleting information about a client's delegated prefix, causing a
> >      denial of service attack as traffic will no longer be routed to
> >      the client.
> >
> > I'd considering reiterating the "full denial of service" capability
> > (which is currently implicit in "send messages to a rouge DHCPv6 server"
> > combined with the list of attacks that a compromised server can
> > undertake).
> >
> >   Some of the readable data nodes in this YANG module may be considered
> >   sensitive or vulnerable in some network environments.  Therefore, it
> >   is important to control read access (e.g., only permitting get, get-
> >   config, or notifications) to these data nodes.  These subtrees and
> >   data nodes can be misused to track the activity of a host:
> >
> > First off, thank you for stating this consideration so clearly.
> >
> > Second, there may be an additional consideration for read-only access --
> > knowledge of what pools of addresses are available for allocation might
> > facilitate network reconaissance (viz. RFC 7707).
> >
> > Section 9.2
> > RFC 8987 is used as in a few YANG reference stanzas, but is not
> > mentioned in the preface before the containing YANG module.  It could
> > arguably be classified as normative based on that usage.
> >
> > Appendix D
> >
> >         case user-class-option-id {
> >           description
> >             "Client class selection based on the value of the
> >             OPTION_USER_CLASS(15) and its user-class-data field.";
> >           leaf user-class-data {
> >             type string;
> >             mandatory true;
> >             description
> >               "Value of the enterprise-number field.";
> >
> > This description doesn't look right.
> >
> > NITS
> >
> > We might check the spelling of "grouping message-stats" (vs "grouping
> > message-statistics") across modules.
> >
> > Section 4.2
> >
> >       leaf rapid-commit {
> >         type boolean;
> >         description
> >           "When set to 'true', Specifies that the pool supports
> >           client-server exchanges involving two messages.";
> >
> > This is in the "grouping resource-config" that may or may not be used
> > within a "pool" container.  A more generic phrasing might be in order.
> >
> >     grouping sol-max-rt-option-group {
> >       [...]
> >         leaf sol-max-rt-value {
> >           type dhc6:timer-seconds32;
> >           description
> >             "sol max rt value";
> >
> > That's a pretty sparse description.
> >
> >     grouping inf-max-rt-option-group {
> >       [...]
> >         leaf inf-max-rt-value {
> >           type dhc6:timer-seconds32;
> >           description
> >             "inf max rt value";
> >
> > (ditto)
> >
> >               leaf max-address-utilization {
> >                 type dhc6:threshold;
> >                 description
> >                   "Maximum amount of the addresses in the
> >                   pool which can be simultaneously allocated,
> >                   calculated as a percentage of the available
> >                   addresses (end-address minus start-address plus
> >                   one).";
> >
> > The analogous prefix-allocation node has "rounded up" stated
> > explicitly.  Should we do so here as well?
> >
> > Section 4.3
> >
> >       leaf reply-sent-count {
> >         type uint32;
> >         config "false";
> >         description
> >           "Number of Reply (7) messages received.";
> >       }
> >       leaf release-received-count {
> >         type uint32;
> >         config "false";
> >         description
> >           "Number of Release (8) messages sent.";
> >       }
> >       leaf decline-received-count {
> >         type uint32;
> >         config "false";
> >         description
> >           "Number of Decline (9) messages sent.";
> >
> > The descriptions seem out of sync about sent vs received, here.
> >
> > Section 5
> >
> >   As the RPCs for deleting/clearing active address and prefix entries
> >   in the server and relay modules are particularly sensitive, these use
> >   'nacm:default-deny-all'.
> >
> > This looks like a comma splice.
> >
> >   An attacker with read/write access the DHCPv6 server can undertake
> >   various attacks, such as:
> >   [...]
> >   An attacker with read/write access the DHCPv6 relay can undertake
> >   various attacks, such as:
> >
> > s/access/access to/
> >
> >   Some of the readable data nodes in this YANG module may be considered
> >   sensitive or vulnerable in some network environments.  Therefore, it
> >   is important to control read access (e.g., only permitting get, get-
> >   config, or notifications) to these data nodes.  These subtrees and
> >
> > This doesn't match the template at
> > https://trac.ietf.org/trac/ops/wiki/yang-security-guidelines , where the
> > parenthetical is "(e.g., via get, get-config, or notification)" -- the
> > meaning is different!
> >
> > Appendix A.1
> >
> >   The 'max-pd-space-utilization' is set to 80 so that a 'prefix-pool-
> >   utilization-threshold-exceeded' notification will be raised if the
> >   number of prefix allocations exceeds this.
> >
> > the max-pd-space-utilitization is a percentage, so the "number of prefix
> > allocations" needs a unit conversion to be comparable to it.
> >
> > Appendix C
> >
> >         container lease-storage {
> >           description
> >             "Configures how the server will stores leases.";
> >           choice storage-type {
> >             description
> >               "The type storage that will be used for lease
> >               information.";
> >
> > s/type storage/type of storage/
> >
> >             case mysql {
> >               leaf mysql-name {
> >                 type string;
> >                 description
> >                   "Name of the database.";
> >               }
> >               [...]
> >
> >               leaf mysql-port {
> >                 type inet:port-number;
> >                 default 5432;
> > mysql's registered port seems to be 3306 (5432 is assigned for
> > postgres).
> >
> > Appendix D
> >
> >   classifying clients.  So this standard does not try to provide full
> >   specification for class selection, it only shows an example how it
> >   could be defined.
> >
> > s/example/example of/
> >
> >     grouping client-class-id {
> >       description
> >         "Definitions of client message classification for
> >         authorization and assignment purposes.";
> >       leaf client-class-name {
> >         type string;
> >         description
> >           "Unique Identifier for client class identification list
> >           entries.";
> >
> > Shouldn't this be mandatory if it's going to be the only way we refer to
> > class IDs?  I guess the grouping is currently only used in one place,
> > and it's a list key there (so implicitly mandatory), but I still wonder
> > if the grouping is more reusable with "mandatory true".
> >
> >           leaf relay-interface {
> >             type string;
> >             description
> >               "Reference to the interface entry for the incoming
> >               DHCPv6 message.";
> >
> > Whatever we do in the main module for the encoding of the interface
> > entry should be reflected here as well.
> >
> >             leaf vendor-class-data {
> >               type string;
> >               mandatory true;
> >               description
> >                 "Vendor class data to match.";
> >
> > (likewise)
> >
> >             leaf remote-id {
> >               type string;
> >               mandatory true;
> >               description
> >                 "Remote-ID data to match.";
> >
> > (and here)
> >
> >
> >
> > _______________________________________________
> > dhcwg mailing list
> > [email-masked]
> > https://www.ietf.org/mailman/listinfo/dhcwg
>

_______________________________________________
dhcwg mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/dhcwg