From: Amos Jeffries Date: Sat, 6 Mar 2010 14:52:26 +0000 (+1300) Subject: Author: Henrik Nordstrom X-Git-Tag: SQUID_3_0_STABLE25~4 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=38fee48ef1f571e530a35a41c6cf3a299751336e;p=thirdparty%2Fsquid.git Author: Henrik Nordstrom Bug 2845: Rework the http digest auth parser - Validate sanity of digest messages - Properly parse quoted strings --- diff --git a/src/HttpHeaderTools.cc b/src/HttpHeaderTools.cc index 11ae11be27..814df424fa 100644 --- a/src/HttpHeaderTools.cc +++ b/src/HttpHeaderTools.cc @@ -353,25 +353,29 @@ httpHeaderParseQuotedString (const char *start, String *val) { const char *end, *pos; val->clean(); - assert (*start == '"'); + if (*start != '"') { + debugs(66, 2, "failed to parse a quoted-string header field near '" << start << "'"); + return 0; + } pos = start + 1; - while (1) { - if (!(end = index (pos,'"'))) { + while (*pos != '"') { + bool quoted = (*pos == '\\'); + if (quoted) + pos++; + if (!*pos) { debugs(66, 2, "failed to parse a quoted-string header field near '" << start << "'"); + val->clean(); return 0; } - - /* check for quoted-chars */ - if (*(end - 1) != '\\') { - /* done */ - val->append(start + 1, end-start-1); - return 1; - } - - /* try for the end again */ - pos = end + 1; + end = pos + strcspn(pos + quoted, "\"\\") + quoted; + val->append(pos, end-pos); + pos = end; } + /* Make sure it's defined even if empty "" */ + if (!val->buf()) + val->limitInit("", 0); + return 1; } /* diff --git a/src/auth/digest/auth_digest.cc b/src/auth/digest/auth_digest.cc index 5854196764..107f198e57 100644 --- a/src/auth/digest/auth_digest.cc +++ b/src/auth/digest/auth_digest.cc @@ -67,6 +67,33 @@ static MemAllocator *digest_nonce_pool = NULL; CBDATA_TYPE(DigestAuthenticateStateData); +enum http_digest_attr_type { + DIGEST_USERNAME, + DIGEST_REALM, + DIGEST_QOP, + DIGEST_ALGORITHM, + DIGEST_URI, + DIGEST_NONCE, + DIGEST_NC, + DIGEST_CNONCE, + DIGEST_RESPONSE, + DIGEST_ENUM_END +}; + +static const HttpHeaderFieldAttrs DigestAttrs[DIGEST_ENUM_END] = { + {"username", (http_hdr_type)DIGEST_USERNAME}, + {"realm", (http_hdr_type)DIGEST_REALM}, + {"qop", (http_hdr_type)DIGEST_QOP}, + {"algorithm", (http_hdr_type)DIGEST_ALGORITHM}, + {"uri", (http_hdr_type)DIGEST_URI}, + {"nonce", (http_hdr_type)DIGEST_NONCE}, + {"nc", (http_hdr_type)DIGEST_NC}, + {"cnonce", (http_hdr_type)DIGEST_CNONCE}, + {"response", (http_hdr_type)DIGEST_RESPONSE}, +}; + +static HttpHeaderFieldInfo *DigestFieldsInfo = NULL; + /* * * Nonce Functions @@ -507,6 +534,9 @@ digestScheme::done() if (digestauthenticators) helperShutdown(digestauthenticators); + httpHeaderDestroyFieldsInfo(DigestFieldsInfo, DIGEST_ENUM_END); + DigestFieldsInfo = NULL; + authdigest_initialised = 0; if (!shutting_down) { @@ -871,6 +901,7 @@ void AuthDigestConfig::init(AuthConfig * scheme) { if (authenticate) { + DigestFieldsInfo = httpHeaderBuildFieldsInfo(DigestAttrs, DIGEST_ENUM_END); authenticateDigestNonceSetup(); authdigest_initialised = 1; @@ -1093,124 +1124,84 @@ AuthDigestConfig::decode(char const *proxy_auth) String temp(proxy_auth); while (strListGetItem(&temp, ',', &item, &ilen, &pos)) { - if ((p = strchr(item, '=')) && (p - item < ilen)) - ilen = p++ - item; - - if (!strncmp(item, "username", ilen)) { - /* white space */ - - while (xisspace(*p)) - p++; + String value; + size_t nlen; + /* isolate directive name */ + if ((p = (const char *)memchr(item, '=', ilen)) && (p - item < ilen)) { + nlen = p++ - item; + if (!httpHeaderParseQuotedString(p, &value)) + value.limitInit(p, ilen - (p - item)); + } else + nlen = ilen; + + if (!value.buf()) { + debugs(29, 9, "authDigestDecodeAuth: Failed to parse attribute '" << temp << "' in '" << proxy_auth << "'"); + continue; + } - /* quote mark */ - p++; + /* find type */ + http_digest_attr_type type = (http_digest_attr_type)httpHeaderIdByName(item, nlen, DigestFieldsInfo, DIGEST_ENUM_END); + switch (type) { + case DIGEST_USERNAME: safe_free(username); - username = xstrndup(p, strchr(p, '"') + 1 - p); - + username = xstrndup(value.buf(), value.size() + 1); debugs(29, 9, "authDigestDecodeAuth: Found Username '" << username << "'"); - } else if (!strncmp(item, "realm", ilen)) { - /* white space */ - - while (xisspace(*p)) - p++; - - /* quote mark */ - p++; + break; + case DIGEST_REALM: safe_free(digest_request->realm); - digest_request->realm = xstrndup(p, strchr(p, '"') + 1 - p); - + digest_request->realm = xstrndup(value.buf(), value.size() + 1); debugs(29, 9, "authDigestDecodeAuth: Found realm '" << digest_request->realm << "'"); - } else if (!strncmp(item, "qop", ilen)) { - /* white space */ - - while (xisspace(*p)) - p++; - - if (*p == '\"') - /* quote mark */ - p++; + break; + case DIGEST_QOP: safe_free(digest_request->qop); - digest_request->qop = xstrndup(p, strcspn(p, "\" \t\r\n()<>@,;:\\/[]?={}") + 1); - + digest_request->qop = xstrndup(value.buf(), value.size() + 1); debugs(29, 9, "authDigestDecodeAuth: Found qop '" << digest_request->qop << "'"); - } else if (!strncmp(item, "algorithm", ilen)) { - /* white space */ - - while (xisspace(*p)) - p++; - - if (*p == '\"') - /* quote mark */ - p++; + break; + case DIGEST_ALGORITHM: safe_free(digest_request->algorithm); - digest_request->algorithm = xstrndup(p, strcspn(p, "\" \t\r\n()<>@,;:\\/[]?={}") + 1); - + digest_request->algorithm = xstrndup(value.buf(), value.size() + 1); debugs(29, 9, "authDigestDecodeAuth: Found algorithm '" << digest_request->algorithm << "'"); - } else if (!strncmp(item, "uri", ilen)) { - /* white space */ - - while (xisspace(*p)) - p++; - - /* quote mark */ - p++; + break; + case DIGEST_URI: safe_free(digest_request->uri); - digest_request->uri = xstrndup(p, strchr(p, '"') + 1 - p); - + digest_request->uri = xstrndup(value.buf(), value.size() + 1); debugs(29, 9, "authDigestDecodeAuth: Found uri '" << digest_request->uri << "'"); - } else if (!strncmp(item, "nonce", ilen)) { - /* white space */ - - while (xisspace(*p)) - p++; - - /* quote mark */ - p++; + break; + case DIGEST_NONCE: safe_free(digest_request->nonceb64); - digest_request->nonceb64 = xstrndup(p, strchr(p, '"') + 1 - p); - + digest_request->nonceb64 = xstrndup(value.buf(), value.size() + 1); debugs(29, 9, "authDigestDecodeAuth: Found nonce '" << digest_request->nonceb64 << "'"); - } else if (!strncmp(item, "nc", ilen)) { - /* white space */ - - while (xisspace(*p)) - p++; - - xstrncpy(digest_request->nc, p, 9); + break; + case DIGEST_NC: + if (value.size() != 8) { + debugs(29, 9, "authDigestDecodeAuth: Invalid nc '" << value << "' in '" << temp << "'"); + } + xstrncpy(digest_request->nc, value.buf(), value.size() + 1); debugs(29, 9, "authDigestDecodeAuth: Found noncecount '" << digest_request->nc << "'"); - } else if (!strncmp(item, "cnonce", ilen)) { - /* white space */ - - while (xisspace(*p)) - p++; - - /* quote mark */ - p++; + break; + case DIGEST_CNONCE: safe_free(digest_request->cnonce); - digest_request->cnonce = xstrndup(p, strchr(p, '"') + 1 - p); - + digest_request->cnonce = xstrndup(value.buf(), value.size() + 1); debugs(29, 9, "authDigestDecodeAuth: Found cnonce '" << digest_request->cnonce << "'"); - } else if (!strncmp(item, "response", ilen)) { - /* white space */ - - while (xisspace(*p)) - p++; - - /* quote mark */ - p++; + break; + case DIGEST_RESPONSE: safe_free(digest_request->response); - digest_request->response = xstrndup(p, strchr(p, '"') + 1 - p); - + digest_request->response = xstrndup(value.buf(), value.size() + 1); debugs(29, 9, "authDigestDecodeAuth: Found response '" << digest_request->response << "'"); + break; + + default: + debugs(29, 3, "authDigestDecodeAuth: Unknown attribute '" << item << "' in '" << temp << "'"); + } } @@ -1228,69 +1219,93 @@ AuthDigestConfig::decode(char const *proxy_auth) * correct values - 400/401/407 */ - /* first the NONCE count */ + /* 2069 requirements */ - if (digest_request->cnonce && strlen(digest_request->nc) != 8) { - debugs(29, 4, "authenticateDigestDecode: nonce count length invalid"); + /* do we have a username ? */ + if (!username || username[0] == '\0') { + debugs(29, 2, "authenticateDigestDecode: Empty or not present username"); return authDigestLogUsername(username, digest_request); } - /* now the nonce */ - nonce = authenticateDigestNonceFindNonce(digest_request->nonceb64); - - if (!nonce) { - /* we couldn't find a matching nonce! */ - debugs(29, 4, "authenticateDigestDecode: Unexpected or invalid nonce received"); + /* do we have a realm ? */ + if (!digest_request->realm || digest_request->realm[0] == '\0') { + debugs(29, 2, "authenticateDigestDecode: Empty or not present realm"); return authDigestLogUsername(username, digest_request); } - digest_request->nonce = nonce; - authDigestNonceLink(nonce); - - /* check the qop is what we expected. Note that for compatability with - * RFC 2069 we should support a missing qop. Tough. */ - - if (digest_request->qop && strcmp(digest_request->qop, QOP_AUTH) != 0) { - /* we received a qop option we didn't send */ - debugs(29, 4, "authenticateDigestDecode: Invalid qop option received"); + /* and a nonce? */ + if (!digest_request->nonceb64 || digest_request->nonceb64[0] == '\0') { + debugs(29, 2, "authenticateDigestDecode: Empty or not present nonce"); return authDigestLogUsername(username, digest_request); } /* we can't check the URI just yet. We'll check it in the - * authenticate phase */ + * authenticate phase, but needs to be given */ + if (!digest_request->uri || digest_request->uri[0] == '\0') { + debugs(29, 2, "authenticateDigestDecode: Missing URI field"); + return authDigestLogUsername(username, digest_request); + } /* is the response the correct length? */ - if (!digest_request->response || strlen(digest_request->response) != 32) { - debugs(29, 4, "authenticateDigestDecode: Response length invalid"); + debugs(29, 2, "authenticateDigestDecode: Response length invalid"); return authDigestLogUsername(username, digest_request); } - /* do we have a username ? */ - if (!username || username[0] == '\0') { - debugs(29, 4, "authenticateDigestDecode: Empty or not present username"); + /* check the algorithm is present and supported */ + if (!digest_request->algorithm) + digest_request->algorithm = xstrndup("MD5", 4); + else if (strcmp(digest_request->algorithm, "MD5") + && strcmp(digest_request->algorithm, "MD5-sess")) { + debugs(29, 2, "authenticateDigestDecode: invalid algorithm specified!"); return authDigestLogUsername(username, digest_request); } - /* check that we're not being hacked / the username hasn't changed */ - if (nonce->user && strcmp(username, nonce->user->username())) { - debugs(29, 4, "authenticateDigestDecode: Username for the nonce does not equal the username for the request"); - return authDigestLogUsername(username, digest_request); + /* 2617 requirements, indicated by qop */ + if (digest_request->qop) { + + /* check the qop is what we expected. */ + if (strcmp(digest_request->qop, QOP_AUTH) != 0) { + /* we received a qop option we didn't send */ + debugs(29, 2, "authenticateDigestDecode: Invalid qop option received"); + return authDigestLogUsername(username, digest_request); + } + + /* check cnonce */ + if (!digest_request->cnonce || digest_request->cnonce[0] == '\0') { + debugs(29, 2, "authenticateDigestDecode: Missing cnonce field"); + return authDigestLogUsername(username, digest_request); + } + + /* check nc */ + if (strlen(digest_request->nc) != 8 || strspn(digest_request->nc, "0123456789abcdefABCDEF") != 8) { + debugs(29, 2, "authenticateDigestDecode: invalid nonce count"); + return authDigestLogUsername(username, digest_request); + } + } else { + /* cnonce and nc both require qop */ + if (digest_request->cnonce || digest_request->nc) { + debugs(29, 2, "authenticateDigestDecode: missing qop!"); + return authDigestLogUsername(username, digest_request); + } } - /* if we got a qop, did we get a cnonce or did we get a cnonce wihtout a qop? */ - if ((digest_request->qop && !digest_request->cnonce) - || (!digest_request->qop && digest_request->cnonce)) { - debugs(29, 4, "authenticateDigestDecode: qop without cnonce, or vice versa!"); + /** below nonce state dependent **/ + + /* now the nonce */ + nonce = authenticateDigestNonceFindNonce(digest_request->nonceb64); + if (!nonce) { + /* we couldn't find a matching nonce! */ + debugs(29, 2, "authenticateDigestDecode: Unexpected or invalid nonce received"); return authDigestLogUsername(username, digest_request); } - /* check the algorithm is present and supported */ - if (!digest_request->algorithm) - digest_request->algorithm = xstrndup("MD5", 4); - else if (strcmp(digest_request->algorithm, "MD5") - && strcmp(digest_request->algorithm, "MD5-sess")) { - debugs(29, 4, "authenticateDigestDecode: invalid algorithm specified!"); + digest_request->nonce = nonce; + authDigestNonceLink(nonce); + + /* check that we're not being hacked / the username hasn't changed */ + if (nonce->user && strcmp(username, nonce->user->username())) { + debugs(29, 2, "authenticateDigestDecode: Username for the nonce does not equal the username for the request"); return authDigestLogUsername(username, digest_request); }