[apps-discuss] APPSDIR review of draft-ietf-appsawg-http-forwarded-02

Salvatore Loreto <salvatore.loreto@ericsson.com> Mon, 14 May 2012 13:40 UTC

Message-ID: <4FB10B4F.80906@ericsson.com>
Date: Mon, 14 May 2012 16:40:31 +0300
From: Salvatore Loreto <salvatore.loreto@ericsson.com>
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:12.0) Gecko/20120428 Thunderbird/12.0.1
MIME-Version: 1.0
To: "apps-discuss@ietf.org" <apps-discuss@ietf.org>, draft-ietf-appsawg-http-forwarded@tools.ietf.org
Content-Type: multipart/alternative; boundary="------------040207010809050508070206"
Subject: [apps-discuss] APPSDIR review of draft-ietf-appsawg-http-forwarded-02
Precedence: list



I have been selected as the Applications Area Directorate reviewer for 
this draft
(for background on appsdir, please see  
http://trac.tools.ietf.org/area/app/trac/wiki/ApplicationsAreaDirectorate).

Please resolve these comments along with any other Last Call comments 
you may receive.
Please wait for direction from your document shepherd or AD before 
posting a new version of the draft.


Document: draft-ietf-appsawg-http-forwarded-02

Title: Forwarded HTTP Extensions
Reviewer: Salvatore Loreto
Review Date: 2012-05-14
IESG Telechat Date:


Summary:
I have read and reviewed this draft.
The draft is really clear on describing the issues and the proposed 
solution,
it is almost ready for publication  however there are some issues that need
to be addressed before publication.



Major Issues:
-----------
Section 4

the "token" syntax is currently defined to be aligned to the HTTP 
parameter syntax as defined in
draft-ietf-httpbis-p1-messaging-19#section-3.2.4
however this syntax does not allow to insert an IPv4address with the 
optional port
or an IPv6address with or without port.


Minor Issues:
-----------
-Section 4

last paragraph states

    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.


It should also list the possibility to simply add another Forwaded: instance
to the list of headers (as the header field uses HTTP list syntax).


-Section 5.2

    The "for" parameter is used to disclose information about the user
    agent that initiated the request.


s/user agent/client
The sentence is supposed to give a general description of the parameter,
using the term user agent seems to suggest that the parameter is only for
user agents.

    Typically the value of this
    parameter is an IP address, but it MAY also be some other kind of
    identifier.


The normative MAY is not necessary since it is only an informative 
example of usage.


-Section 5.3

    This MAY be used for example by the origin
    server if a reverse proxy is rewriting the "Host" header field to
    some internal host name


The normative MAY is not necessary since it is only an informative 
example of usage.


- Section 5.5

    IANA MUST in such cases be notified, and parameters
    should be registered according to [RFC3864].

it would be better to rephrase it in something like:
"It is possible to register additional parameters using the IANA 
registration policy
described in [RFC3864]"

- Section 6.3

It is not clear to me how  a randomly generate value in Forwarded header 
field
can be used for tracing and debugging, and especially why it would be 
necessary this
header for debugging.
Moreover I agree that in case you do not want to disclose information it 
would be better
don't add the "Forwarded: for" header field at all as a random 
identifier would end up
to provide extra information anyway.


- Section 8.1

It should be clarified that the header is not supposed at all to be 
inserted in an answer
as it would reveal the full proxy chain to the client.
Also it usage with a TRACE request should be clarified.



Nits:
----

- Section 4

    Example:

        Forwarded: For=192.0.2.43,"for=[2001:db8:cafe::17]:47011"

it should be

	Forwarded: For=192.0.2.43,for="[2001:db8:cafe::17]:47011"



- Section 6.1

in the last sentence "an IPv6 adress" ' -> "an IPv6 address"


- Section 7

IPv6 addresses must be transmitted as quoted-string

i.e. for=[2001:db8:cafe::17]  should be for="[2001:db8:cafe::17]"



best regards
/Sal

-- 
Salvatore Loreto, PhD
www.sloreto.com

[apps-discuss] APPSDIR review of draft-ietf-appsa… Salvatore Loreto
Re: [apps-discuss] APPSDIR review of draft-ietf-a… Andreas Petersson
Re: [apps-discuss] APPSDIR review of draft-ietf-a… Salvatore Loreto
Re: [apps-discuss] APPSDIR review of draft-ietf-a… Andreas Petersson