Subject: draft-duke-quic-load-balancers-05.txt

From: Sowmini Varadhan <[email-masked]>
Date: Mon, 28 Oct 2019 15:49:56 -0400

Hi Martin, Nick, 

I read through
  https://www.ietf.org/id/draft-duke-quic-load-balancers-05.txt
and had a few comments and questions..

- (Editorial) s/encoding/encode at line 306 in Section 2.4
 302 2.4.  Load Balancer Chains
 303 
 304    While it is possible to construct a scheme that supports multiple
 305    low-state load balancers in the path, by using different parts of the
 306    connection ID to encoding routing information for each load balancer,
  :

- Section 3.2: Config rotation is one octet (per Section 3.3) and 
  there are 2 bits for config rotation, so it is confusing when Section 3.2
  says
 361    received these, it MUST generate connection IDs with the config
 362    rotation bits set to '0b11' and MUST use the "disable_migration"
  (0b11 is 2 bytes? In general, it would help to provide more context about
  what/when/how this can happen, and why it is needed)

- Section 4 ("Routing Algorithms"). The 0-RTT can have application data.
  Given that Section 4 discusses an "alternate algorithm" for 0-RTT,
  is it possible that the data from 0-RTT gets routed differently than
  the remainder of the packet? 

- (Editorial) The third paragraph in Section 4.1.2 referes to a "figure
  below" that seems to be missing? I am referring to the lines below:
 457 4.1.2.  Server Actions
  :
 468    The figure below clarifies the format.  The first two bits are
 469    reserved for config rotation.  The server can assign the next 6 bit
  :

- General editorial comment: missing a defintions section for acronyms
  like SCID and token. I realize that the Normative reference here is
  [QUIC-TRANSPORT] but it may be helpful to include a cheat-sheet of the
  main concepts like Initial, 0-RTT, Retry, SCID, Token (to name a few)

- Section 4.3.1 refers to "token". Is this the token from the Initial?
  From the Retry? The LB is just a service in the middle, between the
  2 quic endpoints, so it would help to explain which token to use. 
  To be clear, I am referring to:
 604    The load balancer decrypts the server ID using 128-bit AES Electronic
 605    Codebook (ECB) mode, much like QUIC header protection.  The nonce
 606    octets are padded to 16 octets using the as many of the first octets
 607    of the token as necessary.  AES-ECB encrypts this nonce using its key
 608    to generate a mask which it applies to the encrypted server id.

- Editorial: s/decrypts/encrypts in second paragraph of Section 4.3.2:
 631 4.3.2.  Server Actions
  :
 640    The server decrypts the server ID using 128-bit AES Electronic
 641    Codebook (ECB) mode, much like QUIC header protection.  The nonce

- Section 5: this one was particularly hard for me to read. I think it 
  would help if this section of the draft hsd a top-level discussion on 
  why this is needed (i.e., what are the sort of things that can go
  wrong if it is not there? Also, in Figure 5, why do we need to encode
  ODCIL and ODCID in the Retry token, given that they are already there
  in the Retry packet itself?)

--Sowmini

---

From: Martin Duke <[email-masked]>
Date: Tue, 29 Oct 2019 13:25:01 -0700

Hi Sowmini,

Thanks for the review. I'll file a PR for your editorial changes.

Responses inline:

On Mon, Oct 28, 2019 at 12:50 PM Sowmini Varadhan <[email-masked]>
wrote:

> <snip>

0b11 is 2 bits. The '0b' notation indicates each digit is a bit; '0x' would
be hexadecimal, where 2 characters = 1 Bytes.

>   the remainder of the packet?

In section 4, it says: " The load balancer SHOULD route Initial and 0-RTT
packets from the client using an alternate algorithm". The point here is
that the destination CID for these packets is chosen by the client. It may
not be long enough to apply the chosen algorithm, so something else should
be done. However, " This algorithm SHOULD generate consistent results for
Initial and 0RTT packets that arrive with the same source and destination
connection ID" so they all end up at the same place. If the LB doesn't do
this, then the 0RTT packets will be lost. This is suboptimal but doesn't
break the protocol.

<snip>

>  608    to generate a mask which it applies to the encrypted server id.
This is the QUIC-LB authentication token, which I should clarify. In any
case, I don't believe that servers get this token if there's out-of-band
configuration, so I need to fix this.

>   in the Retry packet itself?)

This is for the benefit of the QUIC server. When the server gets an Initial
Packet with a Retry token in it, it needs to extract the ODCID from that
token so that it can send it back in a transport parameter. If the client
got a Retry but then doesn't get the ODCID transport parameter, it means
there's an unauthorized man in the middle and the client should terminate
the connection.

---

From: Sowmini Varadhan <[email-masked]>
Date: Tue, 29 Oct 2019 17:04:28 -0400

>    Thanks for the review. I'll file a PR for your editorial changes.

sounds good, thanks for doing this!

>    0b11 is 2 bits. The '0b' notation indicates each digit is a bit; '0x'
>    would be hexadecimal, where 2 characters = 1 Bytes.

ah, this is the NIC data-sheet notation, that got me by surprise.
If there is no strong objection, can we just list the code-points
as "11" (I think this is the convention followed in other ietf rfcs,
e.g., 3168, for the ECT code-points). 

>    this, then the 0RTT packets will be lost. This is suboptimal but doesn't

ok, perhaps it would be helpful to add some discussion around this
in the draft.

[discussion for "Section 4.3.1 refers to "token"]
>    configuration, so I need to fix this.

Ok.

[discussion around Section 5]
>    This is for the benefit of the QUIC server. When the server gets an
>    Initial Packet with a Retry token in it, it needs to extract the ODCID
>    from that token so that it can send it back in a transport parameter. If
>    the client got a Retry but then doesn't get the ODCID transport parameter,
>    it means there's an unauthorized man in the middle and the client should
>    terminate the connection.

I see. In general, it would help to provide some context in this section 
about why any tweaks in the Retry Service are needed at all, along with 
your explanation above.

Thanks!

--Sowmini