svn commit: r1936804 - apr/apr-util/branches/1.7.x/crypto

Author: covener
Date: Mon Aug  3 12:08:07 2026
New Revision: 1936804

Log:
use timing safe comparison

Submitted By: ylavic
Reviewed By: ylavic, rpluem, covener

Modified:
   apr/apr-util/branches/1.7.x/crypto/apr_crypto.c
   apr/apr-util/branches/1.7.x/crypto/apr_passwd.c

Modified: apr/apr-util/branches/1.7.x/crypto/apr_crypto.c
==============================================================================
--- apr/apr-util/branches/1.7.x/crypto/apr_crypto.c	Mon Aug  3 12:07:22 2026	(r1936803)
+++ apr/apr-util/branches/1.7.x/crypto/apr_crypto.c	Mon Aug  3 12:08:07 2026	(r1936804)
@@ -21,6 +21,7 @@
 #include "apu.h"
 #include "apr_pools.h"
 #include "apr_dso.h"
+#include "apr_version.h"
 #include "apr_strings.h"
 #include "apr_hash.h"
 #include "apr_thread_mutex.h"
@@ -179,19 +180,64 @@ APU_DECLARE(apr_status_t) apr_crypto_mem
     return APR_SUCCESS;
 }
 
+/* Borrow this from APR-1.8 if not available */
+#if !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;
+}
+
+#endif /* !APR_VERSION_AT_LEAST(1,8,0) */
+
 APU_DECLARE(int) apr_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;
+#if APR_VERSION_AT_LEAST(1,8,0)
+    return apr_memeq_timingsafe(buf1, buf2, size);
+#else
+    apr_uint32_t diff = 0;
+    volatile apr_size_t count = size; /* prevent loop unrolling */
+    apr_size_t i = 0;
 
-    for (i = 0; i < size; ++i) {
-        diff |= p1[i] ^ p2[i];
+    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 */
     }
 
-    return 1 & ((diff - 1) >> 8);
+    /* (diff == 0) <=> (diff != 0) ^ 1 */
+    return test_nonzero_timingsafe(diff) ^ 1;
+#endif
 }
 
 APU_DECLARE(apr_crypto_key_rec_t *) apr_crypto_key_rec_make(

Modified: apr/apr-util/branches/1.7.x/crypto/apr_passwd.c
==============================================================================
--- apr/apr-util/branches/1.7.x/crypto/apr_passwd.c	Mon Aug  3 12:07:22 2026	(r1936803)
+++ apr/apr-util/branches/1.7.x/crypto/apr_passwd.c	Mon Aug  3 12:08:07 2026	(r1936804)
@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+#include "apr_version.h"
 #include "apr_strings.h"
 #include "apr_md5.h"
 #include "apr_lib.h"
@@ -39,6 +40,111 @@
 
 static const char * const apr1_id = "$apr1$";
 
+#if APR_VERSION_AT_LEAST(1,8,0)
+
+#define streq_timingsafe    apr_streq_timingsafe
+#define strneq_timingsafe   apr_strneq_timingsafe
+
+#else /* borrow code from APR-1.8 if not available */
+
+/* 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;
+}
+
+static int 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;
+}
+
+static int 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) */
+
 #if !defined(WIN32) && !defined(BEOS) && !defined(NETWARE)
 #if defined(APU_CRYPT_THREADSAFE) || !APR_HAS_THREADS || \
     defined(CRYPT_R_CRYPTD) || defined(CRYPT_R_STRUCT_CRYPT_DATA)
@@ -86,28 +192,33 @@ APU_DECLARE(apr_status_t) apr_password_v
 #if !CRYPT_MISSING
     char *crypt_pw;
 #endif
-    if (hash[0] == '$'
-        && hash[1] == '2'
-        && (hash[2] == 'a' || hash[2] == 'y')
-        && hash[3] == '$') {
+
+    if ((strneq_timingsafe(hash, "$2a$", 4) | /* test both */
+         strneq_timingsafe(hash, "$2y$", 4))) {
+        /*
+         * The hash was created using [apr_]bcrypt encoding.
+         */
         if (_crypt_blowfish_rn(passwd, hash, sample, sizeof(sample)) == NULL)
             return APR_FROM_OS_ERROR(errno);
     }
-    else if (!strncmp(hash, apr1_id, strlen(apr1_id))) {
+    else if (strneq_timingsafe(hash, apr1_id, strlen(apr1_id))) {
         /*
          * The hash was created using our custom algorithm.
          */
         apr_md5_encode(passwd, hash, sample, sizeof(sample));
     }
-    else if (!strncmp(hash, APR_SHA1PW_ID, APR_SHA1PW_IDLEN)) {
-         apr_sha1_base64(passwd, (int)strlen(passwd), sample);
+    else if (strneq_timingsafe(hash, APR_SHA1PW_ID, APR_SHA1PW_IDLEN)) {
+        /*
+         * The hash is a (naked) SHA1.
+         */
+        apr_sha1_base64(passwd, (int)strlen(passwd), sample);
     }
     else {
         /*
          * It's not our algorithm, so feed it to crypt() if possible.
          */
 #if CRYPT_MISSING
-        return (strcmp(passwd, hash) == 0) ? APR_SUCCESS : APR_EMISMATCH;
+        return streq_timingsafe(hash, passwd) ? APR_SUCCESS : APR_EMISMATCH;
 #elif defined(CRYPT_R_CRYPTD)
         apr_status_t rv;
         CRYPTD *buffer = malloc(sizeof(*buffer));
@@ -118,7 +229,7 @@ APU_DECLARE(apr_status_t) apr_password_v
         if (!crypt_pw)
             rv = APR_EMISMATCH;
         else
-            rv = (strcmp(crypt_pw, hash) == 0) ? APR_SUCCESS : APR_EMISMATCH;
+            rv = streq_timingsafe(hash, crypt_pw) ? APR_SUCCESS : APR_EMISMATCH;
         free(buffer);
         return rv;
 #elif defined(CRYPT_R_STRUCT_CRYPT_DATA)
@@ -149,7 +260,7 @@ APU_DECLARE(apr_status_t) apr_password_v
         if (!crypt_pw)
             rv = APR_EMISMATCH;
         else
-            rv = (strcmp(crypt_pw, hash) == 0) ? APR_SUCCESS : APR_EMISMATCH;
+            rv = streq_timingsafe(hash, crypt_pw) ? APR_SUCCESS : APR_EMISMATCH;
         free(buffer);
         return rv;
 #else
@@ -173,14 +284,14 @@ APU_DECLARE(apr_status_t) apr_password_v
                 rv = APR_EMISMATCH;
             }
             else {
-                rv = (strcmp(crypt_pw, hash) == 0) ? APR_SUCCESS : APR_EMISMATCH;
+                rv = streq_timingsafe(hash, crypt_pw) ? APR_SUCCESS : APR_EMISMATCH;
             }
             crypt_mutex_unlock();
             return rv;
         }
 #endif
     }
-    return (strcmp(sample, hash) == 0) ? APR_SUCCESS : APR_EMISMATCH;
+    return streq_timingsafe(hash, sample) ? APR_SUCCESS : APR_EMISMATCH;
 }
 
 static const char * const bcrypt_id = "$2y$";