mirror of https://github.com/wolfSSL/wolfssh.git
wolfsshd: fix Match User/Group directive misparse
Yosuke Shimizu
John Safranek
parent
0256f4c8fb
commit
1d2b95f979
|
|
@ -923,44 +923,86 @@ static int HandleChrootDir(WOLFSSHD_CONFIG* conf, const char* value)
|
|||
}
|
||||
|
||||
|
||||
/* returns WS_SUCCESS on success, helps with adding a restricted case to the
|
||||
* config */
|
||||
static int AddRestrictedCase(WOLFSSHD_CONFIG* config, const char* mtch,
|
||||
const char* value, char** out)
|
||||
/* Parse the value of a Match directive into the user and group applies-to
|
||||
* fields of 'config'. The value is a whitespace separated sequence of
|
||||
* keyword/name pairs, e.g. "User alice Group admins". The token immediately
|
||||
* following a "User" or "Group" keyword is taken literally as the name and is
|
||||
* never re-examined as a keyword, so a principal named like the opposite
|
||||
* keyword (e.g. "Match User Group") is handled by position rather than by a
|
||||
* substring search. A recognized keyword with no following name (e.g. a bare
|
||||
* "Match User") is a configuration error so the admin's intent is not silently
|
||||
* dropped. Unrecognized tokens are ignored to stay lenient toward Match
|
||||
* criteria that are not yet supported. Returns WS_SUCCESS on success. */
|
||||
static int ParseMatchCriteria(WOLFSSHD_CONFIG* config, const char* value)
|
||||
{
|
||||
int ret = WS_SUCCESS;
|
||||
char* pt;
|
||||
const char* pt;
|
||||
|
||||
pt = (char*)XSTRSTR(value, mtch);
|
||||
if (pt != NULL) {
|
||||
int sz, i;
|
||||
if (config == NULL || value == NULL) {
|
||||
ret = WS_BAD_ARGUMENT;
|
||||
}
|
||||
|
||||
pt += (int)XSTRLEN(mtch);
|
||||
sz = (int)XSTRLEN(pt);
|
||||
pt = value;
|
||||
while (ret == WS_SUCCESS && pt != NULL && *pt != '\0') {
|
||||
const char* tok;
|
||||
int tokSz;
|
||||
char** out = NULL;
|
||||
|
||||
/* remove spaces between 'mtch' and the user name */
|
||||
for (i = 0; i < sz; i++) {
|
||||
if (pt[i] != ' ') break;
|
||||
/* skip separators preceding the keyword token */
|
||||
while (WISSPACE((unsigned char)*pt)) {
|
||||
pt++;
|
||||
}
|
||||
if (i == sz) {
|
||||
wolfSSH_Log(WS_LOG_ERROR,
|
||||
"[SSHD] No valid input found with Match");
|
||||
ret = WS_FATAL_ERROR;
|
||||
if (*pt == '\0') {
|
||||
break;
|
||||
}
|
||||
|
||||
if (ret == WS_SUCCESS) {
|
||||
pt += i;
|
||||
sz -= i;
|
||||
/* read the keyword token */
|
||||
tok = pt;
|
||||
while (*pt != '\0' && !WISSPACE((unsigned char)*pt)) {
|
||||
pt++;
|
||||
}
|
||||
tokSz = (int)(pt - tok);
|
||||
|
||||
/* get the actual size of the user name */
|
||||
for (i = 0; i < sz; i++) {
|
||||
if (pt[i] == ' ' || pt[i] == '\r' || pt[i] == '\n') break;
|
||||
/* map the keyword to its applies-to field; ignore anything else */
|
||||
if (tokSz == (int)XSTRLEN("User") &&
|
||||
WSTRNCMP(tok, "User", tokSz) == 0) {
|
||||
out = &config->usrAppliesTo;
|
||||
}
|
||||
else if (tokSz == (int)XSTRLEN("Group") &&
|
||||
WSTRNCMP(tok, "Group", tokSz) == 0) {
|
||||
out = &config->groupAppliesTo;
|
||||
}
|
||||
|
||||
if (out != NULL) {
|
||||
/* skip separators between the keyword and its name */
|
||||
while (WISSPACE((unsigned char)*pt)) {
|
||||
pt++;
|
||||
}
|
||||
sz = i;
|
||||
|
||||
ret = CreateString(out, pt, sz, config->heap);
|
||||
/* the next token is the name, taken literally */
|
||||
tok = pt;
|
||||
while (*pt != '\0' && !WISSPACE((unsigned char)*pt)) {
|
||||
pt++;
|
||||
}
|
||||
tokSz = (int)(pt - tok);
|
||||
|
||||
if (tokSz == 0) {
|
||||
wolfSSH_Log(WS_LOG_ERROR,
|
||||
"[SSHD] Match %s directive is missing a name",
|
||||
(out == &config->usrAppliesTo) ? "User" : "Group");
|
||||
ret = WS_FATAL_ERROR;
|
||||
}
|
||||
|
||||
if (ret == WS_SUCCESS) {
|
||||
/* a repeated keyword replaces the earlier value */
|
||||
if (*out != NULL) {
|
||||
FreeString(out, config->heap);
|
||||
}
|
||||
ret = CreateString(out, tok, tokSz, config->heap);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return ret;
|
||||
}
|
||||
|
||||
|
|
@ -1052,16 +1094,9 @@ static int HandleMatch(WOLFSSHD_CONFIG** conf, const char* value, int valueSz)
|
|||
}
|
||||
}
|
||||
|
||||
/* users the settings apply to */
|
||||
/* parse the User/Group criteria the settings apply to */
|
||||
if (ret == WS_SUCCESS) {
|
||||
ret = AddRestrictedCase(newConf, "User", value,
|
||||
&newConf->usrAppliesTo);
|
||||
}
|
||||
|
||||
/* groups the settings apply to */
|
||||
if (ret == WS_SUCCESS) {
|
||||
ret = AddRestrictedCase(newConf, "Group", value,
|
||||
&newConf->groupAppliesTo);
|
||||
ret = ParseMatchCriteria(newConf, value);
|
||||
}
|
||||
|
||||
/* @TODO handle , separated user/group list */
|
||||
|
|
@ -1077,6 +1112,10 @@ static int HandleMatch(WOLFSSHD_CONFIG** conf, const char* value, int valueSz)
|
|||
(*conf)->next = newConf;
|
||||
(*conf) = newConf;
|
||||
}
|
||||
else {
|
||||
/* newConf was allocated but not linked into the list; free it */
|
||||
wolfSSHD_ConfigFree(newConf);
|
||||
}
|
||||
|
||||
(void)valueSz;
|
||||
return ret;
|
||||
|
|
|
|||
|
|
@ -597,6 +597,329 @@ static int test_MatchUnsupportedSelector(void)
|
|||
return ret;
|
||||
}
|
||||
|
||||
/* A Match directive whose user name contains the substring "Group" (e.g.
|
||||
* "GroupAdmin") must not yield a spurious group applies-to value. The parser is
|
||||
* a positional tokenizer, so "Match User GroupAdmin" sets only the user
|
||||
* applies-to: a lookup by the real user name resolves to the Match node, while
|
||||
* a lookup by the ghost group token ("Admin") must fall back to the global
|
||||
* head. This prevents an unrelated OS account whose primary group equals the
|
||||
* ghost token from inheriting the Match block's auth overrides. */
|
||||
static int test_GetUserConfMatchSubstring(void)
|
||||
{
|
||||
int ret = WS_SUCCESS;
|
||||
WOLFSSHD_CONFIG* head;
|
||||
WOLFSSHD_CONFIG* conf;
|
||||
WOLFSSHD_CONFIG* match;
|
||||
WOLFSSHD_CONFIG* ghost;
|
||||
|
||||
head = wolfSSHD_ConfigNew(NULL);
|
||||
if (head == NULL)
|
||||
ret = WS_MEMORY_E;
|
||||
conf = head;
|
||||
|
||||
#define PCL(s) ParseConfigLine(&conf, s, (int)WSTRLEN(s), 0)
|
||||
if (ret == WS_SUCCESS) ret = PCL("PasswordAuthentication no");
|
||||
|
||||
/* user name embeds the "Group" keyword as a substring */
|
||||
if (ret == WS_SUCCESS) ret = PCL("Match User GroupAdmin");
|
||||
if (ret == WS_SUCCESS) ret = PCL("PasswordAuthentication yes");
|
||||
#undef PCL
|
||||
|
||||
/* lookup by the real user name must resolve to the Match node */
|
||||
if (ret == WS_SUCCESS) {
|
||||
match = wolfSSHD_GetUserConf(head, "GroupAdmin", NULL, NULL, NULL,
|
||||
NULL, NULL, NULL);
|
||||
if (match == NULL || match == head)
|
||||
ret = WS_FATAL_ERROR;
|
||||
}
|
||||
if (ret == WS_SUCCESS) {
|
||||
if (wolfSSHD_ConfigGetPwAuth(match) != 1)
|
||||
ret = WS_FATAL_ERROR;
|
||||
}
|
||||
|
||||
/* lookup by the ghost group token ("Admin") must NOT match; it falls back
|
||||
* to the permissive-denied global head */
|
||||
if (ret == WS_SUCCESS) {
|
||||
ghost = wolfSSHD_GetUserConf(head, NULL, "Admin", NULL, NULL,
|
||||
NULL, NULL, NULL);
|
||||
if (ghost != head)
|
||||
ret = WS_FATAL_ERROR;
|
||||
}
|
||||
if (ret == WS_SUCCESS) {
|
||||
if (wolfSSHD_ConfigGetPwAuth(head) != 0)
|
||||
ret = WS_FATAL_ERROR;
|
||||
}
|
||||
|
||||
wolfSSHD_ConfigFree(head);
|
||||
return ret;
|
||||
}
|
||||
|
||||
/* Symmetric counterpart to test_GetUserConfMatchSubstring: a Match directive
|
||||
* whose group name contains the substring "User" (e.g. "UserStaff") must not
|
||||
* yield a spurious user applies-to value. "Match Group UserStaff" sets only the
|
||||
* group applies-to: a lookup by the real group name resolves to the Match node,
|
||||
* while a lookup by the ghost user token ("Staff") must fall back to the global
|
||||
* head. This locks in the "User" keyword direction of the positional parser. */
|
||||
static int test_GetUserConfMatchSubstringGroup(void)
|
||||
{
|
||||
int ret = WS_SUCCESS;
|
||||
WOLFSSHD_CONFIG* head;
|
||||
WOLFSSHD_CONFIG* conf;
|
||||
WOLFSSHD_CONFIG* match;
|
||||
WOLFSSHD_CONFIG* ghost;
|
||||
|
||||
head = wolfSSHD_ConfigNew(NULL);
|
||||
if (head == NULL)
|
||||
ret = WS_MEMORY_E;
|
||||
conf = head;
|
||||
|
||||
#define PCL(s) ParseConfigLine(&conf, s, (int)WSTRLEN(s), 0)
|
||||
if (ret == WS_SUCCESS) ret = PCL("PasswordAuthentication no");
|
||||
|
||||
/* group name embeds the "User" keyword as a substring */
|
||||
if (ret == WS_SUCCESS) ret = PCL("Match Group UserStaff");
|
||||
if (ret == WS_SUCCESS) ret = PCL("PasswordAuthentication yes");
|
||||
#undef PCL
|
||||
|
||||
/* lookup by the real group name must resolve to the Match node */
|
||||
if (ret == WS_SUCCESS) {
|
||||
match = wolfSSHD_GetUserConf(head, NULL, "UserStaff", NULL, NULL,
|
||||
NULL, NULL, NULL);
|
||||
if (match == NULL || match == head)
|
||||
ret = WS_FATAL_ERROR;
|
||||
}
|
||||
if (ret == WS_SUCCESS) {
|
||||
if (wolfSSHD_ConfigGetPwAuth(match) != 1)
|
||||
ret = WS_FATAL_ERROR;
|
||||
}
|
||||
|
||||
/* lookup by the ghost user token ("Staff") must NOT match; it falls back
|
||||
* to the permissive-denied global head */
|
||||
if (ret == WS_SUCCESS) {
|
||||
ghost = wolfSSHD_GetUserConf(head, "Staff", NULL, NULL, NULL,
|
||||
NULL, NULL, NULL);
|
||||
if (ghost != head)
|
||||
ret = WS_FATAL_ERROR;
|
||||
}
|
||||
if (ret == WS_SUCCESS) {
|
||||
if (wolfSSHD_ConfigGetPwAuth(head) != 0)
|
||||
ret = WS_FATAL_ERROR;
|
||||
}
|
||||
|
||||
wolfSSHD_ConfigFree(head);
|
||||
return ret;
|
||||
}
|
||||
|
||||
/* A principal named exactly like the opposite keyword must be handled by token
|
||||
* position, not by the embedded keyword. "Match User Group" names a user whose
|
||||
* name happens to be "Group": it must set usrAppliesTo="Group" and leave
|
||||
* groupAppliesTo unset. The positional parser consumes the token after the
|
||||
* "User" keyword as the name and never re-examines it as a keyword, so the
|
||||
* "Group" token here is treated as the user name, not the Group keyword. A
|
||||
* lookup by user "Group" must resolve to the Match node, while a lookup by
|
||||
* group "Group" must fall back to the head. This locks the behavior against a
|
||||
* refactor that would treat a name token spelling a keyword as a keyword. */
|
||||
static int test_GetUserConfMatchLiteralKeywordName(void)
|
||||
{
|
||||
int ret = WS_SUCCESS;
|
||||
WOLFSSHD_CONFIG* head;
|
||||
WOLFSSHD_CONFIG* conf;
|
||||
WOLFSSHD_CONFIG* match;
|
||||
WOLFSSHD_CONFIG* ghost;
|
||||
|
||||
head = wolfSSHD_ConfigNew(NULL);
|
||||
if (head == NULL)
|
||||
ret = WS_MEMORY_E;
|
||||
conf = head;
|
||||
|
||||
#define PCL(s) ParseConfigLine(&conf, s, (int)WSTRLEN(s), 0)
|
||||
if (ret == WS_SUCCESS) ret = PCL("PasswordAuthentication no");
|
||||
|
||||
/* user literally named "Group" (the opposite keyword) at end of line */
|
||||
if (ret == WS_SUCCESS) ret = PCL("Match User Group");
|
||||
if (ret == WS_SUCCESS) ret = PCL("PasswordAuthentication yes");
|
||||
#undef PCL
|
||||
|
||||
/* lookup by the user name "Group" must resolve to the Match node */
|
||||
if (ret == WS_SUCCESS) {
|
||||
match = wolfSSHD_GetUserConf(head, "Group", NULL, NULL, NULL,
|
||||
NULL, NULL, NULL);
|
||||
if (match == NULL || match == head)
|
||||
ret = WS_FATAL_ERROR;
|
||||
}
|
||||
if (ret == WS_SUCCESS) {
|
||||
if (wolfSSHD_ConfigGetPwAuth(match) != 1)
|
||||
ret = WS_FATAL_ERROR;
|
||||
}
|
||||
|
||||
/* the user name must NOT have leaked into groupAppliesTo: a lookup by group
|
||||
* "Group" must fall back to the permissive-denied global head */
|
||||
if (ret == WS_SUCCESS) {
|
||||
ghost = wolfSSHD_GetUserConf(head, NULL, "Group", NULL, NULL,
|
||||
NULL, NULL, NULL);
|
||||
if (ghost != head)
|
||||
ret = WS_FATAL_ERROR;
|
||||
}
|
||||
if (ret == WS_SUCCESS) {
|
||||
if (wolfSSHD_ConfigGetPwAuth(head) != 0)
|
||||
ret = WS_FATAL_ERROR;
|
||||
}
|
||||
|
||||
wolfSSHD_ConfigFree(head);
|
||||
return ret;
|
||||
}
|
||||
|
||||
/* A Match directive whose keyword has no following name (e.g. a bare
|
||||
* "Match User") is a configuration error and must fail closed: ParseConfigLine
|
||||
* returns an error rather than silently loading a dead node that would discard
|
||||
* the admin's subsequent settings. The global head must be left untouched. */
|
||||
static int test_GetUserConfMatchBareKeyword(void)
|
||||
{
|
||||
int ret = WS_SUCCESS;
|
||||
int rc;
|
||||
WOLFSSHD_CONFIG* head;
|
||||
WOLFSSHD_CONFIG* conf;
|
||||
|
||||
head = wolfSSHD_ConfigNew(NULL);
|
||||
if (head == NULL)
|
||||
ret = WS_MEMORY_E;
|
||||
conf = head;
|
||||
|
||||
#define PCL(s) ParseConfigLine(&conf, s, (int)WSTRLEN(s), 0)
|
||||
if (ret == WS_SUCCESS) ret = PCL("PasswordAuthentication no");
|
||||
|
||||
/* bare keyword with no name must be rejected */
|
||||
if (ret == WS_SUCCESS) {
|
||||
rc = PCL("Match User");
|
||||
if (rc == WS_SUCCESS)
|
||||
ret = WS_FATAL_ERROR;
|
||||
}
|
||||
#undef PCL
|
||||
|
||||
/* the rejected directive must not have altered the global head */
|
||||
if (ret == WS_SUCCESS) {
|
||||
if (wolfSSHD_ConfigGetPwAuth(head) != 0)
|
||||
ret = WS_FATAL_ERROR;
|
||||
}
|
||||
|
||||
wolfSSHD_ConfigFree(head);
|
||||
return ret;
|
||||
}
|
||||
|
||||
/* A keyword repeated on one Match line ("Match User a User b") replaces the
|
||||
* earlier name: the parser frees the first usrAppliesTo ("a") before storing
|
||||
* the second ("b"). The final value must be "b", so a lookup by user "b"
|
||||
* resolves to the Match node while a lookup by the replaced name "a" falls back
|
||||
* to the global head. This exercises the free-then-reallocate path (run under
|
||||
* ASan to catch a leak or double-free regression). */
|
||||
static int test_GetUserConfMatchRepeatedKeyword(void)
|
||||
{
|
||||
int ret = WS_SUCCESS;
|
||||
WOLFSSHD_CONFIG* head;
|
||||
WOLFSSHD_CONFIG* conf;
|
||||
WOLFSSHD_CONFIG* match;
|
||||
WOLFSSHD_CONFIG* old;
|
||||
|
||||
head = wolfSSHD_ConfigNew(NULL);
|
||||
if (head == NULL)
|
||||
ret = WS_MEMORY_E;
|
||||
conf = head;
|
||||
|
||||
#define PCL(s) ParseConfigLine(&conf, s, (int)WSTRLEN(s), 0)
|
||||
if (ret == WS_SUCCESS) ret = PCL("PasswordAuthentication no");
|
||||
|
||||
/* same keyword twice; the second name must replace the first */
|
||||
if (ret == WS_SUCCESS) ret = PCL("Match User a User b");
|
||||
if (ret == WS_SUCCESS) ret = PCL("PasswordAuthentication yes");
|
||||
#undef PCL
|
||||
|
||||
/* lookup by the replacement name "b" must resolve to the Match node */
|
||||
if (ret == WS_SUCCESS) {
|
||||
match = wolfSSHD_GetUserConf(head, "b", NULL, NULL, NULL,
|
||||
NULL, NULL, NULL);
|
||||
if (match == NULL || match == head ||
|
||||
wolfSSHD_ConfigGetPwAuth(match) != 1)
|
||||
ret = WS_FATAL_ERROR;
|
||||
}
|
||||
|
||||
/* lookup by the replaced name "a" must NOT match; it falls back to head */
|
||||
if (ret == WS_SUCCESS) {
|
||||
old = wolfSSHD_GetUserConf(head, "a", NULL, NULL, NULL,
|
||||
NULL, NULL, NULL);
|
||||
if (old != head)
|
||||
ret = WS_FATAL_ERROR;
|
||||
}
|
||||
if (ret == WS_SUCCESS) {
|
||||
if (wolfSSHD_ConfigGetPwAuth(head) != 0)
|
||||
ret = WS_FATAL_ERROR;
|
||||
}
|
||||
|
||||
wolfSSHD_ConfigFree(head);
|
||||
return ret;
|
||||
}
|
||||
|
||||
/* A single Match line carrying both criteria ("Match User alice Group admins")
|
||||
* is parsed as two keyword/name pairs, setting both usrAppliesTo="alice" and
|
||||
* groupAppliesTo="admins" on one node. Both the user and the group lookup must
|
||||
* resolve to that node, while unrelated names fall back to the global head.
|
||||
* This locks in that the parser still extracts both names when both keywords
|
||||
* share a line. */
|
||||
static int test_GetUserConfMatchUserAndGroup(void)
|
||||
{
|
||||
int ret = WS_SUCCESS;
|
||||
WOLFSSHD_CONFIG* head;
|
||||
WOLFSSHD_CONFIG* conf;
|
||||
WOLFSSHD_CONFIG* byUsr;
|
||||
WOLFSSHD_CONFIG* byGrp;
|
||||
WOLFSSHD_CONFIG* other;
|
||||
|
||||
head = wolfSSHD_ConfigNew(NULL);
|
||||
if (head == NULL)
|
||||
ret = WS_MEMORY_E;
|
||||
conf = head;
|
||||
|
||||
#define PCL(s) ParseConfigLine(&conf, s, (int)WSTRLEN(s), 0)
|
||||
if (ret == WS_SUCCESS) ret = PCL("PasswordAuthentication no");
|
||||
|
||||
/* both criteria on one line */
|
||||
if (ret == WS_SUCCESS) ret = PCL("Match User alice Group admins");
|
||||
if (ret == WS_SUCCESS) ret = PCL("PasswordAuthentication yes");
|
||||
#undef PCL
|
||||
|
||||
/* lookup by the user criterion must resolve to the Match node */
|
||||
if (ret == WS_SUCCESS) {
|
||||
byUsr = wolfSSHD_GetUserConf(head, "alice", NULL, NULL, NULL,
|
||||
NULL, NULL, NULL);
|
||||
if (byUsr == NULL || byUsr == head ||
|
||||
wolfSSHD_ConfigGetPwAuth(byUsr) != 1)
|
||||
ret = WS_FATAL_ERROR;
|
||||
}
|
||||
|
||||
/* lookup by the group criterion must resolve to the same Match node */
|
||||
if (ret == WS_SUCCESS) {
|
||||
byGrp = wolfSSHD_GetUserConf(head, NULL, "admins", NULL, NULL,
|
||||
NULL, NULL, NULL);
|
||||
if (byGrp == NULL || byGrp == head ||
|
||||
wolfSSHD_ConfigGetPwAuth(byGrp) != 1)
|
||||
ret = WS_FATAL_ERROR;
|
||||
}
|
||||
|
||||
/* unrelated user and group must fall back to the permissive-denied head */
|
||||
if (ret == WS_SUCCESS) {
|
||||
other = wolfSSHD_GetUserConf(head, "bob", "staff", NULL, NULL,
|
||||
NULL, NULL, NULL);
|
||||
if (other != head)
|
||||
ret = WS_FATAL_ERROR;
|
||||
}
|
||||
if (ret == WS_SUCCESS) {
|
||||
if (wolfSSHD_ConfigGetPwAuth(head) != 0)
|
||||
ret = WS_FATAL_ERROR;
|
||||
}
|
||||
|
||||
wolfSSHD_ConfigFree(head);
|
||||
return ret;
|
||||
}
|
||||
|
||||
/* Bounded recursion through Include directives: a self-including config
|
||||
* must fail with WS_BAD_ARGUMENT once the depth limit is hit, and the
|
||||
* config object must remain usable so a subsequent load of a normal
|
||||
|
|
@ -1100,6 +1423,12 @@ const TEST_CASE testCases[] = {
|
|||
TEST_DECL(test_ConfigCopy),
|
||||
TEST_DECL(test_GetUserConfMatchOverride),
|
||||
TEST_DECL(test_MatchUnsupportedSelector),
|
||||
TEST_DECL(test_GetUserConfMatchSubstring),
|
||||
TEST_DECL(test_GetUserConfMatchSubstringGroup),
|
||||
TEST_DECL(test_GetUserConfMatchLiteralKeywordName),
|
||||
TEST_DECL(test_GetUserConfMatchBareKeyword),
|
||||
TEST_DECL(test_GetUserConfMatchRepeatedKeyword),
|
||||
TEST_DECL(test_GetUserConfMatchUserAndGroup),
|
||||
TEST_DECL(test_CAKeysFileDiffers),
|
||||
TEST_DECL(test_IncludeRecursionBound),
|
||||
TEST_DECL(test_GetUserAuthTypes),
|
||||
|
|
|
|||
Loading…
Reference in New Issue