]> git.ipfire.org Git - thirdparty/apache/httpd.git/commitdiff
Merge r1933389 from trunk:
authorEric Covener <covener@apache.org>
Tue, 28 Apr 2026 18:10:32 +0000 (18:10 +0000)
committerEric Covener <covener@apache.org>
Tue, 28 Apr 2026 18:10:32 +0000 (18:10 +0000)
Add ap_*_timingsafe() constant-time comparison functions:

* include/httpd.h: Declare ap_memeq_timingsafe(), ap_streq_timingsafe(), ap_strneq_timingsafe().

* server/util.c: Implement, wrapping apr_*_timingsafe() if APR >= 1.8, with a fallback to copied-in versions.

* modules/aaa/mod_auth_digest.c: Replace apr_crypto_equals() with ap_memeq_timingsafe(). Remove apr_crypto.h include.

* modules/session/mod_session_crypto.c: Replace local ap_crypto_equals() with ap_memeq_timingsafe(). Remove the local implementation and macro alias.

* include/ap_mmn.h: Bump MMN minor.

Submitted by:  jorton
Reviewed by: jorton, rpluem, covener

Github: closes #639

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1933444 13f79535-47bb-0310-9956-ffa450edef68

.github/workflows/linux.yml
configure.in
include/ap_mmn.h
include/httpd.h
modules/aaa/mod_auth_digest.c
modules/session/mod_session_crypto.c
server/util.c

index 6bc8e5683727ae15f3fb3a5531420a804cafa264..3d4d935fca0eeab78bc3dc3f76cb2dc1a4975d06 100644 (file)
@@ -103,12 +103,12 @@ jobs:
               CLEAR_CACHE=1
           # -------------------------------------------------------------------------
           # Oldest supported APR/APR-util branches:
-          - name: APR 1.4.8, APR-util 1.6.3
+          - name: APR 1.4.8, APR-util 1.3.12
             config: --enable-mods-shared=reallyall
             env: |
               TEST_APR_TARBALL=yes
               APR_VERSION=1.4.8
-              APU_VERSION=1.6.3
+              APU_VERSION=1.3.12
               APU_CONFIG="--with-crypto --with-ldap"
           # -------------------------------------------------------------------------
           - name: Pool-debug
index d49cb28465a2c2c77ae37100f3355011f1f5cd30..d2a009d790204c8507cf74e94136bbd501c81abc 100644 (file)
@@ -431,10 +431,10 @@ if test "${apu_found}" = "yes"; then
     # we need to add the APR includes to CPPFLAGS
     apu_ckver_CPPFLAGS="$CPPFLAGS"
     CPPFLAGS="$CPPFLAGS `$apr_config --includes`"
-    APACHE_CHECK_APxVER([apu], 1, 6)
+    APACHE_CHECK_APxVER([apu], 1, 3)
     CPPFLAGS="$apu_ckver_CPPFLAGS"
   else
-    APACHE_CHECK_APxVER([apu], 1, 6)
+    APACHE_CHECK_APxVER([apu], 1, 3)
   fi
 fi
 
index 8164fe06511e348cf68fbf0e38b6490112cf07da..d00c6d1ec9cc086999dc967bec3d1120b013f0db 100644 (file)
  * 20120211.139 (2.4.63-dev) Add dav_get_base_path() to mod_dav
  * 20120211.140 (2.4.64-dev) Add ap_set_time_process_request() to scoreboard.h
  * 20120211.141 (2.4.64-dev) add ap_stat_check() to httpd.h
+ * 20120211.142 (2.4.64-dev) Add ap_*_timingsafe() to httpd.h
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20120211
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 141                 /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 142                 /* 0...n */
 
 /**
  * Determine if the server's current MODULE_MAGIC_NUMBER is at least a
index 573ef18381d5decd2ad1717bc4052b0d4e7a29b1..342d00a022712d786040b846d9b8e787ff61ad73 100644 (file)
@@ -2129,6 +2129,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 &quot; with \\&quot; .
  * @param p The pool to allocate memory from
index 3ef5f256de00dad3f89348776842a51a2eef6803..f10af0da81e57aec3d7b16462800ca81e4df225d 100644 (file)
@@ -73,7 +73,6 @@
 #include "apr_shm.h"
 #include "apr_rmm.h"
 #include "ap_provider.h"
-#include "apr_crypto.h" /* for apr_crypto_equals */
 
 #include "mod_auth.h"
 
@@ -1437,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 (!apr_crypto_equals(hash, resp->nonce+NONCE_TIME_LEN, NONCE_HASH_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);
@@ -1787,7 +1786,7 @@ static int authenticate_digest_user(request_rec *r)
 
     if (resp->message_qop == NULL) {
         /* old (rfc-2069) style digest */
-        if (!apr_crypto_equals(resp->digest, old_digest(r, resp), MD5_DIGEST_LEN)) {
+        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);
@@ -1822,7 +1821,7 @@ static int authenticate_digest_user(request_rec *r)
             /* we failed to allocate a client struct */
             return HTTP_INTERNAL_SERVER_ERROR;
         }
-        if (!apr_crypto_equals(resp->digest, exp_digest, MD5_DIGEST_LEN)) {
+        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);
index fe39f2c9174f3e9cbe9bb1b29c682525b3f2bf04..b33847b0186b82d5cbac322fec1cfb8a92f37729 100644 (file)
@@ -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;
index 11d0e405004e2320eb691f611981a2da7da92042..0cdf543499d0ee5e962b37fdd574131f8b03f711 100644 (file)
@@ -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) */