From 12b0a43a66e000b75644c3bfcea2cb91f8194182 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 5 Jun 2024 14:36:04 -0700 Subject: [PATCH] RSA Verify Refactor 1. If the signature to verify is short, pad it to the key length with leading zeros. Some implementations of SSH will prune leading zeros from an mpint value as a string for the signature. 2. Change some of the GetSize() and play with pointers to using GetStringRef() or GetMpint(). 3. Added an RSA private key for testing with PuTTY client. --- keys/putty_rsa.ppk | 26 ++++ keys/putty_rsa.pub | 1 + src/internal.c | 341 ++++++++++++++++++++++----------------------- wolfssh/internal.h | 5 +- 4 files changed, 194 insertions(+), 179 deletions(-) create mode 100644 keys/putty_rsa.ppk create mode 100644 keys/putty_rsa.pub diff --git a/keys/putty_rsa.ppk b/keys/putty_rsa.ppk new file mode 100644 index 000000000..c12c5fbea --- /dev/null +++ b/keys/putty_rsa.ppk @@ -0,0 +1,26 @@ +PuTTY-User-Key-File-3: ssh-rsa +Encryption: none +Comment: rsa-key-20240604 +Public-Lines: 6 +AAAAB3NzaC1yc2EAAAADAQABAAABAQDEbENolVsJ9W/mfKF1G+j/xKiL0g+BhVLH +JP3fOYpXRur5x5kdselmlnklpnzqxQcp+5uv89XfqhILDMNJRhffIKvOYa2AHdEg +ML/FjtLwgiruM6sCA+NZ1MbBHRUqzsPdMlEqZp0kMBpldtUgcwNoyT3TD0zxPNk7 +ZaVl5KTZi3c5KBr11SpT5HsxPLRGN0XwjEZpxu6nfPAdg4R1/rW1vJDHJfU/ZvJb +GcJvDls5OWvFMbaGzhq/JgWmrSRYjYlKpNBnGpvCm61ZbABBoUVyWUGbNgWjeVjw +apW/Ycw9Mb9+u3jVtFPquU3loMXDyXGslclhwH/k8pJjt+g7jAN1 +Private-Lines: 14 +AAABAGyMC8Bq8VGSkhFEhJFMKDnX+vCx2CHShMlKtwU6LipHJal9VS9k1z/7Hd3h +oJy431mjEwlsbZ/Zw3jZx73hf2WuD2PQ9OmdEKmCZygM4qNIu+LBKNrHPUeyX1fu +83ihpPnDSblt1Z9e+edigSsahCLPO1w8019pKf86D+o8LaGOCgWrgAhxzlESQSHj +d5c7C08qOTjOTfSCrUGX6X8vbuVN62sejd7stw/hznNSfKXxGNS36U4PAFA3ISkD +TD3ZYKNDHogfxWbnQdQBykw90OQCn/k05U1ibih4dE7o2C+1Nd+gJBfoUFoz0DcT +LILn9MC7TazgFvfsZ/8eV9hPZm0AAACBAOiNzN6TJvvE2rE0/NFTjXw6Rr0DpCDE +px3IOHaX6sDR+w8h7/Hu1VdnAhNPZndslzW8B0x+yfIE3jxUds3rl5sF4Q54/POj +PnPSNrdP6xFFznxen6TyLxg4DNnlirBBQRPFg6dqtv3SKenVyGLWuzOgCV+oajBh +vnXHJIIMSFRHAAAAgQDYOeymt6Ubi5shUNHpTfvbRMh08Uhlb6R2wkDDBLcDJEHd +h0+4nlNC3I/5OMyGrtPa0zwdEdUNTOKXT3sHC5g/mCOvh3Nk2pcMBr2kK4nR2jKK +oDY6czAlHk3Egd1WAz00Vm+DRKlKOzkPbnYk66cbtmIOPfyBoMv3Ce/wtWM0YwAA +AIEA2hkI2Px9OgtDRjl9Q/ACzTrEytucBtr8sbfDEB9xJo6KfQSvSM+JTs6ZwyDq +xGYnAgfExL6jAziHuDoPOY2ypk9narnVvbT7YnR/unI7w2hKOA4wzwDg2ttjTd2H +p/TeCUiHrrVPe6Q9KkfXMFngbYnt11nN5p6JFKOuzMLg224= +Private-MAC: 8eead3c876b6feb64a80d9d7573ffc1ab89bb272091a38fe85c962d47400c7cb diff --git a/keys/putty_rsa.pub b/keys/putty_rsa.pub new file mode 100644 index 000000000..447b7cc45 --- /dev/null +++ b/keys/putty_rsa.pub @@ -0,0 +1 @@ +ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDEbENolVsJ9W/mfKF1G+j/xKiL0g+BhVLHJP3fOYpXRur5x5kdselmlnklpnzqxQcp+5uv89XfqhILDMNJRhffIKvOYa2AHdEgML/FjtLwgiruM6sCA+NZ1MbBHRUqzsPdMlEqZp0kMBpldtUgcwNoyT3TD0zxPNk7ZaVl5KTZi3c5KBr11SpT5HsxPLRGN0XwjEZpxu6nfPAdg4R1/rW1vJDHJfU/ZvJbGcJvDls5OWvFMbaGzhq/JgWmrSRYjYlKpNBnGpvCm61ZbABBoUVyWUGbNgWjeVjwapW/Ycw9Mb9+u3jVtFPquU3loMXDyXGslclhwH/k8pJjt+g7jAN1 rsa-key-20240604 diff --git a/src/internal.c b/src/internal.c index c67c123fd..6435cafbc 100644 --- a/src/internal.c +++ b/src/internal.c @@ -6229,24 +6229,19 @@ static int DoUserAuthRequestRsa(WOLFSSH* ssh, WS_UserAuthData_PublicKey* pk, enum wc_HashType hashId, byte* digest, word32 digestSz) { - enum wc_HashType enmhashId = hashId; - byte *checkDigest = NULL; - byte *encDigest = NULL; - int checkDigestSz; const byte* publicKeyType; - word32 publicKeyTypeSz = 0; - const byte* n; - word32 nSz = 0; - const byte* e = NULL; - word32 eSz = 0; const byte* sig; + word32 publicKeyTypeSz = 0; word32 sigSz; + word32 encDigestSz; word32 i = 0; int ret = WS_SUCCESS; - RsaKey *key_ptr = NULL; - -#ifndef WOLFSSH_SMALL_STACK - byte s_checkDigest[MAX_ENCODED_SIG_SZ]; +#ifdef WOLFSSH_SMALL_STACK + byte* encDigest = NULL; + RsaKey* key = NULL; +#else + byte encDigest[MAX_ENCODED_SIG_SZ]; + RsaKey key[1]; #endif WLOG(WS_LOG_DEBUG, "Entering DoUserAuthRequestRsa()"); @@ -6257,24 +6252,22 @@ static int DoUserAuthRequestRsa(WOLFSSH* ssh, WS_UserAuthData_PublicKey* pk, ret = WS_BAD_ARGUMENT; } - if (ret == WS_SUCCESS) { #ifdef WOLFSSH_SMALL_STACK - checkDigest = (byte*)WMALLOC(MAX_ENCODED_SIG_SZ, ssh->ctx->heap, - DYNTYPE_BUFFER); - if (checkDigest == NULL) + if (ret == WS_SUCCESS) { + encDigest = (byte*)WMALLOC(MAX_ENCODED_SIG_SZ, + ssh->ctx->heap, DYNTYPE_BUFFER); + if (encDigest == NULL) ret = WS_MEMORY_E; -#else - checkDigest = s_checkDigest; -#endif - key_ptr = (RsaKey*)WMALLOC(sizeof(RsaKey), ssh->ctx->heap, - DYNTYPE_PUBKEY); - if (key_ptr == NULL) + } + if (ret == WS_SUCCESS) { + key = (RsaKey*)WMALLOC(sizeof(RsaKey), ssh->ctx->heap, DYNTYPE_PUBKEY); + if (key == NULL) ret = WS_MEMORY_E; } +#endif if (ret == WS_SUCCESS) { - WMEMSET(checkDigest, 0, MAX_ENCODED_SIG_SZ); - ret = wc_InitRsaKey(key_ptr, ssh->ctx->heap); + ret = wc_InitRsaKey(key, ssh->ctx->heap); if (ret == 0) { ret = WS_SUCCESS; } @@ -6296,19 +6289,27 @@ static int DoUserAuthRequestRsa(WOLFSSH* ssh, WS_UserAuthData_PublicKey* pk, } } + /* Load up the key. */ if (ret == WS_SUCCESS) { - ret = GetMpint(&eSz, &e, pk->publicKey, pk->publicKeySz, &i); - } + const byte* n = NULL; + word32 nSz = 0; + const byte* e = NULL; + word32 eSz = 0; - if (ret == WS_SUCCESS) { - ret = GetMpint(&nSz, &n, pk->publicKey, pk->publicKeySz, &i); - } + if (ret == WS_SUCCESS) { + ret = GetMpint(&eSz, &e, pk->publicKey, pk->publicKeySz, &i); + } - if (ret == WS_SUCCESS) { - ret = wc_RsaPublicKeyDecodeRaw(n, nSz, e, eSz, key_ptr); - if (ret != 0) { - WLOG(WS_LOG_DEBUG, "Could not decode public key"); - ret = WS_CRYPTO_FAILED; + if (ret == WS_SUCCESS) { + ret = GetMpint(&nSz, &n, pk->publicKey, pk->publicKeySz, &i); + } + + if (ret == WS_SUCCESS) { + ret = wc_RsaPublicKeyDecodeRaw(n, nSz, e, eSz, key); + if (ret != 0) { + WLOG(WS_LOG_DEBUG, "Could not decode public key"); + ret = WS_CRYPTO_FAILED; + } } } @@ -6334,55 +6335,22 @@ static int DoUserAuthRequestRsa(WOLFSSH* ssh, WS_UserAuthData_PublicKey* pk, } if (ret == WS_SUCCESS) { - checkDigestSz = wc_RsaSSL_Verify(sig, sigSz, checkDigest, - MAX_ENCODED_SIG_SZ, key_ptr); - if (checkDigestSz <= 0) { - WLOG(WS_LOG_DEBUG, "Could not verify signature"); - ret = WS_CRYPTO_FAILED; - } + encDigestSz = wc_EncodeSignature(encDigest, + digest, digestSz, wc_HashGetOID(hashId)); + ret = wolfSSH_RsaVerify(sig, sigSz, encDigest, encDigestSz, + key, ssh->ctx->heap, "DoUserAuthRequestRsa"); } - if (ret == WS_SUCCESS) { - word32 encDigestSz; - volatile int compare; - volatile int sizeCompare; + wc_FreeRsaKey(key); #ifdef WOLFSSH_SMALL_STACK - encDigest = (byte*)WMALLOC(MAX_ENCODED_SIG_SZ, ssh->ctx->heap, - DYNTYPE_BUFFER); - if (encDigest == NULL) - ret = WS_MEMORY_E; - - if (ret == WS_SUCCESS) -#else - byte s_encDigest[MAX_ENCODED_SIG_SZ]; - encDigest = s_encDigest; -#endif - { - WMEMSET(encDigest, 0, MAX_ENCODED_SIG_SZ); - encDigestSz = wc_EncodeSignature(encDigest, digest, - wc_HashGetDigestSize(enmhashId), - wc_HashGetOID(enmhashId)); - - compare = ConstantCompare(encDigest, checkDigest, - encDigestSz); - sizeCompare = encDigestSz != (word32)checkDigestSz; - - if ((compare == 0) && (sizeCompare == 0)) - ret = WS_SUCCESS; - else - ret = WS_RSA_E; - } + if (key) { + WFREE(key, ssh->ctx->heap, DYNTYPE_PUBKEY); } - if (key_ptr != NULL) { - wc_FreeRsaKey(key_ptr); - WFREE(key_ptr, ssh->ctx->heap, DYNTYPE_PUBKEY); - } -#ifdef WOLFSSH_SMALL_STACK - if (checkDigest) - WFREE(checkDigest, ssh->ctx->heap, DYNTYPE_BUFFER); - if (encDigest) + if (encDigest) { WFREE(encDigest, ssh->ctx->heap, DYNTYPE_BUFFER); + } #endif + WLOG(WS_LOG_DEBUG, "Leaving DoUserAuthRequestRsa(), ret = %d", ret); return ret; } @@ -6394,21 +6362,19 @@ static int DoUserAuthRequestRsaCert(WOLFSSH* ssh, WS_UserAuthData_PublicKey* pk, enum wc_HashType hashId, byte* digest, word32 digestSz) { - enum wc_HashType enmhashId = hashId; - byte *checkDigest = NULL; - byte *encDigest = NULL; - int checkDigestSz; const byte* publicKeyType; - word32 publicKeyTypeSz = 0; const byte* sig; - word32 sigSz = 0; + word32 publicKeyTypeSz = 0; + word32 sigSz; + word32 encDigestSz; word32 i = 0; int ret = WS_SUCCESS; - RsaKey *key_ptr = NULL; - -#ifndef WOLFSSH_SMALL_STACK - byte s_checkDigest[MAX_ENCODED_SIG_SZ]; - RsaKey s_key; +#ifdef WOLFSSH_SMALL_STACK + byte* encDigest = NULL; + RsaKey* key = NULL; +#else + byte encDigest[MAX_ENCODED_SIG_SZ]; + RsaKey key[1]; #endif WLOG(WS_LOG_DEBUG, "Entering DoUserAuthRequestRsaCert()"); @@ -6419,27 +6385,28 @@ static int DoUserAuthRequestRsaCert(WOLFSSH* ssh, WS_UserAuthData_PublicKey* pk, ret = WS_BAD_ARGUMENT; } - if (ret == WS_SUCCESS) { #ifdef WOLFSSH_SMALL_STACK - checkDigest = (byte*)WMALLOC(MAX_ENCODED_SIG_SZ, ssh->ctx->heap, - DYNTYPE_BUFFER); - key_ptr = (RsaKey*)WMALLOC(sizeof(RsaKey), ssh->ctx->heap, - DYNTYPE_PUBKEY); - if (checkDigest == NULL || key_ptr == NULL) - ret = WS_MEMORY_E; -#else - checkDigest = s_checkDigest; - key_ptr = &s_key; -#endif + if (ret == WS_SUCCESS) { + encDigest = (byte*)WMALLOC(MAX_ENCODED_SIG_SZ, + ssh->ctx->heap, DYNTYPE_BUFFER); + if (encDigest == NULL) + ret = WS_MEMORY_E; + } + if (ret == WS_SUCCESS) { + key = (RsaKey*)WMALLOC(sizeof(RsaKey), ssh->ctx->heap, DYNTYPE_PUBKEY); + if (key == NULL) + ret = WS_MEMORY_E; } +#endif if (ret == WS_SUCCESS) { - ret = wc_InitRsaKey(key_ptr, ssh->ctx->heap); + ret = wc_InitRsaKey(key, ssh->ctx->heap); if (ret == 0) { ret = WS_SUCCESS; } } + /* Load up the key. */ if (ret == WS_SUCCESS) { byte* pub = NULL; word32 pubSz; @@ -6462,8 +6429,12 @@ static int DoUserAuthRequestRsaCert(WOLFSSH* ssh, WS_UserAuthData_PublicKey* pk, } if (ret == 0) { - word32 idx = 0; - ret = wc_RsaPublicKeyDecode(pub, &idx, key_ptr, pubSz); + i = 0; + ret = wc_RsaPublicKeyDecode(pub, &i, key, pubSz); + if (ret != 0) { + WLOG(WS_LOG_DEBUG, "Could not decode public key"); + ret = WS_CRYPTO_FAILED; + } } if (pub != NULL) @@ -6471,13 +6442,8 @@ static int DoUserAuthRequestRsaCert(WOLFSSH* ssh, WS_UserAuthData_PublicKey* pk, wc_FreeDecodedCert(&cert); } - if (ret != 0) { - WLOG(WS_LOG_DEBUG, "Could not decode public key"); - ret = WS_CRYPTO_FAILED; - } - if (ret == WS_SUCCESS) { - int keySz = wc_RsaEncryptSize(key_ptr) * 8; + int keySz = wc_RsaEncryptSize(key) * 8; if (keySz < 2048) { WLOG(WS_LOG_DEBUG, "Key size too small (%d)", keySz); ret = WS_CERT_KEY_SIZE_E; @@ -6486,69 +6452,42 @@ static int DoUserAuthRequestRsaCert(WOLFSSH* ssh, WS_UserAuthData_PublicKey* pk, if (ret == WS_SUCCESS) { i = 0; - /* First check that the signature's public key type matches the one - * we are expecting. */ - ret = GetSize(&publicKeyTypeSz, pk->signature, pk->signatureSz, &i); + /* Check that the signature's pubkey type matches the expected one. */ + ret = GetStringRef(&publicKeyTypeSz, &publicKeyType, + pk->signature, pk->signatureSz, &i); } if (ret == WS_SUCCESS) { - publicKeyType = pk->signature + i; - i += publicKeyTypeSz; - WOLFSSH_UNUSED(publicKeyType); - } - - if (ret == WS_SUCCESS) - ret = GetSize(&sigSz, pk->signature, pk->signatureSz, &i); + if (publicKeyTypeSz != pk->publicKeyTypeSz && + WMEMCMP(publicKeyType, pk->publicKeyType, publicKeyTypeSz) != 0) { - if (ret == WS_SUCCESS) { - sig = pk->signature + i; - checkDigestSz = wc_RsaSSL_Verify(sig, sigSz, checkDigest, - MAX_ENCODED_SIG_SZ, key_ptr); - if (checkDigestSz <= 0) { - WLOG(WS_LOG_DEBUG, "Could not verify signature"); - ret = WS_CRYPTO_FAILED; + WLOG(WS_LOG_DEBUG, + "Signature's type does not match public key type"); + ret = WS_INVALID_ALGO_ID; } } if (ret == WS_SUCCESS) { - word32 encDigestSz; - volatile int compare; - volatile int sizeCompare; -#ifdef WOLFSSH_SMALL_STACK - encDigest = (byte*)WMALLOC(MAX_ENCODED_SIG_SZ, ssh->ctx->heap, - DYNTYPE_BUFFER); - if (encDigest == NULL) - ret = WS_MEMORY_E; - - if (ret == WS_SUCCESS) -#else - byte s_encDigest[MAX_ENCODED_SIG_SZ]; - encDigest = s_encDigest; -#endif - { - encDigestSz = wc_EncodeSignature(encDigest, digest, - wc_HashGetDigestSize(enmhashId), - wc_HashGetOID(enmhashId)); - - compare = ConstantCompare(encDigest, checkDigest, - encDigestSz); - sizeCompare = encDigestSz != (word32)checkDigestSz; + ret = GetMpint(&sigSz, &sig, pk->signature, pk->signatureSz, &i); + } - if ((compare == 0) && (sizeCompare == 0)) - ret = WS_SUCCESS; - else - ret = WS_RSA_E; - } + if (ret == WS_SUCCESS) { + encDigestSz = wc_EncodeSignature(encDigest, + digest, digestSz, wc_HashGetOID(hashId)); + ret = wolfSSH_RsaVerify(sig, sigSz, encDigest, encDigestSz, + key, ssh->ctx->heap, "DoUserAuthRequestRsaCert"); } - if (key_ptr) - wc_FreeRsaKey(key_ptr); + wc_FreeRsaKey(key); #ifdef WOLFSSH_SMALL_STACK - if (key_ptr) - WFREE(key_ptr, ssh->ctx->heap, DYNTYPE_PUBKEY); - if (checkDigest) - WFREE(checkDigest, ssh->ctx->heap, DYNTYPE_BUFFER); + if (key) { + WFREE(key, ssh->ctx->heap, DYNTYPE_PUBKEY); + } + if (encDigest) { + WFREE(encDigest, ssh->ctx->heap, DYNTYPE_BUFFER); + } #endif + WLOG(WS_LOG_DEBUG, "Leaving DoUserAuthRequestRsaCert(), ret = %d", ret); return ret; } @@ -10523,38 +10462,86 @@ static INLINE byte SigTypeForId(byte id) /* * wolfSSH_RsaVerify * sig - signature to verify - * sigSz - signature to verify size - * digest - encoded digest for verification - * digestSz - encoded digest size + * sigSz - signature size + * encDigest - encoded digest for verification + * encDigestSz - encoded digest size * key - key used to sign and verify signature * heap - allocation heap * loc - calling function for logging + * + * Takes the provided digest of type digestId and converts it to an + * encoded digest. Then verifies the signature, comparing the output + * digest and compares it. */ -int wolfSSH_RsaVerify(byte *sig, word32 sigSz, - const byte* digest, word32 digestSz, +int wolfSSH_RsaVerify(const byte *sig, word32 sigSz, + const byte* encDigest, word32 encDigestSz, RsaKey* key, void* heap, const char* loc) { - byte* check; + byte* checkSig = NULL; + int checkDigestSz; + word32 keySz; int ret = WS_SUCCESS; +#ifdef WOLFSSH_SMALL_STACK + byte* checkDigest = NULL; +#else + byte checkDigest[MAX_ENCODED_SIG_SZ]; +#endif - check = (byte*)WMALLOC(digestSz, heap, DYNTYPE_TEMP); - if (check == NULL) { - ret = WS_MEMORY_E; + keySz = (word32)wc_RsaEncryptSize(key); + + if (ret == WS_SUCCESS) { + checkSig = (byte*)WMALLOC(keySz, heap, DYNTYPE_TEMP); + if (checkSig == NULL) + ret = WS_MEMORY_E; } - else { - int checkSz; +#ifdef WOLFSSH_SMALL_STACK + if (ret == WS_SUCCESS) { + checkDigest = (byte*)WMALLOC(MAX_ENCODED_SIG_SZ, heap, DYNTYPE_TEMP); + if (checkDigest == NULL) + ret = WS_MEMORY_E; + } +#endif + + /* Normalize the peer's signature. Some SSH implementations remove + * leading zeros on the signatures they encode. We need to pad the + * front of the signature to the key size. */ + if (ret == WS_SUCCESS) { + word32 offset; + + if (keySz > sigSz) { + offset = keySz - sigSz; + } + else { + sigSz = keySz; + offset = 0; + } + + WMEMSET(checkSig, 0, offset); + WMEMCPY(checkSig + offset, sig, sigSz); + } + + if (ret == WS_SUCCESS) { + volatile int sizeCompare; + volatile int compare; - checkSz = wc_RsaSSL_Verify(sig, sigSz, check, digestSz, key); - if (checkSz < 0 - || (word32)checkSz != digestSz - || WMEMCMP(digest, check, digestSz) != 0) { - WLOG(WS_LOG_DEBUG, "%s: %s", loc, "Bad RSA Sign Verify"); + checkDigestSz = wc_RsaSSL_Verify(checkSig, keySz, + checkDigest, MAX_ENCODED_SIG_SZ, key); + + sizeCompare = checkDigestSz > 0 && encDigestSz != (word32)checkDigestSz; + compare = ConstantCompare(encDigest, checkDigest, encDigestSz); + + if (checkDigestSz < 0 || sizeCompare || compare) { + WLOG(WS_LOG_DEBUG, "%s: %s", loc, "Bad RSA Verify"); ret = WS_RSA_E; } - ForceZero(check, digestSz); - WFREE(check, heap, DYNTYPE_TEMP); } +#ifdef WOLFSSH_SMALL_STACK + if (checkDigest) + WFREE(checkDigest, heap, DYNTYPE_TEMP); +#endif + if (checkSig) + WFREE(checkSig, heap, DYNTYPE_TEMP); return ret; } #endif /* WOLFSSH_NO_RSA */ diff --git a/wolfssh/internal.h b/wolfssh/internal.h index 0ed5db5c4..680555de6 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -1259,8 +1259,9 @@ WOLFSSH_LOCAL int wsScpSendCallback(WOLFSSH*, int, const char*, char*, word32, WOLFSSH_LOCAL int wolfSSH_CleanPath(WOLFSSH* ssh, char* in); #ifndef WOLFSSH_NO_RSA -WOLFSSH_LOCAL int wolfSSH_RsaVerify(byte *sig, word32 sigSz, - const byte* digest, word32 digestSz, +WOLFSSH_LOCAL int wolfSSH_RsaVerify( + const byte *sig, word32 sigSz, + const byte* encDigest, word32 encDigestSz, RsaKey* key, void* heap, const char* loc); #endif WOLFSSH_LOCAL void DumpOctetString(const byte*, word32);