auth-int qop is ignored for Digest authentication

net/http/http_auth_handler_digest.cc

 // Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/http/http_auth_handler_digest.h"
 
 #include <string>
 
 #include "base/logging.h"
 #include "base/md5.h"
 #include "base/rand_util.h"
 #include "base/string_util.h"
 #include "base/stringprintf.h"
 #include "base/utf_string_conversions.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_util.h"
 #include "net/http/http_auth.h"
 #include "net/http/http_request_info.h"
 #include "net/http/http_util.h"
 
-// TODO(eroman): support qop=auth-int
-
 namespace net {
 
 // Digest authentication is specified in RFC 2617.
 // The expanded derivations are listed in the tables below.
 
 //==========+==========+==========================================+
 //    qop   |algorithm |               response                   |
 //==========+==========+==========================================+
 //    ?     |  ?, md5, | MD5(MD5(A1):nonce:MD5(A2))               |
 //          | md5-sess |                                          |
@@ skipping 25 matching lines @@
   if (fixed_cnonce_)
     return std::string(domain);
   std::string cnonce;
   cnonce.reserve(16);
   for (int i = 0; i < 16; ++i)
     cnonce.push_back(domain[base::RandInt(0, 15)]);
   return cnonce;
 }
 
 // static
 std::string HttpAuthHandlerDigest::QopToString(int qop) {

[eroman, 2010/11/11 23:02:58]

I think this function should be changed to either just take a
QualityOfProtection parameter, or else to return a comma-separated string.

My preference is the former, since it makes it obvious we are dealing with just
a single value, and the caller should have already decided on the policy of
which QOP to use.

-  switch (qop) {
-    case QOP_AUTH:
-      return "auth";
-    case QOP_AUTH_INT:
-      return "auth-int";
-    default:
-      return "";
+  // qop is a bitmask, so report highest priority one first.
+  if (qop & QOP_AUTH_INT) {
+    return "auth-int";
+  } else if (qop & QOP_AUTH) {

[eroman, 2010/11/11 23:02:58]

style: we avoid "else" when using returns.
As in:

if (xxx)
  return AAA;
if (yyy)
  return BBB;

+    return "auth";
+  } else {
+    return "";
   }
 }
 
 // static
 std::string HttpAuthHandlerDigest::AlgorithmToString(int algorithm) {
   switch (algorithm) {
     case ALGORITHM_MD5:
       return "MD5";
     case ALGORITHM_MD5_SESS:
       return "MD5-sess";
@@ skipping 59 matching lines @@
     const std::string& cnonce,
     const std::string& nc) const {
   // ha1 = MD5(A1)
   // TODO(eroman): is this the right encoding?
   std::string ha1 = MD5String(UTF16ToUTF8(username) + ":" + realm_ + ":" +
                               UTF16ToUTF8(password));
   if (algorithm_ == HttpAuthHandlerDigest::ALGORITHM_MD5_SESS)
     ha1 = MD5String(ha1 + ":" + nonce_ + ":" + cnonce);
 
   // ha2 = MD5(A2)
   // TODO(eroman): need to add MD5(req-entity-body) for qop=auth-int.

[eroman, 2010/11/11 23:02:58]

Can you add an assertion here that qop is not auth-int?

   std::string ha2 = MD5String(method + ":" + path);
 
   std::string nc_part;
   if (qop_ != HttpAuthHandlerDigest::QOP_UNSPECIFIED) {
     nc_part = nc + ":" + cnonce + ":" + QopToString(qop_) + ":";
   }
 
   return MD5String(ha1 + ":" + nonce_ + ":" + nc_part + ha2);
 }
 
@@ skipping 136 matching lines @@
       algorithm_ = ALGORITHM_MD5;
     } else if (LowerCaseEqualsASCII(value, "md5-sess")) {
       algorithm_ = ALGORITHM_MD5_SESS;
     } else {
       DVLOG(1) << "Unknown value of algorithm";
       return false;  // FAIL -- unsupported value of algorithm.
     }
   } else if (LowerCaseEqualsASCII(name, "qop")) {
     // Parse the comma separated list of qops.
     HttpUtil::ValuesIterator qop_values(value.begin(), value.end(), ',');
+    int qop = QOP_UNSPECIFIED;
     while (qop_values.GetNext()) {
       if (LowerCaseEqualsASCII(qop_values.value(), "auth")) {
-        qop_ |= QOP_AUTH;
+        qop |= QOP_AUTH;
       } else if (LowerCaseEqualsASCII(qop_values.value(), "auth-int")) {
-        qop_ |= QOP_AUTH_INT;
+        qop |= QOP_AUTH_INT;
       }
     }
+    // TODO(cbentzel): Since auth-int isn't currently supported, fail
+    // parsing if it is the only qop option available.
+    // http://crbug.com/45194
+    if (qop == QOP_AUTH_INT)
+      return false;

[eroman, 2010/11/11 23:02:58]

I suggest outputting some sort of warning or log message.

+    qop_ = (qop & ~QOP_AUTH_INT);
   } else {
     DVLOG(1) << "Skipping unrecognized digest property";
     // TODO(eroman): perhaps we should fail instead of silently skipping?
   }
   return true;
 }
 
 HttpAuthHandlerDigest::Factory::Factory() {
 }
 
@@ skipping 12 matching lines @@
   //                 method and only constructing when valid.
   scoped_ptr<HttpAuthHandler> tmp_handler(
       new HttpAuthHandlerDigest(digest_nonce_count));
   if (!tmp_handler->InitFromChallenge(challenge, target, origin, net_log))
     return ERR_INVALID_RESPONSE;
   handler->swap(tmp_handler);
   return OK;
 }
 
 }  // namespace net