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 <msk@cloudmark.com> 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.
> 
> _______________________________________________
> apps-discuss mailing list
> apps-discuss@ietf.org
> https://www.ietf.org/mailman/listinfo/apps-discuss