Skip to content

Commit

Permalink
PR Review
Browse files Browse the repository at this point in the history 
1. Rename and move CleanupUserAuthRequestPublicKey() as
   wolfSSH_KEY_clean().
2. Restrict the number of keys allowed in an OpenSSH key package to 1.
3. Initialize the Rsa and Ecc keys right before getting them.
4. New error code for an OpenSSH key format error when decoding.
  • Loading branch information
@ejohnstown
ejohnstown committed Nov 16, 2023
1 parent dba9a6c commit 9214981
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 70 deletions.
109 changes: 40 additions & 69 deletions src/internal.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@
Set when all ECDH algorithms are disabled. Set to disable use of all ECDH
algorithms for key agreement. Setting this will force all ECDH key agreement
algorithms off.
WOLFSSH_KEY_QUANTITY_REQ
Number of keys required to be in an OpenSSH-style key wrapper.
*/

static const char sshProtoIdStr[] = "SSH-2.0-wolfSSHv"
Expand Down Expand Up @@ -425,6 +427,9 @@ const char* GetErrorString(int err)
case WS_KEY_CHECK_VAL_E:
return "key check value error";

case WS_KEY_FORMAT_E:
return "key format wrong error";

default:
return "Unknown error code";
}
Expand Down Expand Up @@ -872,58 +877,22 @@ typedef struct WS_KeySignature {
} WS_KeySignature;


static int wolfSSH_KEY_init(WS_KeySignature* key, byte keyId, void* heap)
static void wolfSSH_KEY_clean(WS_KeySignature* key)
{
int ret = WS_SUCCESS;

if (key != NULL) {
WMEMSET(key, 0, sizeof(*key));
key->keySigId = keyId;

switch (keyId) {
if (key->keySigId == ID_SSH_RSA) {
#ifndef WOLFSSH_NO_RSA
case ID_SSH_RSA:
ret = wc_InitRsaKey(&key->ks.rsa.key, heap);
break;
#endif
#ifndef WOLFSSH_NO_ECDSA
case ID_ECDSA_SHA2_NISTP256:
ret = wc_ecc_init_ex(&key->ks.ecc.key, heap, INVALID_DEVID);
break;
wc_FreeRsaKey(&key->ks.rsa.key);
#endif
default:
ret = WS_INVALID_ALGO_ID;
}
}

return ret;
}


static int wolfSSH_KEY_clean(WS_KeySignature* key)
{
int ret = WS_SUCCESS;

if (key != NULL) {
switch (key->keySigId) {
#ifndef WOLFSSH_NO_RSA
case ID_SSH_RSA:
wc_FreeRsaKey(&key->ks.rsa.key);
break;
#endif
else if (key->keySigId == ID_ECDSA_SHA2_NISTP256 ||
key->keySigId == ID_ECDSA_SHA2_NISTP384 ||
key->keySigId == ID_ECDSA_SHA2_NISTP521) {
#ifndef WOLFSSH_NO_ECDSA
case ID_ECDSA_SHA2_NISTP256:
case ID_ECDSA_SHA2_NISTP384:
case ID_ECDSA_SHA2_NISTP521:
wc_ecc_free(&key->ks.ecc.key);
break;
wc_ecc_free(&key->ks.ecc.key);
#endif
default:
ret = WS_INVALID_ALGO_ID;
}
}

return ret;
}


Expand Down Expand Up @@ -1083,7 +1052,9 @@ static int GetOpenSshKeyRsa(RsaKey* key,
{
int ret;

ret = GetMpintToMp(&key->n, buf, len, idx);
ret = wc_InitRsaKey(key, NULL);
if (ret == WS_SUCCESS)
ret = GetMpintToMp(&key->n, buf, len, idx);
if (ret == WS_SUCCESS)
ret = GetMpintToMp(&key->e, buf, len, idx);
if (ret == WS_SUCCESS)
Expand Down Expand Up @@ -1116,7 +1087,9 @@ static int GetOpenSshKeyEcc(ecc_key* key,
word32 nameSz = 0, privSz = 0, pubSz = 0;
int ret;

ret = GetStringRef(&nameSz, &name, buf, len, idx); /* curve name */
ret = wc_ecc_init(key);
if (ret == WS_SUCCESS)
ret = GetStringRef(&nameSz, &name, buf, len, idx); /* curve name */
if (ret == WS_SUCCESS)
ret = GetStringRef(&pubSz, &pub, buf, len, idx); /* Q */
if (ret == WS_SUCCESS)
Expand All @@ -1141,7 +1114,7 @@ static int GetOpenSshKey(WS_KeySignature *key,
{
const char AuthMagic[] = "openssh-key-v1";
const byte* str = NULL;
word32 keyCount = 0, strSz = 0, i;
word32 keyCount = 0, strSz, i;
int ret = WS_SUCCESS;

if (WSTRCMP(AuthMagic, (const char*)buf) != 0) {
Expand All @@ -1162,13 +1135,25 @@ static int GetOpenSshKey(WS_KeySignature *key,
if (ret == WS_SUCCESS)
ret = GetUint32(&keyCount, buf, len, idx); /* key count */

for (i = 0; i < keyCount; i++) {
if (ret == WS_SUCCESS)
ret = GetStringRef(&strSz, &str, buf, len, idx); /* public buf */
if (ret == WS_SUCCESS) {
if (keyCount != WOLFSSH_KEY_QUANTITY_REQ) {
ret = WS_KEY_FORMAT_E;
}
}

if (ret == WS_SUCCESS) {
for (i = 0; i < keyCount; i++) {
strSz = 0;
if (ret == WS_SUCCESS) {
ret = GetStringRef(&strSz, &str, buf, len, idx);
/* public buf */
}
}
}

if (keyCount > 0) {
if (ret == WS_SUCCESS) {
strSz = 0;
ret = GetStringRef(&strSz, &str, buf, len, idx);
/* list of private keys */
}
Expand All @@ -1194,7 +1179,7 @@ static int GetOpenSshKey(WS_KeySignature *key,
str, strSz, &subIdx);
if (ret == WS_SUCCESS) {
keyId = NameToId((const char*)subStr, subStrSz);
ret = wolfSSH_KEY_init(key, keyId, NULL);
key->keySigId = keyId;
}
if (ret == WS_SUCCESS) {
switch (keyId) {
Expand Down Expand Up @@ -1241,6 +1226,9 @@ static int GetOpenSshKey(WS_KeySignature *key,
}
}
}
else {
ret = WS_KEY_FORMAT_E;
}
}

return ret;
Expand Down Expand Up @@ -1273,7 +1261,7 @@ int IdentifyOpenSshKey(const byte* in, word32 inSz, void* heap)
}
else {
WMEMSET(key, 0, sizeof(*key));
key->keySigId = ID_UNKNOWN;
key->keySigId = ID_NONE;

ret = GetOpenSshKey(key, in, inSz, &idx);

Expand Down Expand Up @@ -12068,23 +12056,6 @@ static int BuildUserAuthRequestPublicKey(WOLFSSH* ssh,
}


static void CleanupUserAuthRequestPublicKey(WS_KeySignature* keySig)
{
if (keySig != NULL) {
if (keySig->keySigId == ID_SSH_RSA) {
#ifndef WOLFSSH_NO_RSA
wc_FreeRsaKey(&keySig->ks.rsa.key);
#endif
}
else if (keySig->keySigId == ID_ECDSA_SHA2_NISTP256 ||
keySig->keySigId == ID_ECDSA_SHA2_NISTP384 ||
keySig->keySigId == ID_ECDSA_SHA2_NISTP521) {
#ifndef WOLFSSH_NO_ECDSA
wc_ecc_free(&keySig->ks.ecc.key);
#endif
}
}
}
#endif


Expand Down Expand Up @@ -12234,7 +12205,7 @@ int SendUserAuthRequest(WOLFSSH* ssh, byte authType, int addSig)
}

if (authId == ID_USERAUTH_PUBLICKEY)
CleanupUserAuthRequestPublicKey(keySig_ptr);
wolfSSH_KEY_clean(keySig_ptr);

if (ret == WS_SUCCESS) {
ret = wolfSSH_SendPacket(ssh);
Expand Down
4 changes: 4 additions & 0 deletions src/ssh.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1824,6 +1824,10 @@ int wolfSSH_ReadKey_file(const char* name,
format = WOLFSSH_FORMAT_SSH;
in[inSz] = 0;
}
#if 0
else if (WSTRNSTR((const char*)in, PrivBeginOpenSSH, inSz) != NULL &&
WSTRNSTR((const char*)in, PrivEndOpenSSH, inSz) != NULL) {
#endif
else if (WSTRNSTR((const char*)in, PrivBeginOpenSSH, inSz) != NULL) {
*isPrivate = 1;
format = WOLFSSH_FORMAT_OPENSSH;
Expand Down
3 changes: 2 additions & 1 deletion wolfssh/error.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,9 @@ enum WS_ErrorCodes {
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_KEY_FORMAT_E = -1092, /* OpenSSH key format fail */

WS_LAST_E = -1091 /* Update this to indicate last error */
WS_LAST_E = -1092 /* Update this to indicate last error */
};


Expand Down
3 changes: 3 additions & 0 deletions wolfssh/internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,9 @@ enum {
#ifndef WOLFSSH_MAX_PUB_KEY_ALGO
#define WOLFSSH_MAX_PUB_KEY_ALGO (WOLFSSH_MAX_PVT_KEYS + 2)
#endif
#ifndef WOLFSSH_KEY_QUANTITY_REQ
#define WOLFSSH_KEY_QUANTITY_REQ 1
#endif

WOLFSSH_LOCAL byte NameToId(const char*, word32);
WOLFSSH_LOCAL const char* IdToName(byte);
Expand Down

0 comments on commit 9214981

Please sign in to comment.