Merge pull request wolfSSL#917 from yosuke-wolfssl/f_2471

Fix DoKexDhReply() to reject the server's pub key if no PublicKeyCheck callback is registered

Author: JacobBarthelmeh

src/internal.c

@@ -5818,8 +5818,8 @@ static int DoKexDhReply(WOLFSSH* ssh, byte* buf, word32 len, word32* idx)
             }
         }
         else {
-            WLOG(WS_LOG_DEBUG, "DKDR: no public key check callback, accepted");
-            ret = WS_SUCCESS;
+            WLOG(WS_LOG_DEBUG, "DKDR: no public key check callback, rejected");
+            ret = WS_PUBKEY_REJECTED_E;
         }
     }
 

tests/api.c

@@ -1158,6 +1158,15 @@ static int sftpUserAuth(byte authType, WS_UserAuthData* authData, void* ctx)
     return ret;
 }
 
+static int AcceptAnyServerHostKey(const byte* pubKey, word32 pubKeySz,
+        void* ctx)
+{
+    (void)pubKey;
+    (void)pubKeySz;
+    (void)ctx;
+    return 0;
+}
+
 /* performs connection to port, sets WOLFSSH_CTX and WOLFSSH on success
  * caller needs to free ctx and ssh when done
  */
@@ -1180,6 +1189,7 @@ static void sftp_client_connect(WOLFSSH_CTX** ctx, WOLFSSH** ssh, int port)
         return;
     }
 
+    wolfSSH_CTX_SetPublicKeyCheck(*ctx, AcceptAnyServerHostKey);
     wolfSSH_SetUserAuth(*ctx, sftpUserAuth);
     *ssh = wolfSSH_new(*ctx);
     if (*ssh == NULL) {
@@ -1888,6 +1898,7 @@ static void keyboard_client_connect(WOLFSSH_CTX** ctx, WOLFSSH** ssh, int port)
         return;
     }
 
+    wolfSSH_CTX_SetPublicKeyCheck(*ctx, AcceptAnyServerHostKey);
     wolfSSH_SetUserAuth(*ctx, keyboardUserAuth);
     *ssh = wolfSSH_new(*ctx);
     if (*ssh == NULL) {

tests/auth.c

@@ -549,6 +549,15 @@ static THREAD_RETURN WOLFSSH_THREAD pubkey_server_thread(void* args)
     WOLFSSL_RETURN_FROM_THREAD(0);
 }
 
+static int AcceptAnyServerHostKey(const byte* pubKey, word32 pubKeySz,
+        void* ctx)
+{
+    (void)pubKey;
+    (void)pubKeySz;
+    (void)ctx;
+    return 0;
+}
+
 /* Run one pubkey auth attempt.
  * sCtx   – server context (authorised key hash)
  * cCtx   – client context (key material to present)
@@ -578,6 +587,7 @@ static int run_pubkey_test(PubkeyServerCtx* sCtx, PubkeyClientCtx* cCtx,
 
     clientCtx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL);
     AssertNotNull(clientCtx);
+    wolfSSH_CTX_SetPublicKeyCheck(clientCtx, AcceptAnyServerHostKey);
     wolfSSH_SetUserAuth(clientCtx, clientPubkeyUserAuth);
 
     clientSsh = wolfSSH_new(clientCtx);
@@ -977,6 +987,7 @@ static int basic_client_connect(WOLFSSH_CTX** ctx, WOLFSSH** ssh, int port)
         return WS_BAD_ARGUMENT;
     }
 
+    wolfSSH_CTX_SetPublicKeyCheck(*ctx, AcceptAnyServerHostKey);
     wolfSSH_SetUserAuth(*ctx, keyboardUserAuth);
     *ssh = wolfSSH_new(*ctx);
     if (*ssh == NULL) {

tests/regress.c

@@ -749,8 +749,8 @@ static void FreeKexReplyHarness(KexReplyHarness* harness)
     }
 }
 
-static void InitKexReplyHarness(KexReplyHarness* harness,
-        const char* keyAlgo, byte mutateReply)
+static void InitKexReplyHarnessEx(KexReplyHarness* harness,
+        const char* keyAlgo, byte mutateReply, byte skipPublicKeyCheck)
 {
     byte keyBuf[2048];
     word32 keySz;
@@ -781,7 +781,9 @@ static void InitKexReplyHarness(KexReplyHarness* harness,
 
     wolfSSH_SetUserAuth(harness->clientCtx, RegressionClientUserAuth);
     wolfSSH_SetUserAuth(harness->serverCtx, RegressionServerUserAuth);
-    wolfSSH_CTX_SetPublicKeyCheck(harness->clientCtx, AcceptAnyServerHostKey);
+    if (!skipPublicKeyCheck) {
+        wolfSSH_CTX_SetPublicKeyCheck(harness->clientCtx, AcceptAnyServerHostKey);
+    }
 
     keySz = LoadFileBuffer(REGRESS_SERVER_KEY_PATH, keyBuf, sizeof(keyBuf));
     AssertTrue(keySz > 0);
@@ -802,6 +804,12 @@ static void InitKexReplyHarness(KexReplyHarness* harness,
             WS_SUCCESS);
 }
 
+static void InitKexReplyHarness(KexReplyHarness* harness,
+        const char* keyAlgo, byte mutateReply)
+{
+    InitKexReplyHarnessEx(harness, keyAlgo, mutateReply, 0);
+}
+
 static int IsHandshakeRetryable(int err)
 {
     return err == WS_WANT_READ || err == WS_WANT_WRITE ||
@@ -903,6 +911,33 @@ static void TestKexDhReplyRejectsRsaSha2_512SigNameDowngrade(void)
 }
 #endif
 
+static void AssertHandshakeRejectsWithNoPublicKeyCheck(const char* keyAlgo)
+{
+    KexReplyHarness harness;
+    KexReplyRunResult result;
+
+    InitKexReplyHarnessEx(&harness, keyAlgo, 0, 1 /* skipPublicKeyCheck */);
+    RunKexReplyHandshake(&harness, &result);
+
+    AssertFalse(result.clientSuccess);
+    AssertTrue(result.clientRet == WS_FATAL_ERROR);
+    AssertTrue(result.clientErr != WS_WANT_READ && result.clientErr != WS_WANT_WRITE);
+    AssertIntEQ(result.clientErr, WS_PUBKEY_REJECTED_E);
+    AssertFalse(harness.client->connectState >= CONNECT_KEYED);
+
+    FreeKexReplyHarness(&harness);
+}
+
+static void TestKexDhReplyRejectsNoPublicKeyCheck(void)
+{
+#ifndef WOLFSSH_NO_RSA_SHA2_256
+    AssertHandshakeRejectsWithNoPublicKeyCheck("rsa-sha2-256");
+#endif
+#ifndef WOLFSSH_NO_RSA_SHA2_512
+    AssertHandshakeRejectsWithNoPublicKeyCheck("rsa-sha2-512");
+#endif
+}
+
 #endif /* KEXDH_REPLY_REGRESS_KEX_ALGO */
 
 static void AssertChannelOpenFailResponse(const ChannelOpenHarness* harness,
@@ -1667,6 +1702,7 @@ int main(int argc, char** argv)
     #ifndef WOLFSSH_NO_RSA_SHA2_512
     TestKexDhReplyRejectsRsaSha2_512SigNameDowngrade();
     #endif
+    TestKexDhReplyRejectsNoPublicKeyCheck();
 #endif
 
 #ifdef WOLFSSH_SFTP