client: harden password and prompt handling

apps/wolfssh/common.c

@@ -652,6 +652,7 @@ int ClientUserAuth(byte authType,
         ret = WOLFSSH_USERAUTH_SUCCESS;
     }
     else if (authType == WOLFSSH_USERAUTH_PASSWORD) {
+        ClientClearPassword();
         printf("Password: ");
         fflush(stdout);
         ClientSetEcho(0);
@@ -675,12 +676,23 @@ int ClientUserAuth(byte authType,
             authData->sf.password.passwordSz =
                 (word32)strlen((const char*)userPassword);
         }
+        else {
+            ClientClearPassword();
+        }
     }
 
     return ret;
 }
 
 
+/* This app does not link examples/client/common.c, so it has its own copy.
+ * It answers no keyboard-interactive, so only the password needs wiping. */
+void ClientClearPassword(void)
+{
+    wc_ForceZero(userPassword, sizeof(userPassword));
+}
+
+
 /* type = 2 : shell / execute command settings
  * type = 0 : password
  * type = 1 : restore default
@@ -908,7 +920,7 @@ void ClientFreeBuffers(void)
 
     wc_ForceZero(userPrivateKeyBuf, sizeof(userPrivateKeyBuf));
     userPrivateKeySz = 0;
-    wc_ForceZero(userPassword, sizeof(userPassword));
+    ClientClearPassword();
 }
 
 

apps/wolfssh/common.h

@@ -31,6 +31,9 @@ WOLFSSH_LOCAL int ClientUserAuth(byte authType, WS_UserAuthData* authData,
 WOLFSSH_LOCAL int ClientPublicKeyCheck(const byte* pubKey, word32 pubKeySz,
         void* ctx);
 WOLFSSH_LOCAL void ClientIPOverride(int flag);
+/* Wipes the password ClientUserAuth() last read. The library sends it from
+ * that buffer after the callback returns, so call this once connect is done. */
+WOLFSSH_LOCAL void ClientClearPassword(void);
 WOLFSSH_LOCAL void ClientFreeBuffers(void);
 WOLFSSH_LOCAL int ClientParseDestination(const char* in, char** user,
         char** hostname, word16* port);

apps/wolfssh/wolfssh.c

@@ -1273,6 +1273,9 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args)
 #endif
 
     MODES_STORE();
+    /* No stdio buffer keeps a copy of a typed password. */
+    if (setvbuf(stdin, NULL, _IONBF, 0) != 0)
+        fprintf(stderr, "Couldn't unbuffer stdin.\n");
 
     ((func_args*)args)->return_code = 0;
 
@@ -1380,6 +1383,7 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args)
 #endif
 
     ret = NonBlockSSH_connect(ssh);
+    ClientClearPassword();
     if (ret != WS_SUCCESS)
         err_sys("Couldn't connect SSH stream.");
 

examples/client/client.c

@@ -1013,6 +1013,10 @@ THREAD_RETURN WOLFSSH_THREAD client_test(void* args)
     }
     myoptind = 0;      /* reset for test cases */
 
+#ifdef WOLFSSH_TERM
+    ClientUnbufferStdin();
+#endif
+
     if (username == NULL)
         err_sys("client requires a username parameter.");
 
@@ -1225,6 +1229,7 @@ THREAD_RETURN WOLFSSH_THREAD client_test(void* args)
         ret = wolfSSH_connect(ssh);
     else
         ret = NonBlockSSH_connect(ssh);
+    ClientClearPassword();
     if (ret != WS_SUCCESS) {
         ClientFreeBuffers(pubKeyName, privKeyName, NULL);
         wolfSSH_free(ssh);

examples/client/common.c

@@ -444,6 +444,23 @@ int ClientPublicKeyCheck(const byte* pubKey, word32 pubKeySz, void* ctx)
 }
 
 
+#if defined(WOLFSSH_TERM) && defined(WOLFSSH_KEYBOARD_INTERACTIVE)
+/* Prints a server-supplied string with a '.' in place of each byte that is
+ * a control character other than tab or newline, DEL, or 0x80 and up. */
+static void ClientPrintSafe(const byte* str)
+{
+    byte c;
+
+    for (; *str != 0; str++) {
+        c = *str;
+        if ((c < 0x20 && c != '\t' && c != '\n') || c >= 0x7F)
+            c = '.';
+        putchar(c);
+    }
+}
+#endif
+
+
 int ClientUserAuth(byte authType,
                       WS_UserAuthData* authData,
                       void* ctx)
@@ -510,6 +527,7 @@ int ClientUserAuth(byte authType,
         ret = WOLFSSH_USERAUTH_SUCCESS;
     }
     else if (authType == WOLFSSH_USERAUTH_PASSWORD) {
+        ClientClearPassword();
         if (defaultPassword != NULL) {
             passwordSz = (word32)strlen(defaultPassword);
             if (passwordSz > (word32)sizeof(userPassword))
@@ -544,18 +562,23 @@ int ClientUserAuth(byte authType,
             authData->sf.password.password = userPassword;
             authData->sf.password.passwordSz = passwordSz;
         }
+        else {
+            ClientClearPassword();
+        }
     }
 #if defined(WOLFSSH_TERM) && defined(WOLFSSH_KEYBOARD_INTERACTIVE)
     else if (authType == WOLFSSH_USERAUTH_KEYBOARD) {
         word32 promptCount = authData->sf.keyboard.promptCount;
 
         if (authData->sf.keyboard.promptName &&
             authData->sf.keyboard.promptName[0] != '\0') {
-            printf("%s\n", authData->sf.keyboard.promptName);
+            ClientPrintSafe(authData->sf.keyboard.promptName);
+            printf("\n");
         }
         if (authData->sf.keyboard.promptInstruction &&
             authData->sf.keyboard.promptInstruction[0] != '\0') {
-            printf("%s\n", authData->sf.keyboard.promptInstruction);
+            ClientPrintSafe(authData->sf.keyboard.promptInstruction);
+            printf("\n");
         }
 
         /* The server may send another INFO_REQUEST at any time. */
@@ -589,14 +612,15 @@ int ClientUserAuth(byte authType,
          * but test the one this function actually returns. */
         for (entry = 0; ret == WOLFSSH_USERAUTH_SUCCESS && entry < promptCount;
                 entry++) {
-            printf("%s", authData->sf.keyboard.prompts[entry]);
+            ClientPrintSafe(authData->sf.keyboard.prompts[entry]);
             if (!authData->sf.keyboard.promptEcho[entry]) {
                 ClientSetEcho(0);
             }
             if (WFGETS((char*)userPassword, sizeof(userPassword), stdin)
                     == NULL) {
                 fprintf(stderr, "Getting response failed.\n");
                 ClientSetEcho(1);
+                wc_ForceZero(userPassword, sizeof(userPassword));
                 ret = WOLFSSH_USERAUTH_FAILURE;
                 break;
             }
@@ -613,6 +637,7 @@ int ClientUserAuth(byte authType,
             WFFLUSH(stdout);
             keyboardResponses[entry] =
                 (byte*) WSTRDUP((char*)userPassword, NULL, 0);
+            wc_ForceZero(userPassword, sizeof(userPassword));
             if (keyboardResponses[entry] == NULL) {
                 ret = WS_MEMORY_E;
                 break;
@@ -633,7 +658,32 @@ int ClientUserAuth(byte authType,
 }
 
 
+/* The wolfssh app has its own copy in apps/wolfssh/common.c, with no
+ * keyboard-interactive responses to free. The keyboard loop above wipes
+ * userPassword directly: this would free the responses authData still
+ * points at. */
+void ClientClearPassword(void)
+{
+#ifdef WOLFSSH_KEYBOARD_INTERACTIVE
+    ClientFreeKeyboardResponses();
+#endif
+    wc_ForceZero(userPassword, sizeof(userPassword));
+}
+
+
 #ifdef WOLFSSH_TERM
+void ClientUnbufferStdin(void)
+{
+    static int done = 0;
+
+    if (!done) {
+        done = 1;
+        if (setvbuf(stdin, NULL, _IONBF, 0) != 0)
+            fprintf(stderr, "Couldn't unbuffer stdin.\n");
+    }
+}
+
+
 /* type = 2 : shell / execute command settings
  * type = 0 : password
  * type = 1 : restore default
@@ -1245,12 +1295,9 @@ void ClientFreeBuffers(const char* pubKeyName, const char* privKeyName,
         userPrivateKeyTypeSz = 0;
     }
 
-#ifdef WOLFSSH_KEYBOARD_INTERACTIVE
-    ClientFreeKeyboardResponses();
-#endif
+    ClientClearPassword();
     wc_ForceZero(userPrivateKeyBuf, sizeof(userPrivateKeyBuf));
     userPrivateKeySz = 0;
-    wc_ForceZero(userPassword, sizeof(userPassword));
     pubKeyLoaded = 0;
 }
 

examples/client/common.h

@@ -32,10 +32,18 @@ int ClientSetPrivateKey(const char* privKeyName, int userEcc,
         void* heap, const char* tpmKeyAuth);
 int ClientUseCert(const char* certName, void* heap);
 int ClientSetEcho(int type);
+/* Makes stdin unbuffered, so no stdio buffer keeps a copy of a typed
+ * password or response. Call before any other stdin I/O; only the first
+ * call in a process acts. */
+void ClientUnbufferStdin(void);
 int ClientUserAuth(byte authType,
         WS_UserAuthData* authData, void* ctx);
 int ClientPublicKeyCheck(const byte* pubKey, word32 pubKeySz, void* ctx);
 void ClientIPOverride(int flag);
+/* Wipes the password and keyboard-interactive responses ClientUserAuth()
+ * last read. The library sends them after the callback returns, so call
+ * this once connect is done. */
+void ClientClearPassword(void);
 void ClientFreeBuffers(const char* pubKeyName, const char* privKeyName,
         void* heap);
 #ifdef WOLFSSH_TPM

examples/scpclient/scpclient.c

@@ -196,6 +196,10 @@ THREAD_RETURN WOLFSSH_THREAD scp_client(void* args)
 
     myoptind = 0;      /* reset for test cases */
 
+#ifdef WOLFSSH_TERM
+    ClientUnbufferStdin();
+#endif
+
     if (username == NULL)
         err_sys("client requires a username parameter.");
 

examples/sftpclient/sftpclient.c

@@ -1722,6 +1722,10 @@ THREAD_RETURN WOLFSSH_THREAD sftpclient_test(void* args)
     }
     myoptind = 0;      /* reset for test cases */
 
+#ifdef WOLFSSH_TERM
+    ClientUnbufferStdin();
+#endif
+
     if (username == NULL)
         err_sys("client requires a username parameter.");
 
@@ -1896,6 +1900,7 @@ THREAD_RETURN WOLFSSH_THREAD sftpclient_test(void* args)
         ret = wolfSSH_SFTP_connect(ssh);
     else
         ret = NonBlockSSH_connect();
+    ClientClearPassword();
     if (ret != WS_SUCCESS) {
         /* the return is a generic failure code; the real cause is kept in
          * the session error */

scripts/sshclient.test

@@ -136,10 +136,9 @@ wait_for_output() {
 
 # Run the client with its stdin coming from a fifo.
 #
-# The client answers its prompts with stdio, which buffers everything that
-# is ready to be read, so anything written along with a prompt's answer is
-# swallowed with it and never reaches the session. Writing each piece only
-# once the client has asked for it keeps them in separate reads. The
+# The client reads its prompt answers from an unbuffered stdin, so the
+# session data can follow the password at once and is left in the fifo for
+# the session. Each answer is still written only once its prompt is up. The
 # echoserver only ends the session when it receives a 0x03, so every
 # session has to send one or both ends wait for the other forever.
 #
@@ -161,11 +160,6 @@ run_client() {
 
         wait_for_output "Password:" || exit 1
         printf 'upthehill\n'
-
-        # Let the client consume the password before sending the session
-        # data. A single read that catches both loses the data.
-        sleep 2
-
         printf 'hello\003'
     ) > "$input_file" 2>/dev/null &
     input_pid=$!