From 8be3ef87f0308fe733fc36f3edbee4e3aa618a37 Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Tue, 1 Sep 2026 10:19:49 +0000 Subject: [PATCH] mod_auth_digest: Use siphash for the nonce hash where available: * modules/aaa/mod_auth_digest.c (gen_nonce_hash): Use apr_siphash24_auth() where APU 1.6 or later provides it, keeping the SHA-1 hash otherwise. Take a pool for the message buffer, and length-prefix the realm. * test/modules/aaa/test_002_nonce.py (test_digest_021): Take the length of the hash from the nonce rather than assuming SHA-1's. Co-Authored-By: Claude Fable 5 GitHub: closes #730 git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1937716 13f79535-47bb-0310-9956-ffa450edef68 --- modules/aaa/mod_auth_digest.c | 59 +++++++++++++++++++++++++++--- test/modules/aaa/test_002_nonce.py | 13 +++++-- 2 files changed, 63 insertions(+), 9 deletions(-) diff --git a/modules/aaa/mod_auth_digest.c b/modules/aaa/mod_auth_digest.c index 8d4e1ddaf3..2e59b7a397 100644 --- a/modules/aaa/mod_auth_digest.c +++ b/modules/aaa/mod_auth_digest.c @@ -44,6 +44,7 @@ * opposite of sharing: the two would have to be configured explicitly. */ +#include "apu_version.h" #include "apr_sha1.h" #include "apr_base64.h" #include "apr_lib.h" @@ -82,6 +83,21 @@ #error mod_auth_digest requires APR with random and shared memory support #endif +/* The nonce is authenticated with a keyed hash, so that a client cannot + * forge one. apr_siphash() is a MAC, and is what that calls for; it is + * available in APU 1.6 / APR 2.0 and later. Older APR falls back to the + * original SHA-1 of the secret followed by the message, which is not a + * sound construction - a Merkle-Damgard hash keyed by a prefix can be + * extended without the key - but which is not attackable here, since the + * padding such an extension needs cannot survive the opaque being parsed + * as a hex number before the nonce is checked. */ +#if APU_MAJOR_VERSION > 1 || (APU_MAJOR_VERSION == 1 && APU_MINOR_VERSION >= 6) +#define DIGEST_SIPHASH_NONCE 1 +#include "apr_siphash.h" +#else +#define DIGEST_SIPHASH_NONCE 0 +#endif + /* struct to hold the configuration info */ typedef struct digest_config_struct { @@ -99,9 +115,16 @@ typedef struct digest_config_struct { #define NEXTNONCE_DELTA apr_time_from_sec(30) /* The server nonce has fixed length and is the concatenation of: - * base64(apr_time_t timestamp) + hex(SHA1(realm+time[+opaque])) */ + * base64(apr_time_t timestamp) + hex(keyed hash of realm+time[+opaque]) + * The hash is half as long with siphash, which produces 64 bits: enough for + * a value which can only be attacked by presenting it to the server, there + * being no way to test a candidate offline. */ #define NONCE_TIME_LEN (((sizeof(apr_time_t)+2)/3)*4) +#if DIGEST_SIPHASH_NONCE +#define NONCE_HASH_LEN (2*APR_SIPHASH_DSIZE) +#else #define NONCE_HASH_LEN (2*APR_SHA1_DIGESTSIZE) +#endif #define NONCE_LEN (int )(NONCE_TIME_LEN + NONCE_HASH_LEN) /* Evaluates to true if nonce string is valid. Since the time part of * the nonce is a base64 encoding of an apr_time_t (8 bytes), it @@ -113,6 +136,10 @@ typedef struct digest_config_struct { #define SECRET_LEN 20 #define RETAINED_DATA_ID "mod_auth_digest" +#if DIGEST_SIPHASH_NONCE && SECRET_LEN < APR_SIPHASH_KSIZE +#error the secret is too short to key siphash +#endif + /* client list definitions */ @@ -1093,11 +1120,31 @@ static int init_digest_request(request_rec *r) /* Writes the hash part of the server nonce to hash, which must be of * minimum size (NONCE_HASH_LEN+1). */ -static void gen_nonce_hash(char hash[NONCE_HASH_LEN+1], const char *timestr, const char *opaque, +static void gen_nonce_hash(apr_pool_t *p, char hash[NONCE_HASH_LEN+1], + const char *timestr, const char *opaque, const server_rec *server, - const digest_config_rec *conf, + const digest_config_rec *conf, const char *realm) { +#if DIGEST_SIPHASH_NONCE + unsigned char mac[APR_SIPHASH_DSIZE]; + const char *msg; + + /* siphash takes the whole message at once, having no streaming + * interface, so the fields are joined here rather than fed in one at a + * time. The realm is length-prefixed, which keeps the boundaries + * between the fields unambiguous whatever they contain: without it a + * realm ending in what another realm's timestamp begins with would + * hash the same. An absent opaque is the empty string, which is what + * the challenge side passes for it too. */ + msg = apr_psprintf(p, "%" APR_SIZE_T_FMT ":%s:%s:%s", + (apr_size_t)strlen(realm), realm, timestr, + opaque ? opaque : ""); + + apr_siphash24_auth(mac, msg, strlen(msg), secret); + + ap_bin2hex(mac, APR_SIPHASH_DSIZE, hash); +#else unsigned char sha1[APR_SHA1_DIGESTSIZE]; apr_sha1_ctx_t ctx; @@ -1113,6 +1160,7 @@ static void gen_nonce_hash(char hash[NONCE_HASH_LEN+1], const char *timestr, con apr_sha1_final(sha1, &ctx); ap_bin2hex(sha1, APR_SHA1_DIGESTSIZE, hash); +#endif } @@ -1136,7 +1184,7 @@ static const char *gen_nonce(apr_pool_t *p, apr_time_t now, const char *opaque, t.time = apr_atomic_inc32(otn_counter) + 1; } apr_base64_encode_binary(nonce, t.arr, sizeof(t.arr)); - gen_nonce_hash(nonce+NONCE_TIME_LEN, nonce, opaque, server, conf, realm); + gen_nonce_hash(p, nonce+NONCE_TIME_LEN, nonce, opaque, server, conf, realm); return nonce; } @@ -1415,7 +1463,8 @@ static int check_nonce(request_rec *r, digest_header_rec *resp, tmp = resp->nonce[NONCE_TIME_LEN]; resp->nonce[NONCE_TIME_LEN] = '\0'; apr_base64_decode_binary(nonce_time.arr, resp->nonce); - gen_nonce_hash(hash, resp->nonce, resp->opaque, r->server, conf, ap_auth_name(r)); + gen_nonce_hash(r->pool, hash, resp->nonce, resp->opaque, r->server, conf, + ap_auth_name(r)); resp->nonce[NONCE_TIME_LEN] = tmp; resp->nonce_time = nonce_time.time; diff --git a/test/modules/aaa/test_002_nonce.py b/test/modules/aaa/test_002_nonce.py index 3c6079def4..5d176f33bc 100644 --- a/test/modules/aaa/test_002_nonce.py +++ b/test/modules/aaa/test_002_nonce.py @@ -42,17 +42,22 @@ class TestDigestNonce: env.httpd_error_log.ignore_recent(lognos=["AH01776"]) def test_digest_021_garbage_nonce_hash_is_stale(self, env): - # A nonce must still look like "b64(time)+sha1hex(hash)" (VALID_NONCE - # in mod_auth_digest.c checks length and the '=' padding boundary) to + # A nonce must still look like "b64(time)+hex(hash)" (VALID_NONCE in + # mod_auth_digest.c checks length and the '=' padding boundary) to # even be considered for a hash check; something that doesn't match # that shape is instead rejected as a malformed header (see # test_digest_010). Here we keep the genuine time-prefix (so the # shape is valid) but replace the whole hash suffix with garbage, to # hit check_nonce()'s "hash is not %s" path distinctly from # test_digest_020's single-flipped-character tamper. + # + # The hash length depends on which keyed hash the module was built + # with, so take it from the nonce rather than assuming: the time is + # base64 of 8 bytes and so ends with the only '=' in the string. challenge = self.challenge(env, "default") - time_prefix = challenge.nonce[:-40] - challenge.nonce = time_prefix + ("f" * 40) + time_prefix = challenge.nonce[:challenge.nonce.index('=') + 1] + hash_len = len(challenge.nonce) - len(time_prefix) + challenge.nonce = time_prefix + ("f" * hash_len) r = self.authenticate(env, "default", challenge) assert r.response["status"] == 401 new_challenge = dc.DigestChallenge.parse(r.response["header"]["www-authenticate"])