From d2550e0b754dee32211e3602c4d4d37a90832187 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 17 Aug 2026 11:00:08 -0700 Subject: [PATCH 1/2] Test overlapping sshd_config Match blocks OpenSSH resolves sshd_config one keyword at a time, scanning every Match block that applies. wolfSSHD_GetUserConf returns the first matching block whole, so a setting made only in a later matching block is dropped. - add test_GetUserConfMatchOverlapCompose, covering a user matched by both a Match User and a Match Group block, in either order - add sshd_match_overlap_test.sh, the same case against a live daemon, where the group block's ForceCommand is the setting that goes missing - both fail until per keyword composition lands, so the unit test runs last and the script stays commented out of run_all_sshd_tests.sh --- apps/wolfsshd/test/run_all_sshd_tests.sh | 4 + apps/wolfsshd/test/sshd_match_overlap_test.sh | 116 ++++++++ apps/wolfsshd/test/test_configuration.c | 278 ++++++++++++++++++ 3 files changed, 398 insertions(+) create mode 100755 apps/wolfsshd/test/sshd_match_overlap_test.sh diff --git a/apps/wolfsshd/test/run_all_sshd_tests.sh b/apps/wolfsshd/test/run_all_sshd_tests.sh index 65cc89aa9..c79643747 100755 --- a/apps/wolfsshd/test/run_all_sshd_tests.sh +++ b/apps/wolfsshd/test/run_all_sshd_tests.sh @@ -406,6 +406,10 @@ else # these tests require setting up an sshd if [ "$USING_LOCAL_HOST" == 1 ]; then run_test "sshd_forcedcmd_test.sh" + # Left out of the suite on purpose: it asserts OpenSSH's per keyword + # Match composition, which wolfSSHD_GetUserConf does not implement yet, + # so it fails today. Add it back once that lands. + # run_test "sshd_match_overlap_test.sh" run_test "sshd_window_full_test.sh" run_test "sshd_empty_password_test.sh" run_test "sshd_permitroot_test.sh" diff --git a/apps/wolfsshd/test/sshd_match_overlap_test.sh b/apps/wolfsshd/test/sshd_match_overlap_test.sh new file mode 100755 index 000000000..97c711b15 --- /dev/null +++ b/apps/wolfsshd/test/sshd_match_overlap_test.sh @@ -0,0 +1,116 @@ +#!/bin/bash + +# sshd local test +# +# OpenSSH resolves sshd_config one keyword at a time: "For each keyword, the +# first obtained value will be used", scanning every Match block that applies to +# the connection. A user who matches both a Match User block and a Match Group +# block therefore gets the settings from both, and the outcome does not depend +# on which block is written first. +# +# This test writes the same two blocks in both orders. Only the group block sets +# ForceCommand, so a shell exec must be refused either way. wolfsshd resolves +# the whole first matching block instead of composing per keyword, so the +# user-first order currently drops the forced command and the exec succeeds. +# +# Expects ./authorized_keys_test to exist, as the other tests here do. Create it +# with ./create_authorized_test_file.sh when running this script on its own. + +if [ -z "$1" ] || [ -z "$2" ]; then + echo "expecting host and port as arguments" + echo "$0 127.0.0.1 22222" + exit 1 +fi + +# Not named PWD: the shell rewrites that variable on every cd, so a saved copy +# would not survive a cd made here or by anything this script sources. +TESTDIR=`pwd` +USER=`whoami` +GROUP=`id -gn $USER` +TEST_PORT="$2" +TEST_HOST="$1" +source ./start_sshd.sh + +# Stop the daemon on every exit path: a failed scenario exits non-zero straight +# out of the script, and a root daemon left holding the shared test port would +# answer for every later test in the suite. stop_wolfsshd clears PID, so this is +# a no-op after each explicit stop below. +trap stop_wolfsshd EXIT + +# The client chdir's to the wolfSSH root before it parses its arguments, so the +# key paths have to be absolute rather than relative to this directory. +TEST_CLIENT="$TESTDIR/../../../examples/client/client" +PRIVATE_KEY="$TESTDIR/../../../keys/hansel-key-ecc.der" +PUBLIC_KEY="$TESTDIR/../../../keys/hansel-key-ecc.pub" + +# writes a config named $1 holding the two overlapping blocks. $2 selects which +# block is written first, "user" or "group". Both orders name the same two +# blocks with the same settings. +write_config() { + cat < "$1" +Port $TEST_PORT +Protocol 2 +LoginGraceTime 600 +PermitRootLogin yes +PasswordAuthentication yes +PermitEmptyPasswords no +UsePrivilegeSeparation no +UseDNS no +HostKey $TESTDIR/../../../keys/server-key.pem +AuthorizedKeysFile $TESTDIR/authorized_keys_test +EOF + + if [ "$2" == "user" ]; then + cat <> "$1" + +Match User $USER + PermitEmptyPasswords no + +Match Group $GROUP + ForceCommand internal-sftp +EOF + else + cat <> "$1" + +Match Group $GROUP + ForceCommand internal-sftp + +Match User $USER + PermitEmptyPasswords no +EOF + fi +} + +# runs a shell exec against the daemon started from config $1 and fails when the +# command runs, which means the group block's ForceCommand was not applied +# +# The marker is split with an empty quoted string so the command the client +# sends and the output the remote shell produces are not the same text. A debug +# build logs the command it sends, and grepping for a marker that appears +# verbatim in that log reports a shell that never ran as one that did. +check_forced_cmd() { + start_wolfsshd "$1" + + RESULT=$( $TEST_CLIENT -c 'echo overlap""_shell_ran' -u $USER \ + -i $PRIVATE_KEY -j $PUBLIC_KEY -h $TEST_HOST -p $TEST_PORT ) + echo $RESULT + echo $RESULT | grep overlap_shell_ran + FOUND=$? + + stop_wolfsshd + + if [ "$FOUND" == 0 ]; then + echo "$2: shell login should fail, the Match Group block sets" + echo "ForceCommand internal-sftp and the user is in that group" + return 1 + fi + return 0 +} + +write_config sshd_config_test_match_overlap_user "user" +check_forced_cmd sshd_config_test_match_overlap_user "user block first" || exit 1 + +write_config sshd_config_test_match_overlap_group "group" +check_forced_cmd sshd_config_test_match_overlap_group "group block first" || exit 1 + +exit 0 diff --git a/apps/wolfsshd/test/test_configuration.c b/apps/wolfsshd/test/test_configuration.c index 326843663..41c8e3635 100644 --- a/apps/wolfsshd/test/test_configuration.c +++ b/apps/wolfsshd/test/test_configuration.c @@ -1348,6 +1348,279 @@ static int test_GetUserConfMatchNoInherit(void) } +/* Builds "global, Match User alice, Match Group nopw" or the reverse order, + * depending on 'userFirst'. Only the group block sets PasswordAuthentication; + * only the user block sets AuthorizedKeysFile. Returns the head on success. */ +static WOLFSSHD_CONFIG* BuildOverlapConfig(int userFirst) +{ + int ret = WS_SUCCESS; + WOLFSSHD_CONFIG* head; + WOLFSSHD_CONFIG* conf; + + head = wolfSSHD_ConfigNew(NULL); + if (head == NULL) + return NULL; + conf = head; + +#define PCL(s) ParseConfigLine(&conf, s, (int)WSTRLEN(s), 0) + if (ret == WS_SUCCESS) ret = PCL("PasswordAuthentication yes"); + + if (userFirst) { + if (ret == WS_SUCCESS) ret = PCL("Match User alice"); + if (ret == WS_SUCCESS) ret = PCL("AuthorizedKeysFile .ssh/alice_keys"); + if (ret == WS_SUCCESS) ret = PCL("Match Group nopw"); + if (ret == WS_SUCCESS) ret = PCL("PasswordAuthentication no"); + } + else { + if (ret == WS_SUCCESS) ret = PCL("Match Group nopw"); + if (ret == WS_SUCCESS) ret = PCL("PasswordAuthentication no"); + if (ret == WS_SUCCESS) ret = PCL("Match User alice"); + if (ret == WS_SUCCESS) ret = PCL("AuthorizedKeysFile .ssh/alice_keys"); + } +#undef PCL + + if (ret != WS_SUCCESS) { + wolfSSHD_ConfigFree(head); + head = NULL; + } + return head; +} + + +/* OpenSSH resolves sshd_config one keyword at a time: "For each keyword, the + * first obtained value will be used", scanning every Match block that applies + * to the connection. A user who matches two blocks therefore gets the union of + * what those blocks set, and the result does not depend on the order the blocks + * appear in. + * + * This locks in that behaviour for wolfSSHD_GetUserConf: a restriction named in + * only one of the matching blocks must survive, whichever block that is, and a + * keyword named in both must resolve to the value from the earlier block. + * Returning a single whole block drops half of the administrator's policy. */ +static int test_GetUserConfMatchOverlapCompose(void) +{ + int ret = WS_SUCCESS; + int fail = WS_SUCCESS; + WOLFSSHD_CONFIG* head = NULL; + WOLFSSHD_CONFIG* conf; + WOLFSSHD_CONFIG* match; + const char* keys; + const char* cmd; + const char* grps[1]; + + /* alice is in nopw, so both blocks apply to her. The group block's + * PasswordAuthentication no must hold even though the user block, which + * never mentions the keyword, also matches. */ + if (ret == WS_SUCCESS) { + Log(" Testing scenario: later Match Group restriction, user block " + "first."); + head = BuildOverlapConfig(1); + if (head == NULL) { + ret = WS_MEMORY_E; + } + + if (ret == WS_SUCCESS) { + grps[0] = "nopw"; + match = wolfSSHD_GetUserConf(head, "alice", grps, 1, NULL, NULL, + NULL, NULL, NULL); + if (match == NULL) { + ret = WS_FATAL_ERROR; + } + if (ret == WS_SUCCESS && wolfSSHD_ConfigGetPwAuth(match) != 0) { + Log(" [dropped PasswordAuthentication no]"); + ret = WS_FATAL_ERROR; + } + /* the user block's own setting must survive too */ + if (ret == WS_SUCCESS) { + keys = wolfSSHD_ConfigGetAuthKeysFile(match); + if (keys == NULL || XSTRCMP(keys, ".ssh/alice_keys") != 0) { + Log(" [dropped AuthorizedKeysFile]"); + ret = WS_FATAL_ERROR; + } + } + } + Log((ret == WS_SUCCESS) ? " PASSED.\n" : " FAILED.\n"); + wolfSSHD_ConfigFree(head); + head = NULL; + if (ret != WS_SUCCESS) { + /* keep going so every scenario gets reported, not just the + * first one that fails */ + fail = ret; + ret = WS_SUCCESS; + } + } + + /* same two blocks, opposite order, same answer */ + if (ret == WS_SUCCESS) { + Log(" Testing scenario: same blocks, group block first."); + head = BuildOverlapConfig(0); + if (head == NULL) { + ret = WS_MEMORY_E; + } + + if (ret == WS_SUCCESS) { + grps[0] = "nopw"; + match = wolfSSHD_GetUserConf(head, "alice", grps, 1, NULL, NULL, + NULL, NULL, NULL); + if (match == NULL) { + ret = WS_FATAL_ERROR; + } + if (ret == WS_SUCCESS && wolfSSHD_ConfigGetPwAuth(match) != 0) { + Log(" [dropped PasswordAuthentication no]"); + ret = WS_FATAL_ERROR; + } + if (ret == WS_SUCCESS) { + keys = wolfSSHD_ConfigGetAuthKeysFile(match); + if (keys == NULL || XSTRCMP(keys, ".ssh/alice_keys") != 0) { + Log(" [dropped AuthorizedKeysFile]"); + ret = WS_FATAL_ERROR; + } + } + } + Log((ret == WS_SUCCESS) ? " PASSED.\n" : " FAILED.\n"); + wolfSSHD_ConfigFree(head); + head = NULL; + if (ret != WS_SUCCESS) { + /* keep going so every scenario gets reported, not just the + * first one that fails */ + fail = ret; + ret = WS_SUCCESS; + } + } + + /* a user who matches only one of the two blocks is unaffected: nopw's + * restriction must not leak to a user outside the group */ + if (ret == WS_SUCCESS) { + Log(" Testing scenario: single matching block is unchanged."); + head = BuildOverlapConfig(1); + if (head == NULL) { + ret = WS_MEMORY_E; + } + + if (ret == WS_SUCCESS) { + grps[0] = "users"; + match = wolfSSHD_GetUserConf(head, "alice", grps, 1, NULL, NULL, + NULL, NULL, NULL); + if (match == NULL || wolfSSHD_ConfigGetPwAuth(match) != 1) { + ret = WS_FATAL_ERROR; + } + if (ret == WS_SUCCESS) { + keys = wolfSSHD_ConfigGetAuthKeysFile(match); + if (keys == NULL || XSTRCMP(keys, ".ssh/alice_keys") != 0) + ret = WS_FATAL_ERROR; + } + } + + /* and a user who matches neither keeps the global values */ + if (ret == WS_SUCCESS) { + grps[0] = "users"; + match = wolfSSHD_GetUserConf(head, "bob", grps, 1, NULL, NULL, + NULL, NULL, NULL); + if (match != head || wolfSSHD_ConfigGetPwAuth(match) != 1) { + ret = WS_FATAL_ERROR; + } + } + Log((ret == WS_SUCCESS) ? " PASSED.\n" : " FAILED.\n"); + wolfSSHD_ConfigFree(head); + head = NULL; + if (ret != WS_SUCCESS) { + /* keep going so every scenario gets reported, not just the + * first one that fails */ + fail = ret; + ret = WS_SUCCESS; + } + } + + /* ForceCommand resolves through the same lookup, so a confinement set only + * in the group block must apply to a user whose own block matched first */ + if (ret == WS_SUCCESS) { + Log(" Testing scenario: ForceCommand from the later group block."); + 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("Match User alice"); + if (ret == WS_SUCCESS) ret = PCL("AuthorizedKeysFile .ssh/alice_keys"); + if (ret == WS_SUCCESS) ret = PCL("Match Group jailed"); + if (ret == WS_SUCCESS) ret = PCL("ForceCommand internal-sftp"); +#undef PCL + + if (ret == WS_SUCCESS) { + grps[0] = "jailed"; + match = wolfSSHD_GetUserConf(head, "alice", grps, 1, NULL, NULL, + NULL, NULL, NULL); + if (match == NULL) { + ret = WS_FATAL_ERROR; + } + if (ret == WS_SUCCESS) { + cmd = wolfSSHD_ConfigGetForcedCmd(match); + if (cmd == NULL || XSTRCMP(cmd, "internal-sftp") != 0) { + Log(" [dropped ForceCommand, resolved to %s]", + (cmd == NULL) ? "none" : cmd); + ret = WS_FATAL_ERROR; + } + } + } + Log((ret == WS_SUCCESS) ? " PASSED.\n" : " FAILED.\n"); + wolfSSHD_ConfigFree(head); + head = NULL; + if (ret != WS_SUCCESS) { + /* keep going so every scenario gets reported, not just the + * first one that fails */ + fail = ret; + ret = WS_SUCCESS; + } + } + + /* when both matching blocks set the same keyword the earlier one wins, + * which is the "first obtained value" half of the rule */ + if (ret == WS_SUCCESS) { + Log(" Testing scenario: earlier block wins a direct conflict."); + 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("ForceCommand /bin/global"); + if (ret == WS_SUCCESS) ret = PCL("Match User alice"); + if (ret == WS_SUCCESS) ret = PCL("ForceCommand /bin/alice"); + if (ret == WS_SUCCESS) ret = PCL("Match Group jailed"); + if (ret == WS_SUCCESS) ret = PCL("ForceCommand internal-sftp"); +#undef PCL + + if (ret == WS_SUCCESS) { + grps[0] = "jailed"; + match = wolfSSHD_GetUserConf(head, "alice", grps, 1, NULL, NULL, + NULL, NULL, NULL); + if (match == NULL) { + ret = WS_FATAL_ERROR; + } + if (ret == WS_SUCCESS) { + cmd = wolfSSHD_ConfigGetForcedCmd(match); + if (cmd == NULL || XSTRCMP(cmd, "/bin/alice") != 0) + ret = WS_FATAL_ERROR; + } + } + Log((ret == WS_SUCCESS) ? " PASSED.\n" : " FAILED.\n"); + wolfSSHD_ConfigFree(head); + head = NULL; + if (ret != WS_SUCCESS) { + /* keep going so every scenario gets reported, not just the + * first one that fails */ + fail = ret; + ret = WS_SUCCESS; + } + } + + return (fail != WS_SUCCESS) ? fail : ret; +} + + /* A Match block inside an Include'd file must survive a later Match block in * the including file, and the include must not export its Match scope: a * directive after the Include belongs to the global config, the way OpenSSH @@ -6245,6 +6518,11 @@ const TEST_CASE testCases[] = { TEST_DECL(test_CheckPublicKeyUnixOrdering), #endif #endif + /* Runs last: the first failing case stops the run, and this one is a known + * gap. wolfSSHD_GetUserConf returns the first matching Match block whole + * instead of composing per keyword, so a setting made only in a later + * matching block is dropped. */ + TEST_DECL(test_GetUserConfMatchOverlapCompose), }; int main(int argc, char** argv) From a405baf7404240f5ea7755227cb4030007878b08 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 17 Aug 2026 13:55:23 -0700 Subject: [PATCH 2/2] Compose sshd_config Match blocks per keyword wolfSSHD_GetUserConf returned the first matching Match block whole, so a keyword named only in a later matching block was dropped and the outcome depended on the order the blocks were written. - track in a new setMask which keywords a node set itself, so a value inherited from the globals can be told from one the block named, with a compile time check that no option tag shifts out of the mask - resolve into a fresh config seeded from the globals, letting every matching block contribute the keywords no earlier block claimed - the resolved config now belongs to the caller, so wolfsshd and the auth paths free it and the tests compare values rather than node identity - put sshd_match_overlap_test.sh back in the suite Issue: ZD-22324 --- apps/wolfsshd/auth.c | 4 + apps/wolfsshd/auth.h | 2 + apps/wolfsshd/configuration.c | 232 ++++++++++--- apps/wolfsshd/configuration.h | 5 + apps/wolfsshd/test/run_all_sshd_tests.sh | 5 +- apps/wolfsshd/test/sshd_match_overlap_test.sh | 6 +- apps/wolfsshd/test/test_configuration.c | 304 +++++++++--------- apps/wolfsshd/wolfsshd.c | 2 + 8 files changed, 351 insertions(+), 209 deletions(-) diff --git a/apps/wolfsshd/auth.c b/apps/wolfsshd/auth.c index 1b7a74155..21f901784 100644 --- a/apps/wolfsshd/auth.c +++ b/apps/wolfsshd/auth.c @@ -2262,6 +2262,7 @@ static int DoCheckUser(const char* usr, WOLFSSHD_AUTH* auth, int isRoot) wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Login as root not permitted"); ret = WOLFSSH_USERAUTH_REJECTED; } + wolfSSHD_ConfigFree(usrConf); } if (ret == WOLFSSH_USERAUTH_SUCCESS) { @@ -2752,6 +2753,8 @@ static int RequestAuthentication(WS_UserAuthData* authData, } + wolfSSHD_ConfigFree(usrConf); + if (wolfSSHD_AuthReducePermissions(authCtx) != WS_SUCCESS) { /* stop everything if not able to reduce permissions level */ exit(1); @@ -2839,6 +2842,7 @@ int DefaultUserAuthTypes(WOLFSSH* ssh, void* ctx) } else { ret = wolfSSHD_GetUserAuthTypes(usrConf); + wolfSSHD_ConfigFree(usrConf); } return ret; diff --git a/apps/wolfsshd/auth.h b/apps/wolfsshd/auth.h index 637cdda7b..fe0d4b8ea 100644 --- a/apps/wolfsshd/auth.h +++ b/apps/wolfsshd/auth.h @@ -93,6 +93,8 @@ const char* wolfSSHD_AuthMergeForcedCmd(const char* configCmd, int wolfSSHD_AuthSetCertForcedCmd(WOLFSSHD_AUTH* auth, const byte* cmd, word32 cmdSz); #endif +/* Wraps wolfSSHD_GetUserConf with this user's resolved group set. The result is + * a newly allocated config the caller frees with wolfSSHD_ConfigFree(). */ WOLFSSHD_CONFIG* wolfSSHD_AuthGetUserConf(const WOLFSSHD_AUTH* auth, const char* usr, const char* host, const char* localAdr, word16* localPort, const char* RDomain, diff --git a/apps/wolfsshd/configuration.c b/apps/wolfsshd/configuration.c index cff184ec8..a06e6a6f8 100644 --- a/apps/wolfsshd/configuration.c +++ b/apps/wolfsshd/configuration.c @@ -96,6 +96,10 @@ struct WOLFSSHD_CONFIG { char* authorizedUPNDomains; /* allowlist of UPN realms for cert auth */ WOLFSSHD_CONFIG* next; /* next config in list */ WOLFSSHD_CONFIG* head; /* global config the Match nodes branch from */ + /* one bit per OPT_ tag, set when this node set that keyword itself rather + * than inheriting it. Drives the per keyword Match composition done by + * wolfSSHD_GetUserConf. */ + word32 setMask; long loginTimer; word16 port; byte usePrivilegeSeparation:2; @@ -254,7 +258,8 @@ WOLFSSHD_CONFIG* wolfSSHD_ConfigNew(void* heap) /* on success return a newly create WOLFSSHD_CONFIG structure that has the * same values set as the input 'conf'. User and group match values are not - * copied */ + * copied, and neither is setMask: the copy inherits values but has set nothing + * of its own, which is what lets composition tell the two apart. */ static WOLFSSHD_CONFIG* wolfSSHD_ConfigCopy(WOLFSSHD_CONFIG* conf) { int ret = WS_SUCCESS; @@ -434,6 +439,17 @@ enum { NUM_OPTIONS = 27 }; +/* bit in WOLFSSHD_CONFIG.setMask recording that option 'o' was set on a node. + * setMask is a word32, so NUM_OPTIONS must stay at or below 32. */ +#define OPT_BIT(o) ((word32)1 << (o)) + +/* A tag past bit 31 shifts out of setMask. The shift count wraps on the usual + * targets rather than trapping, so OPT_BIT(32) would quietly alias onto + * OPT_AUTH_KEYS_FILE and let composition claim a keyword no Match block named. + * Break the build instead. Kept local rather than using wc_static_assert so + * this file does not gain a minimum wolfSSL version. */ +typedef char wolfsshd_opt_bits_fit[(NUM_OPTIONS <= 32) ? 1 : -1]; + static const CONFIG_OPTION options[NUM_OPTIONS] = { {OPT_AUTH_KEYS_FILE, "AuthorizedKeysFile"}, {OPT_PRIV_SEP, "UsePrivilegeSeparation"}, @@ -1233,6 +1249,15 @@ static int HandleConfigOption(WOLFSSHD_CONFIG** conf, int opt, const char* value, const char* full, int fullSz, int depth) { int ret = WS_BAD_ARGUMENT; + WOLFSSHD_CONFIG* target; + + if (conf == NULL || *conf == NULL) { + return WS_BAD_ARGUMENT; + } + + /* the node the setting lands on. Match replaces the caller's cursor, so + * remember where we started. */ + target = *conf; switch (opt) { case OPT_AUTH_KEYS_FILE: @@ -1334,6 +1359,12 @@ static int HandleConfigOption(WOLFSSHD_CONFIG** conf, int opt, break; } + /* Match sets no keyword of its own, and Include's own lines were already + * recorded on whichever node they landed on. */ + if (ret == WS_SUCCESS && opt != OPT_MATCH && opt != OPT_INCLUDE) { + target->setMask |= OPT_BIT(opt); + } + return ret; } @@ -1483,61 +1514,174 @@ static int ConfigLoad(WOLFSSHD_CONFIG* conf, const char* filename, int depth) } -/* returns the config associated with the user */ -WOLFSSHD_CONFIG* wolfSSHD_GetUserConf(const WOLFSSHD_CONFIG* conf, - const char* usr, const char** grps, word32 grpCount, const char* host, - const char* localAdr, word16* localPort, const char* RDomain, - const char* adr) +/* Replaces *dst with a copy of 'src', freeing whatever *dst held. A NULL 'src' + * just clears *dst. Returns WS_SUCCESS on success. */ +static int ComposeString(char** dst, const char* src, void* heap) { - WOLFSSHD_CONFIG* ret; - WOLFSSHD_CONFIG* current; - int matches; + int ret = WS_SUCCESS; + + FreeString(dst, heap); + if (src != NULL) { + ret = CreateString(dst, src, (int)WSTRLEN(src), heap); + } + + return ret; +} + + +/* Applies to 'dst' every keyword that 'src' set itself and that 'dst' has not + * already taken from an earlier block. Returns WS_SUCCESS on success. */ +static int ConfigComposeFrom(WOLFSSHD_CONFIG* dst, const WOLFSSHD_CONFIG* src) +{ + int ret = WS_SUCCESS; + word32 take = src->setMask & ~dst->setMask; + + if (take & OPT_BIT(OPT_AUTH_KEYS_FILE)) { + ret = ComposeString(&dst->authKeysFile, src->authKeysFile, dst->heap); + dst->authKeysFileSet = src->authKeysFileSet; + } + if (ret == WS_SUCCESS && (take & OPT_BIT(OPT_BANNER))) { + ret = ComposeString(&dst->banner, src->banner, dst->heap); + } + if (ret == WS_SUCCESS && (take & OPT_BIT(OPT_CHROOT_DIR))) { + ret = ComposeString(&dst->chrootDir, src->chrootDir, dst->heap); + } + if (ret == WS_SUCCESS && (take & OPT_BIT(OPT_HOST_KEY))) { + ret = ComposeString(&dst->hostKeyFile, src->hostKeyFile, dst->heap); + } + if (ret == WS_SUCCESS && (take & OPT_BIT(OPT_HOST_CERT))) { + ret = ComposeString(&dst->hostCertFile, src->hostCertFile, dst->heap); + } + if (ret == WS_SUCCESS && (take & OPT_BIT(OPT_TRUSTED_USER_CA_KEYS))) { + ret = ComposeString(&dst->userCAKeysFile, src->userCAKeysFile, + dst->heap); + } + if (ret == WS_SUCCESS && (take & OPT_BIT(OPT_FORCE_CMD))) { + ret = ComposeString(&dst->forceCmd, src->forceCmd, dst->heap); + } + if (ret == WS_SUCCESS && (take & OPT_BIT(OPT_PIDFILE))) { + ret = ComposeString(&dst->pidFile, src->pidFile, dst->heap); + } + if (ret == WS_SUCCESS && (take & OPT_BIT(OPT_AUTHORIZED_UPN_DOMAINS))) { + ret = ComposeString(&dst->authorizedUPNDomains, + src->authorizedUPNDomains, dst->heap); + } + + if (ret == WS_SUCCESS) { + if (take & OPT_BIT(OPT_LOGIN_GRACE_TIME)) { + dst->loginTimer = src->loginTimer; + } + if (take & OPT_BIT(OPT_PORT)) { + dst->port = src->port; + } + if (take & OPT_BIT(OPT_PRIV_SEP)) { + dst->usePrivilegeSeparation = src->usePrivilegeSeparation; + } + if (take & OPT_BIT(OPT_PASSWORD_AUTH)) { + dst->passwordAuth = src->passwordAuth; + } + if (take & OPT_BIT(OPT_PUBKEY_AUTH)) { + dst->pubKeyAuth = src->pubKeyAuth; + } + if (take & OPT_BIT(OPT_PERMIT_ROOT)) { + dst->permitRootLogin = src->permitRootLogin; + } + if (take & OPT_BIT(OPT_PERMIT_EMPTY_PW)) { + dst->permitEmptyPasswords = src->permitEmptyPasswords; + } + if (take & OPT_BIT(OPT_STRICT_MODES)) { + dst->strictModes = src->strictModes; + } + + dst->setMask |= take; + } + + return ret; +} + + +/* returns 1 when the Match block 'node' applies to the connection described by + * 'usr' and the group list, and 0 otherwise. A node carrying no selector at all + * is the global config, never a Match candidate. */ +static int MatchApplies(const WOLFSSHD_CONFIG* node, const char* usr, + const char** grps, word32 grpCount) +{ + int matches = 0; word32 i; - /* default to return head of list */ - ret = current = (WOLFSSHD_CONFIG*)conf; - while (current != NULL) { - /* A node is a Match candidate only if it carries at least one - * selector. Every non-NULL selector on the node must match for the - * node to apply, so a combined 'Match User X Group Y' is treated as a - * conjunction the same way OpenSSH treats a Match line. A NULL - * selector acts as a wildcard. */ - matches = 0; - if (current->usrAppliesTo != NULL || current->groupAppliesTo != NULL) { - matches = 1; - - if (current->usrAppliesTo != NULL) { - if (usr == NULL || - XSTRCMP(current->usrAppliesTo, usr) != 0) { - matches = 0; - } - } + /* Every non-NULL selector on the node must match, so a combined + * 'Match User X Group Y' is a conjunction the same way OpenSSH treats a + * Match line. A NULL selector acts as a wildcard. */ + if (node->usrAppliesTo != NULL || node->groupAppliesTo != NULL) { + matches = 1; - /* The group selector matches when it equals any group the user - * belongs to, primary or supplementary, mirroring how OpenSSH - * evaluates 'Match Group'. An empty group list matches no group - * selector, so combined blocks fail closed. */ - if (matches && current->groupAppliesTo != NULL) { + if (node->usrAppliesTo != NULL) { + if (usr == NULL || XSTRCMP(node->usrAppliesTo, usr) != 0) { matches = 0; - if (grps != NULL) { - for (i = 0; i < grpCount; i++) { - if (grps[i] == NULL) - continue; - if (XSTRCMP(current->groupAppliesTo, grps[i]) == 0) { - matches = 1; - break; - } + } + } + + /* The group selector matches when it equals any group the user belongs + * to, primary or supplementary, mirroring how OpenSSH evaluates + * 'Match Group'. An empty group list matches no group selector, so + * combined blocks fail closed. */ + if (matches && node->groupAppliesTo != NULL) { + matches = 0; + if (grps != NULL) { + for (i = 0; i < grpCount; i++) { + if (grps[i] == NULL) + continue; + if (XSTRCMP(node->groupAppliesTo, grps[i]) == 0) { + matches = 1; + break; } } } } + } - if (matches) { - ret = current; - break; + return matches; +} + + +/* returns the config associated with the user, or NULL on failure. The result + * is a new config the caller frees with wolfSSHD_ConfigFree(). */ +WOLFSSHD_CONFIG* wolfSSHD_GetUserConf(const WOLFSSHD_CONFIG* conf, + const char* usr, const char** grps, word32 grpCount, const char* host, + const char* localAdr, word16* localPort, const char* RDomain, + const char* adr) +{ + WOLFSSHD_CONFIG* ret; + const WOLFSSHD_CONFIG* current; + int rc = WS_SUCCESS; + + if (conf == NULL) { + return NULL; + } + + /* Start from the global values. They carry setMask 0, so any keyword a + * matching block sets outranks them. */ + ret = wolfSSHD_ConfigCopy((WOLFSSHD_CONFIG*)conf); + if (ret == NULL) { + return NULL; + } + + /* OpenSSH resolves one keyword at a time over every Match block that + * applies, "the first obtained value will be used". Walking the list in + * order and only taking keywords not already taken does the same, so a + * setting made in just one of several matching blocks still applies. */ + for (current = conf; current != NULL; current = current->next) { + if (MatchApplies(current, usr, grps, grpCount)) { + rc = ConfigComposeFrom(ret, current); + if (rc != WS_SUCCESS) { + break; + } } + } - current = current->next; + if (rc != WS_SUCCESS) { + wolfSSHD_ConfigFree(ret); + ret = NULL; } /* @TODO */ diff --git a/apps/wolfsshd/configuration.h b/apps/wolfsshd/configuration.h index 5792b4e89..5608f5e34 100644 --- a/apps/wolfsshd/configuration.h +++ b/apps/wolfsshd/configuration.h @@ -76,6 +76,11 @@ byte wolfSSHD_ConfigGetPrivilegeSeparation(const WOLFSSHD_CONFIG* conf); long wolfSSHD_ConfigGetGraceTime(const WOLFSSHD_CONFIG* conf); byte wolfSSHD_ConfigGetPwAuth(const WOLFSSHD_CONFIG* conf); byte wolfSSHD_ConfigGetPubKeyAuth(const WOLFSSHD_CONFIG* conf); +/* Resolves the configuration for one connection. Every Match block that applies + * contributes, one keyword at a time, and the first block to name a keyword + * wins it; keywords no matching block names keep the global value. Returns a + * newly allocated config the caller frees with wolfSSHD_ConfigFree(), or NULL + * on failure. */ WOLFSSHD_CONFIG* wolfSSHD_GetUserConf(const WOLFSSHD_CONFIG* conf, const char* usr, const char** grps, word32 grpCount, const char* host, const char* localAdr, word16* localPort, const char* RDomain, diff --git a/apps/wolfsshd/test/run_all_sshd_tests.sh b/apps/wolfsshd/test/run_all_sshd_tests.sh index c79643747..69e24c011 100755 --- a/apps/wolfsshd/test/run_all_sshd_tests.sh +++ b/apps/wolfsshd/test/run_all_sshd_tests.sh @@ -406,10 +406,7 @@ else # these tests require setting up an sshd if [ "$USING_LOCAL_HOST" == 1 ]; then run_test "sshd_forcedcmd_test.sh" - # Left out of the suite on purpose: it asserts OpenSSH's per keyword - # Match composition, which wolfSSHD_GetUserConf does not implement yet, - # so it fails today. Add it back once that lands. - # run_test "sshd_match_overlap_test.sh" + run_test "sshd_match_overlap_test.sh" run_test "sshd_window_full_test.sh" run_test "sshd_empty_password_test.sh" run_test "sshd_permitroot_test.sh" diff --git a/apps/wolfsshd/test/sshd_match_overlap_test.sh b/apps/wolfsshd/test/sshd_match_overlap_test.sh index 97c711b15..a5e09f85e 100755 --- a/apps/wolfsshd/test/sshd_match_overlap_test.sh +++ b/apps/wolfsshd/test/sshd_match_overlap_test.sh @@ -9,9 +9,9 @@ # on which block is written first. # # This test writes the same two blocks in both orders. Only the group block sets -# ForceCommand, so a shell exec must be refused either way. wolfsshd resolves -# the whole first matching block instead of composing per keyword, so the -# user-first order currently drops the forced command and the exec succeeds. +# ForceCommand, so a shell exec must be refused either way. Resolving the whole +# first matching block instead of composing per keyword would drop the forced +# command in the user-first order and let the exec through. # # Expects ./authorized_keys_test to exist, as the other tests here do. Create it # with ./create_authorized_test_file.sh when running this script on its own. diff --git a/apps/wolfsshd/test/test_configuration.c b/apps/wolfsshd/test/test_configuration.c index 41c8e3635..fc1575c85 100644 --- a/apps/wolfsshd/test/test_configuration.c +++ b/apps/wolfsshd/test/test_configuration.c @@ -383,7 +383,7 @@ static int test_ConfigCopy(void) int ret = WS_SUCCESS; WOLFSSHD_CONFIG* head; WOLFSSHD_CONFIG* conf; - WOLFSSHD_CONFIG* match; + WOLFSSHD_CONFIG* match = NULL; head = wolfSSHD_ConfigNew(NULL); if (head == NULL) @@ -425,11 +425,12 @@ static int test_ConfigCopy(void) if (ret == WS_SUCCESS) ret = PCL("Match User testuser"); #undef PCL - /* retrieve match node from the list head */ + /* resolve the match node; the Match block sets nothing of its own, so + * every global value must come through the copy */ if (ret == WS_SUCCESS) { match = wolfSSHD_GetUserConf(head, "testuser", NULL, 0, NULL, NULL, NULL, NULL, NULL); - if (match == NULL || match == head) + if (match == NULL) ret = WS_FATAL_ERROR; } @@ -524,14 +525,15 @@ static int test_ConfigCopy(void) ret = WS_FATAL_ERROR; } + wolfSSHD_ConfigFree(match); wolfSSHD_ConfigFree(head); return ret; } -/* Verifies a Match block override is returned by wolfSSHD_GetUserConf and - * differs from the global node; RequestAuthentication/DoCheckUser depend on - * this resolution for PwAuth, PermitEmptyPw, PermitRootLogin, and - * AuthKeysFileSet. +/* Verifies a Match block override reaches the config wolfSSHD_GetUserConf + * resolves, and that the global node keeps its own permissive values; + * RequestAuthentication/DoCheckUser depend on this resolution for PwAuth, + * PermitEmptyPw, PermitRootLogin, and AuthKeysFileSet. * * Not covered here: the fail-closed NULL-config branches and the Match-aware * PermitRootLogin modes (prohibit-password, forced-commands-only) need a @@ -543,8 +545,8 @@ static int test_GetUserConfMatchOverride(void) int ret = WS_SUCCESS; WOLFSSHD_CONFIG* head; WOLFSSHD_CONFIG* conf; - WOLFSSHD_CONFIG* match; - WOLFSSHD_CONFIG* other; + WOLFSSHD_CONFIG* match = NULL; + WOLFSSHD_CONFIG* other = NULL; head = wolfSSHD_ConfigNew(NULL); if (head == NULL) @@ -578,11 +580,11 @@ static int test_GetUserConfMatchOverride(void) ret = WS_FATAL_ERROR; } - /* resolving testuser must return the per-user node, not the global head */ + /* resolving testuser must pick up the per-user block */ if (ret == WS_SUCCESS) { match = wolfSSHD_GetUserConf(head, "testuser", NULL, 0, NULL, NULL, NULL, NULL, NULL); - if (match == NULL || match == head) + if (match == NULL) ret = WS_FATAL_ERROR; } @@ -606,15 +608,23 @@ static int test_GetUserConfMatchOverride(void) ret = WS_FATAL_ERROR; } - /* a user with no Match block must fall back to the permissive global head, - * confirming the default behavior is unchanged for non-Match users */ + /* a user with no Match block must fall back to the permissive global + * values, confirming the default behavior is unchanged for non-Match + * users */ if (ret == WS_SUCCESS) { other = wolfSSHD_GetUserConf(head, "otheruser", NULL, 0, NULL, NULL, NULL, NULL, NULL); - if (other != head) + if (other == NULL || + wolfSSHD_ConfigGetPwAuth(other) != 1 || + wolfSSHD_ConfigGetPubKeyAuth(other) != 1 || + wolfSSHD_ConfigGetPermitEmptyPw(other) != 1 || + wolfSSHD_ConfigGetPermitRoot(other) != 1 || + wolfSSHD_ConfigGetAuthKeysFileSet(other) != 0) ret = WS_FATAL_ERROR; } + wolfSSHD_ConfigFree(other); + wolfSSHD_ConfigFree(match); wolfSSHD_ConfigFree(head); return ret; } @@ -692,9 +702,59 @@ static int test_MatchUnsupportedSelector(void) return ret; } +/* Resolves 'usr' against 'head' and compares the resolved ForceCommand with + * 'expect', where NULL expects no forced command. wolfSSHD_GetUserConf composes + * a config the caller owns, so this frees it. Returns WS_SUCCESS on a match. */ +static int CheckForcedCmd(WOLFSSHD_CONFIG* head, const char* usr, + const char** grps, word32 grpCount, const char* expect) +{ + int ret = WS_SUCCESS; + WOLFSSHD_CONFIG* match; + const char* cmd; + + match = wolfSSHD_GetUserConf(head, usr, grps, grpCount, NULL, NULL, + NULL, NULL, NULL); + if (match == NULL) { + return WS_FATAL_ERROR; + } + + cmd = wolfSSHD_ConfigGetForcedCmd(match); + if (expect == NULL) { + if (cmd != NULL) + ret = WS_FATAL_ERROR; + } + else if (cmd == NULL || XSTRCMP(cmd, expect) != 0) { + ret = WS_FATAL_ERROR; + } + + wolfSSHD_ConfigFree(match); + return ret; +} + +/* Same as CheckForcedCmd for the resolved PasswordAuthentication setting. */ +static int CheckPwAuth(WOLFSSHD_CONFIG* head, const char* usr, + const char** grps, word32 grpCount, int expect) +{ + int ret = WS_SUCCESS; + WOLFSSHD_CONFIG* match; + + match = wolfSSHD_GetUserConf(head, usr, grps, grpCount, NULL, NULL, + NULL, NULL, NULL); + if (match == NULL) { + return WS_FATAL_ERROR; + } + + if (wolfSSHD_ConfigGetPwAuth(match) != expect) { + ret = WS_FATAL_ERROR; + } + + wolfSSHD_ConfigFree(match); + return ret; +} + /* A combined 'Match User X Group Y' directive is a conjunction: it applies only * to a user who satisfies BOTH selectors, matching OpenSSH semantics. This - * locks in that wolfSSHD_GetUserConf does not return such a block for a user + * locks in that wolfSSHD_GetUserConf does not apply such a block to a user * who satisfies only one selector (the policy-bypass case), while single * selector 'Match User' and 'Match Group' blocks keep applying on their one * selector alone. */ @@ -703,10 +763,6 @@ static int test_GetUserConfMatchGroupAnd(void) int ret = WS_SUCCESS; WOLFSSHD_CONFIG* head; WOLFSSHD_CONFIG* conf; - WOLFSSHD_CONFIG* combined; - WOLFSSHD_CONFIG* userOnly; - WOLFSSHD_CONFIG* groupOnly; - WOLFSSHD_CONFIG* match; const char* grps[1]; head = wolfSSHD_ConfigNew(NULL); @@ -722,74 +778,53 @@ static int test_GetUserConfMatchGroupAnd(void) * 'alice' AND in group 'admins' */ if (ret == WS_SUCCESS) ret = PCL("Match User alice Group admins"); if (ret == WS_SUCCESS) ret = PCL("ForceCommand /bin/sh"); - if (ret == WS_SUCCESS) combined = conf; /* single selector blocks must keep matching on their one selector */ if (ret == WS_SUCCESS) ret = PCL("Match User bob"); if (ret == WS_SUCCESS) ret = PCL("ForceCommand /bin/bob"); - if (ret == WS_SUCCESS) userOnly = conf; if (ret == WS_SUCCESS) ret = PCL("Match Group staff"); if (ret == WS_SUCCESS) ret = PCL("ForceCommand /bin/staff"); - if (ret == WS_SUCCESS) groupOnly = conf; #undef PCL /* alice in admins satisfies both selectors -> gets the combined block */ if (ret == WS_SUCCESS) { grps[0] = "admins"; - match = wolfSSHD_GetUserConf(head, "alice", grps, 1, NULL, NULL, - NULL, NULL, NULL); - if (match != combined) - ret = WS_FATAL_ERROR; + ret = CheckForcedCmd(head, "alice", grps, 1, "/bin/sh"); } /* alice in a different group satisfies only the user selector -> must NOT - * get the combined block; falls back to the restrictive global head */ + * get the combined block; keeps the restrictive global value */ if (ret == WS_SUCCESS) { grps[0] = "users"; - match = wolfSSHD_GetUserConf(head, "alice", grps, 1, NULL, NULL, - NULL, NULL, NULL); - if (match != head) - ret = WS_FATAL_ERROR; + ret = CheckForcedCmd(head, "alice", grps, 1, "internal-sftp"); } /* carol in admins satisfies only the group selector -> must NOT get the - * combined block; falls back to the restrictive global head */ + * combined block; keeps the restrictive global value */ if (ret == WS_SUCCESS) { grps[0] = "admins"; - match = wolfSSHD_GetUserConf(head, "carol", grps, 1, NULL, NULL, - NULL, NULL, NULL); - if (match != head) - ret = WS_FATAL_ERROR; + ret = CheckForcedCmd(head, "carol", grps, 1, "internal-sftp"); } /* alice with no resolved group must NOT get the combined block: an empty * group set cannot satisfy the group selector, so it fails closed to the - * global head. This is the WIN32 / failed group-lookup path where + * global value. This is the WIN32 / failed group-lookup path where * wolfSSHD_AuthGetUserConf passes an empty group list. */ if (ret == WS_SUCCESS) { - match = wolfSSHD_GetUserConf(head, "alice", NULL, 0, NULL, NULL, - NULL, NULL, NULL); - if (match != head) - ret = WS_FATAL_ERROR; + ret = CheckForcedCmd(head, "alice", NULL, 0, "internal-sftp"); } /* single selector 'Match User bob' still applies on the user alone */ if (ret == WS_SUCCESS) { grps[0] = "anygroup"; - match = wolfSSHD_GetUserConf(head, "bob", grps, 1, NULL, NULL, - NULL, NULL, NULL); - if (match != userOnly) - ret = WS_FATAL_ERROR; + ret = CheckForcedCmd(head, "bob", grps, 1, "/bin/bob"); } /* single selector 'Match Group staff' still applies on the group alone */ if (ret == WS_SUCCESS) { grps[0] = "staff"; - match = wolfSSHD_GetUserConf(head, "anyuser", grps, 1, NULL, NULL, - NULL, NULL, NULL); - if (match != groupOnly) - ret = WS_FATAL_ERROR; + ret = CheckForcedCmd(head, "anyuser", grps, 1, "/bin/staff"); } wolfSSHD_ConfigFree(head); @@ -806,9 +841,6 @@ static int test_GetUserConfMatchSecondaryGroup(void) int ret = WS_SUCCESS; WOLFSSHD_CONFIG* head; WOLFSSHD_CONFIG* conf; - WOLFSSHD_CONFIG* blockWS; - WOLFSSHD_CONFIG* blockComb; - WOLFSSHD_CONFIG* match; const char* grps[3]; head = wolfSSHD_ConfigNew(NULL); @@ -823,12 +855,10 @@ static int test_GetUserConfMatchSecondaryGroup(void) /* single group selector */ if (ret == WS_SUCCESS) ret = PCL("Match Group wireshark"); if (ret == WS_SUCCESS) ret = PCL("ForceCommand /bin/ws"); - if (ret == WS_SUCCESS) blockWS = conf; /* combined user AND group selector */ if (ret == WS_SUCCESS) ret = PCL("Match User john Group admins"); if (ret == WS_SUCCESS) ret = PCL("ForceCommand /bin/adm"); - if (ret == WS_SUCCESS) blockComb = conf; #undef PCL /* wireshark present only in a secondary slot must still select the block */ @@ -836,49 +866,34 @@ static int test_GetUserConfMatchSecondaryGroup(void) grps[0] = "alice"; grps[1] = "staff"; grps[2] = "wireshark"; - match = wolfSSHD_GetUserConf(head, "alice", grps, 3, NULL, NULL, - NULL, NULL, NULL); - if (match != blockWS) - ret = WS_FATAL_ERROR; + ret = CheckForcedCmd(head, "alice", grps, 3, "/bin/ws"); } /* combined block satisfied with the group in a secondary slot */ if (ret == WS_SUCCESS) { grps[0] = "john"; grps[1] = "admins"; - match = wolfSSHD_GetUserConf(head, "john", grps, 2, NULL, NULL, - NULL, NULL, NULL); - if (match != blockComb) - ret = WS_FATAL_ERROR; + ret = CheckForcedCmd(head, "john", grps, 2, "/bin/adm"); } - /* combined block: user matches but group absent -> falls back to head */ + /* combined block: user matches but group absent -> keeps the global */ if (ret == WS_SUCCESS) { grps[0] = "john"; grps[1] = "users"; - match = wolfSSHD_GetUserConf(head, "john", grps, 2, NULL, NULL, - NULL, NULL, NULL); - if (match != head) - ret = WS_FATAL_ERROR; + ret = CheckForcedCmd(head, "john", grps, 2, "internal-sftp"); } - /* combined block: group matches but user differs -> falls back to head */ + /* combined block: group matches but user differs -> keeps the global */ if (ret == WS_SUCCESS) { grps[0] = "bob"; grps[1] = "admins"; - match = wolfSSHD_GetUserConf(head, "bob", grps, 2, NULL, NULL, - NULL, NULL, NULL); - if (match != head) - ret = WS_FATAL_ERROR; + ret = CheckForcedCmd(head, "bob", grps, 2, "internal-sftp"); } - /* user in none of the selector groups -> falls back to head */ + /* user in none of the selector groups -> keeps the global */ if (ret == WS_SUCCESS) { grps[0] = "carol"; - match = wolfSSHD_GetUserConf(head, "carol", grps, 1, NULL, NULL, - NULL, NULL, NULL); - if (match != head) - ret = WS_FATAL_ERROR; + ret = CheckForcedCmd(head, "carol", grps, 1, "internal-sftp"); } wolfSSHD_ConfigFree(head); @@ -897,8 +912,6 @@ static int test_GetUserConfMatchSubstring(void) int ret = WS_SUCCESS; WOLFSSHD_CONFIG* head; WOLFSSHD_CONFIG* conf; - WOLFSSHD_CONFIG* match; - WOLFSSHD_CONFIG* ghost; const char* grps[1]; head = wolfSSHD_ConfigNew(NULL); @@ -914,26 +927,16 @@ static int test_GetUserConfMatchSubstring(void) 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, 0, NULL, NULL, - NULL, NULL, NULL); - if (match == NULL || match == head) - ret = WS_FATAL_ERROR; - } + /* lookup by the real user name must pick up the Match block */ if (ret == WS_SUCCESS) { - if (wolfSSHD_ConfigGetPwAuth(match) != 1) - ret = WS_FATAL_ERROR; + ret = CheckPwAuth(head, "GroupAdmin", NULL, 0, 1); } - /* lookup by the ghost group token ("Admin") must NOT match; it falls back - * to the permissive-denied global head */ + /* lookup by the ghost group token ("Admin") must NOT match; it keeps the + * permissive-denied global value */ if (ret == WS_SUCCESS) { grps[0] = "Admin"; - ghost = wolfSSHD_GetUserConf(head, NULL, grps, 1, NULL, NULL, - NULL, NULL, NULL); - if (ghost != head) - ret = WS_FATAL_ERROR; + ret = CheckPwAuth(head, NULL, grps, 1, 0); } if (ret == WS_SUCCESS) { if (wolfSSHD_ConfigGetPwAuth(head) != 0) @@ -955,8 +958,6 @@ static int test_GetUserConfMatchSubstringGroup(void) int ret = WS_SUCCESS; WOLFSSHD_CONFIG* head; WOLFSSHD_CONFIG* conf; - WOLFSSHD_CONFIG* match; - WOLFSSHD_CONFIG* ghost; const char* grps[1]; head = wolfSSHD_ConfigNew(NULL); @@ -972,26 +973,16 @@ static int test_GetUserConfMatchSubstringGroup(void) if (ret == WS_SUCCESS) ret = PCL("PasswordAuthentication yes"); #undef PCL - /* lookup by the real group name must resolve to the Match node */ + /* lookup by the real group name must pick up the Match block */ if (ret == WS_SUCCESS) { grps[0] = "UserStaff"; - match = wolfSSHD_GetUserConf(head, NULL, grps, 1, 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; + ret = CheckPwAuth(head, NULL, grps, 1, 1); } - /* lookup by the ghost user token ("Staff") must NOT match; it falls back - * to the permissive-denied global head */ + /* lookup by the ghost user token ("Staff") must NOT match; it keeps the + * permissive-denied global value */ if (ret == WS_SUCCESS) { - ghost = wolfSSHD_GetUserConf(head, "Staff", NULL, 0, NULL, NULL, - NULL, NULL, NULL); - if (ghost != head) - ret = WS_FATAL_ERROR; + ret = CheckPwAuth(head, "Staff", NULL, 0, 0); } if (ret == WS_SUCCESS) { if (wolfSSHD_ConfigGetPwAuth(head) != 0) @@ -1016,8 +1007,6 @@ static int test_GetUserConfMatchLiteralKeywordName(void) int ret = WS_SUCCESS; WOLFSSHD_CONFIG* head; WOLFSSHD_CONFIG* conf; - WOLFSSHD_CONFIG* match; - WOLFSSHD_CONFIG* ghost; const char* grps[1]; head = wolfSSHD_ConfigNew(NULL); @@ -1033,26 +1022,16 @@ static int test_GetUserConfMatchLiteralKeywordName(void) if (ret == WS_SUCCESS) ret = PCL("PasswordAuthentication yes"); #undef PCL - /* lookup by the user name "Group" must resolve to the Match node */ + /* lookup by the user name "Group" must pick up the Match block */ if (ret == WS_SUCCESS) { - match = wolfSSHD_GetUserConf(head, "Group", NULL, 0, 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; + ret = CheckPwAuth(head, "Group", NULL, 0, 1); } /* the user name must NOT have leaked into groupAppliesTo: a lookup by group - * "Group" must fall back to the permissive-denied global head */ + * "Group" must keep the permissive-denied global value */ if (ret == WS_SUCCESS) { grps[0] = "Group"; - ghost = wolfSSHD_GetUserConf(head, NULL, grps, 1, NULL, NULL, - NULL, NULL, NULL); - if (ghost != head) - ret = WS_FATAL_ERROR; + ret = CheckPwAuth(head, NULL, grps, 1, 0); } if (ret == WS_SUCCESS) { if (wolfSSHD_ConfigGetPwAuth(head) != 0) @@ -1111,8 +1090,6 @@ 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) @@ -1127,21 +1104,14 @@ static int test_GetUserConfMatchRepeatedKeyword(void) if (ret == WS_SUCCESS) ret = PCL("PasswordAuthentication yes"); #undef PCL - /* lookup by the replacement name "b" must resolve to the Match node */ + /* lookup by the replacement name "b" must pick up the Match block */ if (ret == WS_SUCCESS) { - match = wolfSSHD_GetUserConf(head, "b", NULL, 0, NULL, NULL, - NULL, NULL, NULL); - if (match == NULL || match == head || - wolfSSHD_ConfigGetPwAuth(match) != 1) - ret = WS_FATAL_ERROR; + ret = CheckPwAuth(head, "b", NULL, 0, 1); } - /* lookup by the replaced name "a" must NOT match; it falls back to head */ + /* lookup by the replaced name "a" must NOT match; it keeps the global */ if (ret == WS_SUCCESS) { - old = wolfSSHD_GetUserConf(head, "a", NULL, 0, NULL, NULL, - NULL, NULL, NULL); - if (old != head) - ret = WS_FATAL_ERROR; + ret = CheckPwAuth(head, "a", NULL, 0, 0); } if (ret == WS_SUCCESS) { if (wolfSSHD_ConfigGetPwAuth(head) != 0) @@ -1248,8 +1218,6 @@ static int test_GetUserConfMatchNoInherit(void) int ret = WS_SUCCESS; WOLFSSHD_CONFIG* head; WOLFSSHD_CONFIG* conf; - WOLFSSHD_CONFIG* aliceConf = NULL; - WOLFSSHD_CONFIG* staffConf = NULL; WOLFSSHD_CONFIG* match = NULL; const char* cmd; const char* grps[1]; @@ -1268,13 +1236,11 @@ static int test_GetUserConfMatchNoInherit(void) if (ret == WS_SUCCESS) ret = PCL("ForceCommand /bin/alice"); if (ret == WS_SUCCESS) ret = PCL("AuthorizedKeysFile .ssh/alice_keys"); if (ret == WS_SUCCESS) ret = PCL("PermitEmptyPasswords no"); - if (ret == WS_SUCCESS) aliceConf = conf; /* the staff block sets one option, everything else must resolve to the * global value rather than to alice's */ if (ret == WS_SUCCESS) ret = PCL("Match Group staff"); if (ret == WS_SUCCESS) ret = PCL("PubkeyAuthentication no"); - if (ret == WS_SUCCESS) staffConf = conf; #undef PCL if (ret == WS_SUCCESS) { @@ -1282,7 +1248,7 @@ static int test_GetUserConfMatchNoInherit(void) grps[0] = "staff"; match = wolfSSHD_GetUserConf(head, "bob", grps, 1, NULL, NULL, NULL, NULL, NULL); - if (match != staffConf) + if (match == NULL) ret = WS_FATAL_ERROR; if (ret == WS_SUCCESS) { @@ -1302,6 +1268,8 @@ static int test_GetUserConfMatchNoInherit(void) if (ret == WS_SUCCESS && wolfSSHD_ConfigGetPubKeyAuth(match) != 0) { ret = WS_FATAL_ERROR; } + wolfSSHD_ConfigFree(match); + match = NULL; if (ret == WS_SUCCESS) { Log(" PASSED.\n"); } @@ -1315,7 +1283,7 @@ static int test_GetUserConfMatchNoInherit(void) grps[0] = "users"; match = wolfSSHD_GetUserConf(head, "alice", grps, 1, NULL, NULL, NULL, NULL, NULL); - if (match != aliceConf) + if (match == NULL) ret = WS_FATAL_ERROR; if (ret == WS_SUCCESS) { @@ -1335,6 +1303,8 @@ static int test_GetUserConfMatchNoInherit(void) if (ret == WS_SUCCESS && wolfSSHD_ConfigGetPubKeyAuth(match) != 1) { ret = WS_FATAL_ERROR; } + wolfSSHD_ConfigFree(match); + match = NULL; if (ret == WS_SUCCESS) { Log(" PASSED.\n"); } @@ -1343,6 +1313,7 @@ static int test_GetUserConfMatchNoInherit(void) } } + wolfSSHD_ConfigFree(match); wolfSSHD_ConfigFree(head); return ret; } @@ -1403,7 +1374,7 @@ static int test_GetUserConfMatchOverlapCompose(void) int fail = WS_SUCCESS; WOLFSSHD_CONFIG* head = NULL; WOLFSSHD_CONFIG* conf; - WOLFSSHD_CONFIG* match; + WOLFSSHD_CONFIG* match = NULL; const char* keys; const char* cmd; const char* grps[1]; @@ -1438,6 +1409,8 @@ static int test_GetUserConfMatchOverlapCompose(void) ret = WS_FATAL_ERROR; } } + wolfSSHD_ConfigFree(match); + match = NULL; } Log((ret == WS_SUCCESS) ? " PASSED.\n" : " FAILED.\n"); wolfSSHD_ConfigFree(head); @@ -1476,6 +1449,8 @@ static int test_GetUserConfMatchOverlapCompose(void) ret = WS_FATAL_ERROR; } } + wolfSSHD_ConfigFree(match); + match = NULL; } Log((ret == WS_SUCCESS) ? " PASSED.\n" : " FAILED.\n"); wolfSSHD_ConfigFree(head); @@ -1509,16 +1484,22 @@ static int test_GetUserConfMatchOverlapCompose(void) if (keys == NULL || XSTRCMP(keys, ".ssh/alice_keys") != 0) ret = WS_FATAL_ERROR; } + wolfSSHD_ConfigFree(match); + match = NULL; } - /* and a user who matches neither keeps the global values */ + /* and a user who matches neither keeps the global values, with none of + * alice's settings leaking across */ if (ret == WS_SUCCESS) { grps[0] = "users"; match = wolfSSHD_GetUserConf(head, "bob", grps, 1, NULL, NULL, NULL, NULL, NULL); - if (match != head || wolfSSHD_ConfigGetPwAuth(match) != 1) { + if (match == NULL || wolfSSHD_ConfigGetPwAuth(match) != 1 || + wolfSSHD_ConfigGetAuthKeysFile(match) != NULL) { ret = WS_FATAL_ERROR; } + wolfSSHD_ConfigFree(match); + match = NULL; } Log((ret == WS_SUCCESS) ? " PASSED.\n" : " FAILED.\n"); wolfSSHD_ConfigFree(head); @@ -1563,6 +1544,8 @@ static int test_GetUserConfMatchOverlapCompose(void) ret = WS_FATAL_ERROR; } } + wolfSSHD_ConfigFree(match); + match = NULL; } Log((ret == WS_SUCCESS) ? " PASSED.\n" : " FAILED.\n"); wolfSSHD_ConfigFree(head); @@ -1605,6 +1588,8 @@ static int test_GetUserConfMatchOverlapCompose(void) if (cmd == NULL || XSTRCMP(cmd, "/bin/alice") != 0) ret = WS_FATAL_ERROR; } + wolfSSHD_ConfigFree(match); + match = NULL; } Log((ret == WS_SUCCESS) ? " PASSED.\n" : " FAILED.\n"); wolfSSHD_ConfigFree(head); @@ -1629,7 +1614,7 @@ static int test_ConfigIncludeMatchChain(void) { int ret; WOLFSSHD_CONFIG* head = NULL; - WOLFSSHD_CONFIG* match; + WOLFSSHD_CONFIG* match = NULL; const char* cmd; const char* incPath = "./include_match.conf"; const char* topPath = "./include_match_top.conf"; @@ -1667,10 +1652,12 @@ static int test_ConfigIncludeMatchChain(void) match = wolfSSHD_GetUserConf(head, "alice", NULL, 0, NULL, NULL, NULL, NULL, NULL); cmd = wolfSSHD_ConfigGetForcedCmd(match); - if (match == head || cmd == NULL || + if (match == NULL || cmd == NULL || XSTRCMP(cmd, "/bin/alice") != 0) { ret = WS_FATAL_ERROR; } + wolfSSHD_ConfigFree(match); + match = NULL; if (ret == WS_SUCCESS) { Log(" PASSED.\n"); } @@ -1684,9 +1671,11 @@ static int test_ConfigIncludeMatchChain(void) match = wolfSSHD_GetUserConf(head, "bob", NULL, 0, NULL, NULL, NULL, NULL, NULL); cmd = wolfSSHD_ConfigGetForcedCmd(match); - if (match == head || cmd == NULL || XSTRCMP(cmd, "/bin/bob") != 0) { + if (match == NULL || cmd == NULL || XSTRCMP(cmd, "/bin/bob") != 0) { ret = WS_FATAL_ERROR; } + wolfSSHD_ConfigFree(match); + match = NULL; if (ret == WS_SUCCESS) { Log(" PASSED.\n"); } @@ -1700,7 +1689,7 @@ static int test_ConfigIncludeMatchChain(void) match = wolfSSHD_GetUserConf(head, "carol", NULL, 0, NULL, NULL, NULL, NULL, NULL); cmd = wolfSSHD_ConfigGetForcedCmd(match); - if (match != head || cmd == NULL || + if (match == NULL || cmd == NULL || XSTRCMP(cmd, "/bin/global") != 0) { ret = WS_FATAL_ERROR; } @@ -1710,6 +1699,8 @@ static int test_ConfigIncludeMatchChain(void) wolfSSHD_ConfigGetPermitEmptyPw(match) != 1) { ret = WS_FATAL_ERROR; } + wolfSSHD_ConfigFree(match); + match = NULL; if (ret == WS_SUCCESS) { Log(" PASSED.\n"); } @@ -1718,6 +1709,7 @@ static int test_ConfigIncludeMatchChain(void) } } + wolfSSHD_ConfigFree(match); wolfSSHD_ConfigFree(head); (void)WREMOVE(NULL, incPath); (void)WREMOVE(NULL, topPath); @@ -6436,6 +6428,7 @@ const TEST_CASE testCases[] = { TEST_DECL(test_MatchUPNToUser), TEST_DECL(test_IncludeRecursionBound), TEST_DECL(test_GetUserConfMatchNoInherit), + TEST_DECL(test_GetUserConfMatchOverlapCompose), TEST_DECL(test_ConfigIncludeMatchChain), TEST_DECL(test_GetUserAuthTypes), TEST_DECL(test_DefaultUserAuthTypesNullArgs), @@ -6518,11 +6511,6 @@ const TEST_CASE testCases[] = { TEST_DECL(test_CheckPublicKeyUnixOrdering), #endif #endif - /* Runs last: the first failing case stops the run, and this one is a known - * gap. wolfSSHD_GetUserConf returns the first matching Match block whole - * instead of composing per keyword, so a setting made only in a later - * matching block is dropped. */ - TEST_DECL(test_GetUserConfMatchOverlapCompose), }; int main(int argc, char** argv) diff --git a/apps/wolfsshd/wolfsshd.c b/apps/wolfsshd/wolfsshd.c index a294661af..45c747cc2 100644 --- a/apps/wolfsshd/wolfsshd.c +++ b/apps/wolfsshd/wolfsshd.c @@ -2585,6 +2585,8 @@ static void* HandleConnection(void* arg) ret = WS_NOT_COMPILED; } } + + wolfSSHD_ConfigFree(usrConf); } error = wolfSSH_get_error(ssh);