Export a constant-time compare to applications

apps/wolfsshd/auth.c

@@ -354,7 +354,7 @@ static int CheckAuthKeysLine(char* line, word32 lineSz, const byte* key,
         /* Constant-time compare to avoid leaking which prefix bytes of an
          * authorized key match a candidate offered by a remote peer. */
         if (keyCandSz != keySz ||
-                ConstantCompare(key, keyCand, keySz) != 0) {
+                wolfSSH_ConstantCompare(key, keyCand, keySz) != 0) {
             ret = WSSHD_AUTH_FAILURE;
         }
     }
@@ -811,7 +811,7 @@ static int CheckPasswordHashUnix(const char* input, const char* stored)
 
             if (hashedInputSz == 0 || hashedInput[0] == '*' ||
                     hashedInputSz != storedSz ||
-                    ConstantCompare((const byte*)hashedInput,
+                    wolfSSH_ConstantCompare((const byte*)hashedInput,
                         (const byte*)stored, storedSz) != 0) {
                 ret = WSSHD_AUTH_FAILURE;
             }

examples/echoserver/echoserver.c

@@ -2772,7 +2772,7 @@ static PwMap* PwMapNew(PwMapList* list, byte type, const byte* username,
         map->type = type;
         if (usernameSz >= sizeof(map->username))
             usernameSz = sizeof(map->username) - 1;
-        WMEMCPY(map->username, username, usernameSz + 1);
+        WMEMCPY(map->username, username, usernameSz);
         map->username[usernameSz] = 0;
         map->usernameSz = usernameSz;
 
@@ -3400,13 +3400,14 @@ static int wsUserAuthResult(byte res,
 
 
 static int userAuthWouldBlock = 0;
-static int wsUserAuth(byte authType,
-                      WS_UserAuthData* authData,
-                      void* ctx)
+/* authHash is WC_SHA256_DIGEST_SIZE bytes of scratch for the digest of the
+ * offered secret; the caller wipes it. */
+static int wsUserAuthCheck(byte authType,
+                           WS_UserAuthData* authData,
+                           void* ctx, byte* authHash)
 {
     PwMapList* list;
     PwMap* map;
-    byte authHash[WC_SHA256_DIGEST_SIZE] = {0};
     int userFound = 0;
 
     if (ctx == NULL) {
@@ -3503,6 +3504,8 @@ static int wsUserAuth(byte authType,
                     WFREE(uuid, NULL, 0);
             }
 
+            wc_FreeDecodedCert(&cert);
+
             /* failed to at least get UUID string */
             if (ret != 0) {
                 return WOLFSSH_USERAUTH_INVALID_PUBLICKEY;
@@ -3529,14 +3532,16 @@ static int wsUserAuth(byte authType,
 
             if (authData->type == WOLFSSH_USERAUTH_PUBLICKEY) {
                 userFound = 1;
-                if (WMEMCMP(map->p, authHash, WC_SHA256_DIGEST_SIZE) == 0) {
+                if (wolfSSH_ConstantCompare(map->p, authHash,
+                        WC_SHA256_DIGEST_SIZE) == 0) {
                     return WOLFSSH_USERAUTH_SUCCESS;
                 }
                 /* Hash mismatch: continue checking other registered keys
                  * for this user (a user may have multiple public keys). */
             }
             else if (authData->type == WOLFSSH_USERAUTH_PASSWORD) {
-                if (WMEMCMP(map->p, authHash, WC_SHA256_DIGEST_SIZE) == 0) {
+                if (wolfSSH_ConstantCompare(map->p, authHash,
+                        WC_SHA256_DIGEST_SIZE) == 0) {
                     return WOLFSSH_USERAUTH_SUCCESS;
                 }
                 else {
@@ -3556,7 +3561,8 @@ static int wsUserAuth(byte authType,
                 }
 
                 /* do keyboard auth prompts */
-                if (WMEMCMP(map->p, authHash, WC_SHA256_DIGEST_SIZE) == 0) {
+                if (wolfSSH_ConstantCompare(map->p, authHash,
+                        WC_SHA256_DIGEST_SIZE) == 0) {
                     return WOLFSSH_USERAUTH_SUCCESS;
                 }
                 else {
@@ -3582,6 +3588,20 @@ static int wsUserAuth(byte authType,
 }
 
 
+static int wsUserAuth(byte authType,
+                      WS_UserAuthData* authData,
+                      void* ctx)
+{
+    byte authHash[WC_SHA256_DIGEST_SIZE] = {0};
+    int ret;
+
+    ret = wsUserAuthCheck(authType, authData, ctx, authHash);
+    wc_ForceZero(authHash, sizeof(authHash));
+
+    return ret;
+}
+
+
 #ifdef WOLFSSH_SFTP
 /*
  * Sets the WOLFSSH object's default SFTP path to the value provided by

examples/tpmcertserver/tpmcertserver.c

@@ -64,11 +64,12 @@ static int TpmCsUserAuth(byte authType, WS_UserAuthData* authData, void* ctx)
     WOLFSSH_UNUSED(ctx);
 
     if (authType == WOLFSSH_USERAUTH_PASSWORD) {
-        const char* pw = (const char*)authData->sf.password.password;
+        const byte* pw = authData->sf.password.password;
         word32 pwSz = authData->sf.password.passwordSz;
 
         if (pwSz == (word32)XSTRLEN(TPMCS_PASSWORD)
-                && XMEMCMP(pw, TPMCS_PASSWORD, pwSz) == 0) {
+                && wolfSSH_ConstantCompare(pw,
+                        (const byte*)TPMCS_PASSWORD, pwSz) == 0) {
             ret = WOLFSSH_USERAUTH_SUCCESS;
         }
     }

src/internal.c

@@ -14865,7 +14865,7 @@ static INLINE int VerifyMac(WOLFSSH* ssh, const byte* in, word32 inSz,
                     ret = wc_HmacUpdate(&hmac, in, inSz);
                 if (ret == WS_SUCCESS)
                     ret = wc_HmacFinal(&hmac, checkMac);
-                if (ret == WS_SUCCESS && ConstantCompare(checkMac, mac, ssh->peerMacSz) != 0)
+                if (ret == WS_SUCCESS && WS_CONSTANTCOMPARE(checkMac, mac, ssh->peerMacSz) != 0)
                     ret = WS_VERIFY_MAC_E;
                 break;
 #endif
@@ -14880,7 +14880,7 @@ static INLINE int VerifyMac(WOLFSSH* ssh, const byte* in, word32 inSz,
                     ret = wc_HmacUpdate(&hmac, in, inSz);
                 if (ret == WS_SUCCESS)
                     ret = wc_HmacFinal(&hmac, checkMac);
-                if (ret == WS_SUCCESS && ConstantCompare(checkMac, mac, ssh->peerMacSz) != 0)
+                if (ret == WS_SUCCESS && WS_CONSTANTCOMPARE(checkMac, mac, ssh->peerMacSz) != 0)
                     ret = WS_VERIFY_MAC_E;
                 break;
 #endif
@@ -14895,7 +14895,7 @@ static INLINE int VerifyMac(WOLFSSH* ssh, const byte* in, word32 inSz,
                     ret = wc_HmacUpdate(&hmac, in, inSz);
                 if (ret == WS_SUCCESS)
                     ret = wc_HmacFinal(&hmac, checkMac);
-                if (ret == WS_SUCCESS && ConstantCompare(checkMac, mac, ssh->peerMacSz) != 0)
+                if (ret == WS_SUCCESS && WS_CONSTANTCOMPARE(checkMac, mac, ssh->peerMacSz) != 0)
                     ret = WS_VERIFY_MAC_E;
                 break;
 #endif
@@ -17196,7 +17196,7 @@ int wolfSSH_RsaVerify(const byte *sig, word32 sigSz,
                 RSA_PUBLIC_ENCRYPT, key, NULL);
 
         if (rsaRet != 0 || checkBlockSz != keySz
-                || ConstantCompare(checkBlock, encBlock, keySz) != 0) {
+                || WS_CONSTANTCOMPARE(checkBlock, encBlock, keySz) != 0) {
             WLOG(WS_LOG_DEBUG, "%s: %s", loc, "Bad RSA Verify");
             ret = WS_RSA_E;
         }

src/misc.c

@@ -109,19 +109,22 @@ STATIC INLINE void wolfSSH_ForceZero(void* mem, size_t len)
 #endif
 
 
+#ifdef WOLFSSH_NO_CONSTCOMPARE
 /* check all length bytes for equality, return 0 on success */
 STATIC INLINE int ConstantCompare(const byte* a, const byte* b,
                                   word32 length)
 {
     word32 i;
-    word32 compareSum = 0;
+    /* volatile keeps the compiler from turning the loop into an early exit */
+    volatile word32 compareSum = 0;
 
     for (i = 0; i < length; i++) {
         compareSum |= a[i] ^ b[i];
     }
 
     return compareSum;
 }
+#endif
 
 
 #undef STATIC

src/ssh.c

@@ -56,6 +56,7 @@
 #include <wolfssl/wolfcrypt/wc_port.h>
 #include <wolfssl/wolfcrypt/error-crypt.h>
 #include <wolfssl/wolfcrypt/random.h>
+#include <limits.h>
 
 #ifdef WOLFSSH_WINDOWS_CERT_STORE
     #include <windows.h>
@@ -4610,6 +4611,15 @@ void wolfSSH_GetStats(WOLFSSH* ssh, word32* txCount, word32* rxCount,
 }
 
 
+int wolfSSH_ConstantCompare(const byte* a, const byte* b, word32 len)
+{
+    if (a == NULL || b == NULL || len > (word32)INT_MAX)
+        return WS_BAD_ARGUMENT;
+
+    return WS_CONSTANTCOMPARE(a, b, len);
+}
+
+
 int wolfSSH_KDF(byte hashId, byte keyId,
                 byte* key, word32 keySz,
                 const byte* k, word32 kSz,

tests/api.c

@@ -32,6 +32,7 @@
 
 #include <stdio.h>
 #include <string.h>
+#include <limits.h>
 #if ((defined(WOLFSSH_SFTP) && !defined(NO_WOLFSSH_CLIENT)) || \
      (defined(WOLFSSH_SCP) && defined(WOLFSSH_HAVE_SYMLINK) && \
       !defined(WOLFSSH_SCP_USER_CALLBACKS))) && \
@@ -8034,6 +8035,26 @@ static void test_wolfSSH_ConvertConsole(void)
 }
 
 
+static void test_wolfSSH_ConstantCompare(void)
+{
+    const byte a[] = { 0x01, 0x02, 0x03, 0x04 };
+    const byte b[] = { 0x01, 0x02, 0x03, 0x04 };
+    const byte first[] = { 0x81, 0x02, 0x03, 0x04 };
+    const byte last[] = { 0x01, 0x02, 0x03, 0x05 };
+
+    AssertIntEQ(0, wolfSSH_ConstantCompare(a, b, sizeof(a)));
+    AssertIntNE(0, wolfSSH_ConstantCompare(a, first, sizeof(a)));
+    AssertIntNE(0, wolfSSH_ConstantCompare(a, last, sizeof(a)));
+    /* Only len bytes take part. */
+    AssertIntEQ(0, wolfSSH_ConstantCompare(a, last, sizeof(a) - 1));
+    AssertIntEQ(0, wolfSSH_ConstantCompare(a, first, 0));
+    AssertIntEQ(WS_BAD_ARGUMENT, wolfSSH_ConstantCompare(NULL, b, sizeof(b)));
+    AssertIntEQ(WS_BAD_ARGUMENT, wolfSSH_ConstantCompare(a, NULL, sizeof(a)));
+    AssertIntEQ(WS_BAD_ARGUMENT,
+            wolfSSH_ConstantCompare(a, b, (word32)INT_MAX + 1));
+}
+
+
 static void test_wstrcat(void)
 {
 #ifndef WSTRING_USER
@@ -9286,6 +9307,7 @@ int wolfSSH_ApiTest(int argc, char** argv)
     #endif /* HAVE_FIPS */
 
     test_wstrcat();
+    test_wolfSSH_ConstantCompare();
     test_wolfSSH_Log_sanitize();
     test_wolfSSH_CTX_new();
     test_server_wolfSSH_new();

wolfssh/misc.h

@@ -36,6 +36,7 @@
 
 
 #define WOLFSSL_V5_8_4 0x05008004
+#define WOLFSSL_V5_9_4 0x05009004
 
 #if (LIBWOLFSSL_VERSION_HEX < WOLFSSL_V5_8_4) || \
     defined(WOLFSSL_NO_FORCE_ZERO)
@@ -48,6 +49,18 @@
     #define WS_FORCEZERO(mem, len) wc_ForceZero((mem), (len))
 #endif
 
+#if (LIBWOLFSSL_VERSION_HEX < WOLFSSL_V5_9_4) || \
+    defined(WOLFSSL_NO_CONST_CMP)
+    #define WOLFSSH_NO_CONSTCOMPARE
+#endif
+
+#ifdef WOLFSSH_NO_CONSTCOMPARE
+    #define WS_CONSTANTCOMPARE(a, b, len) ConstantCompare((a), (b), (len))
+#else
+    #define WS_CONSTANTCOMPARE(a, b, len) \
+        wc_ConstantCompare((a), (b), (int)(len))
+#endif
+
 
 #ifdef NO_INLINE
 
@@ -58,7 +71,9 @@ WOLFSSH_LOCAL word32 min(word32 a, word32 b);
 
 WOLFSSH_LOCAL void ato32(const byte* c, word32* u32);
 WOLFSSH_LOCAL void c32toa(word32 u32, byte* c);
+#ifdef WOLFSSH_NO_CONSTCOMPARE
 WOLFSSH_LOCAL int ConstantCompare(const byte* a, const byte* b, word32 length);
+#endif
 #ifdef WOLFSSH_NO_FORCEZERO
 WOLFSSH_LOCAL void wolfSSH_ForceZero(void* mem, size_t len);
 #endif

wolfssh/ssh.h

@@ -764,7 +764,9 @@ typedef struct WS_UserAuthData {
  * For WOLFSSH_USERAUTH_PUBLICKEY, the callback must check the offered
  * public key against the user's authorized keys; returning success on an
  * unchecked key authorizes an attacker-supplied key. The library verifies
- * the signature, not the key's authorization. */
+ * the signature, not the key's authorization.
+ *
+ * Compare secrets with wolfSSH_ConstantCompare(), not memcmp(). */
 typedef int (*WS_CallbackUserAuth)(byte authType, WS_UserAuthData* authData,
         void* ctx);
 WOLFSSH_API void wolfSSH_SetUserAuth(WOLFSSH_CTX* ctx, WS_CallbackUserAuth cb);
@@ -782,6 +784,15 @@ WOLFSSH_API void wolfSSH_SetUserAuthResultCtx(WOLFSSH* ssh,
         void* userAuthResultCtx);
 WOLFSSH_API void* wolfSSH_GetUserAuthResultCtx(WOLFSSH* ssh);
 
+/* Compares len bytes of a and b in time that depends only on len, using
+ * wolfCrypt's wc_ConstantCompare() where the wolfSSL build provides it.
+ * Returns 0 when they match, nonzero when they differ, and WS_BAD_ARGUMENT
+ * for a NULL a or b or a len over INT_MAX. Intended for userauth callbacks
+ * comparing passwords, responses or digests; check lengths separately, as
+ * the compare cannot hide them. */
+WOLFSSH_API int wolfSSH_ConstantCompare(const byte* a, const byte* b,
+        word32 len);
+
 /* Public Key Check Callback
  *
  * A client-side callback that decides whether to trust the server's host