Skip to content

Commit f60b00c

Browse files
yosuke-wolfsslejohnstown
authored andcommitted
ossh: check the key-type parse before the ID lookup
- GetOpenSshPublicKey() calls NameToId() and enters the key-type switch only when GetStringRef() returns WS_SUCCESS, and returns that result otherwise. - publicKeyType starts NULL and keyId starts ID_UNKNOWN. - tests/api.c adds test_GetOpenSshPublicKey_type(), gated on WOLFSSH_TPM and WOLFSSH_TEST_INTERNAL, covering a truncated type string, a truncated length prefix, an empty type, an unsupported type and a well-formed ssh-rsa blob. - Each case asserts idx alongside the return code: UINT32_SZ for a truncated type string, 0 for a truncated length prefix, and the full blob size for the empty type, the unsupported type and the ssh-rsa key. Issue: F-11650
1 parent faca3bc commit f60b00c

2 files changed

Lines changed: 112 additions & 39 deletions

File tree

src/ossh.c

Lines changed: 44 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -471,50 +471,55 @@ static int GetOpenSshPublicKeyRsa(RsaKey* key, const byte* buf, word32 len,
471471
int GetOpenSshPublicKey(WS_KeySignature *key,
472472
const byte* buf, word32 len, word32* idx)
473473
{
474-
int ret = WS_SUCCESS;
475-
const byte* publicKeyType;
474+
int ret;
475+
const byte* publicKeyType = NULL;
476476
word32 publicKeyTypeSz = 0;
477-
byte keyId;
477+
byte keyId = ID_UNKNOWN;
478478

479479
ret = GetStringRef(&publicKeyTypeSz, &publicKeyType, buf, len, idx);
480-
keyId = NameToId((const char*)publicKeyType, publicKeyTypeSz);
481480

482-
switch (keyId) {
483-
#ifndef WOLFSSH_NO_RSA
484-
case ID_SSH_RSA:
485-
ret = GetOpenSshPublicKeyRsa(&key->ks.rsa.key, buf, len, idx);
486-
break;
487-
#endif
488-
#ifndef WOLFSSH_NO_ECDSA
489-
case ID_ECDSA_SHA2_NISTP256:
490-
case ID_ECDSA_SHA2_NISTP384:
491-
case ID_ECDSA_SHA2_NISTP521:
492-
ret = GetOpenSshPublicKeyEcc(&key->ks.ecc.key, buf, len, idx);
493-
break;
494-
#endif
495-
#ifndef WOLFSSH_NO_ED25519
496-
case ID_ED25519:
497-
ret = GetOpenSshKeyPublicEd25519(&key->ks.ed25519.key, buf, len, idx);
498-
break;
499-
#endif
500-
#ifndef WOLFSSH_NO_MLDSA
501-
case ID_MLDSA44:
502-
ret = GetOpenSshKeyPublicMlDsa(&key->ks.mldsa.key, buf, len,
503-
idx, WC_ML_DSA_44);
504-
break;
505-
case ID_MLDSA65:
506-
ret = GetOpenSshKeyPublicMlDsa(&key->ks.mldsa.key, buf, len,
507-
idx, WC_ML_DSA_65);
508-
break;
509-
case ID_MLDSA87:
510-
ret = GetOpenSshKeyPublicMlDsa(&key->ks.mldsa.key, buf, len,
511-
idx, WC_ML_DSA_87);
512-
break;
513-
#endif
514-
default:
515-
ret = WS_UNIMPLEMENTED_E;
516-
break;
481+
if (ret == WS_SUCCESS) {
482+
keyId = NameToId((const char*)publicKeyType, publicKeyTypeSz);
483+
484+
switch (keyId) {
485+
#ifndef WOLFSSH_NO_RSA
486+
case ID_SSH_RSA:
487+
ret = GetOpenSshPublicKeyRsa(&key->ks.rsa.key, buf, len, idx);
488+
break;
489+
#endif
490+
#ifndef WOLFSSH_NO_ECDSA
491+
case ID_ECDSA_SHA2_NISTP256:
492+
case ID_ECDSA_SHA2_NISTP384:
493+
case ID_ECDSA_SHA2_NISTP521:
494+
ret = GetOpenSshPublicKeyEcc(&key->ks.ecc.key, buf, len, idx);
495+
break;
496+
#endif
497+
#ifndef WOLFSSH_NO_ED25519
498+
case ID_ED25519:
499+
ret = GetOpenSshKeyPublicEd25519(&key->ks.ed25519.key, buf, len,
500+
idx);
501+
break;
502+
#endif
503+
#ifndef WOLFSSH_NO_MLDSA
504+
case ID_MLDSA44:
505+
ret = GetOpenSshKeyPublicMlDsa(&key->ks.mldsa.key, buf, len,
506+
idx, WC_ML_DSA_44);
507+
break;
508+
case ID_MLDSA65:
509+
ret = GetOpenSshKeyPublicMlDsa(&key->ks.mldsa.key, buf, len,
510+
idx, WC_ML_DSA_65);
511+
break;
512+
case ID_MLDSA87:
513+
ret = GetOpenSshKeyPublicMlDsa(&key->ks.mldsa.key, buf, len,
514+
idx, WC_ML_DSA_87);
515+
break;
516+
#endif
517+
default:
518+
ret = WS_UNIMPLEMENTED_E;
519+
break;
520+
}
517521
}
522+
518523
return ret;
519524
}
520525

tests/api.c

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2270,6 +2270,71 @@ static void test_LoadTpmSshKey_NoTrailingNewline(void)
22702270

22712271
#endif /* WOLFSSH_TPM && FILESYSTEM && !USER_FILESYSTEM */
22722272

2273+
#if defined(WOLFSSH_TPM) && defined(WOLFSSH_TEST_INTERNAL)
2274+
2275+
/* The key type is read with GetStringRef(), which sets the length from the
2276+
* wire but leaves the pointer alone when the name runs past the buffer. */
2277+
static void test_GetOpenSshPublicKey_type(void)
2278+
{
2279+
/* "ssh" carrying a length of 7. */
2280+
static const byte truncType[] = {
2281+
0x00, 0x00, 0x00, 0x07, 's', 's', 'h'
2282+
};
2283+
/* Too short to hold the length prefix. */
2284+
static const byte truncLen[] = { 0x00, 0x00 };
2285+
/* Parses, but names no key. */
2286+
static const byte emptyType[] = { 0x00, 0x00, 0x00, 0x00 };
2287+
/* A known SSH key type that wolfSSH's NameIdMap does not carry. */
2288+
static const byte unsupportedType[] = {
2289+
0x00, 0x00, 0x00, 0x07, 's', 's', 'h', '-', 'd', 's', 's'
2290+
};
2291+
#ifndef WOLFSSH_NO_RSA
2292+
/* string "ssh-rsa", mpint e, mpint n. */
2293+
static const byte rsaKey[] = {
2294+
0x00, 0x00, 0x00, 0x07, 's', 's', 'h', '-', 'r', 's', 'a',
2295+
0x00, 0x00, 0x00, 0x03, 0x01, 0x00, 0x01,
2296+
0x00, 0x00, 0x00, 0x09,
2297+
0x00, 0xC5, 0x1A, 0x37, 0x8B, 0x42, 0x9D, 0xE0, 0x6F
2298+
};
2299+
#endif
2300+
WS_KeySignature keySig;
2301+
word32 idx;
2302+
2303+
WMEMSET(&keySig, 0, sizeof(keySig));
2304+
2305+
/* On failure idx keeps whatever was consumed, as elsewhere in the tree. */
2306+
idx = 0;
2307+
AssertIntEQ(GetOpenSshPublicKey(&keySig, truncType,
2308+
(word32)sizeof(truncType), &idx), WS_BUFFER_E);
2309+
AssertIntEQ(idx, UINT32_SZ);
2310+
2311+
idx = 0;
2312+
AssertIntEQ(GetOpenSshPublicKey(&keySig, truncLen,
2313+
(word32)sizeof(truncLen), &idx), WS_BUFFER_E);
2314+
AssertIntEQ(idx, 0);
2315+
2316+
idx = 0;
2317+
AssertIntEQ(GetOpenSshPublicKey(&keySig, emptyType,
2318+
(word32)sizeof(emptyType), &idx), WS_UNIMPLEMENTED_E);
2319+
AssertIntEQ(idx, (word32)sizeof(emptyType));
2320+
2321+
idx = 0;
2322+
AssertIntEQ(GetOpenSshPublicKey(&keySig, unsupportedType,
2323+
(word32)sizeof(unsupportedType), &idx), WS_UNIMPLEMENTED_E);
2324+
AssertIntEQ(idx, (word32)sizeof(unsupportedType));
2325+
2326+
#ifndef WOLFSSH_NO_RSA
2327+
idx = 0;
2328+
AssertIntEQ(wc_InitRsaKey(&keySig.ks.rsa.key, NULL), 0);
2329+
AssertIntEQ(GetOpenSshPublicKey(&keySig, rsaKey,
2330+
(word32)sizeof(rsaKey), &idx), WS_SUCCESS);
2331+
AssertIntEQ(idx, (word32)sizeof(rsaKey));
2332+
AssertIntEQ(wc_FreeRsaKey(&keySig.ks.rsa.key), 0);
2333+
#endif
2334+
}
2335+
2336+
#endif /* WOLFSSH_TPM && WOLFSSH_TEST_INTERNAL */
2337+
22732338

22742339
static void test_wolfSSH_ReadKey_badPad(void)
22752340
{
@@ -7749,6 +7814,9 @@ int wolfSSH_ApiTest(int argc, char** argv)
77497814
#if defined(WOLFSSH_TPM) && !defined(NO_FILESYSTEM) && \
77507815
!defined(NO_WRITE_TEMP_FILES) && !defined(WOLFSSH_USER_FILESYSTEM)
77517816
test_LoadTpmSshKey_NoTrailingNewline();
7817+
#endif
7818+
#if defined(WOLFSSH_TPM) && defined(WOLFSSH_TEST_INTERNAL)
7819+
test_GetOpenSshPublicKey_type();
77527820
#endif
77537821
test_wolfSSH_ReadKey_shortBuffer();
77547822
test_wolfSSH_ReadKey_noTrailingNewline();

0 commit comments

Comments
 (0)