Merge branch 'bugfix/fix_http_auth_without_qop_v5.2' into 'release/v5.2'

fix(esp_http_client): Fix http digest auth without qop (v5.2) See merge request espressif/esp-idf!28757
espressif · Feb 16, 2024 · 3549a15 · 3549a15
2 parents 4beadd4 + 302661e
commit 3549a15
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 3 deletions.
diff --git a/components/esp_http_client/esp_http_client.c b/components/esp_http_client/esp_http_client.c
@@ -603,7 +603,6 @@ static esp_err_t esp_http_client_prepare(esp_http_client_handle_t client)
             client->auth_data->uri = client->connection_info.path;
             client->auth_data->cnonce = ((uint64_t)esp_random() << 32) + esp_random();
             auth_response = http_auth_digest(client->connection_info.username, client->connection_info.password, client->auth_data);
-            client->auth_data->nc ++;
 #endif
         }
 

diff --git a/components/esp_http_client/lib/http_auth.c b/components/esp_http_client/lib/http_auth.c
@@ -111,19 +111,38 @@ char *http_auth_digest(const char *username, const char *password, esp_http_auth
             goto _digest_exit;
         }
     } else {
+        /* Although as per RFC-2617, "qop" directive is optional in order to maintain backward compatibality, it is recommended
+           to use it if the server indicated that qop is supported. This enhancement was introduced to protect against attacks
+           like chosen-plaintext attack. */
+        ESP_LOGW(TAG, "\"qop\" directive not found. This may lead to attacks like chosen-plaintext attack");
         // response=MD5(HA1:nonce:HA2)
         if (md5_printf(digest, "%s:%s:%s", ha1, auth_data->nonce, ha2) <= 0) {
             goto _digest_exit;
         }
     }
     int rc = asprintf(&auth_str, "Digest username=\"%s\", realm=\"%s\", nonce=\"%s\", uri=\"%s\", algorithm=\"MD5\", "
-             "response=\"%s\", qop=%s, nc=%08x, cnonce=%016"PRIx64,
-             username, auth_data->realm, auth_data->nonce, auth_data->uri, digest, auth_data->qop, auth_data->nc, auth_data->cnonce);
+                      "response=\"%s\"", username, auth_data->realm, auth_data->nonce, auth_data->uri, digest);
     if (rc < 0) {
         ESP_LOGE(TAG, "asprintf() returned: %d", rc);
         ret = ESP_FAIL;
         goto _digest_exit;
     }
+
+    if (auth_data->qop) {
+        rc = asprintf(&temp_auth_str, ", qop=%s, nc=%08x, cnonce=\"%016"PRIx64"\"", auth_data->qop, auth_data->nc, auth_data->cnonce);
+        if (rc < 0) {
+            ESP_LOGE(TAG, "asprintf() returned: %d", rc);
+            ret = ESP_FAIL;
+            goto _digest_exit;
+        }
+        auth_str = http_utils_append_string(&auth_str, temp_auth_str, strlen(temp_auth_str));
+        if (!auth_str) {
+            ret = ESP_FAIL;
+            goto _digest_exit;
+        }
+        free(temp_auth_str);
+        auth_data->nc ++;
+    }
     if (auth_data->opaque) {
         rc = asprintf(&temp_auth_str, "%s, opaque=\"%s\"", auth_str, auth_data->opaque);
         // Free the previous memory allocated for `auth_str`