diff --git a/apps/wolfsshd/auth.c b/apps/wolfsshd/auth.c index 062455d4d..b33ba4817 100644 --- a/apps/wolfsshd/auth.c +++ b/apps/wolfsshd/auth.c @@ -1253,6 +1253,14 @@ static int RequestAuthentication(WS_UserAuthData* authData, } + if (ret == WOLFSSH_USERAUTH_SUCCESS && + authData->type == WOLFSSH_USERAUTH_PUBLICKEY && + wolfSSHD_ConfigGetPubKeyAuth(usrConf) != 1) { + wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Public key authentication not " + "allowed by configuration!"); + ret = WOLFSSH_USERAUTH_REJECTED; + } + #ifdef WOLFSSL_FPKI if (ret == WOLFSSH_USERAUTH_SUCCESS && authData->type == WOLFSSH_USERAUTH_PUBLICKEY) { @@ -1434,6 +1442,26 @@ int DefaultUserAuth(byte authType, WS_UserAuthData* authData, void* ctx) } +/* Builds the bit mask of authentication methods advertised to a peer based on + * the resolved per-user configuration. A method is only offered when its + * corresponding config option is enabled, so PasswordAuthentication no and + * PubkeyAuthentication no remove the method from the advertisement. Returns 0 + * when both are disabled (no methods advertised). */ +WOLFSSHD_STATIC int wolfSSHD_GetUserAuthTypes(const WOLFSSHD_CONFIG* usrConf) +{ + int ret = 0; + + if (wolfSSHD_ConfigGetPwAuth(usrConf) == 1) { + ret |= WOLFSSH_USERAUTH_PASSWORD; + } + if (wolfSSHD_ConfigGetPubKeyAuth(usrConf) == 1) { + ret |= WOLFSSH_USERAUTH_PUBLICKEY; + } + + return ret; +} + + int DefaultUserAuthTypes(WOLFSSH* ssh, void* ctx) { WOLFSSHD_CONFIG* usrConf; @@ -1453,10 +1481,7 @@ int DefaultUserAuthTypes(WOLFSSH* ssh, void* ctx) ret = WS_BAD_ARGUMENT; } else { - if (wolfSSHD_ConfigGetPwAuth(usrConf) == 1) { - ret |= WOLFSSH_USERAUTH_PASSWORD; - } - ret |= WOLFSSH_USERAUTH_PUBLICKEY; + ret = wolfSSHD_GetUserAuthTypes(usrConf); } return ret; diff --git a/apps/wolfsshd/auth.h b/apps/wolfsshd/auth.h index 6d3d706c0..e4b6178d0 100644 --- a/apps/wolfsshd/auth.h +++ b/apps/wolfsshd/auth.h @@ -91,5 +91,6 @@ 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 wolfSSHD_GetUserAuthTypes(const WOLFSSHD_CONFIG* usrConf); #endif #endif /* WOLFAUTH_H */ diff --git a/apps/wolfsshd/configuration.c b/apps/wolfsshd/configuration.c index 91dd6480c..54f204ab5 100644 --- a/apps/wolfsshd/configuration.c +++ b/apps/wolfsshd/configuration.c @@ -32,11 +32,8 @@ /* functions for parsing out options from a config file and for handling loading * key/certs using the env. filesystem */ -#ifdef WOLFSSHD_UNIT_TEST -#define WOLFSSHD_STATIC -#else -#define WOLFSSHD_STATIC static -#endif +/* WOLFSSHD_STATIC is defined in configuration.h so configuration.c and auth.c + * share the same test-visibility convention. */ #include #include @@ -217,6 +214,7 @@ WOLFSSHD_CONFIG* wolfSSHD_ConfigNew(void* heap) /* default values */ ret->port = 22; ret->passwordAuth = 1; + ret->pubKeyAuth = 1; ret->loginTimer = 120; } return ret; @@ -388,9 +386,10 @@ enum { OPT_TRUSTED_USER_CA_KEYS = 21, OPT_PIDFILE = 22, OPT_BANNER = 23, + OPT_PUBKEY_AUTH = 24, }; enum { - NUM_OPTIONS = 24 + NUM_OPTIONS = 25 }; static const CONFIG_OPTION options[NUM_OPTIONS] = { @@ -407,6 +406,7 @@ static const CONFIG_OPTION options[NUM_OPTIONS] = { {OPT_LOGIN_GRACE_TIME, "LoginGraceTime"}, {OPT_HOST_KEY, "HostKey"}, {OPT_PASSWORD_AUTH, "PasswordAuthentication"}, + {OPT_PUBKEY_AUTH, "PubkeyAuthentication"}, {OPT_PORT, "Port"}, {OPT_PERMIT_ROOT, "PermitRootLogin"}, {OPT_USE_DNS, "UseDNS"}, @@ -553,6 +553,32 @@ static int HandlePwAuth(WOLFSSHD_CONFIG* conf, const char* value) return ret; } +/* returns WS_SUCCESS on success */ +static int HandlePubKeyAuth(WOLFSSHD_CONFIG* conf, const char* value) +{ + int ret = WS_SUCCESS; + + if (conf == NULL || value == NULL) { + ret = WS_BAD_ARGUMENT; + } + + if (ret == WS_SUCCESS) { + if (WSTRCMP(value, "no") == 0) { + wolfSSH_Log(WS_LOG_INFO, + "[SSHD] public key authentication disabled"); + conf->pubKeyAuth = 0; + } + else if (WSTRCMP(value, "yes") == 0) { + conf->pubKeyAuth = 1; + } + else { + ret = WS_BAD_ARGUMENT; + } + } + + return ret; +} + #define WOLFSSH_PROTOCOL_VERSION 2 /* returns WS_SUCCESS on success */ @@ -1032,6 +1058,9 @@ static int HandleConfigOption(WOLFSSHD_CONFIG** conf, int opt, case OPT_PASSWORD_AUTH: ret = HandlePwAuth(*conf, value); break; + case OPT_PUBKEY_AUTH: + ret = HandlePubKeyAuth(*conf, value); + break; case OPT_PORT: ret = HandlePort(*conf, value); break; @@ -1467,6 +1496,17 @@ byte wolfSSHD_ConfigGetPwAuth(const WOLFSSHD_CONFIG* conf) return ret; } +byte wolfSSHD_ConfigGetPubKeyAuth(const WOLFSSHD_CONFIG* conf) +{ + byte ret = 0; + + if (conf != NULL) { + ret = conf->pubKeyAuth; + } + + return ret; +} + byte wolfSSHD_ConfigGetPermitRoot(const WOLFSSHD_CONFIG* conf) { byte ret = 0; diff --git a/apps/wolfsshd/configuration.h b/apps/wolfsshd/configuration.h index 064db6bbb..4df45ebac 100644 --- a/apps/wolfsshd/configuration.h +++ b/apps/wolfsshd/configuration.h @@ -23,6 +23,15 @@ typedef struct WOLFSSHD_CONFIG WOLFSSHD_CONFIG; +/* Expose otherwise-static wolfsshd internals to the unit tests while keeping + * them file-local in normal builds. Shared so configuration.c and auth.c use + * the same convention. */ +#ifdef WOLFSSHD_UNIT_TEST +#define WOLFSSHD_STATIC +#else +#define WOLFSSHD_STATIC static +#endif + #include "auth.h" /* 0 so that privilege separation is default on after struct memset'd on init */ @@ -52,6 +61,7 @@ byte wolfSSHD_ConfigGetPermitRoot(const WOLFSSHD_CONFIG* conf); 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); WOLFSSHD_CONFIG* wolfSSHD_GetUserConf(const WOLFSSHD_CONFIG* conf, const char* usr, const char* grp, const char* host, const char* localAdr, word16* localPort, const char* RDomain, diff --git a/apps/wolfsshd/test/test_configuration.c b/apps/wolfsshd/test/test_configuration.c index e1a2b2f6d..d7d96de96 100644 --- a/apps/wolfsshd/test/test_configuration.c +++ b/apps/wolfsshd/test/test_configuration.c @@ -194,6 +194,10 @@ static int test_ConfigDefaults(void) if (wolfSSHD_ConfigGetPwAuth(conf) == 0) ret = WS_FATAL_ERROR; } + if (ret == WS_SUCCESS) { + if (wolfSSHD_ConfigGetPubKeyAuth(conf) == 0) + ret = WS_FATAL_ERROR; + } wolfSSHD_ConfigFree(conf); return ret; @@ -242,6 +246,11 @@ static int test_ParseConfigLine(void) {"Password auth yes", "PasswordAuthentication yes", 0}, {"Password auth invalid", "PasswordAuthentication wolfsshd", 1}, + /* Public key auth tests. */ + {"Pubkey auth no", "PubkeyAuthentication no", 0}, + {"Pubkey auth yes", "PubkeyAuthentication yes", 0}, + {"Pubkey auth invalid", "PubkeyAuthentication wolfsshd", 1}, + /* Include files tests. */ {"Include file bad", "Include sshd_config.d/test.bad", 1}, {"Include file exists", "Include sshd_config.d/01-test.conf", 0}, @@ -312,6 +321,9 @@ static int test_ConfigCopy(void) if (ret == WS_SUCCESS) ret = PCL("Port 2222"); if (ret == WS_SUCCESS) ret = PCL("LoginGraceTime 30"); if (ret == WS_SUCCESS) ret = PCL("PasswordAuthentication yes"); + /* set to the non-default value so a dropped copy (which would leave the + * wolfSSHD_ConfigNew default of 1) is caught */ + if (ret == WS_SUCCESS) ret = PCL("PubkeyAuthentication no"); if (ret == WS_SUCCESS) ret = PCL("PermitEmptyPasswords yes"); if (ret == WS_SUCCESS) ret = PCL("PermitRootLogin yes"); if (ret == WS_SUCCESS) ret = PCL("UsePrivilegeSeparation sandbox"); @@ -389,6 +401,12 @@ static int test_ConfigCopy(void) if (wolfSSHD_ConfigGetPwAuth(match) == 0) ret = WS_FATAL_ERROR; } + /* pubKeyAuth was set to the non-default 'no' (0) on the head, so the copy + * must carry 0; a dropped copy would surface as the default 1 here */ + if (ret == WS_SUCCESS) { + if (wolfSSHD_ConfigGetPubKeyAuth(match) != 0) + ret = WS_FATAL_ERROR; + } if (ret == WS_SUCCESS) { if (wolfSSHD_ConfigGetPermitEmptyPw(match) == 0) ret = WS_FATAL_ERROR; @@ -448,14 +466,17 @@ static int test_GetUserConfMatchOverride(void) * global head node unchanged. */ if (ret == WS_SUCCESS) ret = PCL("Match User testuser"); if (ret == WS_SUCCESS) ret = PCL("PasswordAuthentication no"); + if (ret == WS_SUCCESS) ret = PCL("PubkeyAuthentication no"); if (ret == WS_SUCCESS) ret = PCL("PermitEmptyPasswords no"); if (ret == WS_SUCCESS) ret = PCL("PermitRootLogin no"); if (ret == WS_SUCCESS) ret = PCL("AuthorizedKeysFile .ssh/match_keys"); #undef PCL - /* the global head node must keep the permissive values */ + /* the global head node must keep the permissive values (pubKeyAuth keeps + * its default of 1, proving the Match override did not leak to the head) */ if (ret == WS_SUCCESS) { if (wolfSSHD_ConfigGetPwAuth(head) != 1 || + wolfSSHD_ConfigGetPubKeyAuth(head) != 1 || wolfSSHD_ConfigGetPermitEmptyPw(head) != 1 || wolfSSHD_ConfigGetPermitRoot(head) != 1) ret = WS_FATAL_ERROR; @@ -473,6 +494,7 @@ static int test_GetUserConfMatchOverride(void) * ones RequestAuthentication and DoCheckUser will now enforce */ if (ret == WS_SUCCESS) { if (wolfSSHD_ConfigGetPwAuth(match) != 0 || + wolfSSHD_ConfigGetPubKeyAuth(match) != 0 || wolfSSHD_ConfigGetPermitEmptyPw(match) != 0 || wolfSSHD_ConfigGetPermitRoot(match) != 0) ret = WS_FATAL_ERROR; @@ -855,12 +877,80 @@ static int test_CAKeysFileDiffers(void) 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 + * security-relevant cases where pubkey is disabled and where both are disabled + * (no methods advertised, mask == 0). */ +static int test_GetUserAuthTypes(void) +{ + int ret = WS_SUCCESS; + int i; + + static const struct { + const char* desc; + int pwAuth; /* 1 = leave enabled, 0 = PasswordAuthentication no */ + int pubKeyAuth; /* 1 = leave enabled, 0 = PubkeyAuthentication no */ + int expected; + } vectors[] = { + {"both enabled advertises both", 1, 1, + WOLFSSH_USERAUTH_PASSWORD | WOLFSSH_USERAUTH_PUBLICKEY}, + {"pubkey disabled advertises password only", 1, 0, + WOLFSSH_USERAUTH_PASSWORD}, + {"password disabled advertises pubkey only", 0, 1, + WOLFSSH_USERAUTH_PUBLICKEY}, + {"both disabled advertises nothing", 0, 0, 0}, + }; + const int numVectors = (int)(sizeof(vectors) / sizeof(*vectors)); + WOLFSSHD_CONFIG* conf; + + for (i = 0; i < numVectors && ret == WS_SUCCESS; ++i) { + Log(" Testing scenario: %s.", vectors[i].desc); + + conf = wolfSSHD_ConfigNew(NULL); + if (conf == NULL) { + Log(" FAILED.\n"); + ret = WS_MEMORY_E; + break; + } + + /* both options default to enabled in wolfSSHD_ConfigNew, so only the + * disabled cases need an explicit directive */ + if (vectors[i].pwAuth == 0) { + ret = ParseConfigLine(&conf, "PasswordAuthentication no", + (int)WSTRLEN("PasswordAuthentication no")); + } + if (ret == WS_SUCCESS && vectors[i].pubKeyAuth == 0) { + ret = ParseConfigLine(&conf, "PubkeyAuthentication no", + (int)WSTRLEN("PubkeyAuthentication no")); + } + + if (ret == WS_SUCCESS) { + if (wolfSSHD_GetUserAuthTypes(conf) != vectors[i].expected) { + Log(" FAILED.\n"); + ret = WS_FATAL_ERROR; + } + else { + Log(" PASSED.\n"); + } + } + else { + Log(" FAILED.\n"); + } + + wolfSSHD_ConfigFree(conf); + } + + return ret; +} + const TEST_CASE testCases[] = { TEST_DECL(test_ConfigDefaults), TEST_DECL(test_ParseConfigLine), TEST_DECL(test_ConfigCopy), TEST_DECL(test_GetUserConfMatchOverride), TEST_DECL(test_CAKeysFileDiffers), + TEST_DECL(test_GetUserAuthTypes), TEST_DECL(test_ConfigFree), #ifdef WOLFSSL_BASE64_ENCODE TEST_DECL(test_CheckAuthKeysLine), diff --git a/src/internal.c b/src/internal.c index a66226917..f2bd01f4a 100644 --- a/src/internal.c +++ b/src/internal.c @@ -16255,6 +16255,7 @@ int SendUserAuthRequest(WOLFSSH* ssh, byte authType, int addSig) static int GetAllowedAuth(WOLFSSH* ssh, char* authStr) { int typeAllowed = 0; + int authStrSz; if (ssh == NULL || authStr == NULL) return WS_BAD_ARGUMENT; @@ -16285,8 +16286,16 @@ static int GetAllowedAuth(WOLFSSH* ssh, char* authStr) } #endif - /* remove last comma from the list */ - return (int)XSTRLEN(authStr) - 1; + /* Remove the trailing comma from the list. An empty list (no auth methods + * allowed, e.g. a server callback that returns 0) has length 0 and must not + * underflow to a negative size; return 0 so SendUserAuthFailure emits a + * valid USERAUTH_FAILURE with an empty name-list per RFC 4252. */ + authStrSz = (int)XSTRLEN(authStr); + if (authStrSz > 0) { + authStrSz--; + } + + return authStrSz; } int SendUserAuthFailure(WOLFSSH* ssh, byte partialSuccess) @@ -18248,6 +18257,11 @@ int wolfSSH_TestDoUserAuthRequest(WOLFSSH* ssh, byte* buf, word32 len, return DoUserAuthRequest(ssh, buf, len, idx); } +int wolfSSH_TestSendUserAuthFailure(WOLFSSH* ssh, byte partialSuccess) +{ + return SendUserAuthFailure(ssh, partialSuccess); +} + int wolfSSH_TestHighwaterCheck(WOLFSSH* ssh, byte side) { return HighwaterCheck(ssh, side); diff --git a/tests/unit.c b/tests/unit.c index 5d1acdcd9..fbc3978df 100644 --- a/tests/unit.c +++ b/tests/unit.c @@ -2070,6 +2070,128 @@ static int test_DoUserAuthRequest_serviceName(void) } +/* userAuthTypesCb that advertises no methods (returns mask 0). Mirrors a + * wolfsshd configuration with both PasswordAuthentication no and + * PubkeyAuthentication no. */ +static int UnitAuthTypesReturnZero(WOLFSSH* ssh, void* ctx) +{ + (void)ssh; + (void)ctx; + return 0; +} + +/* Regression test for the 0-mask case (issue 4115 follow-up). When the + * userAuthTypesCb advertises no methods, SendUserAuthFailure must still emit a + * well-formed USERAUTH_FAILURE carrying an empty "authentications that can + * continue" name-list (RFC 4252 Section 5.1) and return WS_SUCCESS, instead of + * underflowing the name-list length to -1 and dropping the connection. + * + * Asserts: + * 1. wolfSSH_TestSendUserAuthFailure() returns WS_SUCCESS (not negative). + * 2. Exactly one packet is emitted (connection not dropped). + * 3. The packet's message id is MSGID_USERAUTH_FAILURE. + * 4. The name-list length field is 0 (empty method list). + * + * The control case (a permissive callback advertising publickey+password) + * confirms the same path produces a non-empty name-list, so the empty result + * is specific to the 0-mask input rather than the test always seeing 0. */ +static int test_SendUserAuthFailure_emptyMethods(void) +{ + WOLFSSH_CTX* ctx = NULL; + WOLFSSH* ssh = NULL; + int result = 0; + int i; + struct { + WS_CallbackUserAuthTypes cb; + int expectEmpty; /* 1 = name-list must be empty, 0 = non-empty */ + const char* label; + } cases[] = { + { UnitAuthTypesReturnZero, 1, "no methods advertised" }, + { NULL, 0, "default methods advertised" }, + }; + word32 off = LENGTH_SZ + PAD_LENGTH_SZ; + + for (i = 0; i < (int)(sizeof(cases)/sizeof(cases[0])); i++) { + word32 nameListSz; + int capMsgId; + int ret; + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + if (ctx == NULL) { result = -600; break; } + if (cases[i].cb != NULL) { + wolfSSH_SetUserAuthTypes(ctx, cases[i].cb); + } + wolfSSH_SetIOSend(ctx, CaptureIoSendAuthSvc); + + ssh = wolfSSH_new(ctx); + if (ssh == NULL) { result = -601; break; } + + s_authSvcCaptureSz = 0; + s_authSvcSendCount = 0; + WMEMSET(s_authSvcCapture, 0, sizeof(s_authSvcCapture)); + + ret = wolfSSH_TestSendUserAuthFailure(ssh, 0); + + if (ret != WS_SUCCESS) { + printf("SendUserAuthFailure[%s]: ret=%d expected WS_SUCCESS\n", + cases[i].label, ret); + result = -602 - i; + break; + } + + if (s_authSvcSendCount != 1) { + printf("SendUserAuthFailure[%s]: expected 1 send, got %u\n", + cases[i].label, s_authSvcSendCount); + result = -610 - i; + break; + } + + capMsgId = CaptureMsgId(s_authSvcCapture, s_authSvcCaptureSz); + if (capMsgId != MSGID_USERAUTH_FAILURE) { + printf("SendUserAuthFailure[%s]: msgId=%d expected USERAUTH_FAILURE\n", + cases[i].label, capMsgId); + result = -620 - i; + break; + } + + /* name-list length is the 4 bytes following the message id */ + if (s_authSvcCaptureSz < off + MSG_ID_SZ + LENGTH_SZ) { + printf("SendUserAuthFailure[%s]: packet too short (%u)\n", + cases[i].label, s_authSvcCaptureSz); + result = -630 - i; + break; + } + nameListSz = + ((word32)s_authSvcCapture[off + MSG_ID_SZ] << 24) | + ((word32)s_authSvcCapture[off + MSG_ID_SZ + 1] << 16) | + ((word32)s_authSvcCapture[off + MSG_ID_SZ + 2] << 8) | + ((word32)s_authSvcCapture[off + MSG_ID_SZ + 3]); + + if (cases[i].expectEmpty && nameListSz != 0) { + printf("SendUserAuthFailure[%s]: nameListSz=%u expected 0\n", + cases[i].label, nameListSz); + result = -640 - i; + break; + } + if (!cases[i].expectEmpty && nameListSz == 0) { + printf("SendUserAuthFailure[%s]: nameListSz=0 expected non-empty\n", + cases[i].label); + result = -650 - i; + break; + } + + wolfSSH_free(ssh); + ssh = NULL; + wolfSSH_CTX_free(ctx); + ctx = NULL; + } + + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); + return result; +} + + #if !defined(WOLFSSH_NO_RSA) /* 2048-bit RSA private key (PKCS#1 DER). @@ -3541,6 +3663,11 @@ int wolfSSH_UnitTest(int argc, char** argv) (unitResult == 0 ? "SUCCESS" : "FAILED")); testResult = testResult || unitResult; + unitResult = test_SendUserAuthFailure_emptyMethods(); + printf("SendUserAuthFailure_emptyMethods: %s\n", + (unitResult == 0 ? "SUCCESS" : "FAILED")); + testResult = testResult || unitResult; + unitResult = test_IdentifyAsn1Key(); printf("IdentifyAsn1Key: %s\n", (unitResult == 0 ? "SUCCESS" : "FAILED")); testResult = testResult || unitResult; diff --git a/wolfssh/internal.h b/wolfssh/internal.h index 3898076fd..58a46c172 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -1384,6 +1384,8 @@ enum WS_MessageIdLimits { WOLFSSH_API int wolfSSH_TestChannelPutData(WOLFSSH_CHANNEL*, byte*, word32); WOLFSSH_API int wolfSSH_TestDoUserAuthRequest(WOLFSSH* ssh, byte* buf, word32 len, word32* idx); + WOLFSSH_API int wolfSSH_TestSendUserAuthFailure(WOLFSSH* ssh, + byte partialSuccess); WOLFSSH_API int wolfSSH_TestHighwaterCheck(WOLFSSH* ssh, byte side); #ifndef WOLFSSH_NO_DH WOLFSSH_API int wolfSSH_TestKeyAgreeDh_client(WOLFSSH* ssh, byte hashId,