fix(auth): Fix a potential buffer overflow in iperf3 client.

This condition was only possible when configuration authentication
via the libiperf API.

While here, also fix a few other sundry issues:

* Remove arbitrary length limits on username and password.

* Improved error handling.

* Updated error messages for readability.

* Fixed minor typo in some identifiers.

Fixes #996.

5 files changed, 80 insertions, 32 deletions

diff --git a/src/iperf_api.c b/src/iperf_api.c
index 935f616..ed643b7 100644
--- a/src/iperf_api.c
+++ b/src/iperf_api.c
@@ -1294,12 +1294,6 @@ iperf_parse_arguments(struct iperf_test *test, int argc, char **argv)
         else if (iperf_getpass(&client_password, &s, stdin) < 0){
             return -1;
         } 
-
-        if (strlen(client_username) > 20 || strlen(client_password) > 20){
-            i_errno = IESETCLIENTAUTH;
-            return -1;
-        }
-
         if (test_load_pubkey_from_file(client_rsa_public_key) < 0){
             i_errno = IESETCLIENTAUTH;
             return -1;
@@ -1589,15 +1583,18 @@ int test_is_authorized(struct iperf_test *test){
     if (test->settings->authtoken){
         char *username = NULL, *password = NULL;
         time_t ts;
-        decode_auth_setting(test->debug, test->settings->authtoken, test->server_rsa_private_key, &username, &password, &ts);
+        int rc = decode_auth_setting(test->debug, test->settings->authtoken, test->server_rsa_private_key, &username, &password, &ts);
+	if (rc) {
+	    return -1;
+	}
         int ret = check_authentication(username, password, ts, test->server_authorized_users);
         if (ret == 0){
-            iperf_printf(test, report_authetication_successed, username, ts);
+            iperf_printf(test, report_authentication_succeeded, username, ts);
             free(username);
             free(password);
             return 0;
         } else {
-            iperf_printf(test, report_authetication_failed, username, ts);
+            iperf_printf(test, report_authentication_failed, username, ts);
             free(username);
             free(password);
             return -1;
@@ -1760,10 +1757,18 @@ send_parameters(struct iperf_test *test)
 	if (test->repeating_payload)
 	    cJSON_AddNumberToObject(j, "repeating_payload", test->repeating_payload);
 #if defined(HAVE_SSL)
-    if (test->settings->client_username && test->settings->client_password && test->settings->client_rsa_pubkey){
-        encode_auth_setting(test->settings->client_username, test->settings->client_password, test->settings->client_rsa_pubkey, &test->settings->authtoken);
-        cJSON_AddStringToObject(j, "authtoken", test->settings->authtoken);
-    }
+	/* Send authentication parameters */
+	if (test->settings->client_username && test->settings->client_password && test->settings->client_rsa_pubkey){
+	    int rc = encode_auth_setting(test->settings->client_username, test->settings->client_password, test->settings->client_rsa_pubkey, &test->settings->authtoken);
+
+	    if (rc) {
+		cJSON_Delete(j);
+		i_errno = IESENDPARAMS;
+		return -1;
+	    }
+	    
+	    cJSON_AddStringToObject(j, "authtoken", test->settings->authtoken);
+	}
 #endif // HAVE_SSL
 	cJSON_AddStringToObject(j, "client_version", IPERF_VERSION);
 
diff --git a/src/iperf_auth.c b/src/iperf_auth.c
index a2882f9..fde44a6 100644
--- a/src/iperf_auth.c
+++ b/src/iperf_auth.c
@@ -1,5 +1,5 @@
 /*
- * iperf, Copyright (c) 2014-2018, The Regents of the University of
+ * iperf, Copyright (c) 2014-2020, The Regents of the University of
  * California, through Lawrence Berkeley National Laboratory (subject
  * to receipt of any required approvals from the U.S. Dept. of
  * Energy).  All rights reserved.
@@ -43,6 +43,9 @@
 #include <openssl/pem.h>
 #include <openssl/sha.h>
 #include <openssl/buffer.h>
+#include <openssl/err.h>
+
+const char *auth_text_format = "user: %s\npwd:  %s\nts:   %ld";
 
 void sha256(const char *string, char outputBuffer[65])
 {
@@ -238,6 +241,11 @@ int encrypt_rsa_message(const char *plaintext, EVP_PKEY *public_key, unsigned ch
     OPENSSL_free(rsa_buffer);
     BIO_free(bioBuff);
 
+    if (encryptedtext_len < 0) {
+      /* We probably shoudln't be printing stuff like this */
+      fprintf(stderr, "%s\n", ERR_error_string(ERR_get_error(), NULL));
+    }
+
     return encryptedtext_len;  
 }
 
@@ -260,17 +268,35 @@ int decrypt_rsa_message(const unsigned char *encryptedtext, const int encryptedt
     OPENSSL_free(rsa_buffer);
     BIO_free(bioBuff);
 
+    if (plaintext_len < 0) {
+      /* We probably shoudln't be printing stuff like this */
+      fprintf(stderr, "%s\n", ERR_error_string(ERR_get_error(), NULL));
+    }
+
     return plaintext_len;
 }
 
 int encode_auth_setting(const char *username, const char *password, EVP_PKEY *public_key, char **authtoken){
     time_t t = time(NULL);
     time_t utc_seconds = mktime(localtime(&t));
-    char text[150];
-    sprintf (text, "user: %s\npwd:  %s\nts:   %ld", username, password, utc_seconds);
+
+    /*
+     * Compute a pessimistic/conservative estimate of storage required.
+     * It's OK to allocate too much storage but too little is bad.
+     */
+    const int text_len = strlen(auth_text_format) + strlen(username) + strlen(password) + 32;
+    char *text = (char *) calloc(text_len, sizeof(char));
+    if (text == NULL) {
+	return -1;
+    }
+    snprintf(text, text_len, auth_text_format, username, password, utc_seconds);
+
     unsigned char *encrypted = NULL;
     int encrypted_len;
     encrypted_len = encrypt_rsa_message(text, public_key, &encrypted);
+    if (encrypted_len < 0) {
+      return -1;
+    }
     Base64Encode(encrypted, encrypted_len, authtoken);
     OPENSSL_free(encrypted);
 
@@ -285,19 +311,36 @@ int decode_auth_setting(int enable_debug, char *authtoken, EVP_PKEY *private_key
     unsigned char *plaintext = NULL;
     int plaintext_len;
     plaintext_len = decrypt_rsa_message(encrypted_b64, encrypted_len_b64, private_key, &plaintext);
-    plaintext[plaintext_len] = '\0';
     free(encrypted_b64);
+    if (plaintext_len < 0) {
+        return -1;
+    }
+    plaintext[plaintext_len] = '\0';
+
+    char *s_username, *s_password;
+    s_username = (char *) calloc(plaintext_len, sizeof(char));
+    if (s_username == NULL) {
+	return -1;
+    }
+    s_password = (char *) calloc(plaintext_len, sizeof(char));
+    if (s_password == NULL) {
+	free(s_username);
+	return -1;
+    }
+
+    int rc = sscanf((char *) plaintext, auth_text_format, s_username, s_password, ts);
+    if (rc != 3) {
+	free(s_password);
+	free(s_username);
+	return -1;
+    }
 
-    char s_username[20], s_password[20];
-    sscanf ((char *)plaintext,"user: %s\npwd:  %s\nts:   %ld", s_username, s_password, ts);
     if (enable_debug) {
         printf("Auth Token Content:\n%s\n", plaintext);
         printf("Auth Token Credentials:\n--> %s %s\n", s_username, s_password);
     }
-    *username = (char *) calloc(21, sizeof(char));
-    *password = (char *) calloc(21, sizeof(char));
-    strncpy(*username, s_username, 20);
-    strncpy(*password, s_password, 20);
+    *username = s_username;
+    *password = s_password;
     OPENSSL_free(plaintext);
     return (0);
 }
diff --git a/src/iperf_error.c b/src/iperf_error.c
index fd3cccc..c6e5ee4 100644
--- a/src/iperf_error.c
+++ b/src/iperf_error.c
@@ -1,5 +1,5 @@
 /*
- * iperf, Copyright (c) 2014-2019, The Regents of the University of
+ * iperf, Copyright (c) 2014-2020, The Regents of the University of
  * California, through Lawrence Berkeley National Laboratory (subject
  * to receipt of any required approvals from the U.S. Dept. of
  * Energy).  All rights reserved.
@@ -134,10 +134,10 @@ iperf_strerror(int int_errno)
             snprintf(errstr, len, "bad TOS value (must be between 0 and 255 inclusive)");
             break;
         case IESETCLIENTAUTH:
-             snprintf(errstr, len, "you must specify username (max 20 chars), password (max 20 chars) and a path to a valid public rsa client to be used");
+             snprintf(errstr, len, "you must specify a username, password, and path to a valid RSA public key");
             break;
         case IESETSERVERAUTH:
-             snprintf(errstr, len, "you must specify path to a valid private rsa server to be used and a user credential file");
+             snprintf(errstr, len, "you must specify a path to a valid RSA private key and a user credential file");
             break;
 	case IEBADFORMAT:
 	    snprintf(errstr, len, "bad format specifier (valid formats are in the set [kmgtKMGT])");
diff --git a/src/iperf_locale.c b/src/iperf_locale.c
index 0d8a7ec..2dd5a5b 100644
--- a/src/iperf_locale.c
+++ b/src/iperf_locale.c
@@ -1,5 +1,5 @@
 /*--------------------------------------------------------------- 
- * iperf, Copyright (c) 2014-2018, The Regents of the University of
+ * iperf, Copyright (c) 2014-2020, The Regents of the University of
  * California, through Lawrence Berkeley National Laboratory (subject
  * to receipt of any required approvals from the U.S. Dept. of
  * Energy).  All rights reserved.
@@ -272,10 +272,10 @@ const char report_time[] =
 const char report_connecting[] =
 "Connecting to host %s, port %d\n";
 
-const char report_authetication_successed[] =
+const char report_authentication_succeeded[] =
 "Authentication successed for user '%s' ts %ld\n";
 
-const char report_authetication_failed[] =
+const char report_authentication_failed[] =
 "Authentication failed for user '%s' ts %ld\n";
 
 const char report_reverse[] =
diff --git a/src/iperf_locale.h b/src/iperf_locale.h
index ad784a6..15bdcdb 100644
--- a/src/iperf_locale.h
+++ b/src/iperf_locale.h
@@ -1,5 +1,5 @@
 /*
- * iperf, Copyright (c) 2014-2018, The Regents of the University of
+ * iperf, Copyright (c) 2014-2020, The Regents of the University of
  * California, through Lawrence Berkeley National Laboratory (subject
  * to receipt of any required approvals from the U.S. Dept. of
  * Energy).  All rights reserved.
@@ -54,8 +54,8 @@ extern const char report_reverse[] ;
 extern const char report_accepted[] ;
 extern const char report_cookie[] ;
 extern const char report_connected[] ;
-extern const char report_authetication_successed[] ;
-extern const char report_authetication_failed[] ;
+extern const char report_authentication_succeeded[] ;
+extern const char report_authentication_failed[] ;
 extern const char report_window[] ;
 extern const char report_autotune[] ;
 extern const char report_omit_done[] ;