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

[[e-mail]]

Hi Benjamin,

Thanks for your comments. Please see inline below.

BR,
Ian

> On 25. Feb 2022, at 23:18, Benjamin Kaduk <[e-mail]> 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, [e-mail] wrote:
>> 
>>> On 15. Dec 2021, at 01:07, Benjamin Kaduk via Datatracker <[e-mail]> 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