expand test cases, add negative tests, cap string log print outs, add documentation

Author: JacobBarthelmeh

.github/workflows/windows-cert-store-test.yml

@@ -538,6 +538,60 @@ fred-cert.der would be:
 
     $ ./examples/client/client -u fred -J ./keys/fred-cert.der -i ./keys/fred-key.der
 
+WINDOWS CERTIFICATE STORE
+=========================
+
+On Windows, host and user keys can come from the MS Certificate Store instead
+of files. Requires certificate support (`--enable-certs` or `WOLFSSH_CERTS`);
+enable it with the `--enable-windows-cert-store` build option (mingw hosts
+only) or by defining `WOLFSSH_WINDOWS_CERT_STORE`. The build links against
+`crypt32` and `ncrypt`. For the Visual Studio build see the comment block in
+`ide/winvs/user_settings.h`, including the `WOLFSSH_NO_SHA1_SOFT_DISABLE` and
+`WC_SIG_MIN_HASH_TYPE` caveats an RSA store certificate needs (RFC 6187's only
+RSA algorithm, `x509v3-ssh-rsa`, signs with SHA-1); ECDSA store keys need
+neither.
+
+The echoserver and the SFTP client take a `-W store:subject[:flags]` option
+naming the store, the certificate's subject CN, and optionally the store
+location. Accepted location names are CURRENT_USER (the default),
+LOCAL_MACHINE, USERS, CURRENT_SERVICE, SERVICES, CURRENT_USER_GROUP_POLICY,
+LOCAL_MACHINE_GROUP_POLICY and LOCAL_MACHINE_ENTERPRISE, each also accepted
+with a `CERT_SYSTEM_STORE_` prefix or as a number. `-W` supplies both the
+certificate and its private key, so it cannot be combined with `-i`, `-j`, or
+`-J`, and it skips the wolfssh home directory search so file arguments resolve
+against the current directory.
+
+    $ ./examples/echoserver/echoserver -W "My:wolfSSH-Server:LOCAL_MACHINE" -a ./keys/ca-cert-ecc.pem
+
+    $ ./examples/sftpclient/wolfsftp -u testuser -W "My:testuser:CURRENT_USER" -A ./keys/ca-cert-ecc.der -X
+
+wolfSSHd gains these configuration directives, all global only (they are
+rejected inside a `Match` block):
+
+* `HostKeyStore <store>`, `HostKeyStoreSubject <CN>`, and
+  `HostKeyStoreFlags <location>` select the host key from a certificate
+  store. All three must be set together, and they conflict with `HostKey`,
+  `HostCertificate`, and the `-h` command line option.
+* `wolfSSH_TrustedUserCAStore yes|no` loads the client-certificate trust
+  anchors from a Windows store named by `wolfSSH_WinUserPvPara <store>` at
+  the mandatory location `wolfSSH_WinUserDwFlags <location>`
+  (`wolfSSH_WinUserStores` optionally names the provider; only
+  `CERT_STORE_PROV_SYSTEM` is supported). Only certificates with
+  basicConstraints CA:TRUE are loaded, and the OS-managed public trust
+  stores (`Root`, `AuthRoot`, `CA`, ...) are refused: every CA in the named
+  store becomes an SSH login authority, so point it at a store created for
+  this purpose that holds nothing but your own CA.
+* `wolfSSH_TrustedSystemCAKeys yes|no` imports the OS trust store via
+  wolfSSL (`WOLFSSL_SYS_CA_CERTS`) as the client-certificate trust anchors.
+  On CN-binding builds (no FPKI) this additionally requires a per-user
+  `AuthorizedKeysFile` on every config node, so a subject CN match alone can
+  never log in.
+
+Without FPKI, a client certificate is bound to the requested account by a
+case-insensitive subject CN match only; keep the trusted CA set narrow. Note
+also that the config parser requires whitespace between an option name and
+its value; the OpenSSH `Keyword=value` form is rejected.
+
 TPM PUBLIC KEY AUTHENTICATION
 =============================
 

README.md

@@ -2584,9 +2584,12 @@ static int RequestAuthentication(WS_UserAuthData* authData,
         usrConf = wolfSSHD_AuthGetUserConf(authCtx, usr, NULL, NULL, NULL, NULL,
                                            NULL);
         if (usrConf == NULL) {
+            /* bound the untrusted name so it cannot consume the whole
+             * fixed-width log message (control bytes are scrubbed by
+             * wolfSSH_Log itself) */
             wolfSSH_Log(WS_LOG_ERROR,
-                    "[SSHD] Failure to get user configuration for auth (user=%s)",
-                    usr);
+                    "[SSHD] Failure to get user configuration for auth "
+                    "(user=%.32s)", usr);
             ret = WOLFSSH_USERAUTH_FAILURE;
             needFakeCheck = 1;
         }
@@ -2683,7 +2686,11 @@ static int RequestAuthentication(WS_UserAuthData* authData,
          * (checked below) is a stronger binding than a CN name match. A
          * shared AuthorizedKeysFile (an absolute pattern with no %u or %h)
          * resolves to one file for every account and binds the certificate to
-         * nothing, so the name match still has to run. */
+         * nothing, so the name match still has to run. Note the strength of
+         * the file binding rests on the file's integrity: on Windows,
+         * wolfSSHD_OpenSecureFile() performs no ownership or ACL checks, so
+         * the guarantee is only as good as the NTFS ACLs on the profile
+         * directory. */
     #ifdef WOLFSSL_FPKI
         if (authData->sf.publicKey.isCert) {
     #else
@@ -2740,11 +2747,20 @@ static int RequestAuthentication(WS_UserAuthData* authData,
                         current = current->next;
                     }
 
-                    /* a UPN matched but no realm policy is set; note per auth
-                     * attempt so the opt-in gap is visible, no shared state */
+                    /* a UPN matched but no realm policy is set; this states a
+                     * fixed configuration property, so emit it once per
+                     * process rather than on every auth attempt (an attempt
+                     * is peer-triggered and the log callback writes WARN
+                     * unconditionally) */
                     if (upnRealmUnchecked) {
-                        wolfSSH_Log(WS_LOG_WARN, "[SSHD] AuthorizedUPNDomains "
-                            "not set; certificate UPN domain is not checked");
+                        static int upnRealmWarned = 0;
+
+                        if (!upnRealmWarned) {
+                            upnRealmWarned = 1;
+                            wolfSSH_Log(WS_LOG_WARN,
+                                "[SSHD] AuthorizedUPNDomains not set; "
+                                "certificate UPN domain is not checked");
+                        }
                     }
                 #else
                     /* Without FPKI compare subject CN with user name. Only
@@ -2760,18 +2776,30 @@ static int RequestAuthentication(WS_UserAuthData* authData,
                             WSTRNCASECMP(usr, dCert->subjectCN,
                                 (size_t)dCert->subjectCNLen) == 0) {
                         usrMatch = 1;
-                        /* note per auth attempt so the weaker binding is
-                         * visible without -d, no shared state */
-                        wolfSSH_Log(WS_LOG_WARN, "[SSHD] certificate bound to "
-                            "user by subject CN only; no issuer constraint is "
-                            "applied, keep the trusted user CA set narrow");
+                        /* states a fixed build/configuration property, so
+                         * emit once per process; a per-attempt WARN would
+                         * let a peer grow the log with every attempt now
+                         * that the log callback writes WARN without -d */
+                        {
+                            static int cnBindWarned = 0;
+
+                            if (!cnBindWarned) {
+                                cnBindWarned = 1;
+                                wolfSSH_Log(WS_LOG_WARN,
+                                    "[SSHD] certificate bound to user by "
+                                    "subject CN only; no issuer constraint");
+                                wolfSSH_Log(WS_LOG_WARN,
+                                    "[SSHD] keep the trusted user CA set "
+                                    "narrow");
+                            }
+                        }
                     }
                 #endif
 
                     if (usrMatch == 0) {
                         wolfSSH_Log(WS_LOG_ERROR, "[SSHD] incorrect user cert "
                             "sent; certificate identity does not match the "
-                            "requested user (user=%s)", usr);
+                            "requested user (user=%.32s)", usr);
                         ret = WOLFSSH_USERAUTH_INVALID_PUBLICKEY;
                     }
                 }
@@ -2800,7 +2828,8 @@ static int RequestAuthentication(WS_UserAuthData* authData,
                     wolfSSHD_ConfigGetUserCAKeysFile(usrConf))) {
                 wolfSSH_Log(WS_LOG_ERROR,
                     "[SSHD] Per-user TrustedUserCAKeys override is not enforced "
-                    "for certificate authentication; rejecting (user=%s)", usr);
+                    "for certificate authentication; rejecting (user=%.32s)",
+                    usr);
                 ret = WOLFSSH_USERAUTH_REJECTED;
             }
             else {
@@ -2838,7 +2867,7 @@ static int RequestAuthentication(WS_UserAuthData* authData,
                 wolfSSH_Log(WS_LOG_ERROR,
                     "[SSHD] Certificate authentication cannot bind the requested "
                     "user without FPKI or AuthorizedKeysFile; rejecting "
-                    "(user=%s)", usr);
+                    "(user=%.32s)", usr);
                 ret = WOLFSSH_USERAUTH_REJECTED;
             #endif
             }