From 0e642232d662ec8742bbabf525496ca330e6835c Mon Sep 17 00:00:00 2001 From: Paul van Tilburg Date: Wed, 28 Feb 2018 10:16:07 +0100 Subject: [PATCH 1/3] Only parse the digest challenge part of the response header It is allowed to provide multiple authentication schemes with optional challenges in the WWW-Authenticate headers. This fixes that it always took the first scheme, and thus failed on a header such as: WWW-Authenticate: NTLM, Digest .... Also frameworks such as Faraday group multiple WWW-Authenticate headers in a response, for example WWW-Authenticate: Digest ... WWW-Authenticate: NTLM into: WWW-Authenticate: Digest ..., NTLM so there is a single way of retrieving header data. This commit changes the parser to pick out only the digest challenge part and the authentication parameters until it encounters another scheme (if any). --- lib/net/http/digest_auth.rb | 4 ++-- test/test_net_http_digest_auth.rb | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/net/http/digest_auth.rb b/lib/net/http/digest_auth.rb index 98feecb..391f4ab 100644 --- a/lib/net/http/digest_auth.rb +++ b/lib/net/http/digest_auth.rb @@ -79,9 +79,9 @@ def auth_header uri, www_authenticate, method, iis = false user = CGI.unescape uri.user password = CGI.unescape uri.password - www_authenticate =~ /^(\w+) (.*)/ + www_authenticate =~ /Digest ((?:\w+=(?:"[^"]+?"|[^\s,]+)(?:,\s?)?)+)/ - challenge = $2 + challenge = $1 params = {} challenge.gsub(/(\w+)="(.*?)"/) { params[$1] = $2 } diff --git a/test/test_net_http_digest_auth.rb b/test/test_net_http_digest_auth.rb index 7fe0e61..c5b2454 100644 --- a/test/test_net_http_digest_auth.rb +++ b/test/test_net_http_digest_auth.rb @@ -54,6 +54,12 @@ def test_auth_header_iis assert_equal expected, @da.auth_header(@uri, @header, 'GET', true) end + def test_auth_header_multi + multi_header = "NTLM, #{@header}, Basic realm=\"www.example.com\"" + + assert_equal expected, @da.auth_header(@uri, multi_header, 'GET') + end + def test_auth_header_no_qop @header.sub! ' qop="auth",', '' From 8b116bc19da3908fc82e11823b990094cc68cdcc Mon Sep 17 00:00:00 2001 From: Paul van Tilburg Date: Wed, 28 Feb 2018 10:37:11 +0100 Subject: [PATCH 2/3] Add some robustness for not so complete RFC (2617) complient headers This allows for HTTP servers to provide headers where the challenge parameter values are unquoted or when there is no spacing after the parameter separator. --- lib/net/http/digest_auth.rb | 2 +- test/test_net_http_digest_auth.rb | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/net/http/digest_auth.rb b/lib/net/http/digest_auth.rb index 391f4ab..3a12d62 100644 --- a/lib/net/http/digest_auth.rb +++ b/lib/net/http/digest_auth.rb @@ -84,7 +84,7 @@ def auth_header uri, www_authenticate, method, iis = false challenge = $1 params = {} - challenge.gsub(/(\w+)="(.*?)"/) { params[$1] = $2 } + challenge.gsub(/(\w+)=(?:"([^"]+?)"|([^\s,]+))/) { params[$1] = $2 || $3 } challenge =~ /algorithm="?(.*?)"?([, ]|$)/ diff --git a/test/test_net_http_digest_auth.rb b/test/test_net_http_digest_auth.rb index c5b2454..b1cbdde 100644 --- a/test/test_net_http_digest_auth.rb +++ b/test/test_net_http_digest_auth.rb @@ -60,6 +60,16 @@ def test_auth_header_multi assert_equal expected, @da.auth_header(@uri, multi_header, 'GET') end + def test_auth_header_unquoted_unspaced + header = [ + 'Digest qop=auth', + 'realm="www.example.com"', + 'nonce=4107baa081a592a6021660200000cd6c5686ff5f579324402b374d83e2c9' + ].join ',' + + assert_equal expected, @da.auth_header(@uri, header, 'GET') + end + def test_auth_header_no_qop @header.sub! ' qop="auth",', '' From acbf4d478e39c3dd9ee615bba30e490474f4fea4 Mon Sep 17 00:00:00 2001 From: Paul van Tilburg Date: Wed, 21 Mar 2018 10:27:13 +0100 Subject: [PATCH 3/3] Raise an error if the auth header has a syntax error This also includes the case where there is no Digest authentication in the auth header at all. Adds a small remark about this to the documentation of Net::HTTP::DigestAuth#auth_header. --- lib/net/http/digest_auth.rb | 13 ++++++++++--- test/test_net_http_digest_auth.rb | 20 ++++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/lib/net/http/digest_auth.rb b/lib/net/http/digest_auth.rb index 3a12d62..6d3ce40 100644 --- a/lib/net/http/digest_auth.rb +++ b/lib/net/http/digest_auth.rb @@ -70,6 +70,10 @@ def initialize ignored = :ignored # # See Net::HTTP::DigestAuth for a complete example. # + # Raises an Net::HTTP::DigestAuth::Error if there is a syntax error + # in the +www_authenticate+ header or it does not include the Digest + # authentication method. + # # IIS servers handle the "qop" parameter of digest authentication # differently so you may need to set +iis+ to true for such servers. @@ -79,9 +83,12 @@ def auth_header uri, www_authenticate, method, iis = false user = CGI.unescape uri.user password = CGI.unescape uri.password - www_authenticate =~ /Digest ((?:\w+=(?:"[^"]+?"|[^\s,]+)(?:,\s?)?)+)/ - - challenge = $1 + if www_authenticate =~ /Digest ((?:\w+=(?:"[^"]+?"|[^\s,]+)(?:,\s?)?)+)/ + challenge = $1 + else + raise Error, "Digest auth method not found or syntax error in " + + "auth header: #{www_authenticate}" + end params = {} challenge.gsub(/(\w+)=(?:"([^"]+?)"|([^\s,]+))/) { params[$1] = $2 || $3 } diff --git a/test/test_net_http_digest_auth.rb b/test/test_net_http_digest_auth.rb index b1cbdde..c5a09e2 100644 --- a/test/test_net_http_digest_auth.rb +++ b/test/test_net_http_digest_auth.rb @@ -128,6 +128,26 @@ def test_auth_header_quoted_algorithm assert_equal expected, @da.auth_header(@uri, @header, 'GET') end + def test_auth_header_error + no_digest_auth_header = "Basic realm=\"www.example.com\"" + erroneous_header = @header.gsub('=', '') + + e = assert_raises Net::HTTP::DigestAuth::Error do + @da.auth_header(@uri, no_digest_auth_header, 'GET') + end + + assert_equal "Digest auth method not found or syntax error in " + + "auth header: #{no_digest_auth_header}", + e.message + + e = assert_raises Net::HTTP::DigestAuth::Error do + @da.auth_header(@uri, erroneous_header, 'GET') + end + + assert_equal "Digest auth method not found or syntax error in " + + "auth header: #{erroneous_header}", e.message + end + def test_make_cnonce da = Net::HTTP::DigestAuth.new