wolfsshd: mark AuthorizedKeysFile as explicitly set in public setter

Author: yosuke-wolfssl

apps/wolfsshd/configuration.c

@@ -1064,7 +1064,6 @@ static int HandleConfigOption(WOLFSSHD_CONFIG** conf, int opt,
 
     switch (opt) {
         case OPT_AUTH_KEYS_FILE:
-            (*conf)->authKeysFileSet = 1;
             ret = wolfSSHD_ConfigSetAuthKeysFile(*conf, value);
             break;
         case OPT_PRIV_SEP:
@@ -1381,21 +1380,29 @@ int wolfSSHD_ConfigGetAuthKeysFileSet(const WOLFSSHD_CONFIG* conf)
 int wolfSSHD_ConfigSetAuthKeysFile(WOLFSSHD_CONFIG* conf, const char* file)
 {
     int ret = WS_SUCCESS;
+    char* newFile = NULL;
 
     if (conf == NULL) {
         ret = WS_BAD_ARGUMENT;
     }
 
+    /* allocate the replacement string first so a failure leaves the existing
+     * authKeysFile and authKeysFileSet untouched rather than half updated */
+    if (ret == WS_SUCCESS && file != NULL) {
+        ret = CreateString(&newFile, file, (int)WSTRLEN(file), conf->heap);
+    }
+
     if (ret == WS_SUCCESS) {
         if (conf->authKeysFile != NULL) {
             FreeString(&conf->authKeysFile, conf->heap);
             conf->authKeysFile = NULL;
         }
 
-        if (file != NULL) {
-            ret = CreateString(&conf->authKeysFile, file,
-                                        (int)WSTRLEN(file), conf->heap);
-        }
+        /* swap in the new file and keep authKeysFileSet consistent with it:
+         * set when a file is explicitly configured so certificate public-key
+         * logins are still checked against it, cleared when removed */
+        conf->authKeysFile = newFile;
+        conf->authKeysFileSet = (file != NULL) ? 1 : 0;
     }
 
     return ret;

apps/wolfsshd/test/test_configuration.c

@@ -305,7 +305,8 @@ static int test_ConfigCopy(void)
         ret = wolfSSHD_ConfigSetHostCertFile(head, "/etc/ssh/host_cert.pub");
     if (ret == WS_SUCCESS)
         ret = wolfSSHD_ConfigSetUserCAKeysFile(head, "/etc/ssh/ca.pub");
-    /* AuthorizedKeysFile must go through PCL so authKeysFileSet flag is set */
+    /* AuthorizedKeysFile via PCL to also exercise the config-parse path; the
+     * authKeysFileSet flag is set either way and must survive the copy */
     if (ret == WS_SUCCESS) ret = PCL("AuthorizedKeysFile .ssh/authorized_keys");
 
     /* scalar fields */
@@ -651,6 +652,62 @@ static int test_IncludeRecursionBound(void)
     return ret;
 }
 
+/* The public wolfSSHD_ConfigSetAuthKeysFile setter must mark the authorized
+ * keys file as explicitly configured, otherwise certificate public-key logins
+ * skip the authorized-keys check and rely on CA validation alone. */
+static int test_ConfigSetAuthKeysFile(void)
+{
+    int ret = WS_SUCCESS;
+    WOLFSSHD_CONFIG* conf;
+
+    conf = wolfSSHD_ConfigNew(NULL);
+    if (conf == NULL)
+        ret = WS_MEMORY_E;
+
+    /* fresh config has no explicit authorized keys file */
+    if (ret == WS_SUCCESS) {
+        if (wolfSSHD_ConfigGetAuthKeysFileSet(conf) != 0)
+            ret = WS_FATAL_ERROR;
+    }
+
+    /* configuring a file through the public setter must set the flag */
+    if (ret == WS_SUCCESS)
+        ret = wolfSSHD_ConfigSetAuthKeysFile(conf, ".ssh/authorized_keys");
+    if (ret == WS_SUCCESS) {
+        if (wolfSSHD_ConfigGetAuthKeysFileSet(conf) == 0)
+            ret = WS_FATAL_ERROR;
+    }
+
+    /* a failed update must leave the existing configuration intact: an
+     * all-whitespace file makes CreateString fail, and both the previously
+     * configured file and the flag must be untouched, not half cleared */
+    if (ret == WS_SUCCESS) {
+        if (wolfSSHD_ConfigSetAuthKeysFile(conf, " ") == WS_SUCCESS)
+            ret = WS_FATAL_ERROR; /* the bad value should have been rejected */
+    }
+    if (ret == WS_SUCCESS) {
+        if (wolfSSHD_ConfigGetAuthKeysFile(conf) == NULL ||
+            XSTRCMP(wolfSSHD_ConfigGetAuthKeysFile(conf),
+                    ".ssh/authorized_keys") != 0)
+            ret = WS_FATAL_ERROR;
+    }
+    if (ret == WS_SUCCESS) {
+        if (wolfSSHD_ConfigGetAuthKeysFileSet(conf) == 0)
+            ret = WS_FATAL_ERROR;
+    }
+
+    /* removing the file must clear the flag again */
+    if (ret == WS_SUCCESS)
+        ret = wolfSSHD_ConfigSetAuthKeysFile(conf, NULL);
+    if (ret == WS_SUCCESS) {
+        if (wolfSSHD_ConfigGetAuthKeysFileSet(conf) != 0)
+            ret = WS_FATAL_ERROR;
+    }
+
+    wolfSSHD_ConfigFree(conf);
+    return ret;
+}
+
 /* Verifies ConfigFree releases all string fields - most useful under ASan. */
 static int test_ConfigFree(void)
 {
@@ -1013,6 +1070,7 @@ const TEST_CASE testCases[] = {
     TEST_DECL(test_MatchUnsupportedSelector),
     TEST_DECL(test_CAKeysFileDiffers),
     TEST_DECL(test_IncludeRecursionBound),
+    TEST_DECL(test_ConfigSetAuthKeysFile),
     TEST_DECL(test_ConfigFree),
 #ifdef WOLFSSL_BASE64_ENCODE
     TEST_DECL(test_CheckAuthKeysLine),