Skip to content

Hold algorithm-list names to RFC 4251 format #1321

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
+64 −1
Open

Hold algorithm-list names to RFC 4251 format #1321

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
26 changes: 25 additions & 1 deletion src/internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4098,7 +4098,31 @@ int CheckAlgoList(const char* list, byte type)
}
else {
byte tokenType = TYPE_OTHER;
byte id = NameToIdType(name, nameSz, &tokenType);
byte id;
word32 j;
word32 atCount = 0;

/* Skipped names still go out in KEXINIT. RFC 4251 Section 6
* and RFC 4250 Section 4.6: at most 64 bytes, printable
* US-ASCII with no space, control code or DEL, at most one
* '@', with text on both sides of it. */
if (nameSz > WOLFSSH_MAX_ALGO_NAME_SZ) {
ret = WS_INVALID_ALGO_ID;
break;
}
for (j = 0; j < nameSz; j++) {
if ((byte)name[j] <= 0x20 || (byte)name[j] >= 0x7F)
break;
if (name[j] == '@' && (++atCount > 1 || j == 0
|| j == nameSz - 1))
break;
}
if (j < nameSz) {
ret = WS_INVALID_ALGO_ID;
break;
}

id = NameToIdType(name, nameSz, &tokenType);

if (id == ID_NONE) {
/* "none" names the null cipher/MAC, not a host key. It
Expand Down
34 changes: 34 additions & 0 deletions tests/api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8692,6 +8692,40 @@ static void test_wolfSSH_CheckAlgoList(void)
WSNPRINTF(listBuf, sizeof(listBuf), "%s,bogus", validCipher);
AssertIntEQ(wolfSSH_CTX_SetAlgoListCipher(ctx, listBuf), WS_SUCCESS);

/* A skipped name still goes out in KEXINIT, so it must be well formed. */
WSNPRINTF(listBuf, sizeof(listBuf), "%s,%064d", validCipher, 0);
AssertIntEQ(wolfSSH_CTX_SetAlgoListCipher(ctx, listBuf), WS_SUCCESS);
WSNPRINTF(listBuf, sizeof(listBuf), "%s,%065d", validCipher, 0);
AssertIntEQ(wolfSSH_CTX_SetAlgoListCipher(ctx, listBuf),
WS_INVALID_ALGO_ID);
WSNPRINTF(listBuf, sizeof(listBuf), "%s,bo gus", validCipher);
AssertIntEQ(wolfSSH_CTX_SetAlgoListCipher(ctx, listBuf),
WS_INVALID_ALGO_ID);
WSNPRINTF(listBuf, sizeof(listBuf), "%s,bo\tgus", validCipher);
AssertIntEQ(wolfSSH_CTX_SetAlgoListCipher(ctx, listBuf),
WS_INVALID_ALGO_ID);
WSNPRINTF(listBuf, sizeof(listBuf), "%s,bo\x7fgus", validCipher);
AssertIntEQ(wolfSSH_CTX_SetAlgoListCipher(ctx, listBuf),
WS_INVALID_ALGO_ID);
WSNPRINTF(listBuf, sizeof(listBuf), "%s,bo\xc3\xa9gus", validCipher);
AssertIntEQ(wolfSSH_CTX_SetAlgoListCipher(ctx, listBuf),
WS_INVALID_ALGO_ID);
WSNPRINTF(listBuf, sizeof(listBuf), "%s,bogus@example.com", validCipher);
AssertIntEQ(wolfSSH_CTX_SetAlgoListCipher(ctx, listBuf), WS_SUCCESS);
WSNPRINTF(listBuf, sizeof(listBuf), "%s,bo@gus@example.com",
validCipher);
AssertIntEQ(wolfSSH_CTX_SetAlgoListCipher(ctx, listBuf),
WS_INVALID_ALGO_ID);
WSNPRINTF(listBuf, sizeof(listBuf), "%s,@example.com", validCipher);
AssertIntEQ(wolfSSH_CTX_SetAlgoListCipher(ctx, listBuf),
WS_INVALID_ALGO_ID);
WSNPRINTF(listBuf, sizeof(listBuf), "%s,bogus@", validCipher);
AssertIntEQ(wolfSSH_CTX_SetAlgoListCipher(ctx, listBuf),
WS_INVALID_ALGO_ID);
WSNPRINTF(listBuf, sizeof(listBuf), "%s,@", validCipher);
AssertIntEQ(wolfSSH_CTX_SetAlgoListCipher(ctx, listBuf),
WS_INVALID_ALGO_ID);

/* A known name in the wrong category still fails the whole list. */
WSNPRINTF(listBuf, sizeof(listBuf), "%s,%s", validCipher, aKexName);
AssertIntEQ(wolfSSH_CTX_SetAlgoListCipher(ctx, listBuf),
Expand Down
2 changes: 2 additions & 0 deletions wolfssh/internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,8 @@ enum NameIdType {
/* Per-field cap on the number of names in a name-list. */
#define WOLFSSH_MAX_NAMELIST_CNT 64
#endif
/* RFC 4251 Section 6 byte cap on one algorithm name. */
#define WOLFSSH_MAX_ALGO_NAME_SZ 64
#ifndef WOLFSSH_DEFAULT_GEXDH_MIN
#define WOLFSSH_DEFAULT_GEXDH_MIN 2048
#endif
Expand Down
3 changes: 3 additions & 0 deletions wolfssh/ssh.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,9 @@ WOLFSSH_API int wolfSSH_ReadCert_file(const char* name,

/* SetAlgoList* validate the list, returning WS_SUCCESS, WS_INVALID_ALGO_ID for
* a bad list, or WS_SSH_CTX_NULL_E / WS_SSH_NULL_E for a NULL ctx / ssh.
* Unknown names are skipped, but every name must be 1 to 64 printable
* US-ASCII bytes with no space and at most one '@', not first or last
* (RFC 4251 Section 6, RFC 4250 Section 4.6).
* Kex/Cipher/Mac reject NULL. Key accepts NULL only on a server, restoring the
* default of deriving the host key list from the loaded private keys; a client
* has no such fallback. KeyAccepted accepts NULL on either side, but that
Expand Down
Loading