Subject: [apps-discuss] WGLC: draft-ietf-appsawg-http-forwarded-02.txt

From: Alexey Melnikov <[email-masked]>
Date: Tue, 01 May 2012 19:26:50 +0100

Dear WG participants,
I would like to initiate WG Last Call on 
draft-ietf-appsawg-http-forwarded-02.txt ("Forwarded HTTP Extension"). 
Please send your reviews, as well as expressions of support regarding 
document readiness for IESG (or not) either to the mailing list, or 
directly to WG chairs (Murray Kucherawy <[email-masked]> and myself). 
Comments like "I've read the document and it is Ok to publish" or "I've 
read the document and it has the following issues" are useful and would 
be gratefully accepted by chairs.

The WGLC will end on Friday, May 18th.

Thank you,
Alexey as an APPSAWG co-chair.

---

From: Ben Niven-Jenkins <[email-masked]>
Date: Wed, 2 May 2012 16:59:15 +0100

Colleagues,

I'm forwarding the following comments on draft-ietf-appsawg-http-forwarded-02 on behalf of John (cc'd) who is our resident HTTP expert but doesn't subscribe to the apps-discuss list.

Thanks
Ben

=== Start of John's comments:
0. First, I approve of the proposal in general: it is a big
improvement over the current situation with multiple non-standardised
headers partially/incompletely implemented by different software.

1. The ABNF gives:

   forwarded-pair = token "=" ( token / quoted-string )
   token = 1*( /[-!#$%&'*+.^_`|~0-9A-Z]/ ) # httpbis-p1 S3.2.4

It is a happy coincidence that an IPv4address without an optional
port happens to fall within the token syntax and therefore does
not need to be a quoted-string, but neither an IPv4address with
the optional port (which involves ":" which is not in tchar) or
an IPv6address with or without port (which involve "[", "]" and ":"
none of which are in tchar) are valid tokens and MUST be sent
as quoted-string.

What might casually seem to some to be the same syntax element
therefore has different quoting/escaping rules depending on its
exact makeup. Generators need to beware. Some might take the view
that it is safest and easiest to simply use quoted-string in all
cases, so consumers also need to beware that even an IPv4address
with no port might be sent as a quoted-string.

I think this should be explicitly called out, given this draft gets
it wrong itself (see below).

2. The example in S4 is both wrong and odd.

  Forwarded: For=192.0.2.43,"for=[2001:db8:cafe::17]:47011"
  Forwarded: proto=https;by=198.51.100.60

By #rule this expands to a 3-element list

  For=192.0.2.43
  "for=[2001:db8:cafe::17]:47011"
  proto=https;by=198.51.100.60

The second element does not match the ABNF: the first quote should
be after the "=".

The third element, while valid, looks like it might have been
intended to be a run-on of the second thus creating a 2-element
list where the second element has 3 parameters. (It's not clear how
a client send an IPv6 request to an IPv4 endpoint though!) If this
is deliberate then fine, but it perhaps makes for a confusing example.

3. S5.1 ("by" parameter) says:

   This is primarily added by reverse proxies that wish to forward this
   information to the backend server.

I would add that this is especially useful for multi-homed proxies
where the address that the backend server sees the forwarded request
coming from is not the same as the address the proxy received the
request on. (Where the two addresses are the same it seems superfluous.)

4. S5.2 ("for" parameter) says:

   The
   last proxy's IP address, and optionally a port number, are, however,
   readily available as the remote IP address of the TCP/IP connection.

As above, the inbound address is not necessarily the same as the
outbound address of the proxy. The "by" parameter where present might
be a more valuable source of the "proxy's IP address".

5. S6.1 (address identifiers) says:

   The IPv6address SHOULD comply with textual representation
   recommendations [RFC5952] (e.g., lowercase, zero compression).

But RFC 5952 defines a standard mechanism for generating the shortest
(most compressed) textual representation of an IPv6 literal.

6. S7 says:

    As an example, the header field

       Forwarded: for=192.0.2.43,for=[2001:db8:cafe::17],for=unknown

This is not valid syntax. The IPv6address contains non-tchars and must
be transmitted as a quoted-string.

7. S7 also says:

   Note that this header field is relevant on a per request basis and
   MUST NOT be cached.

It's not entirely clear where this comes from. Request headers are not
normally cached, unless the origin invokes the Vary mechanism. Is it
intended that this should disable the ability of an origin to say
"Vary: Forwarded"? I can't see why this needs to be done (and in any
case won't work with intermediates unaware of this specification.)

   Also note that this header field MUST NOT be
   preserved across redirects.

This seems to presume an intermediate which follows redirects internally,
preserving the original request state for each sub-request, and presents
the final target as a response to the original incoming request. This
seems to be an inadvisable thing to do in any case. (I had a vague idea
it was prohibited, but can't find specific language in httpbis that
provides guidance in either direction - apart from perhaps the p1-S3.1.1
direction to issue 301 responses to invalid request-lines rather than
transparently fixing them. That seems closely related.)

Instead, it might be worth noting when the incoming Forwarded header
should be preserved or discarded:

* Obviously a directly forwarded request should preserve/extend it.

* If a single incoming request causes an intermediate to make multiple
  related inbound requests, then presumably one is a more direct
  analog of the original request and should preserve/extend the header,
  whereas the others are "internal" and should filter it out.

* Unless of course the intermediate has detected a content mismatch in
  a 304 response and is following the httpbis-p4 S4.1 instructions to
  repeat the request unconditionally, in which case the new request
  is still basically a direct consequence of the origin request and
  the header should probably be kept.

8. Because Via is well established and mandatory and this is new and
optional, consumers should also beware that the elements of Via may
not match the elements of Forwarded: so although proto= may be present
one will probably not be able to infer the corresponding HTTP version
number, or the software version taken from Via of a particular
intermediate identified within Forwarded. This remaining mismatch of
capabilities (which within the X-Forwarded-* family this specification
is intended to address) is perhaps unfortunate.

=== End of John's comments

On 1 May 2012, at 19:26, Alexey Melnikov wrote:

> Dear WG participants,
> I would like to initiate WG Last Call on draft-ietf-appsawg-http-forwarded-02.txt ("Forwarded HTTP Extension"). Please send your reviews, as well as expressions of support regarding document readiness for IESG (or not) either to the mailing list, or directly to WG chairs (Murray Kucherawy <[email-masked]> and myself). Comments like "I've read the document and it is Ok to publish" or "I've read the document and it has the following issues" are useful and would be gratefully accepted by chairs.
> 
> _______________________________________________
> apps-discuss mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/apps-discuss

---

From: Andreas Petersson <[email-masked]>
Date: Fri, 4 May 2012 11:05:15 +0200

Hi,

Thanks for the feedback!
See inlined comments below. 

/Andreas

> it wrong itself (see below).

Right. I missed that those chars isn't part of tchar. 
I will add some warnings about this and also make sure 
to get it right in the examples. :-)

> The second element does not match the ABNF: the first quote should

Typo.

> is deliberate then fine, but it perhaps makes for a confusing example.

It was intended to be a list with three elements, however not entirely
thought through. The by-address in the last element should have a
matching address family with the for-parameter in the second element.

Maybe a simple example would be better in this section and a more
complex example in section 7.1?

> request on. (Where the two addresses are the same it seems superfluous.)

Indeed.

> be a more valuable source of the "proxy's IP address".

Yes, in some use cases it wouldn't matter though.
A note on that would probably be in place though.

> (most compressed) textual representation of an IPv6 literal.

I do not fully understand the complaint here.

The formulation above is however suggested from Dan Wing (@cisco),
if the WG has a better idea on how to recommend textual representation
of IPv6 addresses you are welcome to suggest them.
This was however discussed also on the http-bis mailing list, so I
suggest to read the old discussions first.

> be transmitted as a quoted-string.

Right, this is already mentioned in #1.

>   the header should probably be kept.

The intention was to clarify (and satisfy
draft-ietf-httpbis-p2-semantics-19#section-3.1). Your suggestion here
seems better though.

> is intended to address) is perhaps unfortunate.

Yes, a note on that would be in place. 

(The document do support the possibility to extend this header field 
to also contain stuff like proxy-software and version...)

---

From: Julian Reschke <[email-masked]>
Date: Sat, 05 May 2012 13:06:03 +0200

On 2012-05-04 11:05, Andreas Petersson wrote:
> ...

+1

The alternative would be to use a different syntax, not reusing HTTP 
parameter syntax. I think this would be a mistake, though; it just needs 
even more custom parsers need to be written.

So I think examples and a big fat warning in the prose are the right 
thing to do here.

Best regards, Julian

---

From: Julian Reschke <[email-masked]>
Date: Sat, 05 May 2012 13:10:37 +0200

Hi there,

first of all: thanks for taking the work, Andreas & Martin!

I've got one nit right now:

>    Given that a proxy wishes to add a Forwarded header field to the
>    outgoing request, if the incoming request has no such header field,
>    the proxy simply adds the header with the list of parameters desired.
>    If, on the other hand, the incoming request has such a header field,
>    the proxy adds a comma and the list of parameters.  A proxy MAY
>    remove all Forwarded header fields from a request.  It MUST, however,
>    ensure that the correct header field is updated in case of multiple
>    Forwarded header fields.

As the header field uses HTTP list syntax, it's also possible (and maybe 
simpler) to simply add another instance to the list of headers.

Best regards, Julian

---

From: Willy Tarreau <[email-masked]>
Date: Sat, 5 May 2012 16:41:21 +0200

Hi Julian,

> simpler) to simply add another instance to the list of headers.

That's the point I wanted to add too. Haproxy does this right now, as
well as stunnel (with appropriate patch). It's generally the easiest
solution on gateways which modify as little the input stream as possible.

Best regards,
Willy

---

From: John Sullivan <[email-masked]>
Date: Wed, 9 May 2012 16:17:56 +0100

> I do not fully understand the complaint here.

My apologies - on further reflection I see that the phrase
"zero compression" should be interpreted as "compression of zeroes".
I had misread it as "no compression"/"without compression", which is
completely the opposite of what that RFC recommends.

> suggest to read the old discussions first.

Yup, I see that that wording was suggested there as-is. I
completely agree that following RFC 5952 is the right thing to
do here, but suggest that phrasing it as "compression of zeroes"
avoids an ambiguity that seems to contradict the actual intent.

John
--

---

From: Andreas Petersson <[email-masked]>
Date: Mon, 14 May 2012 17:19:29 +0200

On Tue, 01 May 2012 19:26:50 +0100

> The WGLC will end on Friday, May 18th.

We are closing in on May 18:th now. We have got plenty of good input.

To summarize, I have made a preliminary change log, covering things
that has been discussed this far. I will also mention ideas that we do
not intend to incorporate in this document.

If you have suggested something that is not in the list below I may
have missed that, please send a reminder to me in such case.

If you disagree with something in the list or have other ideas, please
let me know.

*** Intended changes ***

1.1, Section 4: Clearly mention that IPv6-addresses must be quoted.
     Also show this in examples. This also applies to IPv4 addresses
     when the port is specified.

1.2, Section 4: Have a less complex example. Also, make sure that the
     quote is placed on the right side of the "=".

1.3, Section 4: Add a note that a proxy can also add a new
     "Forwarded: .."-line, as this is equivalent. 

1.4, Section 5.*: Remove some MAY-references.

1.5, Section 5.1: Add a note that the by-parameter may be useful in a
     multi-homed environment. 

1.6, Section 5.2: Add a note that in some situations it is more relevant
     to read the address of the last proxy in the last
     Forwarded-by-field.

1.7, Section 5.2: Formulate paragraph 1 to include that the information
     is not only regarding the initiating client. Also change "user
     agent" to "client".

1.8, Section 5.5: Change the requirement to notify IANA into:
>"It is possible to register additional parameters using the IANA
>registration policy described in [RFC3864]"

1.9, Section 6: Require obfport to start with an underscore. 

1.10, Section 6 & 6.3: Include "[:._-]" as valid characters in obfnode
      and obfport. 

1.11, Section 6.1: s/zero compression/compression of zeroes/

1.12, Section 6.1: s/IPv6 adress/IPv6 address/

1.13, Section 7: Add some notes on when the header should be preserved
      or not. Duscussed under #7:
http://www.ietf.org/mail-archive/web/apps-discuss/current/msg05535.html

1.14, Section 7: Note that this header field is not possible to combine
      with the information from the via-header field with certainty.

1.15, Section 7.1: Remove the word "correctly" from: 
      "[...] information might not be correctly updated [...]"

1.16, Section 7.x: Encourage proxies to convert X-Forwarded-*
    when possible. 

1.17, Section 8.2: Add the text W. Tarreau mentions:
      (with the change of must -> should in the first sentence)
> This header field should never be copied into response messages by
> origin servers or intermediaries for whatever reason as it can reveal
> the whole proxy chain to the client. As a side effect, special care
> must be taken in hosting environments not to allow the TRACE request
> where the Forwarded field is used, as it would appear in the body of
> the response message.

1.18, Section 8: Add a section or a note about privacy considerations.

*** Suggestions we intend NOT to incorporate ***
(somewhat incomplete)

2.1, TCP-options. This can be standardized in a separate document.

2.2, Complex transition schemes. 

Best regards,
 Andreas Petersson

---

From: Andreas Petersson <[email-masked]>
Date: Mon, 14 May 2012 17:50:31 +0200

On Mon, 14 May 2012 17:19:29 +0200
> 1.10, Section 6 & 6.3: Include "[:._-]" as valid characters in obfnode

":" is obviously not suited in obfnode/obfport.
I suggest:
obfnode = "_" 1*( ALPHA / DIGIT / "." / "_" / "-" )
obfport = "_" 1*( ALPHA / DIGIT / "." / "_" / "-" )

 /andreas