Hi Rob, 

Thanks for the review. 

Candidate changes to address this review can be tracked at: 
https://tinyurl.com/opsawg-add-latest

Please find inline some inputs in addition to the replies from Alan. 

Cheers,
Med

> -----Message d'origine-----
> De : Rob Wilton (rwilton) <rwil...@cisco.com>
> Envoyé : lundi 19 décembre 2022 17:53
> À : draft-ietf-opsawg-add-encrypted-dns....@ietf.org
> Cc : opsawg@ietf.org
> Objet : AD review of draft-ietf-opsawg-add-encrypted-dns-07
> 
> Hi,
> 
> Thanks for this document.  Here are my AD review comments for
> draft-ietf-opsawg-add-encrypted-dns-07
> 
> Moderate level comments:
> 
> (1) p 2, sec 1.  Introduction
> 
>    This document specifies two new RADIUS attributes: DHCPv6-
> Options
>    (Section 3.1) and DHCPv4-Options (Section 3.2) Attributes.
> These
>    attributes can include DHCP options that are listed under the
> IANA
>    registries that are created in Sections 8.4.1 and 8.4.1.  These
> two
>    attributes are specified in order to accommodate both IPv4 and
> IPv6
>    deployment contexts while taking into account the constraints
> in
>    Section 3.4 of [RFC6158].
> 
> It isn't really clear to me why some of the registries are needed,
> specifically the ones in 8.4.1 and 8.4.2.  Why not allow any v4 or
> v6 DHCP attribute to be carried within the DHCPv6-Options or
> DHCPv4-Options field?
> 

[Med] We considered that design but we went with the current approach to 
address a concern from DHC WG (see 
https://mailarchive.ietf.org/arch/msg/opsawg/7GzmUQbpqett2V0GSlhPY6AL6t0/ and 
follow-ups). Some options do not make sense to encapsulate them. 

> 
> (2) p 4, sec 3.  DHCP Options RADIUS Attributes
> 
>    Absent any explicit configuration on the DHCP server, RADIUS
> supplied
>    data by means of DHCP*-Options Attributes take precedence over
> any
>    local configuration.
> 
> This point may be worth discussing.  Naturally, I would explicit
> configuration to a network device to generally take precedent over
> implicitly learned configuration from the network.
> 

[Med] Alan addressed this one. 

> 
> (3) p 6, sec 3.2.  DHCPv4-Options Attribute
> 
>       Permitted DHCPv4 options in the DHCPv4-Options Attribute are
>       maintained by IANA in the registry created in Section 8.4.2.
> 
> Comparing this text to the description for v6, this description is
> silent on whether multiple instances of the same DHCPv4 option MAY
> be included.  Should that be specified here?
> 

[Med] There are some DHCPv4 specifics. Added this NEW text to explicit the 
behavior: 

  Multiple instances
  of the same DHCPv4 option MAY be included, especially for
  concatenation-requiring options that exceed the maximum DHCPv4
  option size of 255 octets.  The mechanism specified in [RFC3396]
  MUST be used for splitting and concatenating the instances of a
  concatenation-requiring option.

> 
> (4) p 10, sec 7.  Table of Attributes
> 
>    The following table provides a guide as what type of RADIUS
> packets
>    that may contain these attributes, and in what quantity.
> 
> Am I right that this is just a duplication of what is described in
> section 3?  If so, perhaps change "guide" to "informative guide"
> and include text to refer back to the  canonical definition in
> section 3.
> 

[Med] Alan clarified this point.  

> 
> (5) p 13, sec 8.4.3.  Guidelines for the Designated Experts
> 
>    Registration requests that are undetermined for a period longer
> than
>    28 days can be brought to the IESG's attention for resolution.
> 
> I'm wondering whether we need the process related text in this
> document at all, or whether we let IANA apply their standard
> policies?  I may be misinformed, but I'm not aware of many *-
> review mailing lists.
> 

[Med] The latest I'm aware of is 
draft-ietf-drip-rid-37.html#name-new-iana-drip-registry.

> 
> (6) p 15, sec 10.2.  Informative References
> 
>    [I-D.ietf-add-dnr]
>               Boucadair, M., Reddy, T., Wing, D., Cook, N., and T.
>               Jensen, "DHCP and Router Advertisement Options for
> the
>               Discovery of Network-designated Resolvers (DNR)",
> Work in
>               Progress, Internet-Draft, draft-ietf-add-dnr-13, 13
> August
>               2022, <https://www.ietf.org/archive/id/draft-ietf-
> add-dnr-
>               13.txt>.
> 
> Should this be a normative reference?  E.g., if feels like the
> IANA registry values are bound to whatever is published in ietf-
> add-dnr.
> 
> 

[Med] 144/162 are permanent IANA assignments. Please note that the IANA section 
says the following:    

   The initial content of this sub-registry is listed in Table 5.  The
   reference may include the document that registers the option or the
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 
   document that specifies the option. 

IANA registries are authoritative here as well. 

> 
> Minor level comments:
> 
> (7) p 2, sec 1.  Introduction
> 
>    With the advent of encrypted DNS (e.g., DNS-over-HTTPS (DoH)
>    [RFC8484], DNS-over-TLS (DoT) [RFC7858], or DNS-over-QUIC (DoQ)
>    [RFC9250]), additional means are required to provision hosts
> with
>    network-designated encrypted DNS.  To fill that void,
>    [I-D.ietf-add-dnr] leverages existing protocols such as DHCP
> and IPv6
>    Router Advertisement to provide hosts with the required
> information
>    to connect to an encrypted DNS resolver.  However, there are no
>    RADIUS attributes that can be used to populate the discovery
> messages
>    discussed in [I-D.ietf-add-dnr].  The same concern is likely to
> be
>    encountered for future services that are configured using DHCP.
> 
> >From this introduction, I thought that this would be covering
> options for both DHCP and ND, but it looks like only DHCP is
> covered.  Perhaps this introduction text could be tweaked slightly
> to make this clearer?
> 

[Med] Removed the ND mention. 

> 
> (8) p 3, sec 3.  DHCP Options RADIUS Attributes
> 
>    These attributes use the "Long Extended Type" format in order
> to
>    permit the transport of attributes encapsulating more than 253
> octets
>    of data.  DHCP options that can be included in the DHCP*-
> Options
>    RADIUS attributes are limited by the maximum packet size of
> 4096
>    bytes.  In order to accommodate deployments with large options,
>    implementations are RECOMMENDED to support a packet size up to
> 65535
>    bytes.
> 
> I didn't find this text clear.  E.g., limit is 4k but should
> support up to 64K.  Which implementations should support larger
> packet sizes?  Is this RADIUS implementations?
> 

[Med] Yes, this is about RADIUS implementations. Updated the text accordingly. 
Also added a note to refer to some recent RFCs that relaxed the 4096 limit. 

> 
> (9) p 5, sec 3.1.  DHCPv6-Options Attribute
> 
>       This field contains a list of DHCPv6 options.  Multiple
> instances
>       of the same DHCPv6 option MAY be included.  Consistent with
>       Section 17 of [RFC7227], this document does not impose any
> option
>       order when multiple options are present.
> 
> Is there any requirement to merge multiple instances of options
> together, presumably they are logically just concatenated today.
> 

[Med] No new requirements are needed here. We rely on existing DHCPv6 encoding 
procedures.

> 
> (10) p 5, sec 3.1.  DHCPv6-Options Attribute
> 
>       Permitted DHCPv6 options in the DHCPv6-Options Attribute are
>       maintained by IANA in the registry created in Section 8.4.1.
> 
> As per above, presumably there isn't just an DHCPv6 options
> registry that can be reused rather than needing a separate one to
> be setup and maintained.
> 

[Med] Not sure to see if/which change is needed here. 

> 
> (11) p 6, sec 4.1.  Context
> 
>    The RADIUS Attributes suboption [RFC4014] enables a DHCPv4
> relay
>    agent to pass identification and authorization attributes
> received
>    during RADIUS authentication to a DHCPv4 server.  However,
> [RFC4014]
>    defines a frozen set of RADIUS attributes that can be included
> in
>    such a suboption.  This limitation is suboptimal in contexts
> where
>    new services are deployed (e.g., support of encrypted DNS
>    [I-D.ietf-add-dnr]).
> 
> I like 'suboptimal', very diplomatic. ;-)
> 
> 
> (12) p 8, sec 5.  Applicability to Encrypted DNS Provisioning
> 
>          Figure 1: An Example of RADIUS IPv6 Encrypted DNS
> Exchange
> 
> As a minor comment, I wonder whether it would be helpful to also
> include RADIUS client in the NAS box description?
> 

[Med] No problem. Done. 

> 
> (13) p 12, sec 8.4.1.  DHCPv6
> 
>    IANA is requested to create a new sub-registry entitled "DHCPv6
>    Options Permitted in the RADIUS DHCPv6-Options Attribute" in
> the
>    "Dynamic Host Configuration Protocol for IPv6 (DHCPv6)"
> registry
>    [DHCP-RADIUS].
> 
> Do we need to define the definition of columns for this (and the
> v4 equivalent) registries.  E.g., do the values need to match
> another registry?
> 
> 

[Med] Added a note. 


> (14) p 12, sec 8.4.1.  DHCPv6
> 
>                       Table 4: Initial DHCPv6 Options
>                           Permitted in the RADIUS
>                           DHCPv6-Options Attribute
> 
> Is 144 (and 162 for v4) a permanent IANA assignment?  Or should
> the value be bound to that allocated by draft-ietf-add-dnr.
> 
> 

[Med] This is a permanent IANA assignment. 


> Nit level comments:
> 
> (15) p 2, sec 1.  Introduction
> 
>    This document specifies two new RADIUS attributes: DHCPv6-
> Options
>    (Section 3.1) and DHCPv4-Options (Section 3.2) Attributes.
> These
>    attributes can include DHCP options that are listed under the
> IANA
>    registries that are created in Sections 8.4.1 and 8.4.1.  These
> two
>    attributes are specified in order to accommodate both IPv4 and
> IPv6
>    deployment contexts while taking into account the constraints
> in
>    Section 3.4 of [RFC6158].
> 
> Nit, "Sections 8.4.1 and 8.4.1", presumably should be 8.4.1 and
> 8.4.2?
> 

[Med] Fixed. 

> Regards,
> Rob

_________________________________________________________________________________________________________________________

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.

_______________________________________________
OPSAWG mailing list
OPSAWG@ietf.org
https://www.ietf.org/mailman/listinfo/opsawg

Reply via email to