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

fix(esp_http_client): Fix http digest auth without qop (v5.0) See merge request espressif/esp-idf!28759
espressif · Feb 22, 2024 · 5af8e88 · 5af8e88
2 parents 534e3ad + 484de5e
commit 5af8e88
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 4 deletions.
diff --git a/components/esp_http_client/esp_http_client.c b/components/esp_http_client/esp_http_client.c
@@ -537,7 +537,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
@@ -112,14 +112,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;
         }
     }
-    asprintf(&auth_str, "Digest username=\"%s\", realm=\"%s\", nonce=\"%s\", uri=\"%s\", algorithm=\"MD5\", "
-             "response=\"%s\", qop=%s, nc=%08x, cnonce=\"%016llx\"",
-             username, auth_data->realm, auth_data->nonce, auth_data->uri, digest, auth_data->qop, auth_data->nc, auth_data->cnonce);
+    int rc = asprintf(&auth_str, "Digest username=\"%s\", realm=\"%s\", nonce=\"%s\", uri=\"%s\", algorithm=\"MD5\", "
+                      "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) {
         asprintf(&temp_auth_str, "%s, opaque=\"%s\"", auth_str, auth_data->opaque);
         free(auth_str);