commit 6f2841090dffbec1a2b2417e18bb3dc71d62dd2e Author: squidcontrib <56416132+squidcontrib@users.noreply.github.com> Date: 2019-10-20 18:59:08 +0000 Hash Digest noncedata (#491) These commits together 1. Hash the noncedata for Digest nonces before encoding, to match the documentation. 2. Encode Digest nonces using hex, rather than base64. commit 8c1b0acc83b38fef177363e6b1acbbc3cb7db21a Author: squidcontrib <56416132+squidcontrib@users.noreply.github.com> Date: 2020-01-29 06:10:04 +0000 Remove pointer from the input of Digest nonce hashes (#549) This is a follow-up to #491 (b20ce97), which hashed what was previously revealed as plaintext. Removing the pointer from the input to the hash removes the possibility that someone could recover a pointer by reversing a hash. Having the pointer as input was not adding anything: Squid remembers all outstanding nonces, so it really only requires uniqueness, which is already guaranteed by the authenticateDigestNonceFindNonce loop. diff --git a/src/auth/digest/Config.cc b/src/auth/digest/Config.cc index b547bf8..2d25fee 100644 --- a/src/auth/digest/Config.cc +++ b/src/auth/digest/Config.cc @@ -21,13 +21,13 @@ #include "auth/Gadgets.h" #include "auth/State.h" #include "base/LookupTable.h" -#include "base64.h" #include "cache_cf.h" #include "event.h" #include "helper.h" #include "HttpHeaderTools.h" #include "HttpReply.h" #include "HttpRequest.h" +#include "md5.h" #include "mgr/Registration.h" #include "rfc2617.h" #include "sbuf/SBuf.h" @@ -89,7 +89,7 @@ DigestFieldsLookupTable(DIGEST_INVALID_ATTR, DigestAttrs); */ static void authenticateDigestNonceCacheCleanup(void *data); -static digest_nonce_h *authenticateDigestNonceFindNonce(const char *nonceb64); +static digest_nonce_h *authenticateDigestNonceFindNonce(const char *noncehex); static void authenticateDigestNonceDelete(digest_nonce_h * nonce); static void authenticateDigestNonceSetup(void); static void authDigestNonceEncode(digest_nonce_h * nonce); @@ -108,11 +108,14 @@ authDigestNonceEncode(digest_nonce_h * nonce) if (nonce->key) xfree(nonce->key); - nonce->key = xcalloc(base64_encode_len(sizeof(digest_nonce_data)), 1); - struct base64_encode_ctx ctx; - base64_encode_init(&ctx); - size_t blen = base64_encode_update(&ctx, reinterpret_cast(nonce->key), sizeof(digest_nonce_data), reinterpret_cast(&(nonce->noncedata))); - blen += base64_encode_final(&ctx, reinterpret_cast(nonce->key)+blen); + SquidMD5_CTX Md5Ctx; + HASH H; + SquidMD5Init(&Md5Ctx); + SquidMD5Update(&Md5Ctx, reinterpret_cast(&nonce->noncedata), sizeof(nonce->noncedata)); + SquidMD5Final(reinterpret_cast(H), &Md5Ctx); + + nonce->key = xcalloc(sizeof(HASHHEX), 1); + CvtHex(H, static_cast(nonce->key)); } digest_nonce_h * @@ -147,12 +150,12 @@ authenticateDigestNonceNew(void) * * Now for my reasoning: * We will not accept a unrecognised nonce->we have all recognisable - * nonces stored. If we send out unique base64 encodings we guarantee + * nonces stored. If we send out unique encodings we guarantee * that a given nonce applies to only one user (barring attacks or * really bad timing with expiry and creation). Using a random * component in the nonce allows us to loop to find a unique nonce. * We use H(nonce_data) so the nonce is meaningless to the reciever. - * So our nonce looks like base64(H(timestamp,pointertohash,randomdata)) + * So our nonce looks like hex(H(timestamp,pointertohash,randomdata)) * And even if our randomness is not very random we don't really care * - the timestamp and memory pointer also guarantee local uniqueness * in the input to the hash function. @@ -251,7 +254,7 @@ static void authenticateDigestNonceCacheCleanup(void *) { /* - * We walk the hash by nonceb64 as that is the unique key we + * We walk the hash by noncehex as that is the unique key we * use. For big hash tables we could consider stepping through * the cache, 100/200 entries at a time. Lets see how it flies * first. @@ -320,7 +323,7 @@ authDigestNonceUnlink(digest_nonce_h * nonce) } const char * -authenticateDigestNonceNonceb64(const digest_nonce_h * nonce) +authenticateDigestNonceNonceHex(const digest_nonce_h * nonce) { if (!nonce) return NULL; @@ -329,18 +332,18 @@ authenticateDigestNonceNonceb64(const digest_nonce_h * nonce) } static digest_nonce_h * -authenticateDigestNonceFindNonce(const char *nonceb64) +authenticateDigestNonceFindNonce(const char *noncehex) { digest_nonce_h *nonce = NULL; - if (nonceb64 == NULL) + if (noncehex == NULL) return NULL; - debugs(29, 9, "looking for nonceb64 '" << nonceb64 << "' in the nonce cache."); + debugs(29, 9, "looking for noncehex '" << noncehex << "' in the nonce cache."); - nonce = static_cast < digest_nonce_h * >(hash_lookup(digest_nonce_cache, nonceb64)); + nonce = static_cast < digest_nonce_h * >(hash_lookup(digest_nonce_cache, noncehex)); - if ((nonce == NULL) || (strcmp(authenticateDigestNonceNonceb64(nonce), nonceb64))) + if ((nonce == NULL) || (strcmp(authenticateDigestNonceNonceHex(nonce), noncehex))) return NULL; debugs(29, 9, "Found nonce '" << nonce << "'"); @@ -535,12 +538,12 @@ Auth::Digest::Config::fixHeader(Auth::UserRequest::Pointer auth_user_request, Ht debugs(29, 9, "Sending type:" << hdrType << " header: 'Digest realm=\"" << realm << "\", nonce=\"" << - authenticateDigestNonceNonceb64(nonce) << "\", qop=\"" << QOP_AUTH << + authenticateDigestNonceNonceHex(nonce) << "\", qop=\"" << QOP_AUTH << "\", stale=" << (stale ? "true" : "false")); /* in the future, for WWW auth we may want to support the domain entry */ httpHeaderPutStrf(&rep->header, hdrType, "Digest realm=\"" SQUIDSBUFPH "\", nonce=\"%s\", qop=\"%s\", stale=%s", - SQUIDSBUFPRINT(realm), authenticateDigestNonceNonceb64(nonce), QOP_AUTH, stale ? "true" : "false"); + SQUIDSBUFPRINT(realm), authenticateDigestNonceNonceHex(nonce), QOP_AUTH, stale ? "true" : "false"); } /* Initialize helpers and the like for this auth scheme. Called AFTER parsing the @@ -852,10 +855,10 @@ Auth::Digest::Config::decode(char const *proxy_auth, const char *aRequestRealm) break; case DIGEST_NONCE: - safe_free(digest_request->nonceb64); + safe_free(digest_request->noncehex); if (value.size() != 0) - digest_request->nonceb64 = xstrndup(value.rawBuf(), value.size() + 1); - debugs(29, 9, "Found nonce '" << digest_request->nonceb64 << "'"); + digest_request->noncehex = xstrndup(value.rawBuf(), value.size() + 1); + debugs(29, 9, "Found nonce '" << digest_request->noncehex << "'"); break; case DIGEST_NC: @@ -931,7 +934,7 @@ Auth::Digest::Config::decode(char const *proxy_auth, const char *aRequestRealm) } /* and a nonce? */ - if (!digest_request->nonceb64 || digest_request->nonceb64[0] == '\0') { + if (!digest_request->noncehex || digest_request->noncehex[0] == '\0') { debugs(29, 2, "Empty or not present nonce"); rv = authDigestLogUsername(username, digest_request, aRequestRealm); safe_free(username); @@ -1006,7 +1009,7 @@ Auth::Digest::Config::decode(char const *proxy_auth, const char *aRequestRealm) /** below nonce state dependent **/ /* now the nonce */ - nonce = authenticateDigestNonceFindNonce(digest_request->nonceb64); + nonce = authenticateDigestNonceFindNonce(digest_request->noncehex); /* check that we're not being hacked / the username hasn't changed */ if (nonce && nonce->user && strcmp(username, nonce->user->username())) { debugs(29, 2, "Username for the nonce does not equal the username for the request"); @@ -1082,7 +1085,7 @@ Auth::Digest::Config::decode(char const *proxy_auth, const char *aRequestRealm) debugs(29, 9, "username = '" << digest_user->username() << "'\nrealm = '" << digest_request->realm << "'\nqop = '" << digest_request->qop << "'\nalgorithm = '" << digest_request->algorithm << "'\nuri = '" << - digest_request->uri << "'\nnonce = '" << digest_request->nonceb64 << + digest_request->uri << "'\nnonce = '" << digest_request->noncehex << "'\nnc = '" << digest_request->nc << "'\ncnonce = '" << digest_request->cnonce << "'\nresponse = '" << digest_request->response << "'\ndigestnonce = '" << nonce << "'"); diff --git a/src/auth/digest/Config.h b/src/auth/digest/Config.h index 8bee06a..4c97354 100644 --- a/src/auth/digest/Config.h +++ b/src/auth/digest/Config.h @@ -29,7 +29,7 @@ class User; typedef struct _digest_nonce_data digest_nonce_data; typedef struct _digest_nonce_h digest_nonce_h; -/* data to be encoded into the nonce's b64 representation */ +/* data to be encoded into the nonce's hex representation */ struct _digest_nonce_data { time_t creationtime; /* in memory address of the nonce struct (similar purpose to an ETag) */ @@ -58,7 +58,7 @@ struct _digest_nonce_h : public hash_link { void authDigestNonceUnlink(digest_nonce_h * nonce); int authDigestNonceIsValid(digest_nonce_h * nonce, char nc[9]); int authDigestNonceIsStale(digest_nonce_h * nonce); -const char *authenticateDigestNonceNonceb64(const digest_nonce_h * nonce); +const char *authenticateDigestNonceNonceHex(const digest_nonce_h * nonce); int authDigestNonceLastRequest(digest_nonce_h * nonce); void authenticateDigestNonceShutdown(void); void authDigestNoncePurge(digest_nonce_h * nonce); diff --git a/src/auth/digest/UserRequest.cc b/src/auth/digest/UserRequest.cc index 554a144..f14eeab 100644 --- a/src/auth/digest/UserRequest.cc +++ b/src/auth/digest/UserRequest.cc @@ -23,7 +23,7 @@ #include "SquidTime.h" Auth::Digest::UserRequest::UserRequest() : - nonceb64(NULL), + noncehex(NULL), cnonce(NULL), realm(NULL), pszPass(NULL), @@ -46,7 +46,7 @@ Auth::Digest::UserRequest::~UserRequest() { assert(LockCount()==0); - safe_free(nonceb64); + safe_free(noncehex); safe_free(cnonce); safe_free(realm); safe_free(pszPass); @@ -109,11 +109,11 @@ Auth::Digest::UserRequest::authenticate(HttpRequest * request, ConnStateData *, } DigestCalcHA1(digest_request->algorithm, NULL, NULL, NULL, - authenticateDigestNonceNonceb64(digest_request->nonce), + authenticateDigestNonceNonceHex(digest_request->nonce), digest_request->cnonce, digest_user->HA1, SESSIONKEY); SBuf sTmp = request->method.image(); - DigestCalcResponse(SESSIONKEY, authenticateDigestNonceNonceb64(digest_request->nonce), + DigestCalcResponse(SESSIONKEY, authenticateDigestNonceNonceHex(digest_request->nonce), digest_request->nc, digest_request->cnonce, digest_request->qop, sTmp.c_str(), digest_request->uri, HA2, Response); @@ -135,7 +135,7 @@ Auth::Digest::UserRequest::authenticate(HttpRequest * request, ConnStateData *, * used. */ sTmp = HttpRequestMethod(Http::METHOD_GET).image(); - DigestCalcResponse(SESSIONKEY, authenticateDigestNonceNonceb64(digest_request->nonce), + DigestCalcResponse(SESSIONKEY, authenticateDigestNonceNonceHex(digest_request->nonce), digest_request->nc, digest_request->cnonce, digest_request->qop, sTmp.c_str(), digest_request->uri, HA2, Response); @@ -176,7 +176,7 @@ Auth::Digest::UserRequest::authenticate(HttpRequest * request, ConnStateData *, /* check Auth::Pending to avoid loop */ if (!authDigestNonceIsValid(digest_request->nonce, digest_request->nc) && user()->credentials() != Auth::Pending) { - debugs(29, 3, auth_user->username() << "' validated OK but nonce stale: " << digest_request->nonceb64); + debugs(29, 3, auth_user->username() << "' validated OK but nonce stale: " << digest_request->noncehex); /* Pending prevent banner and makes a ldap control */ auth_user->credentials(Auth::Pending); nonce->flags.valid = false; @@ -244,8 +244,8 @@ Auth::Digest::UserRequest::addAuthenticationInfoHeader(HttpReply * rep, int acce nextnonce = authenticateDigestNonceNew(); authDigestUserLinkNonce(digest_user, nextnonce); } - debugs(29, 9, "Sending type:" << type << " header: 'nextnonce=\"" << authenticateDigestNonceNonceb64(nextnonce) << "\""); - httpHeaderPutStrf(&rep->header, type, "nextnonce=\"%s\"", authenticateDigestNonceNonceb64(nextnonce)); + debugs(29, 9, "Sending type:" << type << " header: 'nextnonce=\"" << authenticateDigestNonceNonceHex(nextnonce) << "\""); + httpHeaderPutStrf(&rep->header, type, "nextnonce=\"%s\"", authenticateDigestNonceNonceHex(nextnonce)); } } @@ -276,8 +276,8 @@ Auth::Digest::UserRequest::addAuthenticationInfoTrailer(HttpReply * rep, int acc nonce = authenticateDigestNonceNew(); authDigestUserLinkNonce(digest_user, nonce); } - debugs(29, 9, "Sending type:" << type << " header: 'nextnonce=\"" << authenticateDigestNonceNonceb64(nonce) << "\""); - httpTrailerPutStrf(&rep->header, type, "nextnonce=\"%s\"", authenticateDigestNonceNonceb64(nonce)); + debugs(29, 9, "Sending type:" << type << " header: 'nextnonce=\"" << authenticateDigestNonceNonceHex(nonce) << "\""); + httpTrailerPutStrf(&rep->header, type, "nextnonce=\"%s\"", authenticateDigestNonceNonceHex(nonce)); } } #endif diff --git a/src/auth/digest/UserRequest.h b/src/auth/digest/UserRequest.h index 78e5ee1..91f1338 100644 --- a/src/auth/digest/UserRequest.h +++ b/src/auth/digest/UserRequest.h @@ -44,7 +44,7 @@ public: virtual void startHelperLookup(HttpRequest *request, AccessLogEntry::Pointer &al, AUTHCB *, void *); virtual const char *credentialsStr(); - char *nonceb64; /* "dcd98b7102dd2f0e8b11d0f600bfb0c093" */ + char *noncehex; /* "dcd98b7102dd2f0e8b11d0f600bfb0c093" */ char *cnonce; /* "0a4f113b" */ char *realm; /* = "testrealm@host.com" */ char *pszPass; /* = "Circle Of Life" */ diff --git a/src/auth/digest/Config.cc b/src/auth/digest/Config.cc index 5987637..fdef7df 100644 --- a/src/auth/digest/Config.cc +++ b/src/auth/digest/Config.cc @@ -155,10 +155,10 @@ authenticateDigestNonceNew(void) * really bad timing with expiry and creation). Using a random * component in the nonce allows us to loop to find a unique nonce. * We use H(nonce_data) so the nonce is meaningless to the reciever. - * So our nonce looks like hex(H(timestamp,pointertohash,randomdata)) + * So our nonce looks like hex(H(timestamp,randomdata)) * And even if our randomness is not very random we don't really care - * - the timestamp and memory pointer also guarantee local uniqueness - * in the input to the hash function. + * - the timestamp also guarantees local uniqueness in the input to + * the hash function. */ // NP: this will likely produce the same randomness sequences for each worker // since they should all start within the 1-second resolution of seed value. @@ -168,7 +168,6 @@ authenticateDigestNonceNew(void) /* create a new nonce */ newnonce->nc = 0; newnonce->flags.valid = true; - newnonce->noncedata.self = newnonce; newnonce->noncedata.creationtime = current_time.tv_sec; newnonce->noncedata.randomdata = newRandomData(mt); diff --git a/src/auth/digest/Config.h b/src/auth/digest/Config.h index e7f94ce..56ccaa9 100644 --- a/src/auth/digest/Config.h +++ b/src/auth/digest/Config.h @@ -32,8 +32,6 @@ typedef struct _digest_nonce_h digest_nonce_h; /* data to be encoded into the nonce's hex representation */ struct _digest_nonce_data { time_t creationtime; - /* in memory address of the nonce struct (similar purpose to an ETag) */ - digest_nonce_h *self; uint32_t randomdata; };