Skip to content

Anchor the key type checks in ReadKey_file #1363

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ejohnstown wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Anchor the key type checks in ReadKey_file #1363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 18 additions & 12 deletions src/ssh.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3083,6 +3083,15 @@ static int ReadFileIntoBuffer(const char* name, byte** out, word32* outSz,
}


/* Compares only the first bytes of in, not the secret key body after them. */
static int KeyFileHasPrefix(const byte* in, word32 inSz, const char* prefix)
{
word32 prefixSz = (word32)WSTRLEN(prefix);

return inSz >= prefixSz && WMEMCMP(in, prefix, prefixSz) == 0;
}


/* Reads a key from the file name into a buffer. An SSH algorithm name marks
an SSH format public key and "-----BEGIN " a PEM one, otherwise it is an
ASN.1 private key. The buffer goes to wolfSSH_ReadKey_buffer_ex(). */
Expand All @@ -3104,24 +3113,21 @@ int wolfSSH_ReadKey_file(const char* name,

ret = ReadFileIntoBuffer(name, &in, &inSz, heap);
if (ret == WS_SUCCESS) {
if (WSTRNSTR((const char*)in, "ssh-rsa", inSz) == (const char*)in
|| WSTRNSTR((const char*)in,
"ecdsa-sha2-nistp", inSz) == (const char*)in
|| WSTRNSTR((const char*)in,
"ssh-ed25519", inSz) == (const char*)in
|| WSTRNSTR((const char*)in,
"ssh-mldsa-", inSz) == (const char*)in
|| WSTRNSTR((const char*)in,
"x509v3-ssh-mldsa-", inSz) == (const char*)in) {
if (KeyFileHasPrefix(in, inSz, "ssh-rsa")
|| KeyFileHasPrefix(in, inSz, "ecdsa-sha2-nistp")
|| KeyFileHasPrefix(in, inSz, "ssh-ed25519")
|| KeyFileHasPrefix(in, inSz, "ssh-mldsa")
|| KeyFileHasPrefix(in, inSz, "x509v3-ssh-mldsa-")) {
*isPrivate = 0;
format = WOLFSSH_FORMAT_SSH;
}
else if (WSTRNSTR((const char*)in, PrivBeginOpenSSH, inSz) != NULL) {
else if (KeyFileHasPrefix(in, inSz, PrivBeginOpenSSH)) {
*isPrivate = 1;
format = WOLFSSH_FORMAT_OPENSSH;
}
else if ((WSTRNSTR((const char*)in, PrivBeginPrefix, inSz)
== (const char*)in)
/* The suffix is searched for, not anchored: an EC PARAMETERS block
* can come before the private key. */
else if (KeyFileHasPrefix(in, inSz, PrivBeginPrefix)
&& (WSTRNSTR((const char*)in, PrivSuffix, inSz)
!= NULL)) {
Comment thread
ejohnstown marked this conversation as resolved.
Resolved
*isPrivate = 1;
Expand Down
58 changes: 58 additions & 0 deletions tests/api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2887,6 +2887,63 @@ static void test_wolfSSH_ReadKey_sshNoComment(void)
}


/* wolfSSH_ReadKey_file() must take every ML-DSA public key name as an SSH
* format public key, including the composite ones such as
* "ssh-mldsa44-es256@wolfssl.com", which have no '-' after "ssh-mldsa". */
static void test_wolfSSH_ReadKey_file_mldsaPub(void)
{
#if !defined(NO_FILESYSTEM) && !defined(NO_WRITE_TEMP_FILES) && \
!defined(WOLFSSH_USER_FILESYSTEM)
#ifdef WOLFSSH_ZEPHYR
static const char path[] =
CONFIG_WOLFSSH_SFTP_DEFAULT_DIR "/readkey-mldsa-test.pub";
#else
static const char path[] = "./readkey-mldsa-test.pub";
#endif
static const char* keys[] = {
"ssh-mldsa44-es256@wolfssl.com AAAAB3NzaC1yc2E= comment\n",
"ssh-mldsa65-ed25519@wolfssl.com AAAAB3NzaC1yc2E= comment\n",
"ssh-mldsa87-es384@wolfssl.com AAAAB3NzaC1yc2E= comment\n",
"ssh-mldsa44-ed25519@openssh.com AAAAB3NzaC1yc2E= comment\n",
"ssh-mldsa-44 AAAAB3NzaC1yc2E= comment\n"
};
WFILE* fp = NULL;
byte* out = NULL;
const byte* outType = NULL;
word32 outSz = 0;
word32 outTypeSz = 0;
word32 i;
word32 keySz;
byte isPrivate;

for (i = 0; i < (word32)(sizeof(keys) / sizeof(keys[0])); i++) {
keySz = (word32)WSTRLEN(keys[i]);
AssertIntEQ(WFOPEN(NULL, &fp, path, "wb"), 0);
AssertNotNull(fp);
AssertIntEQ((int)WFWRITE(NULL, keys[i], 1, keySz, fp), (int)keySz);
WFCLOSE(NULL, fp);

out = NULL;
outSz = 0;
isPrivate = 1;
AssertIntEQ(wolfSSH_ReadKey_file(path, &out, &outSz,
&outType, &outTypeSz, &isPrivate, NULL), WS_SUCCESS);
AssertIntEQ(isPrivate, 0);
AssertNotNull(out);
AssertIntGT(outSz, 0);
WFREE(out, NULL, DYNTYPE_PRIVKEY);

/* Zephyr's "w" open does not truncate, so start each key afresh. */
#ifdef WOLFSSH_ZEPHYR
AssertIntEQ(0, fs_unlink(path));
#else
AssertIntEQ(0, remove(path));
#endif
}
#endif
}


#ifdef WOLFSSH_SCP

static int my_ScpRecv(WOLFSSH* ssh, int state, const char* basePath,
Expand Down Expand Up @@ -9355,6 +9412,7 @@ int wolfSSH_ApiTest(int argc, char** argv)
test_wolfSSH_ReadKey_shortBuffer();
test_wolfSSH_ReadKey_noTrailingNewline();
test_wolfSSH_ReadKey_sshNoComment();
test_wolfSSH_ReadKey_file_mldsaPub();
test_wolfSSH_QueryAlgoList();
test_wolfSSH_SetMaxAuthAttempts();
test_wolfSSH_StrictKex();
Expand Down
Loading