From 732aba4bc6a0d7d694f01758b2086749d688a715 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 16 Oct 2023 16:29:17 -0700 Subject: [PATCH] wolfSSH Client with OpenSSH-format Keys 1. Add two error codes for the new key format decoding. 2. Add in some better error and bound checking. 3. Fix ordering on a WOLFSSH_UNUSED and variable declaration. 4. Remove redundant ; from WOLFSSH_UNUSED function-like macro. --- src/internal.c | 151 +++++++++++++++++++++++++++++------------------- src/ssh.c | 5 +- wolfssh/error.h | 4 +- wolfssh/port.h | 2 +- 4 files changed, 98 insertions(+), 64 deletions(-) diff --git a/src/internal.c b/src/internal.c index 0f4407d0..00f4eb49 100644 --- a/src/internal.c +++ b/src/internal.c @@ -419,6 +419,12 @@ const char* GetErrorString(int err) case WS_MATCH_UA_KEY_ID_E: return "unable to match user auth key type"; + case WS_KEY_AUTH_MAGIC_E: + return "key auth magic check error"; + + case WS_KEY_CHECK_VAL_E: + return "key check value error"; + default: return "Unknown error code"; } @@ -1041,8 +1047,8 @@ int IdentifyAsn1Key(const byte* in, word32 inSz, int isPrivate, void* heap) static int GetOpenSshKeyRsa(RsaKey* key, const byte* buf, word32 len, word32* idx) { - const byte* val; - word32 valSz; + const byte* val = NULL; + word32 valSz = 0; mp_int m; GetMpint(&valSz, &val, buf, len, idx); /* n */ @@ -1074,8 +1080,8 @@ static int GetOpenSshKeyRsa(RsaKey* key, static int GetOpenSshKeyEcc(ecc_key* key, const byte* buf, word32 len, word32* idx) { - const byte *name, *priv, *pub; - word32 nameSz, privSz, pubSz; + const byte *name = NULL, *priv = NULL, *pub = NULL; + word32 nameSz = 0, privSz = 0, pubSz = 0; int ret; GetStringRef(&nameSz, &name, buf, len, idx); /* curve name */ @@ -1094,72 +1100,100 @@ static int GetOpenSshKey(WS_KeySignature *key, const byte* buf, word32 len, word32* idx) { const char AuthMagic[] = "openssh-key-v1"; - const byte* str; + const byte* str = NULL; + word32 keyCount = 0, strSz = 0, i; int ret = WS_SUCCESS; - word32 keyCount, i, strSz; if (strcmp(AuthMagic, (const char*)buf) != 0) { - ret = -1; + ret = WS_KEY_AUTH_MAGIC_E; } - strSz = (word32)strlen(AuthMagic); - *idx += strSz + 1; - GetSkip(buf, len, idx); /* ciphername */ - GetSkip(buf, len, idx); /* kdfname */ - GetSkip(buf, len, idx); /* kdfoptions */ + if (ret == WS_SUCCESS) { + *idx += (word32)strlen(AuthMagic) + 1; + ret = GetSkip(buf, len, idx); /* ciphername */ + } - GetUint32(&keyCount, buf, len, idx); /* key count */ + if (ret == WS_SUCCESS) + ret = GetSkip(buf, len, idx); /* kdfname */ + + if (ret == WS_SUCCESS) + ret = GetSkip(buf, len, idx); /* kdfoptions */ + + if (ret == WS_SUCCESS) + ret = GetUint32(&keyCount, buf, len, idx); /* key count */ for (i = 0; i < keyCount; i++) { - GetStringRef(&strSz, &str, buf, len, idx); /* public buf */ + if (ret == WS_SUCCESS) + ret = GetStringRef(&strSz, &str, buf, len, idx); /* public buf */ } - GetStringRef(&strSz, &str, buf, len, idx); /* list of private keys */ + if (keyCount > 0) { + if (ret == WS_SUCCESS) { + ret = GetStringRef(&strSz, &str, buf, len, idx); + /* list of private keys */ + } - if (strSz > 0) { - const byte* subStr; - word32 subStrSz, subIdx = 0, check1 = 0, check2 = ~0; - byte keyId; + if (strSz > 0) { + const byte* subStr = NULL; + word32 subStrSz = 0, subIdx = 0, check1 = 0, check2 = ~0; + byte keyId; - idx = 0; - GetUint32(&check1, str, strSz, &subIdx); /* checkint 1 */ - GetUint32(&check2, str, strSz, &subIdx); /* checkint 2 */ - if (check1 == check2) { - for (i = 0; i < keyCount; i++) { - GetStringRef(&subStrSz, &subStr, str, strSz, &subIdx); - keyId = NameToId((const char*)subStr, subStrSz); - wolfSSH_KEY_init(key, keyId, NULL); - switch (keyId) { - #ifndef WOLFSSH_NO_RSA - case ID_SSH_RSA: - GetOpenSshKeyRsa(&key->ks.rsa.key, - str, strSz, &subIdx); - break; - #endif - #ifndef WOLFSSH_NO_ECDSA - case ID_ECDSA_SHA2_NISTP256: - GetOpenSshKeyEcc(&key->ks.ecc.key, - str, strSz, &subIdx); - break; - #endif - default: - ret = WS_UNIMPLEMENTED_E; - break; + idx = 0; + if (ret == WS_SUCCESS) + ret = GetUint32(&check1, str, strSz, &subIdx); /* checkint 1 */ + if (ret == WS_SUCCESS) + ret = GetUint32(&check2, str, strSz, &subIdx); /* checkint 2 */ + if (ret == WS_SUCCESS) { + if (check1 != check2) { + ret = WS_KEY_CHECK_VAL_E; } - GetSkip(str, strSz, &subIdx); /* comment */ } - /* Padding: Add increasing digits to pad to the nearest block - * size. Default block size is 8, but depends on the encryption - * algo. The private key chunk's length, and the length of the - * comment delimit the end of the encrypted blob. No added - * padding required. */ - if (strSz % 8 == 0) { - if (strSz - subIdx > 0) { - /* The padding starts at 1. */ - check2 = strSz - subIdx; - for (check1 = 1; check1 <= check2; check1++, subIdx++) { - if (check1 != str[subIdx]) { - /* Bad pad value. */ + if (ret == WS_SUCCESS) { + for (i = 0; i < keyCount; i++) { + ret = GetStringRef(&subStrSz, &subStr, + str, strSz, &subIdx); + if (ret == WS_SUCCESS) { + keyId = NameToId((const char*)subStr, subStrSz); + wolfSSH_KEY_init(key, keyId, NULL); + switch (keyId) { + #ifndef WOLFSSH_NO_RSA + case ID_SSH_RSA: + GetOpenSshKeyRsa(&key->ks.rsa.key, + str, strSz, &subIdx); + break; + #endif + #ifndef WOLFSSH_NO_ECDSA + case ID_ECDSA_SHA2_NISTP256: + GetOpenSshKeyEcc(&key->ks.ecc.key, + str, strSz, &subIdx); + break; + #endif + default: + ret = WS_UNIMPLEMENTED_E; + break; + } + if (ret == WS_SUCCESS) + ret = GetSkip(str, strSz, &subIdx); + /* key comment */ + } + } + /* Padding: Add increasing digits to pad to the nearest + * block size. Default block size is 8, but depends on + * the encryption algo. The private key chunk's length, + * and the length of the comment delimit the end of the + * encrypted blob. No added padding required. */ + if (ret == WS_SUCCESS) { + if (strSz % 8 == 0) { + if (strSz - subIdx > 0) { + /* The padding starts at 1. */ + check2 = strSz - subIdx; + for (check1 = 1; + check1 <= check2; + check1++, subIdx++) { + if (check1 != str[subIdx]) { + /* Bad pad value. */ + } + } } } } @@ -1599,9 +1633,6 @@ int wolfSSH_ProcessBuffer(WOLFSSH_CTX* ctx, derSz = (word32)ret; } #endif /* WOLFSSH_CERTS */ - else if (format == WOLFSSH_FORMAT_OPENSSH) { - /* TODO */ - } else { return WS_UNIMPLEMENTED_E; } @@ -11305,6 +11336,7 @@ static int PrepareUserAuthRequestEcc(WOLFSSH* ssh, word32* payloadSz, } else #endif + { ret = wc_EccPrivateKeyDecode(authData->sf.publicKey.privateKey, &idx, &keySig->ks.ecc.key, authData->sf.publicKey.privateKeySz); @@ -11314,6 +11346,7 @@ static int PrepareUserAuthRequestEcc(WOLFSSH* ssh, word32* payloadSz, authData->sf.publicKey.privateKey, authData->sf.publicKey.privateKeySz, &idx); } + } } if (ret == WS_SUCCESS) { diff --git a/src/ssh.c b/src/ssh.c index 738d54d1..80535ddc 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -1594,11 +1594,10 @@ static int DoPemKey(const byte* in, word32 inSz, byte** out, { int ret = WS_SUCCESS; byte* newKey = NULL; + word32 newKeySz = inSz; /* binary will be smaller than PEM */ WOLFSSH_UNUSED(heap); - word32 newKeySz = inSz; /* binary will be smaller than PEM */ - if (*out == NULL) { newKey = (byte*)WMALLOC(newKeySz, heap, DYNTYPE_PRIVKEY); if (newKey == NULL) { @@ -1666,7 +1665,7 @@ static int DoOpenSshKey(const byte* in, word32 inSz, byte** out, } in += strlen(PrivBeginOpenSSH); - inSz -= (strlen(PrivBeginOpenSSH) + strlen(PrivEndOpenSSH) + 2); + inSz -= (word32)(strlen(PrivBeginOpenSSH) + strlen(PrivEndOpenSSH) + 2); ret = Base64_Decode((byte*)in, inSz, newKey, &newKeySz); if (ret == 0) { diff --git a/wolfssh/error.h b/wolfssh/error.h index d964b7ac..10ed54d3 100644 --- a/wolfssh/error.h +++ b/wolfssh/error.h @@ -128,8 +128,10 @@ enum WS_ErrorCodes { WS_CERT_KEY_SIZE_E = -1087, /* Key size error */ WS_CTX_KEY_COUNT_E = -1088, /* Adding too many private keys */ WS_MATCH_UA_KEY_ID_E = -1089, /* Match user auth key key fail */ + WS_KEY_AUTH_MAGIC_E = -1090, /* OpenSSH key auth magic check fail */ + WS_KEY_CHECK_VAL_E = -1091, /* OpenSSH key check value fail */ - WS_LAST_E = -1089 /* Update this to indicate last error */ + WS_LAST_E = -1091 /* Update this to indicate last error */ }; diff --git a/wolfssh/port.h b/wolfssh/port.h index 241fa5c5..a0bfbc28 100644 --- a/wolfssh/port.h +++ b/wolfssh/port.h @@ -1390,7 +1390,7 @@ extern "C" { #ifndef WOLFSSH_UNUSED - #define WOLFSSH_UNUSED(arg) (void)(arg); + #define WOLFSSH_UNUSED(arg) (void)(arg) #endif