From: hno <> Date: Fri, 20 Jun 2003 00:56:59 +0000 (+0000) Subject: 2003-05-18 23:49 hno X-Git-Tag: SQUID_3_0_PRE1~113 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f5292c64df60a9694fc91c30e21da68e7d774392;p=thirdparty%2Fsquid.git 2003-05-18 23:49 hno 2003-05-24 13:08 robertc Bug #630: digest authentication and buggy browsers This patch does four things: * correct signalling of stale digest nonces * auth_param digest check-nonce-count off option to completely disable the nonce count check. Needed to work around Konqueror and certain other broken digest qop implementations. * auth_param digest post_workaround on to work around certain broken browsers sending POST requests with a GET digest response * changes the default of nonce_strictness to off. It is to early to use strict nonce count increase by one checks, and even in the long run this is probably not wanted. Enforcing increase by one does not really add to security, only complexity. --- diff --git a/src/auth/digest/auth_digest.cc b/src/auth/digest/auth_digest.cc index 1e0b1229fc..71382805b6 100644 --- a/src/auth/digest/auth_digest.cc +++ b/src/auth/digest/auth_digest.cc @@ -1,6 +1,6 @@ /* - * $Id: auth_digest.cc,v 1.22 2003/02/26 06:11:41 robertc Exp $ + * $Id: auth_digest.cc,v 1.23 2003/06/19 18:57:00 hno Exp $ * * DEBUG: section 29 Authenticator * AUTHOR: Robert Collins @@ -364,6 +364,18 @@ authDigestNonceIsValid(digest_nonce_h * nonce, char nc[9]) intnc = strtol(nc, NULL, 16); + /* has it already been invalidated ? */ + if (!nonce->flags.valid) { + debug(29, 4) ("authDigestNonceIsValid: Nonce already invalidated\n"); + return 0; + } + + /* is the nonce-count ok ? */ + if (!digestConfig->CheckNonceCount) { + nonce->nc++; + return -1; /* forced OK by configuration */ + } + if ((digestConfig->NonceStrictness && intnc != nonce->nc + 1) || intnc < nonce->nc + 1) { debug(29, 4) ("authDigestNonceIsValid: Nonce count doesn't match\n"); @@ -371,12 +383,6 @@ authDigestNonceIsValid(digest_nonce_h * nonce, char nc[9]) return 0; } - /* has it already been invalidated ? */ - if (!nonce->flags.valid) { - debug(29, 4) ("authDigestNonceIsValid: Nonce already invalidated\n"); - return 0; - } - /* seems ok */ /* increment the nonce count - we've already checked that intnc is a * valid representation for us, so we don't need the test here. @@ -686,11 +692,47 @@ digest_request_h::authenticate(request_t * request, ConnStateData * conn, http_h "squid is = '%s'\n", digest_request->response, Response); if (strcasecmp(digest_request->response, Response)) { - credentials(Failed); - return; + if (digestConfig->PostWorkaround && request->method != METHOD_GET) { + /* Ugly workaround for certain very broken browsers using the + * wrong method to calculate the request-digest on POST request. + * This should be deleted once Digest authentication becomes more + * widespread and such broken browsers no longer are commonly + * used. + */ + DigestCalcResponse(SESSIONKEY, authenticateDigestNonceNonceb64(digest_request->nonce), + digest_request->nc, digest_request->cnonce, digest_request->qop, + RequestMethodStr[METHOD_GET], digest_request->uri, HA2, Response); + + if (strcasecmp(digest_request->response, Response)) { + credentials(Failed); + return; + } else { + const char *useragent = httpHeaderGetStr(&request->header, HDR_USER_AGENT); + + static struct in_addr last_broken_addr = {0}; + + if (memcmp(&last_broken_addr, &request->client_addr, sizeof(last_broken_addr)) != 0) { + debug(29, 1) ("\nDigest POST bug detected from %s using '%s'. Please upgrade browser. See Bug #630 for details.\n", inet_ntoa(request->client_addr), useragent ? useragent : "-"); + last_broken_addr = request->client_addr; + } + } + } else { + credentials(Failed); + return; + } + + /* check for stale nonce */ + if (!authDigestNonceIsValid(digest_request->nonce, digest_request->nc)) { + debug(29, 3) ("authenticateDigestAuthenticateuser: user '%s' validated OK but nonce stale\n", + digest_user->username); + digest_request->flags.nonce_stale = 1; + credentials(Failed); + return; + } } credentials(Ok); + /* password was checked and did match */ debug(29, 4) ("authenticateDigestAuthenticateuser: user '%s' validated OK\n", digest_user->username); @@ -711,16 +753,17 @@ digest_request_h::direction() case Ok: - if (authDigestNonceIsStale(nonce)) - /* send stale response to the client agent */ - return -2; - return 0; case Pending: return -1; case Failed: + + if (digest_request->flags.nonce_stale) + /* nonce is stale, send new challenge */ + return 1; + return -2; } @@ -801,11 +844,7 @@ authenticateDigestFixHeader(auth_user_request_t * auth_user_request, HttpReply * digest_request = dynamic_cast < digest_request_h * >(auth_user_request->state()); assert (digest_request); - if (digest_request->authenticated()) - /* stale indicates that the old nonce can't be used - * and we are providing a new one. - */ - stale = authDigestNonceIsStale(digest_request->nonce); + stale = digest_request->flags.nonce_stale; } /* on a 407 or 401 we always use a new nonce */ @@ -954,8 +993,10 @@ authDigestParse(authScheme * scheme, int n_configured, char *param_str) digestConfig->noncemaxduration = 30 * 60; /* 50 requests */ digestConfig->noncemaxuses = 50; - /* strict nonce count behaviour */ - digestConfig->NonceStrictness = 1; + /* Not strict nonce count behaviour */ + digestConfig->NonceStrictness = 0; + /* Verify nonce count */ + digestConfig->CheckNonceCount = 1; } digestConfig = static_cast < auth_digest_config * >(scheme->scheme_data); @@ -979,6 +1020,10 @@ authDigestParse(authScheme * scheme, int n_configured, char *param_str) parse_int((int *) &digestConfig->noncemaxuses); } else if (strcasecmp(param_str, "nonce_strictness") == 0) { parse_onoff(&digestConfig->NonceStrictness); + } else if (strcasecmp(param_str, "check_nonce_count") == 0) { + parse_onoff(&digestConfig->CheckNonceCount); + } else if (strcasecmp(param_str, "post_workaround") == 0) { + parse_onoff(&digestConfig->PostWorkaround); } else { debug(28, 0) ("unrecognised digest auth scheme parameter '%s'\n", param_str); } @@ -1288,7 +1333,7 @@ authenticateDigestDecodeAuth(auth_user_request_t * auth_user_request, const char /* now the nonce */ nonce = authenticateDigestNonceFindNonce(digest_request->nonceb64); - if ((nonce == NULL) || !(authDigestNonceIsValid(nonce, digest_request->nc))) { + if (!nonce) { /* we couldn't find a matching nonce! */ debug(29, 4) ("authenticateDigestDecode: Unexpected or invalid nonce recieved\n"); authDigestLogUsername(auth_user_request, username); diff --git a/src/auth/digest/auth_digest.h b/src/auth/digest/auth_digest.h index abb3d753dc..415eb5884c 100644 --- a/src/auth/digest/auth_digest.h +++ b/src/auth/digest/auth_digest.h @@ -86,6 +86,9 @@ public: unsigned int authinfo_sent: 1; + +unsigned int nonce_stale: + 1; } flags; @@ -144,6 +147,8 @@ struct _auth_digest_config time_t noncemaxduration; unsigned int noncemaxuses; int NonceStrictness; + int CheckNonceCount; + int PostWorkaround; }; typedef struct _auth_digest_config auth_digest_config; diff --git a/src/authenticate.cc b/src/authenticate.cc index a644ccca51..89d37f623c 100644 --- a/src/authenticate.cc +++ b/src/authenticate.cc @@ -1,6 +1,6 @@ /* - * $Id: authenticate.cc,v 1.57 2003/06/19 18:20:51 hno Exp $ + * $Id: authenticate.cc,v 1.58 2003/06/19 18:56:59 hno Exp $ * * DEBUG: section 29 Authenticator * AUTHOR: Robert Collins @@ -722,10 +722,22 @@ AuthUserRequest::authenticate(auth_user_request_t ** auth_user_request, http_hdr case 1: + if (!request->auth_user_request) { + + (*auth_user_request)->lock() + + ; + request->auth_user_request = *auth_user_request; + } + + /* fallthrough to -2 */ + case -2: /* this ACL check is finished. Unlock. */ (*auth_user_request)->unlock(); + *auth_user_request = NULL; + return AUTH_ACL_CHALLENGE; case -1: diff --git a/src/cf.data.pre b/src/cf.data.pre index 1fb63d208f..82938f996d 100644 --- a/src/cf.data.pre +++ b/src/cf.data.pre @@ -1,6 +1,6 @@ # -# $Id: cf.data.pre,v 1.322 2003/06/19 17:34:10 hno Exp $ +# $Id: cf.data.pre,v 1.323 2003/06/19 18:56:59 hno Exp $ # # # SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -1636,10 +1636,22 @@ DOC_START used. "nonce_strictness" on|off - Determines if squid requires increment-by-1 behaviour for - nonce counts (on - the default), or strictly incrementing - (off - for use when useragents generate nonce counts that - occasionally miss 1 (ie, 1,2,4,6)). + Determines if squid requires strict increment-by-1 behaviour + for nonce counts, or just incrementing (off - for use when + useragents generate nonce counts that occasionally miss 1 + (ie, 1,2,4,6)). Default off. + + "check_nonce_count" on|off + This directive if set to off can disable the nonce count check + completely to work around buggy digest qop implementations in + certain mainstream browser versions. Default on to check the + nonce count to protect from authentication replay attacks. + + "post_workaround" on|off + This is a workaround to certain buggy browsers who sends + an incorrect request digest in POST requests when reusing + the same nonce as aquired earlier on a GET request. + === NTLM scheme options follow === diff --git a/src/client_side.cc b/src/client_side.cc index bd5ef6fc87..7ac80e0b4f 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -1,6 +1,6 @@ /* - * $Id: client_side.cc,v 1.639 2003/06/19 18:18:46 hno Exp $ + * $Id: client_side.cc,v 1.640 2003/06/19 18:56:59 hno Exp $ * * DEBUG: section 33 Client-side Routines * AUTHOR: Duane Wessels @@ -487,10 +487,12 @@ clientPrepareLogWithRequestDetails(request_t * request, AccessLogEntry * aLogEnt aLogEntry->hier = request->hier; if (request->auth_user_request) { - aLogEntry->cache.authuser = - xstrdup(authenticateUserRequestUsername(request-> - auth_user_request)); + if (authenticateUserRequestUsername(request->auth_user_request)) + aLogEntry->cache.authuser = + xstrdup(authenticateUserRequestUsername(request->auth_user_request)); + authenticateAuthUserRequestUnlock(request->auth_user_request); + request->auth_user_request = NULL; }