Subject: Re: [Add] WGLC for draft-ietf-add-dnr

From: Ben Schwartz <[email-masked]>
Date: Thu, 3 Mar 2022 16:52:37 -0500

I guess I'm going to repeat my complaint about the provided binary formats:

* They are capable of representing numerous differently invalid options,
and are therefore error-prone.  In memory-unsafe languages, they are
needlessly dangerous to implement.
* The three versions (DHCPv4, DHCPv6, and RA) are each sufficiently
different that they cannot easily share an implementation with either of
the others.

After the option code, length, and (for RA) lifetime fields, I propose that
we use the following format for all versions:
1. A 1-byte unsigned integer indicating the number of IP addresses
2. That many IP addresses (v4 or v6 depending on context)
3. The SvcPriority, ADN, and SvcParams, encoded as in SVCB RDATA

This is easier to specify, more compact, and has fewer ways to go wrong.

On Thu, Feb 24, 2022 at 10:57 AM Deen, Glenn <Glenn_Deen=
[email-masked]> wrote:

> The ADD draft DHCP and Router Advertisement Options for the Discovery of
> Network-designated Resolvers (DNR)
> https://datatracker.ietf.org/doc/draft-ietf-add-dnr/ is ready for WGLC.
>
> WGLC will run until March 14th, please post all comments to the ADD
> mailing list.
>
> ADD Co-chairs
> Glenn Deen & David Lawrence
>
>
> --
> Add mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/add
>

---

From: "Martin Thomson" <[email-masked]>
Date: Fri, 04 Mar 2022 09:44:39 +1100

On Fri, Mar 4, 2022, at 08:52, Ben Schwartz wrote:
> After the option code, length, and (for RA) lifetime fields, I propose 
> that we use the following format for all versions:

This seems entirely reasonable to me.

I would have said that - ordinarily - encoding differences between DHCPv4 and DHCPv6 are not a huge concern, but when a simple and elegant option exists (as above), without significant downsides...

---

From: <[email-masked]>
Date: Fri, 4 Mar 2022 07:56:36 +0000

Hi Ben,

That was considered in the past, but we need also to listen to the guidelines that are specific to each of these protocols. For example, a reco from DHC is to start with fixed length fields, this is why we have SvcPriority first not the order you propose.

Likewise, SvcPriority is not present in RA because this can be inferred from the order appearance while this is not possible for DHCP as already recorded in the doc:

   The DHCP options defined in Sections 4<https://datatracker.ietf.org/doc/html/draft-ietf-add-dnr#section-4> and 5<https://datatracker.ietf.org/doc/html/draft-ietf-add-dnr#section-5> follow the option
   ordering guidelines in Section 17 of [RFC7227]<https://datatracker.ietf.org/doc/html/rfc7227#section-17>.

   “Option order, either the order among many DHCPv6 options or the order

   of multiple instances of the same option, SHOULD NOT be significant.

   New documents MUST NOT assume any specific option processing order.”

Likewise, we don’t want to create a precedent for self-delimiting variable length fields but followed the guidelines from these groups to have an explicit length field for each variable length.

We need to be careful here because we are in an area where we need to take into account what the owners of these protocols maintain the protocol, not only what would be perfect for a specific application.

Cheers,
Med

De : Add <[email-masked]> De la part de Ben Schwartz
Envoyé : jeudi 3 mars 2022 22:53
À : Deen, Glenn <Glenn_Deen=[email-masked]>
Cc : [email-masked]
Objet : Re: [Add] WGLC for draft-ietf-add-dnr

* They are capable of representing numerous differently invalid options, and are therefore error-prone.  In memory-unsafe languages, they are needlessly dangerous to implement.
* The three versions (DHCPv4, DHCPv6, and RA) are each sufficiently different that they cannot easily share an implementation with either of the others.

After the option code, length, and (for RA) lifetime fields, I propose that we use the following format for all versions:
1. A 1-byte unsigned integer indicating the number of IP addresses
2. That many IP addresses (v4 or v6 depending on context)
3. The SvcPriority, ADN, and SvcParams, encoded as in SVCB RDATA

On Thu, Feb 24, 2022 at 10:57 AM Deen, Glenn <Glenn_Deen=[email-masked]<mailto:[email-masked]>> wrote:
The ADD draft DHCP and Router Advertisement Options for the Discovery of Network-designated Resolvers (DNR) https://datatracker.ietf.org/doc/draft-ietf-add-dnr/ is ready for WGLC.

WGLC will run until March 14th, please post all comments to the ADD mailing list.

ADD Co-chairs

--
Add mailing list
[email-masked]<mailto:[email-masked]>
https://www.ietf.org/mailman/listinfo/add

_________________________________________________________________________________________________________________________

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.

---

From: Chris Box <[email-masked]>
Date: Wed, 9 Mar 2022 17:36:01 +0000

Hi everyone.

I've assessed DNR against relevant parts of draft-ietf-add-requirements.
Here's the first.

    +=============+===================================================+
    | Requirement | Description                                       |
    +=============+===================================================+
    | R1.1        | Discovery SHOULD provide a local network the      |
    |             | ability to announce to clients a set of, or       |
    |             | absence of, designated resolvers.                 |

It's not clear to me how you use DNR to advertise the *absence *of
local encrypted DNS. Are we saying the router should send a frame with
zero option length (if that is considered a multiple of 16), and zero
ADN length? If yes, we should write that. If that's a bad design, we
should think about what is better.

    +-------------+---------------------------------------------------+
    | R1.3        | Discovery SHOULD support all encrypted DNS        |
    |             | protocols standardised by the IETF.               |

The requirement is approximately met, by virtue of building on SVCB.
However I think the draft should normatively depend on
draft-ietf-add-svcb-dns rather than the generic
draft-ietf-dnsop-svcb-https.

There's a point to clarify. It is not stated whether it is useful or
valid for the SVCB Service Priority to be zero (i.e. AliasMode). I
suspect not since it would mean the mandatory SvcParams could not be
populated, and may require the client to perform an additional
bootstrapping query, potentially over Do53.

    | R2.1        | Networks SHOULD be able to announce one or more   |
    |             | designated encrypted DNS resolvers using existing |
    |             | mechanisms such as DHCPv4, DHCPv6, IPv6 Router    |
    |             | Advertisement, and the Point-to-Point Protocol.   |

Met for the first three mechanisms, but PPP is missing. I'll come back
to that in a minute.

    +-------------+---------------------------------------------------+
    | R2.2        | The format for resolver designation SHOULD be     |
    |             | specified such that provisioning mechanisms       |
    |             | defined outside of the IETF can advertise         |
    |             | encrypted DNS resolvers.                          |
    +-------------+---------------------------------------------------+
    | R2.3        | This format SHOULD convey, at minimum, the        |
    |             | information the client needs to make contact with |
    |             | each designated resolver.                         |
    +-------------+---------------------------------------------------+
    | R2.4        | This format MAY convey additional resolver        |
    |             | information.                                      |

Not met, and it seems to be an easy thing to add. Doing so could make
it easier and more consistent for use over, for example, 3GPP mobile
networks.

What should it look like? If we follow the example of the provided
DHCP formats, it would require two blob formats, one each for IPv4 and
IPv6.

 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|       Service Priority        |         Addr Length           |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
~                      ipv6-address(es)                         ~
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|          ADN Length           |                               |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+                               +
~                  authentication-domain-name                   ~
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
~                 Service Parameters (SvcParams)                ~
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|       Service Priority        |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|  Addr Length  |               |
+-+-+-+-+-+-+-+-+               +
~        IPv4 Address(es)       ~
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|   ADN Length  |               |
+-+-+-+-+-+-+-+-+               +
~  authentication-domain-name   ~
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
~Service Parameters (SvcParams) ~
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

I am more drawn to Ben's suggestion:

> 3. The SvcPriority, ADN, and SvcParams, encoded as in SVCB RDATA

Which, for those who like pictures, probably means something along the
lines of this.

0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|  # of Addrs   |               |
+-+-+-+-+-+-+-+-+               +
~          Address(es)          ~
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|       Service Priority        |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
~  Authentication-domain-name   ~
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
~Service Parameters (SvcParams) ~
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

Whichever generic format we select, it could then also be applied to
PPP IPCP as a new extension:

  0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 |    Type TBA   |    Length     |      Encrypted-DNS-Server
 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   Encrypted-DNS-Server (cont)   |
 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

Now I'll cover the final set of requirements.

    +-------------+---------------------------------------------------+
    | R5.1        | Discovery MUST NOT worsen a client's security or  |
    |             | privacy posture.                                  |

Section 7 discusses security. I can't spot any way in which DNR makes it worse.

    +-------------+---------------------------------------------------+
    | R5.2        | Threat modelling MUST assume that there is a      |
    |             | passive eavesdropping attacker on the local       |
    |             | network.                                          |

Section 7.3 has two sentences on this so it's been considered, but I
don't know if it needs expanding.

    +-------------+---------------------------------------------------+
    | R5.3        | Threat modelling MUST assume that an attacker can |
    |             | actively attack from outside the local network.   |

Section 7.1 considers active attack from both inside and outside.
However I suggest the word "CPE" is made more generic to refer to any
such router at the boundary of the local network.

    +-------------+---------------------------------------------------+
    | R5.4        | Attackers MUST NOT be able to redirect encrypted  |
    |             | DNS traffic to themselves when they would not     |
    |             | otherwise handle DNS traffic.                     |
    +-------------+---------------------------------------------------+

This seems to be met.

So in summary I would prefer to see some changes before it leaves the WG.
To make this easier, can we please maintain it in
https://github.com/ietf-wg-add/draft-ietf-add-dnr?

Thanks
Chris

---

From: Ben Schwartz <[email-masked]>
Date: Wed, 9 Mar 2022 13:00:38 -0500

On Wed, Mar 9, 2022 at 12:36 PM Chris Box <[email-masked]> wrote:

> It's not clear to me how you use DNR to advertise the *absence *of local encrypted DNS. Are we saying the router should send a frame with zero option length (if that is considered a multiple of 16), and zero ADN length? If yes, we should write that. If that's a bad design, we should think about what is better.
>
> I don't think an explicit "absence" indication is needed.  Omitting the
DNR announcement seems sufficient for any use case I can think of.

> The requirement is approximately met, by virtue of building on SVCB. However I think the draft should normatively depend on draft-ietf-add-svcb-dns rather than the generic draft-ietf-dnsop-svcb-https.
>
> That seems right.

> There's a point to clarify. It is not stated whether it is useful or valid for the SVCB Service Priority to be zero (i.e. AliasMode). I suspect not since it would mean the mandatory SvcParams could not be populated, and may require the client to perform an additional bootstrapping query, potentially over Do53.
>
> I agree we need to make the text clearer here, and I think we should allow
AliasMode.

Consider a gateway that wants to issue a DNR result for an external
resolver, $RESOLVER.  It can do this by resolving SVCB for _dns.$RESOLVER,
and using this to populate DNR.  However, this SVCB resolution has a
limited TTL (e.g. cloudflare-dns.com has a TTL of 5 minutes).  To be able
to issue DHCP responses to new network clients without delay, the gateway
has to keep this SVCB record in its local cache at all times (constantly
refreshing), and issue DHCP responses that expire when that record expires,
which could be much shorter than an ordinary DHCP lease.  If SVCB
resolution fails (e.g. due to an upstream outage), that gateway has to do
something, but I'm not sure what.

In addition to the complexity and inefficiency of this arrangement, it also
creates a "thundering herd" problem.  In a simple implementation, all
devices on the network will have their DHCP information expire
simultaneously (at the end of the SVCB TTL), leading to a brief spike in
DHCP activity.

If AliasMode (or an equivalent) is allowed, the gateway just tells the
devices "use $RESOLVER" and lets them sort it out.  This pushes more
complexity onto the clients, but if the clients already support DDR then
implementation is trivial.

---

From: Ben Schwartz <[email-masked]>
Date: Wed, 9 Mar 2022 13:39:59 -0500

On Fri, Mar 4, 2022 at 2:56 AM <[email-masked]> wrote:

> Hi Ben,
>
>
>
> That was considered in the past, but we need also to listen to the
> guidelines that are specific to each of these protocols. For example, a
> reco from DHC is to start with fixed length fields, this is why we have
> SvcPriority first not the order you propose.
>

I don't think we should attempt to apply these guidelines on our own.  If
they are indeed applicable in this case, I'd like to get that confirmed
from some relevant subject matter experts.

Likewise, SvcPriority is not present in RA because this can be inferred
> from the order appearance while this is not possible for DHCP as already
> recorded in the doc:
>
>
>
>    The DHCP options defined in Sections 4
> <https://datatracker.ietf.org/doc/html/draft-ietf-add-dnr#section-4> and 5
> <https://datatracker.ietf.org/doc/html/draft-ietf-add-dnr#section-5>
> follow the option
>
>    ordering guidelines in Section 17 of [RFC7227]
> <https://datatracker.ietf.org/doc/html/rfc7227#section-17>.
>
>
>
Thanks for noting this.  A similar rule applies to to IPv6 RA (
https://www.rfc-editor.org/rfc/rfc4861#section-9):

   Options in Neighbor Discovery packets can appear in any order;
   receivers MUST be prepared to process them independently of their
   order.

Evidently we'll need to change that format in the draft.

Likewise, we don’t want to create a precedent for self-delimiting variable
> length fields but followed the guidelines from these groups to have an
> explicit length field for each variable length.
>

Where is that guidance written?

Here are some examples of concatenated self-delimiting variable-length
fields being used:
* IPv6 NDP Options
  - DNSSL domains (https://www.rfc-editor.org/rfc/rfc8106.html#section-5.2)
  - PvD domains (
https://www.rfc-editor.org/rfc/rfc8801.html#name-pvd-option-for-router-adver
)
  - CARD Sub-Options (https://www.rfc-editor.org/rfc/rfc4066#section-5.1.2.2
)
* DHCP
  - DNS Search List (https://www.rfc-editor.org/rfc/rfc3397.html#section-2)
  - SIP Domain List (https://www.rfc-editor.org/rfc/rfc6011.html#section-4.1
)
  - RDNSS (https://www.rfc-editor.org/rfc/rfc6731.html#section-4.2)
  - Mobility Service (https://www.rfc-editor.org/rfc/rfc5678.html#section-3)

> We need to be careful here because we are in an area where we need to take
> into account what the owners of these protocols maintain the protocol, not
> only what would be perfect for a specific application.
>

I agree.  If we're in doubt about their expectations, let's get more input
from them.

---

From: <[email-masked]>
Date: Thu, 10 Mar 2022 08:31:53 +0000

Ben, 

This comment is unfair because we did already reported to the list that we approached DHCP experts (https://mailarchive.ietf.org/arch/msg/dhcwg/gZ_EbiPVOt-3tARDeQHYLHbh8R8/) and that we followed the feedback we received from them. This is ACKed in the document: 

   Thanks to Bernie Volz for the review of the DHCP part.

For the RA part, here is what I sent to the Chairs back in September 2021:

====

I’m not sure we need to delay the WGLC but whatever the decision taken, I suggest the WGLC call to be also cced to both dhc and 6man WGs. I don’t expect much comments from dhc as they already reviewed the draft but there will always something to hear from 6man.

====

Cheers,

Med

De : Ben Schwartz <[email-masked]> 
Envoyé : mercredi 9 mars 2022 19:40
À : BOUCADAIR Mohamed INNOV/NET <[email-masked]>
Cc : Deen, Glenn <[email-masked]>; [email-masked]
Objet : Re: [Add] WGLC for draft-ietf-add-dnr

On Fri, Mar 4, 2022 at 2:56 AM <[email-masked] <mailto:[email-masked]> > wrote:

Hi Ben, 

I don't think we should attempt to apply these guidelines on our own.  If they are indeed applicable in this case, I'd like to get that confirmed from some relevant subject matter experts.

---

From: Ben Schwartz <[email-masked]>
Date: Thu, 10 Mar 2022 11:59:13 -0500

On Thu, Mar 10, 2022 at 3:31 AM <[email-masked]> wrote:

> Ben,
>
>
>
> This comment is unfair because we did already reported to the list that we
> approached DHCP experts
>

I am aware that your design was based on expert input, and I apologize if
it sounded like I was suggesting otherwise.

> (https://mailarchive.ietf.org/arch/msg/dhcwg/gZ_EbiPVOt-3tARDeQHYLHbh8R8/)
> and that we followed the feedback we received from them.
>

Thanks for sharing this link!  I don't remember seeing it.

I think this guidance is excellent but it isn't phrased as a strict rule,
and I don't think we should treat it as law.  For example, on counts vs.
lengths, it says

> There's also a question as to whether for an array (list) of fixed length
values, whether a length or count is better (of course, one can easily be
converted to the other). I prefer a length as that is more consistent; but
this is something to discuss too.

I think this is a case where that discussion is worthwhile.

Otherwise, I think my proposed format is consistent with this guidance,
treating the SVCB-format data as a "final variable-length fragment".

---

From: <[email-masked]>
Date: Thu, 17 Mar 2022 07:02:56 +0000

Hi Ben, 

Please see inline. 

Cheers,

Med

Envoyé : jeudi 10 mars 2022 17:59

On Thu, Mar 10, 2022 at 3:31 AM <[email-masked] <mailto:[email-masked]> > wrote:

Ben, 

This comment is unfair because we did already reported to the list that we approached DHCP experts

I am aware that your design was based on expert input, and I apologize if it sounded like I was suggesting otherwise.

(https://mailarchive.ietf.org/arch/msg/dhcwg/gZ_EbiPVOt-3tARDeQHYLHbh8R8/) and that we followed the feedback we received from them.

[Med] No problem, but this is unfortunate as I shared that information in a discussion I had with you back in 05/21: https://mailarchive.ietf.org/arch/msg/add/5UInLn5guFvMVCXZhXtNbfFsOmg/. As we didn’t have any follow-up, we thought that you were OK with the explanation and proceeded with other changes agreed in that thread. We reported that as well to the list. 

I think this guidance is excellent but it isn't phrased as a strict rule, and I don't think we should treat it as law.  For example, on counts vs. lengths, it says

> There's also a question as to whether for an array (list) of fixed length values, whether a length or count is better (of course, one can easily be converted to the other). I prefer a length as that is more consistent; but this is something to discuss too.

[Med] As reported back in 05/21 we could maintain “Count” that we had in the early design but we went with “Length” because that was a preference expressed by the DHC expert. That preference is not recorded in RFC7227 and the plan triggered by the discussion I had with Bernie was to edit an update to 7227.   

[Med] Agree, but that discussion should be with the maintainers of these protocols. That’s what we tried to do with dhc and hope to see with 6man. FYI, we went for “count” in draft-ietf-ipsecme-add-ike because that was a preference of the IKE expert. I don’t think this is an issue, but rather another way to convey the same information.  

Otherwise, I think my proposed format is consistent with this guidance, treating the SVCB-format data as a "final variable-length fragment".

[Med] Not sure about this as you are putting a fixed length field after a variable one.