From 76ba2f3daa0c75dd051e635978847dadd859ff38 Mon Sep 17 00:00:00 2001
From: Roman Gaufman <roman@tetherit.io>
Date: Fri, 17 Jul 2026 18:14:41 +1000
Subject: [PATCH] Fix malformed Digest Authorization header for multi-value qop
 (#441)

parse_challenge_param split the WWW-Authenticate challenge on every comma,
including the one inside a quoted qop list such as qop="auth,auth-int". That
mis-parsed qop as %{"auth} and produced a malformed request header:

  Authorization: Digest ..., qop="auth, response="...

(an unclosed qop quote that swallows the response= field). Lenient servers
ignore it; stricter ones reject the request, and some embedded camera web
servers choke on it entirely.

- parse_challenge_param: quote-aware split, so qop="auth,auth-int" is kept
  intact rather than broken at the inner comma.
- calc_cred: send exactly one unquoted qop-value (RFC 2617 message-qop),
  preferring "auth", and compute the response digest with that same value.

Adds test_digest_auth_multi_value_qop.

Fixes #441
---
 lib/httpclient/auth.rb | 28 ++++++++++++++++++++--------
 test/test_auth.rb      | 21 +++++++++++++++++++++
 2 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/lib/httpclient/auth.rb b/lib/httpclient/auth.rb
index 6b7b8b37..3a037375 100644
--- a/lib/httpclient/auth.rb
+++ b/lib/httpclient/auth.rb
@@ -393,6 +393,17 @@ def calc_cred(req, user, passwd, param)
       a_1 = "#{user}:#{param['realm']}:#{passwd}"
       a_2 = "#{method}:#{path}"
       qop = param['qop']
+      if qop
+        # A server may offer several qop-options as a comma-separated list
+        # (RFC 2617/7616: WWW-Authenticate carries qop="auth,auth-int"). The
+        # Authorization request header, however, sends exactly ONE unquoted
+        # qop-value (message-qop = "qop" "=" qop-value). Pick "auth" when it is
+        # offered (auth-int also requires hashing the entity body), else the
+        # first option. Both the response digest and the header below then use
+        # this single value, so they stay consistent. See issue #441.
+        qop_options = qop.split(',').map(&:strip)
+        qop = qop_options.include?('auth') ? 'auth' : qop_options.first
+      end
       nonce = param['nonce']
       cnonce = nil
       if qop || param['algorithm'] =~ /MD5-sess/
@@ -412,7 +423,7 @@ def calc_cred(req, user, passwd, param)
         @nonce_count += 1
         message_digest << ('%08x' % @nonce_count)
         message_digest << cnonce
-        message_digest << param['qop']
+        message_digest << qop
       end
       message_digest << Digest::MD5.hexdigest(a_2)
       header = []
@@ -425,7 +436,7 @@ def calc_cred(req, user, passwd, param)
       end
       if qop
         header << "nc=#{'%08x' % @nonce_count}"
-        header << "qop=#{param['qop']}"
+        header << "qop=#{qop}"
       end
       header << "response=\"#{Digest::MD5.hexdigest(message_digest.join(":"))}\""
       header << "algorithm=#{algorithm}"
@@ -442,12 +453,13 @@ def generate_cnonce
 
     def parse_challenge_param(param_str)
       param = {}
-      param_str.scan(/\s*([^\,]+(?:\\.[^\,]*)*)/).each do |str|
-        key, value = str[0].scan(/\A([^=]+)=(.*)\z/)[0]
-        if /\A"(.*)"\z/ =~ value
-          value = $1.gsub(/\\(.)/, '\1')
-        end
-        param[key] = value
+      # Split into `key = value` pairs. A value is either a quoted string (which
+      # may itself contain commas, e.g. qop="auth,auth-int") or a bare token.
+      # Splitting naively on every comma breaks a quoted comma-separated list
+      # such as qop="auth,auth-int" into qop=%{"auth} + junk, which then yields a
+      # malformed Authorization header (qop="auth, response="...). See issue #441.
+      param_str.scan(/(\w+)\s*=\s*(?:"((?:[^"\\]|\\.)*)"|([^,]*))/).each do |key, quoted, bare|
+        param[key] = quoted ? quoted.gsub(/\\(.)/, '\1') : bare.strip
       end
       param
     end
diff --git a/test/test_auth.rb b/test/test_auth.rb
index 95c17188..dd55001f 100644
--- a/test/test_auth.rb
+++ b/test/test_auth.rb
@@ -320,6 +320,27 @@ def test_perfer_digest
     assert_match(/^Authorization: Digest/, str)
   end
 
+  def test_digest_auth_multi_value_qop
+    # RFC 2617/7616: a server MAY offer several qop-options as a quoted,
+    # comma-separated list in WWW-Authenticate (qop="auth,auth-int"), but the
+    # client's Authorization header carries exactly ONE unquoted qop-value.
+    # Regression for issue #441: parse_challenge_param split on the comma INSIDE
+    # the quotes, so the request header came out malformed as
+    #   qop="auth, response="...  (unclosed qop quote swallowing response=)
+    # which strict servers reject.
+    c = HTTPClient.new
+    c.set_auth('http://example.com/', 'admin', 'admin')
+    c.test_loopback_http_response << %{HTTP/1.0 401 Unauthorized\nWWW-Authenticate: Digest realm="foo", qop="auth,auth-int", nonce="nonce", opaque="xyz", stale=false\nContent-Length: 2\n\nNG}
+    c.test_loopback_http_response << "HTTP/1.0 200 OK\nContent-Length: 2\n\nOK"
+    c.debug_dev = str = ''.dup
+    c.get_content('http://example.com/')
+    auth = str[/^Authorization: Digest.*$/]
+    assert(auth, 'no Authorization header sent')
+    assert_match(/(?:\A|, )qop=auth(?:,|\z)/, auth)   # single, unquoted qop-value
+    assert_no_match(/qop="/, auth)                    # never a quoted qop in the request
+    assert_match(/, response="[0-9a-f]{32}"/, auth)   # response= is its own well-formed field
+  end
+
   def test_digest_sess_auth
     c = HTTPClient.new
     c.set_auth("http://localhost:#{serverport}/", 'admin', 'admin')