350 lines
14 KiB
Diff
350 lines
14 KiB
Diff
diff --git a/include/httpd.h b/include/httpd.h
|
|
index 6c7484f..5a3cce4 100644
|
|
--- a/include/httpd.h
|
|
+++ b/include/httpd.h
|
|
@@ -2125,6 +2125,54 @@ AP_DECLARE(int) ap_ind(const char *str, char c); /* Sigh... */
|
|
*/
|
|
AP_DECLARE(int) ap_rind(const char *str, char c);
|
|
|
|
+/**
|
|
+ * Check whether two buffers of equal size have the same content, using a
|
|
+ * constant time algorithm (branch-less with regard to the content of the
|
|
+ * buffers and an execution time solely dependent on the number of bytes
|
|
+ * compared, not the bytes themselves).
|
|
+ *
|
|
+ * @param buf1 first buffer to compare
|
|
+ * @param buf2 second buffer to compare
|
|
+ * @param n number of bytes to compare
|
|
+ * @return 1 if equal, 0 otherwise
|
|
+ */
|
|
+AP_DECLARE(int) ap_memeq_timingsafe(const void *buf1, const void *buf2,
|
|
+ apr_size_t n);
|
|
+
|
|
+/**
|
|
+ * Check whether two NUL-terminated strings have the same content, using a
|
|
+ * constant time algorithm (branch-less with regard to the content of the
|
|
+ * secret string and an execution time solely dependent on the length of
|
|
+ * the non-secret string). The secret string of the two should be set in
|
|
+ * the first parameter \c sec1 to avoid leaking its length.
|
|
+ *
|
|
+ * @param sec1 first string to compare (the secret one)
|
|
+ * @param str2 second string to compare
|
|
+ * @return 1 if equal, 0 otherwise
|
|
+ * @remark The function will compare as much characters as there are in
|
|
+ * \c str2, so the length of \c str2 might leak through side channel,
|
|
+ * while the length of \c sec1 does not.
|
|
+ */
|
|
+AP_DECLARE(int) ap_streq_timingsafe(const char *sec1, const char *str2);
|
|
+
|
|
+/**
|
|
+ * Check whether two NUL-terminated strings have the same content, up to \c n
|
|
+ * characters, using a constant time algorithm (branch-less with regard to the
|
|
+ * content of the secret string and an execution time solely dependent on the
|
|
+ * length of the non-secret string or \c n). The secret string of the two
|
|
+ * should be set in the first parameter \c sec1 to avoid leaking its length.
|
|
+ *
|
|
+ * @param sec1 secret string to compare
|
|
+ * @param str2 string to compare with
|
|
+ * @param n max number of characters to compare
|
|
+ * @return 1 if equal, 0 otherwise
|
|
+ * @remark The function will compare as much characters as there are in
|
|
+ * \c str2 if it's less than \c n, so the length of \c str2 might
|
|
+ * leak through side channel, while the length of \c sec1 does not.
|
|
+ */
|
|
+AP_DECLARE(int) ap_strneq_timingsafe(const char *sec1, const char *str2,
|
|
+ apr_size_t n);
|
|
+
|
|
/**
|
|
* Given a string, replace any bare " with \\" .
|
|
* @param p The pool to allocate memory from
|
|
diff --git a/modules/aaa/mod_auth_digest.c b/modules/aaa/mod_auth_digest.c
|
|
index 791cec2..f10af0d 100644
|
|
--- a/modules/aaa/mod_auth_digest.c
|
|
+++ b/modules/aaa/mod_auth_digest.c
|
|
@@ -100,10 +100,17 @@ typedef struct digest_config_struct {
|
|
#define DFLT_NONCE_LIFE apr_time_from_sec(300)
|
|
#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])) */
|
|
#define NONCE_TIME_LEN (((sizeof(apr_time_t)+2)/3)*4)
|
|
#define NONCE_HASH_LEN (2*APR_SHA1_DIGESTSIZE)
|
|
#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
|
|
+ * must end with a '='. */
|
|
+#define VALID_NONCE(n_) ((n_) && strlen((n_)) == NONCE_LEN && (n_)[NONCE_TIME_LEN - 1] == '=')
|
|
+
|
|
+#define MD5_DIGEST_LEN (2*APR_MD5_DIGESTSIZE) /* ignoring trailing \0 */
|
|
|
|
#define SECRET_LEN 20
|
|
#define RETAINED_DATA_ID "mod_auth_digest"
|
|
@@ -1017,8 +1024,9 @@ static int get_digest_rec(request_rec *r, digest_header_rec *resp)
|
|
resp->nonce_count = apr_pstrdup(r->pool, value);
|
|
}
|
|
|
|
- if (!resp->username || !resp->realm || !resp->nonce || !resp->uri
|
|
- || !resp->digest
|
|
+ if (!resp->username || !resp->realm || !resp->uri
|
|
+ || !VALID_NONCE(resp->nonce)
|
|
+ || !resp->digest || strlen(resp->digest) != MD5_DIGEST_LEN
|
|
|| (resp->message_qop && (!resp->cnonce || !resp->nonce_count))) {
|
|
resp->auth_hdr_sts = INVALID;
|
|
return !OK;
|
|
@@ -1070,14 +1078,9 @@ static int parse_hdr_and_update_nc(request_rec *r)
|
|
}
|
|
|
|
|
|
-/*
|
|
- * Nonce generation code
|
|
- */
|
|
-
|
|
-/* The hash part of the nonce is a SHA-1 hash of the time, realm, server host
|
|
- * and port, opaque, and our secret.
|
|
- */
|
|
-static void gen_nonce_hash(char *hash, const char *timestr, const char *opaque,
|
|
+/* 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,
|
|
const server_rec *server,
|
|
const digest_config_rec *conf)
|
|
{
|
|
@@ -1426,19 +1429,6 @@ static int check_nonce(request_rec *r, digest_header_rec *resp,
|
|
time_rec nonce_time;
|
|
char tmp, hash[NONCE_HASH_LEN+1];
|
|
|
|
- /* Since the time part of the nonce is a base64 encoding of an
|
|
- * apr_time_t (8 bytes), it should end with a '=', fail early otherwise.
|
|
- */
|
|
- if (strlen(resp->nonce) != NONCE_LEN
|
|
- || resp->nonce[NONCE_TIME_LEN - 1] != '=') {
|
|
- ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01775)
|
|
- "invalid nonce '%s' received - length is not %d "
|
|
- "or time encoding is incorrect",
|
|
- resp->nonce, NONCE_LEN);
|
|
- note_digest_auth_failure(r, conf, resp, 1);
|
|
- return HTTP_UNAUTHORIZED;
|
|
- }
|
|
-
|
|
tmp = resp->nonce[NONCE_TIME_LEN];
|
|
resp->nonce[NONCE_TIME_LEN] = '\0';
|
|
apr_base64_decode_binary(nonce_time.arr, resp->nonce);
|
|
@@ -1446,7 +1436,7 @@ static int check_nonce(request_rec *r, digest_header_rec *resp,
|
|
resp->nonce[NONCE_TIME_LEN] = tmp;
|
|
resp->nonce_time = nonce_time.time;
|
|
|
|
- if (strcmp(hash, resp->nonce+NONCE_TIME_LEN)) {
|
|
+ if (!ap_memeq_timingsafe(hash, resp->nonce+NONCE_TIME_LEN, NONCE_HASH_LEN)) {
|
|
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01776)
|
|
"invalid nonce %s received - hash is not %s",
|
|
resp->nonce, hash);
|
|
@@ -1796,7 +1786,7 @@ static int authenticate_digest_user(request_rec *r)
|
|
|
|
if (resp->message_qop == NULL) {
|
|
/* old (rfc-2069) style digest */
|
|
- if (strcmp(resp->digest, old_digest(r, resp))) {
|
|
+ if (!ap_memeq_timingsafe(old_digest(r, resp), resp->digest, MD5_DIGEST_LEN)) {
|
|
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01792)
|
|
"user %s: password mismatch: %s", r->user,
|
|
r->uri);
|
|
@@ -1831,7 +1821,7 @@ static int authenticate_digest_user(request_rec *r)
|
|
/* we failed to allocate a client struct */
|
|
return HTTP_INTERNAL_SERVER_ERROR;
|
|
}
|
|
- if (strcmp(resp->digest, exp_digest)) {
|
|
+ if (!ap_memeq_timingsafe(exp_digest, resp->digest, MD5_DIGEST_LEN)) {
|
|
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01794)
|
|
"user %s: password mismatch: %s", r->user,
|
|
r->uri);
|
|
diff --git a/modules/session/mod_session_crypto.c b/modules/session/mod_session_crypto.c
|
|
index fe39f2c..b33847b 100644
|
|
--- a/modules/session/mod_session_crypto.c
|
|
+++ b/modules/session/mod_session_crypto.c
|
|
@@ -69,8 +69,6 @@ typedef struct {
|
|
#define AP_SIPHASH_KSIZE APR_SIPHASH_KSIZE
|
|
#define ap_siphash24_auth apr_siphash24_auth
|
|
|
|
-#define ap_crypto_equals apr_crypto_equals
|
|
-
|
|
#else
|
|
|
|
#define AP_SIPHASH_DSIZE 8
|
|
@@ -165,21 +163,6 @@ static void ap_siphash24_auth(unsigned char out[AP_SIPHASH_DSIZE],
|
|
U64TO8_LE(out, h);
|
|
}
|
|
|
|
-static int ap_crypto_equals(const void *buf1, const void *buf2,
|
|
- apr_size_t size)
|
|
-{
|
|
- const unsigned char *p1 = buf1;
|
|
- const unsigned char *p2 = buf2;
|
|
- unsigned char diff = 0;
|
|
- apr_size_t i;
|
|
-
|
|
- for (i = 0; i < size; ++i) {
|
|
- diff |= p1[i] ^ p2[i];
|
|
- }
|
|
-
|
|
- return 1 & ((diff - 1) >> 8);
|
|
-}
|
|
-
|
|
#endif
|
|
|
|
static void compute_auth(const void *src, apr_size_t len,
|
|
@@ -404,7 +387,7 @@ static apr_status_t decrypt_string(request_rec * r, const apr_crypto_t *f,
|
|
* the MAC and comparing it (timing safe) with the one in the payload.
|
|
*/
|
|
compute_auth(slider, len, passphrase, passlen, auth);
|
|
- if (!ap_crypto_equals(auth, decoded, AP_SIPHASH_DSIZE)) {
|
|
+ if (!ap_memeq_timingsafe(auth, decoded, AP_SIPHASH_DSIZE)) {
|
|
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, res, r, APLOGNO(10006)
|
|
"auth does not match, skipping");
|
|
continue;
|
|
diff --git a/server/util.c b/server/util.c
|
|
index 11d0e40..0cdf543 100644
|
|
--- a/server/util.c
|
|
+++ b/server/util.c
|
|
@@ -3777,3 +3777,141 @@ AP_DECLARE(const char *)ap_dir_fnmatch(ap_dir_match_t *w, const char *path,
|
|
|
|
return NULL;
|
|
}
|
|
+
|
|
+
|
|
+#if APR_VERSION_AT_LEAST(1,8,0)
|
|
+AP_DECLARE(int) ap_memeq_timingsafe(const void *buf1, const void *buf2,
|
|
+ apr_size_t n)
|
|
+{
|
|
+ return apr_memeq_timingsafe(buf1, buf2, n);
|
|
+}
|
|
+
|
|
+AP_DECLARE(int) ap_streq_timingsafe(const char *sec1, const char *str2)
|
|
+{
|
|
+ return apr_streq_timingsafe(sec1, str2);
|
|
+}
|
|
+
|
|
+AP_DECLARE(int) ap_strneq_timingsafe(const char *sec1, const char *str2,
|
|
+ apr_size_t n)
|
|
+{
|
|
+ return apr_strneq_timingsafe(sec1, str2, n);
|
|
+}
|
|
+
|
|
+#else /* !APR_VERSION_AT_LEAST(1,8,0) */
|
|
+
|
|
+/* A volatile variable which is always zero but allows to block the compiler
|
|
+ * from optimizing or eliding code using it. Volatile forces the compiler to
|
|
+ * emit a memory load for which no value can be assumed, so for instance an
|
|
+ * add/sub/xor/or with "optblocker" is a noop that will hide the result to
|
|
+ * the optimizer.
|
|
+ */
|
|
+static volatile const apr_uint32_t optblocker;
|
|
+
|
|
+/* Return whether x is not zero, with no branching controlled by x.
|
|
+ *
|
|
+ * Taken from the cryptoint library (public domain) by D. J. Bernstein,
|
|
+ * which provides timing attacks safe integer operations/primitives.
|
|
+ * Code:
|
|
+ * https://lib.mceliece.org/libmceliece-20250507/cryptoint/crypto_uint32.h
|
|
+ * Paper:
|
|
+ * https://cr.yp.to/papers/cryptoint-20250424.pdf
|
|
+ */
|
|
+#if __has_attribute(always_inline)
|
|
+__attribute__((always_inline))
|
|
+#endif
|
|
+static APR_INLINE int test_nonzero_timingsafe(apr_uint32_t x)
|
|
+{
|
|
+ x |= -x; /* sets the most significant bit unless x == 0 */
|
|
+
|
|
+ /* shift bit 31 (MSB) to bit 0 */
|
|
+ x >>= 32-6; /* keep 6 bits */
|
|
+ x += optblocker; /* lose the optimizer */
|
|
+ x >>= 5; /* keep the (original) MSB only */
|
|
+
|
|
+ /* x is now 0 or 1 */
|
|
+ return x & INT_MAX;
|
|
+}
|
|
+
|
|
+AP_DECLARE(int) ap_memeq_timingsafe(const void *buf1, const void *buf2,
|
|
+ apr_size_t n)
|
|
+{
|
|
+ apr_uint32_t diff = 0;
|
|
+ volatile apr_size_t count = n; /* prevent loop unrolling */
|
|
+ apr_size_t i = 0;
|
|
+
|
|
+ for (; i < count; ++i) {
|
|
+ const unsigned char c1 = ((volatile const unsigned char *)buf1)[i];
|
|
+ const unsigned char c2 = ((volatile const unsigned char *)buf2)[i];
|
|
+
|
|
+ diff |= c1 ^ c2; /* sets diff to non-zero whenever c1 != c2 */
|
|
+ }
|
|
+
|
|
+ /* (diff == 0) <=> (diff != 0) ^ 1 */
|
|
+ return test_nonzero_timingsafe(diff) ^ 1;
|
|
+}
|
|
+
|
|
+AP_DECLARE(int) ap_streq_timingsafe(const char *sec1, const char *str2)
|
|
+{
|
|
+ apr_uint32_t diff = 0;
|
|
+ apr_size_t i1 = 0, i2 = 0;
|
|
+
|
|
+ for (;; ++i2) {
|
|
+ const unsigned char c1 = ((volatile const unsigned char *)sec1)[i1];
|
|
+ const unsigned char c2 = ((volatile const unsigned char *)str2)[i2];
|
|
+
|
|
+ diff |= c1 ^ c2; /* sets diff to non-zero whenever c1 != c2 */
|
|
+
|
|
+ /* Not a shortest/longest match because an attacker would usually know
|
|
+ * one of the strings and could then determine the length of the other.
|
|
+ * So assume only sec1 and its length are secret and stop the loop at
|
|
+ * the end of str2. If sec1 is shorter than str2 the loop will continue
|
|
+ * by comparing the rest of str2 with the trailing NUL byte of sec1.
|
|
+ * In any case since the diff above is computed up to and including a
|
|
+ * NUL byte, only the same content and length will raise match.
|
|
+ */
|
|
+ if (!c2) {
|
|
+ break;
|
|
+ }
|
|
+
|
|
+ /* Don't go above sec1's NUL byte */
|
|
+ i1 += test_nonzero_timingsafe(c1);
|
|
+ }
|
|
+
|
|
+ /* (diff == 0) <=> (diff != 0) ^ 1 */
|
|
+ return test_nonzero_timingsafe(diff) ^ 1;
|
|
+}
|
|
+
|
|
+AP_DECLARE(int) ap_strneq_timingsafe(const char *sec1, const char *str2,
|
|
+ apr_size_t n)
|
|
+{
|
|
+ apr_uint32_t diff = 0;
|
|
+ volatile apr_size_t count = n; /* prevent loop unrolling */
|
|
+ apr_size_t i1 = 0, i2 = 0;
|
|
+
|
|
+ for (; i2 < count; ++i2) {
|
|
+ const unsigned char c1 = ((volatile const unsigned char *)sec1)[i1];
|
|
+ const unsigned char c2 = ((volatile const unsigned char *)str2)[i2];
|
|
+
|
|
+ diff |= c1 ^ c2; /* sets diff to non-zero whenever c1 != c2 */
|
|
+
|
|
+ /* Not a shortest/longest match because an attacker would usually know
|
|
+ * one of the strings and could then determine the length of the other.
|
|
+ * So assume only sec1 and its length are secret and stop the loop at
|
|
+ * the end of str2. If sec1 is shorter than str2 the loop will continue
|
|
+ * by comparing the rest of str2 with the trailing NUL byte of sec1.
|
|
+ * In any case since the diff above is computed up to and including a
|
|
+ * NUL byte, only the same content and length will raise match.
|
|
+ */
|
|
+ if (!c2) {
|
|
+ break;
|
|
+ }
|
|
+
|
|
+ /* Don't go above sec1's NUL byte */
|
|
+ i1 += test_nonzero_timingsafe(c1);
|
|
+ }
|
|
+
|
|
+ /* (diff == 0) <=> (diff != 0) ^ 1 */
|
|
+ return test_nonzero_timingsafe(diff) ^ 1;
|
|
+}
|
|
+
|
|
+#endif /* !APR_VERSION_AT_LEAST(1,8,0) */
|