LWP::Authen::Ntlm does not correctly support HTTP 407 [rt.cpan.org #111692]

[oalders]

Migrated from rt.cpan.org#111692 (status was 'new')

Requestors:

• A.Guertin@F5.com

From a.guertin@f5.com on 2016-02-03 00:40:12:

The source code seems to indicate that this library is expected to work for both 401 and 407 since it checks whether it needs to use the �Proxy-Authorization� header or the �Authorization� header in this line:

my $auth_header = $proxy ? "Proxy-Authorization" : "Authorization�;

but does not correctly parse the HTTP response from the server. When scanning the response, the authenticate subroutine only checks for �WWW-Authenticate� headers and not �Proxy-Authenticate� headers. As specified by the HTTP Authentication RFC (RFC 7235), HTTP 407 responses should use the �Proxy-Authenticate� header and not the �WWW-Authenticate� header (see https://tools.ietf.org/html/rfc7235#section-3.2).

I have not tested this, but I also believe that it would be safer for the decision to use the �Proxy-Authorization� header to be based on the HTTP response code (�Authorization� to respond to a HTTP 401, �Proxy-Authorization� to response to a HTTP 407). If a proxy forwards a 401 to the client, this library will incorrectly use the �Proxy-Authorization� header.

From a.guertin@f5.com on 2016-02-03 01:28:57:

I believe I was mistaken about how to choose the authorization header. It seems that the variable $proxy in the line:

    my $auth_header = $proxy ? "Proxy-Authorization" : "Authorization�;


Is set by checking the value of the response code in UserAgent.pm:

    my $proxy = ($code == &HTTP::Status::RC_PROXY_AUTHENTICATION_REQUIRED);

So my last comment in the bug can be disregarded.




On 2/2/16, 4:40 PM, "Bugs in libwww-perl via RT" <bug-libwww-perl@rt.cpan.org> wrote:

>
>Greetings,
>
>This message has been automatically generated in response to the
>creation of a trouble ticket regarding:
>	"LWP::Authen::Ntlm does not correctly support HTTP 407", 
>a summary of which appears below.
>
>There is no need to reply to this message right now.  Your ticket has been
>assigned an ID of [rt.cpan.org #111692].  Your ticket is accessible
>on the web at:
>
>    https://rt.cpan.org/Ticket/Display.html?id=111692
>
>Please include the string:
>
>         [rt.cpan.org #111692]
>
>in the subject line of all future correspondence about this issue. To do so, 
>you may reply to this message.
>
>                        Thank you,
>                        bug-libwww-perl@rt.cpan.org
>
>-------------------------------------------------------------------------
>The source code seems to indicate that this library is expected to work for both 401 and 407 since it checks whether it needs to use the �Proxy-Authorization� header or the �Authorization� header in this line:
>
>
>
>my $auth_header = $proxy ? "Proxy-Authorization" : "Authorization�;
>
>
>
>but does not correctly parse the HTTP response from the server. When scanning the response, the authenticate subroutine only checks for �WWW-Authenticate� headers and not �Proxy-Authenticate� headers. As specified by the HTTP Authentication RFC (RFC 7235), HTTP 407 responses should use the �Proxy-Authenticate� header and not the �WWW-Authenticate� header (see https://tools.ietf.org/html/rfc7235#section-3.2).
>
>
>
>I have not tested this, but I also believe that it would be safer for the decision to use the �Proxy-Authorization� header to be based on the HTTP response code (�Authorization� to respond to a HTTP 401, �Proxy-Authorization� to response to a HTTP 407). If a proxy forwards a 401 to the client, this library will incorrectly use the �Proxy-Authorization� header.

[damif512]

Hi,

I don't have a complete understanding of the whole libwww-perl project however regarding this issue opened since years, the following fix from Wataru Taniguchi works very well : https://github.com/wataniguchi/NTLM-proxy-auth
There's just a minor fix needed in the ntlm_reset sub I just reported, cf. wataniguchi/NTLM-proxy-auth#1
If someone is able to review and include it in future releases it would be great.

Thank you,

[oalders]

If someone could put together a pull request with the change and an accompanying test, I'd be happy to review it.

I didn't know about that repo with the diff. The diff looks to be fairly large. If we could pare it down to only the required changes, that would make for an easier review.

[damif512]

Hi oalders,

It seems that a similar pull request was opened years ago and is still opened : #49
The diff from Wataru Taniguchi for LWP::Authen::Ntlm is largely based on this pull request as he wrote in the README.

[oalders]

Thanks @damif512, that PR predates my involvement here. I wasn't aware of it. If someone want to fix the merge conflicts, we can look at shipping the fix. Ideally it would include a test as well.