wolfsshd: implement PubkeyAuthentication config directive

Author: yosuke-wolfssl

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;

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 */

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 <wolfssh/ssh.h>
 #include <wolfssh/internal.h>
@@ -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;

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,

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),

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);