Proxying UDP in HTTP
draft-ietf-masque-connect-udp-15

# Internet AD comments for {draft-ietf-masque-connect-udp-14}
CC @ekline

## Comments

### S3.5

* Should the 2xx discussion here include additional reference to the extended
  commentary in masque-h3-datagram#section-3.2?

### S7

* I *really* wish there could be more said about tunnel security.  I was
  tempted to try to formulate some DISCUSS on the matter, but I cannot
  seem to see where any more extensive text was ever written for the CONNECT
  method.

  Nevertheless, there are plenty of concerns that could be mentioned, like
  what should a proxy do if the target_host is one if its own IP addresses,
  or a link-local address, and so on.

  I'm not entirely convinced that it suffices to claim that because the
  tunnel is not an IP-in-IP tunnel certain considerations don't apply.
  Anything and everything can be tunneled over UDP, especially if RFC 8086
  GRE-in-UDP is what the payloads carry (or ESP, of course).  It seems like
  most of the points raised in RFC 6169 apply; perhaps worth a mention.

  But as I said, I think all these concerns apply equally to CONNECT (in
  principle, anyway), and the text here is commensurate with the text for
  its TCP cousin (that I could find, anyway).

I support Zahed's DISCUSS.

I also concur with Paul's comment about MUST/SHOULD weirdness, and Rob's comment about the choice of the well-known name registered here.

Looks good, just two comments:

    [... MUST ....]
    Clients SHOULD validate the requirements above;

This is a rather odd construct. The requirements contains MUST items,
which are now downgraded via a SHOULD encapsulation. Is this construct
really needed, or can it be removed?

    In IPv4, the Don't Fragment (DF) bit MUST be set if possible

When is this "not possible" ?

Thank you to Catherine Meadows for the SECDIR review.

** Section 3.  The MASQUE charter says “The primary goal of this working group is to develop mechanism(s) that allow configuring and concurrently running multiple proxied stream- and datagram-based flows inside an HTTPS connection.”  I’m not sure this is necessarily the right section, but the text should repeat the charter constraint somewhere  -- that this mechanism is only appropriate if the client sends it over HTTPS.

** Section 3.1.
   UDP proxies MUST NOT introduce fragmentation at the IP layer when
   forwarding HTTP Datagrams onto a UDP socket.  In IPv4, the Don't
   Fragment (DF) bit MUST be set if possible, to prevent fragmentation
   on the path.  

Why is the “if possible” caveat needed on the guidance to set the DF bit?

** Section 7.
   HTTP
   servers that support UDP proxying ought to restrict its use to
   authenticated users.

Did the WG discuss stronger guidance than “ought”, perhaps SHOULD?  If not, is there is reason why NOT to be more prescriptive?

** Section 7

Thanks for ongoing discussion around the SECDIR Review.  I had difficulty following the distinctions between the theory and the practice of the TCP vs. UDP.  I’m also not sure if everyone’s data would back-up the claim that only open ports are targeted. 

I recommend replacing the paragraph starting with “UDP proxies have similar properties to TCP proxies …” with more narrow text around specific DoS attack behaviors.  Something roughly as follows (by all means edit it) …

NEW
UDP proxies share many similarities to TCP CONNECT proxies when considering them as infrastructure for abuse to enable denial-of-service attacks.  TCP CONNECT and UDP proxies can obfuscate the source address of the attack traffic (from the proxy client).  In the case of a stateless volumetric attack (e.g., TCP SYN flood or a UDP flood) both types of proxies pass the traffic to the target host.  With stateful volumetric attacks (e.g., HTTP flooding) being sent over a TCP CONNECT proxy, the proxy will only send data if it gets TCP SYN-ACKs from the target.  Once the path to the target_host or the target_host itself is flooded or otherwise resource exhausted, it will stop responding, and so the proxy will stop sending data for that connection. UDP doesn't have this session setup, so a UDP proxy could continue sending data to the target_host.

Thank you to Al Morton for the useful OpsDir review, and to the authors for working with Al to address the comments -- I think that it noticeably improved the document. 
I think that the document is both useful and clear -- but, while copying my ballot comments from my tablet into the tool I read Rob's comments, and strongly support his "what is this masque thing of which you speak?" (well, how will anyone understand this is 10 years, and would e.g: .well-known/udp-proxy be better?) and also his "a picture is worth a thousand words comment -- having a simple ASCII art diagram explaining the players would make this much much simpler for folk like myself to understand this mechanism.

Based on this PR (https://github.com/ietf-wg-masque/draft-ietf-masque-connect-udp/pull/178), I am clearing my discuss but keeping the comments (hopefully the author will response to the comments as he promised :-)).

Thanks a lot for working on this specification. I think this specification will be very useful.

I don't have other major issues other than mentioned in the discuss points. Below are some comments which I believe will improve the document if addressed.

- Section 3: s/pct-encoding/percent-encoding and would be good to add a reference to pct-encoding (https://datatracker.ietf.org/doc/html/rfc3986)

 - Section 3.1: it took a bit of time for me to understand if we are redefining the "intermediaries" here again. We have already described what is an intermediary in the context of this document. Hence, I would suggest that we just write - if the recipient is an intermediary, it forwards ....

 - Section 3.1: what exactly "fail the request" means? is it only a notification to some waiting processes/methods or sending the error message or both? can we be more specific here?

 - Section 5 : reference to the QUIC DATAGRAM frame would be nicer.

 - Section 5 : it says -

 		"Therefore, endpoints MUST NOT send HTTP Datagrams with a Payload field longer than 65527 using Context ID zero. An endpoint that receives a DATAGRAM capsule using Context ID zero whose Payload field is longer than 65527 MUST abort the stream. "

 	I assume the HTTP Datagram and a DATAGRAM capsule is referring to the same thing, but this is confusing. Can use HTTP DATAGRAM in the second sentence as well? The DATAGRAM capsule kind of appeared in this section all on a sudden.

In this section to me it seems HTTP DATAGRAM and DATAGRAM capsule is interchangeably used, if I am correct then I think it is worth mentioning this for the sake of better understanding.

- Section 5 : it says

	"If a UDP proxy knows it can only send out UDP packets of a certain length due to its underlying link MTU, it SHOULD discard incoming DATAGRAM capsules using Context ID zero whose Payload field is longer than that limit without buffering the capsule contents."

     It is not clear why "SHOULD" is used here? what are the other option the UDP proxy has in this case?

- Section 3.6: if this section should be removed then [STRUCT-FIELD] must not be in the normative section, actually should be removed from the references. Either at this stage of this document we can remove this section or we can add a note to remove normative reference when this section is removed.

# Éric Vyncke, INT AD, comments for draft-ietf-masque-connect-udp-14
CC @evyncke

Thank you for the work put into this document. 

Please find below some non-blocking COMMENT points (but replies would be appreciated even if only for my own education).

Special thanks to Eric Kinnear for the shepherd's detailed write-up including the WG consensus and the justification of the intended status. 

I hope that this helps to improve the document,

Regards,

-éric


## COMMENTS

### Non-explicit well-know

I second Rob's comment on the use of '.well-known/masque' rather than '.well-known/proxy/udp'. This also applies to the examples in section 2.

### Section 3.1 fragments

What happens when the UDP proxy cannot transmit a received UDP payload without fragmenting it ?

### Section 3.1 DNS

Should the UDP proxy prefers AAAA vs. A ?

### Section 8.1

This document leaves the door open to proxying other protocols, so why is "connect-udp" used if the specs could cover other protocols than UDP ?

### Section 8.2

As written above, I support Rob's point about using "masque".

## Notes

This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
[`ietf-comments` tool][ICT] to automatically convert this review into
individual GitHub issues. 

[ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
[ICT]: https://github.com/mnot/ietf-comments

Having reviewed this before reading through other positions, I did walk away with an uneasy feeling about the security section and was struggling to find a way to articulate it.  Having said that, and having read the comments in other ballot positions however, I must support Erik's comments, though I to couldn't really find a way to formulate a discuss.  Sorry for this comment is a little obscure!  I think a wider description of the security issues around this though would make me more comfortable.

# GEN AD review of draft-ietf-masque-connect-udp-14

CC @larseggert

Thanks to Christer Holmberg for the General Area Review Team (Gen-ART) review
(https://mailarchive.ietf.org/arch/msg/gen-art/FOZ1oMqVJD5c0FQlSPgxnoG5qVE).

## Comments

### Section 3.1, paragraph 0
```
  3.1.  UDP Proxy Handling
```
This section as well as Section 6 later in the document have some overlap with
the guidance in RFC8085, specifically, its Section 3. It might be good to align
the guidance.

## Nits

All comments below are about very minor potential issues that you may choose to
address in some way - or ignore - as you see fit. Some were flagged by
automated tools (via https://github.com/larseggert/ietf-reviewtool), so there
will likely be some false positives. There is no need to let me know what you
did with these suggestions.

### Grammar/style

#### Section 3.1, paragraph 3
```
her the destination is reachable. Therefore it needs to respond to the reques
                                  ^^^^^^^^^
```
A comma may be missing after the conjunctive/linking adverb "Therefore".

#### Section 3.6, paragraph 2
```
located within a given HTTP namespace but MAY be allocated in any order. The
                                     ^^^^
```
Use a comma before "but" if it connects two independent clauses (unless they
are closely connected and short).

#### Section 9.1, paragraph 12
```
ke to thank Eric Rescorla for suggesting to use an HTTP method to proxy UDP.
                              ^^^^^^^^^^^^^^^^^
```
The verb "suggesting" is used with the gerund form.

## Notes

This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
[`ietf-comments` tool][ICT] to automatically convert this review into
individual GitHub issues. Review generated by the [`ietf-reviewtool`][IRT].

[ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
[ICT]: https://github.com/mnot/ietf-comments
[IRT]: https://github.com/larseggert/ietf-reviewtool

[Updated with a further comment after reviewing the HTTP Datagram draft.]

Hi David,

Thank you for document.  I have only two minor comments:

(1) I note that MASQUE turns up in the WG name, but not in the draft name, and yet it is used in the example URLs and the also the .well-known protocol name.  Hence, I was wondering whether in future, once the MASQUE WG is no more, whether folks will find it harder to connect the .well-known/masque definition back to the protocol spec, and whether choosing a name like udp-proxy might be better?  Of course, the IANA .well-known registry can link the protocol name back to the RFC that defines it.

(2)
   target_host supports using DNS names, IPv6 literals and IPv4
   literals.  Note that this URI Template expansion requires using pct-
   encoding, so for example if the target_host is "2001:db8::42", it
   will be encoded in the URI as "2001%3Adb8%3A%3A42".

Am I right to presume zone-identifier allowed in the IPv4/6 literals?  Is it worth explicitly mentioning this?

(3) They say that a picture is worth a thousand words.  I wonder whether a simple diagram in the introduction to show the relationship between the client, proxy and server may be beneficial to help quickly align the casual reader?

(4) 
   To allow negotiation of a tunnel for UDP over HTTP, this document
   defines the "connect-udp" HTTP Upgrade Token.  The resulting UDP
   tunnels use the Capsule Protocol (see Section 3.2 of [HTTP-DGRAM])
   with HTTP Datagram in the format defined in Section 5.

This text implies that it using "connect-udp" will always make use of the Capsule protocol (say, even when proxying over HTTP/3).  Is this correct, or should this text perhaps be clarified?

All of these are just suggestions and I'm happy to leave it to the authors/WG on how they want to treat them.

Regards,
Rob