diff --git a/apps/wolfsshd/auth.c b/apps/wolfsshd/auth.c index d79d21508..a70b08233 100644 --- a/apps/wolfsshd/auth.c +++ b/apps/wolfsshd/auth.c @@ -1387,6 +1387,70 @@ static int CAKeysFileDiffers(const char* a, const char* b) } +/* Returns 1 when the certificate UPN @ in name[0..nameSz) + * authorizes login as 'usr'. allowList is a whitespace/comma list of permitted + * realms; NULL/empty matches the local part only, else domain must be listed. */ +#if defined(WOLFSSL_FPKI) || defined(WOLFSSHD_UNIT_TEST) +WOLFSSHD_STATIC int MatchUPNToUser(const char* usr, const char* name, + int nameSz, const char* allowList) +{ + int ret = 0; + int idx = 0; + int domainSz; + int tokSz; + const char* domain; + const char* p; + const char* tok; + + if (usr != NULL && name != NULL && nameSz >= 0) { + /* locate the '@' separating the local part from the domain */ + for (idx = 0; idx < nameSz; idx++) { + if (name[idx] == '@') { + break; + } + } + + /* the local part must equal the requested user name exactly */ + if ((int)XSTRLEN(usr) == idx && XSTRNCMP(usr, name, idx) == 0) { + if (allowList == NULL || *allowList == '\0') { + /* no allowlist configured: keep local-part-only matching */ + ret = 1; + } + else if (idx < nameSz) { + /* an allowlist is set: the UPN domain must be present and + * listed as an exact, case-insensitive match */ + domain = name + idx + 1; + domainSz = nameSz - idx - 1; + + p = allowList; + while (*p != '\0' && ret == 0) { + /* skip separators preceding the realm token */ + while (*p == ' ' || *p == '\t' || *p == '\r' || + *p == '\n' || *p == ',') { + p++; + } + + tok = p; + while (*p != '\0' && *p != ' ' && *p != '\t' && + *p != '\r' && *p != '\n' && *p != ',') { + p++; + } + tokSz = (int)(p - tok); + + if (tokSz > 0 && tokSz == domainSz && + WSTRNCASECMP(tok, domain, (size_t)domainSz) == 0) { + ret = 1; + } + } + } + } + } + + return ret; +} +#endif /* WOLFSSL_FPKI || WOLFSSHD_UNIT_TEST */ + + /* * @TODO this will take a pipe or equivalent to talk to a privileged thread * rather than having WOLFSSHD_AUTH directly with privilege separation. @@ -1538,31 +1602,35 @@ static int RequestAuthentication(WS_UserAuthData* authData, } else { int usrMatch = 0; + int upnRealmUnchecked = 0; DNS_entry* current = dCert->altNames; + const char* upnDomains = + wolfSSHD_ConfigGetAuthorizedUPNDomains(usrConf); while (current != NULL) { if (current->type == ASN_OTHER_TYPE && current->oidSum == UPN_OID) { - /* found UPN oid, check name against user */ - int idx; - - for (idx = 0; idx < current->len; idx++) { - if (current->name[idx] == '@') break; - /* UPN format is @ - * since currently not doing any checks on - * domain it is not treated as an error if only - * the user name is present without the domain - */ - } - - if ((int)XSTRLEN(usr) == idx && - XSTRNCMP(usr, current->name, idx) == 0) { + /* bind the cert identity to the requested user; + * MatchUPNToUser also enforces the realm allowlist + * when AuthorizedUPNDomains is set */ + if (MatchUPNToUser(usr, current->name, current->len, + upnDomains)) { usrMatch = 1; + if (upnDomains == NULL || *upnDomains == '\0') { + upnRealmUnchecked = 1; + } } } current = current->next; } + /* a UPN matched but no realm policy is set; warn per auth + * attempt so the opt-in gap is visible, no shared state */ + if (upnRealmUnchecked) { + wolfSSH_Log(WS_LOG_WARN, "[SSHD] AuthorizedUPNDomains " + "not set; certificate UPN domain is not checked"); + } + if (usrMatch == 0) { wolfSSH_Log(WS_LOG_ERROR, "[SSHD] incorrect user cert " "sent"); diff --git a/apps/wolfsshd/auth.h b/apps/wolfsshd/auth.h index c487a68ca..be947808f 100644 --- a/apps/wolfsshd/auth.h +++ b/apps/wolfsshd/auth.h @@ -100,6 +100,8 @@ int CheckPasswordHashUnix(const char* input, char* stored); int CheckAuthKeysLine(char* line, word32 lineSz, const byte* key, word32 keySz); int CAKeysFileDiffers(const char* a, const char* b); +int MatchUPNToUser(const char* usr, const char* name, int nameSz, + const char* allowList); int wolfSSHD_GetUserAuthTypes(const WOLFSSHD_CONFIG* usrConf); #endif #endif /* WOLFAUTH_H */ diff --git a/apps/wolfsshd/configuration.c b/apps/wolfsshd/configuration.c index c16c63b16..ccce41322 100644 --- a/apps/wolfsshd/configuration.c +++ b/apps/wolfsshd/configuration.c @@ -77,6 +77,7 @@ struct WOLFSSHD_CONFIG { char* authKeysFile; char* forceCmd; char* pidFile; + char* authorizedUPNDomains; /* allowlist of UPN realms for cert auth */ WOLFSSHD_CONFIG* next; /* next config in list */ long loginTimer; word16 port; @@ -314,6 +315,13 @@ static WOLFSSHD_CONFIG* wolfSSHD_ConfigCopy(WOLFSSHD_CONFIG* conf) newConf->heap); } + if (ret == WS_SUCCESS && conf->authorizedUPNDomains) { + ret = CreateString(&newConf->authorizedUPNDomains, + conf->authorizedUPNDomains, + (int)WSTRLEN(conf->authorizedUPNDomains), + newConf->heap); + } + if (ret == WS_SUCCESS) { newConf->loginTimer = conf->loginTimer; newConf->port = conf->port; @@ -357,6 +365,7 @@ void wolfSSHD_ConfigFree(WOLFSSHD_CONFIG* conf) FreeString(¤t->pidFile, heap); FreeString(¤t->userCAKeysFile, heap); FreeString(¤t->forceCmd, heap); + FreeString(¤t->authorizedUPNDomains, heap); FreeString(¤t->usrAppliesTo, heap); FreeString(¤t->groupAppliesTo, heap); @@ -399,9 +408,10 @@ enum { OPT_BANNER = 23, OPT_PUBKEY_AUTH = 24, OPT_STRICT_MODES = 25, + OPT_AUTHORIZED_UPN_DOMAINS = 26, }; enum { - NUM_OPTIONS = 26 + NUM_OPTIONS = 27 }; static const CONFIG_OPTION options[NUM_OPTIONS] = { @@ -431,6 +441,7 @@ static const CONFIG_OPTION options[NUM_OPTIONS] = { {OPT_PIDFILE, "PidFile"}, {OPT_BANNER, "Banner"}, {OPT_STRICT_MODES, "StrictModes"}, + {OPT_AUTHORIZED_UPN_DOMAINS, "AuthorizedUPNDomains"}, }; /* returns WS_SUCCESS on success */ @@ -1152,27 +1163,27 @@ static int HandleMatch(WOLFSSHD_CONFIG** conf, const char* value, int valueSz) } -/* returns WS_SUCCESS on success */ -static int HandleForcedCommand(WOLFSSHD_CONFIG* conf, const char* value, - int valueSz) +/* returns WS_SUCCESS on success. Replaces *dst with a length-bounded copy of + * the full line remainder so a whitespace separated multi value list is kept + * rather than truncated at the first token. */ +static int SetListString(char** dst, const char* value, int valueSz, + void* heap) { int ret = WS_SUCCESS; - if (conf == NULL || value == NULL) { + if (dst == NULL || value == NULL) { ret = WS_BAD_ARGUMENT; } if (ret == WS_SUCCESS) { - if (conf->forceCmd != NULL) { - FreeString(&conf->forceCmd, conf->heap); - conf->forceCmd = NULL; + if (*dst != NULL) { + FreeString(dst, heap); + *dst = NULL; } - ret = CreateString(&conf->forceCmd, value, valueSz, conf->heap); + ret = CreateString(dst, value, valueSz, heap); } - - (void)valueSz; return ret; } @@ -1259,7 +1270,8 @@ static int HandleConfigOption(WOLFSSHD_CONFIG** conf, int opt, ret = HandleMatch(conf, full, fullSz); break; case OPT_FORCE_CMD: - ret = HandleForcedCommand(*conf, full, fullSz); + ret = SetListString(&(*conf)->forceCmd, full, fullSz, + (*conf)->heap); break; case OPT_TRUSTED_USER_CA_KEYS: /* TODO: Add logic to check if file exists? */ @@ -1274,6 +1286,10 @@ static int HandleConfigOption(WOLFSSHD_CONFIG** conf, int opt, case OPT_STRICT_MODES: ret = HandleStrictModes(*conf, value); break; + case OPT_AUTHORIZED_UPN_DOMAINS: + ret = SetListString(&(*conf)->authorizedUPNDomains, full, fullSz, + (*conf)->heap); + break; default: break; } @@ -1629,6 +1645,17 @@ char* wolfSSHD_ConfigGetUserCAKeysFile(const WOLFSSHD_CONFIG* conf) return ret; } +char* wolfSSHD_ConfigGetAuthorizedUPNDomains(const WOLFSSHD_CONFIG* conf) +{ + char* ret = NULL; + + if (conf != NULL) { + ret = conf->authorizedUPNDomains; + } + + return ret; +} + static int SetFileString(char** dst, const char* src, void* heap) { int ret = WS_SUCCESS; diff --git a/apps/wolfsshd/configuration.h b/apps/wolfsshd/configuration.h index 40b541a27..e05a42d1f 100644 --- a/apps/wolfsshd/configuration.h +++ b/apps/wolfsshd/configuration.h @@ -49,6 +49,7 @@ char* wolfSSHD_ConfigGetChroot(const WOLFSSHD_CONFIG* conf); char* wolfSSHD_ConfigGetHostKeyFile(const WOLFSSHD_CONFIG* conf); char* wolfSSHD_ConfigGetHostCertFile(const WOLFSSHD_CONFIG* conf); char* wolfSSHD_ConfigGetUserCAKeysFile(const WOLFSSHD_CONFIG* conf); +char* wolfSSHD_ConfigGetAuthorizedUPNDomains(const WOLFSSHD_CONFIG* conf); int wolfSSHD_ConfigSetHostKeyFile(WOLFSSHD_CONFIG* conf, const char* file); int wolfSSHD_ConfigSetHostCertFile(WOLFSSHD_CONFIG* conf, const char* file); int wolfSSHD_ConfigSetUserCAKeysFile(WOLFSSHD_CONFIG* conf, const char* file); diff --git a/apps/wolfsshd/test/create_sshd_config.sh b/apps/wolfsshd/test/create_sshd_config.sh index 19be76acd..574bdedea 100755 --- a/apps/wolfsshd/test/create_sshd_config.sh +++ b/apps/wolfsshd/test/create_sshd_config.sh @@ -26,9 +26,27 @@ PermitEmptyPasswords no UsePrivilegeSeparation no UseDNS no -TrustedUserCAKeys $PWD/../../../keys/ca-cert-ecc.pem -HostKey $PWD/../../../keys/server-key.pem +TrustedUserCAKeys $PWD/../../../keys/ca-cert-ecc.pem +HostKey $PWD/../../../keys/server-key.pem +HostCertificate $PWD/../../../keys/server-cert.pem +AuthorizedUPNDomains example + +EOF + +cat < sshd_config_test_x509_upn_bad +Port 22222 +Protocol 2 +LoginGraceTime 600 +PermitRootLogin yes +PasswordAuthentication yes +PermitEmptyPasswords no +UsePrivilegeSeparation no +UseDNS no + +TrustedUserCAKeys $PWD/../../../keys/ca-cert-ecc.pem +HostKey $PWD/../../../keys/server-key.pem HostCertificate $PWD/../../../keys/server-cert.pem +AuthorizedUPNDomains other.example EOF diff --git a/apps/wolfsshd/test/run_all_sshd_tests.sh b/apps/wolfsshd/test/run_all_sshd_tests.sh index 51236726c..1450cccc6 100755 --- a/apps/wolfsshd/test/run_all_sshd_tests.sh +++ b/apps/wolfsshd/test/run_all_sshd_tests.sh @@ -411,6 +411,16 @@ else printf "Shutting down test wolfSSHd\n" stop_wolfsshd fi + + # negative test: a certificate UPN realm outside AuthorizedUPNDomains must + # be rejected. Needs the dedicated bad-domain config, so only runs when we + # control the local daemon. + if [ "$USING_LOCAL_HOST" == 1 ]; then + start_wolfsshd "sshd_config_test_x509_upn_bad" + run_test "sshd_x509_upn_fail.sh" + printf "Shutting down test wolfSSHd\n" + stop_wolfsshd + fi fi # Teardown safety net: the start/stop pairs above stop each daemon they start, diff --git a/apps/wolfsshd/test/sshd_x509_upn_fail.sh b/apps/wolfsshd/test/sshd_x509_upn_fail.sh new file mode 100755 index 000000000..6c8745843 --- /dev/null +++ b/apps/wolfsshd/test/sshd_x509_upn_fail.sh @@ -0,0 +1,64 @@ +#!/bin/sh + +# Negative test for AuthorizedUPNDomains. run_all_sshd_tests.sh starts the +# daemon with sshd_config_test_x509_upn_bad, whose AuthorizedUPNDomains is +# "other.example", while the client certificate carries the UPN realm +# "example". The wolfSSHd UPN domain check must therefore reject the cert. + +PWD=`pwd` + +# The UPN domain check is compiled only when wolfSSL is built with FPKI. Probe +# the daemon binary's help output, which prints an FPKI marker under the same +# build guard, and skip when the check is not present. +if ! ../wolfsshd "-?" 2>&1 | grep -q "FPKI"; then + echo "wolfSSHd built without FPKI; UPN domain check not compiled in, skipping" + exit 77 +fi + +# Count existing rejection lines first so a stale match left in the appended +# log (start_sshd.sh uses 'wolfsshd -E ./log.txt', which never truncates) is +# not mistaken for this run's rejection. +BEFORE=`sudo grep -c "incorrect user cert sent" ./log.txt 2>/dev/null` +BEFORE=${BEFORE:-0} + +cd ../../.. + +if [ -z "$1" ] || [ -z "$2" ] || [ -z "$3" ]; then + echo "expecting host, port and user as arguments" + echo "$0 127.0.0.1 22222 user" + exit 1 +fi + +TEST_CLIENT="./examples/client/client" +PRIVATE_KEY="./keys/$3-key.der" +PUBLIC_KEY="./keys/$3-cert.der" +CA_CERT="./keys/ca-cert-ecc.der" + +echo "$TEST_CLIENT -X -c 'pwd' -u $3 -i $PRIVATE_KEY -J $PUBLIC_KEY -A $CA_CERT -h \"$1\" -p \"$2\" (expecting rejection)" +$TEST_CLIENT -X -c 'pwd' -u "$3" -i "$PRIVATE_KEY" -J "$PUBLIC_KEY" -A "$CA_CERT" -h "$1" -p "$2" +RESULT=$? + +cd $PWD + +# Give the daemon child a moment to flush its rejection to the log. +sleep 1 + +# FPKI is confirmed compiled in, so a disallowed realm must be rejected for the +# UPN reason. The daemon logs "incorrect user cert sent" in that case; log.txt +# is root-owned (daemon ran via sudo), so read it with sudo. Require both a +# non-zero client exit and a NEW rejection line (after > before) so neither a +# stale log line nor an unrelated client failure can produce a false pass. +AFTER=`sudo grep -c "incorrect user cert sent" ./log.txt 2>/dev/null` +AFTER=${AFTER:-0} +if [ "$RESULT" -ne 0 ] && [ "$AFTER" -gt "$BEFORE" ]; then + echo "authentication correctly rejected for disallowed UPN realm" + exit 0 +fi + +if [ "$RESULT" -eq 0 ]; then + echo "ERROR: authentication succeeded for a UPN realm outside AuthorizedUPNDomains" + exit 1 +fi + +echo "ERROR: client failed but daemon did not log a UPN realm rejection" +exit 1 diff --git a/apps/wolfsshd/test/test_configuration.c b/apps/wolfsshd/test/test_configuration.c index aa5c410de..2b205c02e 100644 --- a/apps/wolfsshd/test/test_configuration.c +++ b/apps/wolfsshd/test/test_configuration.c @@ -329,6 +329,7 @@ static int test_ConfigCopy(void) if (ret == WS_SUCCESS) ret = PCL("HostKey /etc/ssh/ssh_host_key"); if (ret == WS_SUCCESS) ret = PCL("ForceCommand /bin/restricted"); if (ret == WS_SUCCESS) ret = PCL("PidFile /var/run/sshd.pid"); + if (ret == WS_SUCCESS) ret = PCL("AuthorizedUPNDomains corp.example"); /* string fields via public setters */ if (ret == WS_SUCCESS) @@ -448,6 +449,12 @@ static int test_ConfigCopy(void) if (wolfSSHD_ConfigGetStrictModes(match) != 0) ret = WS_FATAL_ERROR; } + if (ret == WS_SUCCESS) { + if (wolfSSHD_ConfigGetAuthorizedUPNDomains(match) == NULL || + XSTRCMP(wolfSSHD_ConfigGetAuthorizedUPNDomains(match), + "corp.example") != 0) + ret = WS_FATAL_ERROR; + } wolfSSHD_ConfigFree(head); return ret; @@ -1618,6 +1625,119 @@ static int test_CAKeysFileDiffers(void) return ret; } +/* Parses an AuthorizedUPNDomains line and confirms the stored value is returned + * by the getter, locking in the new config option's plumbing. */ +static int test_ConfigParseAuthorizedUPNDomains(void) +{ + int ret = WS_SUCCESS; + WOLFSSHD_CONFIG* conf; + const char* line = "AuthorizedUPNDomains corp.example other.example"; + char* got; + + conf = wolfSSHD_ConfigNew(NULL); + if (conf == NULL) { + ret = WS_MEMORY_E; + } + + if (ret == WS_SUCCESS) { + Log(" Testing scenario: parse AuthorizedUPNDomains."); + ret = ParseConfigLine(&conf, line, (int)WSTRLEN(line), 0); + if (ret == WS_SUCCESS) { + got = wolfSSHD_ConfigGetAuthorizedUPNDomains(conf); + if (got == NULL || + XSTRCMP(got, "corp.example other.example") != 0) { + ret = WS_FATAL_ERROR; + } + } + Log(ret == WS_SUCCESS ? " PASSED.\n" : " FAILED.\n"); + } + + if (conf != NULL) { + wolfSSHD_ConfigFree(conf); + } + + return ret; +} + +/* Exercises the UPN-to-user binding helper directly: local-part matching with + * no allowlist (back-compat), and the realm allowlist enforcement that fixes + * the cross-domain authentication bypass. */ +static int test_MatchUPNToUser(void) +{ + int ret = WS_SUCCESS; + int i; + int sz; + + /* name buffers are not necessarily NUL terminated in a real cert, so the + * helper is length bounded. For vectors with nameSz == 0, the test derives + * the length from WSTRLEN(name); a non-zero nameSz bounds a longer buffer to + * prove bytes past it are ignored. */ + static const struct { + const char* desc; + const char* usr; + const char* name; + const char* allowList; + int expect; + int nameSz; + } vectors[] = { + /* no allowlist: local part only, any/absent domain is accepted */ + {"no allowlist, domain ignored", "alice", "alice@other.example", NULL, + 1, 0}, + {"no allowlist, bare local part", "alice", "alice", NULL, 1, 0}, + {"no allowlist, wrong local part", "bob", "alice@corp.example", NULL, + 0, 0}, + {"empty allowlist, domain ignored", "alice", "alice@x.example", "", 1, + 0}, + + /* allowlist set: realm must be present and listed */ + {"allowlist match", "alice", "alice@corp.example", "corp.example", 1, + 0}, + {"allowlist mismatch", "alice", "alice@other.example", "corp.example", + 0, 0}, + {"allowlist, missing domain", "alice", "alice", "corp.example", 0, 0}, + {"allowlist, empty domain", "alice", "alice@", "corp.example", 0, 0}, + {"allowlist, wrong local part", "bob", "alice@corp.example", + "corp.example", 0, 0}, + {"allowlist multi, first", "alice", "alice@corp.example", + "corp.example other.example", 1, 0}, + {"allowlist multi, second", "alice", "alice@other.example", + "corp.example other.example", 1, 0}, + {"allowlist comma separated", "alice", "alice@other.example", + "corp.example,other.example", 1, 0}, + {"allowlist case-insensitive", "alice", "alice@CORP.EXAMPLE", + "corp.example", 1, 0}, + {"allowlist tab separated", "alice", "alice@other.example", + "corp.example\tother.example", 1, 0}, + {"allowlist mixed ws separators", "alice", "alice@other.example", + "corp.example\t\r\nother.example", 1, 0}, + + /* length-bounded: bytes past nameSz must be ignored. With the full + * string these would flip the result, so expect=1 only holds if the + * helper honors nameSz instead of reading to the NUL. */ + {"bounded domain ignores trailing", "alice", "alice@corp.exampleEVIL", + "corp.example", 1, 18}, /* 18 == strlen("alice@corp.example") */ + {"bounded '@' search ignores trailing", "alice", + "alice.bob@corp.example", NULL, 1, 5}, /* 5 == strlen("alice") */ + }; + const int numVectors = (int)(sizeof(vectors) / sizeof(*vectors)); + + for (i = 0; i < numVectors && ret == WS_SUCCESS; ++i) { + sz = (vectors[i].nameSz != 0) ? vectors[i].nameSz + : (int)WSTRLEN(vectors[i].name); + Log(" Testing scenario: %s.", vectors[i].desc); + if (MatchUPNToUser(vectors[i].usr, vectors[i].name, sz, + vectors[i].allowList) != vectors[i].expect) { + Log(" FAILED.\n"); + ret = WS_FATAL_ERROR; + } + else { + Log(" PASSED.\n"); + } + } + + return ret; +} + /* Exercises the auth-method advertisement logic used by DefaultUserAuthTypes: * a method is only offered when its config option is enabled. Covers all four * permutations of PasswordAuthentication and PubkeyAuthentication, including the @@ -1914,6 +2034,8 @@ const TEST_CASE testCases[] = { TEST_DECL(test_GetUserConfMatchGroupAnd), TEST_DECL(test_GetUserConfMatchSecondaryGroup), TEST_DECL(test_CAKeysFileDiffers), + TEST_DECL(test_ConfigParseAuthorizedUPNDomains), + TEST_DECL(test_MatchUPNToUser), TEST_DECL(test_IncludeRecursionBound), TEST_DECL(test_GetUserAuthTypes), TEST_DECL(test_ConfigSetAuthKeysFile), diff --git a/apps/wolfsshd/wolfsshd.c b/apps/wolfsshd/wolfsshd.c index b25a3f2fb..9c17fa15a 100644 --- a/apps/wolfsshd/wolfsshd.c +++ b/apps/wolfsshd/wolfsshd.c @@ -196,6 +196,12 @@ static void ShowUsage(void) printf(" -D Run in foreground (do not detach)\n"); printf(" -h host private key file to use\n"); printf(" -E append to log file\n"); +#ifdef WOLFSSL_FPKI + /* build-capability note, separated from the option list; also greppable by + * test scripts, for the cert UPN domain check (AuthorizedUPNDomains) */ + printf("\n"); + printf("Build features: FPKI certificate UPN domain checking\n"); +#endif }