Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash with esp_http_client Digest without qop (IDFGH-11876) #12962

Closed
3 tasks done
trumpton opened this issue Jan 11, 2024 · 4 comments
Closed
3 tasks done

Crash with esp_http_client Digest without qop (IDFGH-11876) #12962

trumpton opened this issue Jan 11, 2024 · 4 comments
Assignees
Labels
Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Type: Bug bugs in IDF

Comments

@trumpton
Copy link

trumpton commented Jan 11, 2024

Answers checklist.

  • I have read the documentation ESP-IDF Programming Guide and the issue is not addressed there.
  • I have updated my IDF branch (master or release) to the latest version and checked that the issue is present there.
  • I have searched the issue tracker for a similar issue and not found a similar issue.

IDF version.

v5.1

Espressif SoC revision.

Chip is ESP32D0WDQ5 (revision 3)

Operating System used.

Linux

How did you build your project?

VS Code IDE

If you are using Windows, please specify command line type.

None

Development Kit.

ESP32 WROVER KIT 1

Power Supply used.

USB

What is the expected behavior?

When accessing a website that has a Digest challenge with no 'qop' field in the Authorization header, the expected behaviour is to continue with the login, providing the responses to the challenge and ultimately load the web page.

The excerpt below is acquiring a remote status via Google Chrome (which works), not through the ESP32 device (which does not):

GET /api/v1/status HTTP/1.1
Host: 192.168.11.7
Connection: keep-alive
Cache-Control: max-age=0
Authorization: Digest username="maker", realm="Printer API", nonce="62c64da7002b3f9f", uri="/api/v1/status", response="52f0fae12fcc6862505ff5770adcadc5"
Upgrade-Insecure-Requests: 1
User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/105.0.0.0 Safari/537.36
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.9
Accept-Encoding: gzip, deflate
Accept-Language: en-GB,en;q=0.9,fr-FR;q=0.8,fr;q=0.7,en-US;q=0.6

HTTP/1.1 401 Unauthorized
Content-Type: text/plain
Connection: keep-alive
Content-Length: 20
WWW-Authenticate: Digest realm="Printer API", nonce="62c64da7002b4006", stale=true

401: Unauthorized


GET /api/v1/status HTTP/1.1
Host: 192.168.11.7
Connection: keep-alive
Cache-Control: max-age=0
Authorization: Digest username="maker", realm="Printer API", nonce="62c64da7002b4006", uri="/api/v1/status", response="6c7c8d6f1706c6deb81c05cf5b39f430"
Upgrade-Insecure-Requests: 1
User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/105.0.0.0 Safari/537.36
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.9
Accept-Encoding: gzip, deflate
Accept-Language: en-GB,en;q=0.9,fr-FR;q=0.8,fr;q=0.7,en-US;q=0.6

HTTP/1.1 200 OK
Content-Type: application/json
Connection: keep-alive
Transfer-Encoding: chunked

{"storage":{"path":"/usb/","name":"usb","read_only":false},"printer":{"state":"FINISHED","temp_bed":15.7,"target_bed":0.0,"temp_nozzle":16.0,"target_nozzle":0.0,"axis_z":70.9,"axis_x":241.0,"axis_y":170.0,"flow":100,"speed":100,"fan_hotend":0,"fan_print":0}}

What is the actual behavior?

The application crashes with a Guru Meditation Error: Core 1 panic'ed (LoadProhibited). Exception was unhandled when the following http transaction is performed:

GET /api/v1/status HTTP/1.1
User-Agent: ESP32 HTTP Client/1.0
Host: 192.168.11.7
Accept-Encoding: identity
Content-Length: 0

HTTP/1.1 401 Unauthorized
Content-Type: text/plain
Connection: keep-alive
Content-Length: 20
WWW-Authenticate: Digest realm="Printer API", nonce="62c64da7002b420a", stale=false

401: Unauthorized

This is caused by the following line in http_auth.c:

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);

Note: auth_data->qop is null, which is what actually causes the crash.

qop is specified in RFC7616 in a section which identifies a series of parameters which MAY be supplied:

qop - Indicates what "quality of protection" the client has applied to the message. Its value MUST be one of the alternatives the server indicated it supports in the WWW-Authenticate header field. These values affect the computation of the response. Note that this is a single token, not a quoted list of alternatives as in WWW-Authenticate.

This effectively says that if the parameter is supplied, it must be in a specific format - it does not mandate that the parameter is there.

Steps to reproduce.

Connect to a Digest authenticated web server that does not provide 'qop' fields in the Authorization header, and attempt to retrieve the web page - I used a Prusa MK4 printer status query.

void fetchStatus() {

  static esp_http_client_config_t config = {
    .host = "192.168.11.7",
    .port = 80,
    .username = "maker",
    .password = "mypassword",
    .auth_type = HTTP_AUTH_TYPE_NONE,
    .path = "/api/v1/status", 
    .method = HTTP_METHOD_GET,
  };

  esp_http_client_handle_t client = esp_http_client_init(&config);

  if (!client) {
    return ;
  }

  // Reject receiving gzip/compressed responses
  esp_http_client_set_header(client, "Accept-Encoding", "identity"); 

  esp_err_t err ;
  err = esp_http_client_perform(client);

    //////////////////////////////////
   // Crash happens here!!!  //
  /////////////////////////////////

  int status = esp_http_client_get_status_code(client) ;

  if (err == ESP_OK) {
      int len = esp_http_client_get_content_length(client) ;
      log_i("HTTP GET Status = %d, content_length = %d", status, len) ;
  }

  esp_http_client_close(client) ;

}

Debug Logs.

No response

More Information.

Irrespective of whether the input request is correctly formatted or not, the library should not crash - i.e. all %s submissions to printf should not pass null strings.

If a request is incorrectly formatted, the library should handle in a sensible way, either by reporting that processing has been rejected, or make an assumption with regards to the missing parameter.

@trumpton trumpton added the Type: Bug bugs in IDF label Jan 11, 2024
@espressif-bot espressif-bot added the Status: Opened Issue is new label Jan 11, 2024
@github-actions github-actions bot changed the title Crash with esp_http_client Digest without qop Crash with esp_http_client Digest without qop (IDFGH-11876) Jan 11, 2024
@trumpton
Copy link
Author

trumpton commented Jan 12, 2024

Further investigation shows that the nc must also be included in this issue.

nonce-count
This MUST be specified if a qop directive is sent (see above), and
MUST NOT be specified if the server did not send a qop directive in
the WWW-Authenticate header field. The nc-value is the hexadecimal
count of the number of requests (including the current request)
that the client has sent with the nonce value in this request. For
example, in the first request sent in response to a given nonce
value, the client sends "nc=00000001". The purpose of this
directive is to allow the server to detect request replays by
maintaining its own copy of this count - if the same nc-value is
seen twice, then the request is a replay. See the description
below of the construction of the request-digest value.

Currently, the nc is included autmatically in the response in http_auth.c irrespective of whether it is required. The upshot of this is that if the qop is removed from the response when it is not required (i.e. null), the inclusion of the nc is in violation of the standard, and for me resulted in a 400 bad request response from the server.

The following http_auth.c code fixes the problems:

    asprintf(&auth_str, "Digest username=\"%s\", realm=\"%s\", nonce=\"%s\", uri=\"%s\", "
                        "algorithm=\"MD5\", response=\"%s\", cnonce=\"%016llx\"",
             username, auth_data->realm, auth_data->nonce, auth_data->uri, digest, auth_data->cnonce);

    if (auth_data->qop) {
        // add qop an nc.  note: nc MUST NOT be specified if the server did not send a qop directive
        asprintf(&temp_auth_str, "%s, qop=%s, nc=%08x", auth_str, auth_data->qop, auth_data->nc);
        free(auth_str);
        auth_str = temp_auth_str;
        auth_data->nc++ ;
    }

Note that I have included the nc++ here, and it should be removed from the esp_http_client.c file. It is only necessary to increment nc when it is used, and if qop is not provided, nc is not required.

@hmalpani
Copy link
Collaborator

Hello @trumpton
Can you try the following patch and let me know if it fixes the issue?
IDFGH_11867.txt

Thanks!

@espressif-bot espressif-bot added Status: In Progress Work is in progress and removed Status: Opened Issue is new labels Jan 15, 2024
@hmalpani
Copy link
Collaborator

Hello @trumpton
Did the patch I shared above helped to solve the issue?

@trumpton
Copy link
Author

trumpton commented Jan 20, 2024

I can confirm that the application no longer crashes, and when capturing the output with Wireshark, I now see a correctly formatted header.

GET /api/v1/status HTTP/1.1
User-Agent: ESP32 HTTP Client/1.0
Host: 192.168.11.7
Accept-Encoding: identity
Connection: keep-alive
Content-Length: 0
HTTP/1.1 401 Unauthorized
Content-Type: text/plain
Connection: keep-alive
Content-Length: 20
WWW-Authenticate: Digest realm="Printer API", nonce="62c64da700360138", stale=false

401: Unauthorized

GET /api/v1/status HTTP/1.1
User-Agent: ESP32 HTTP Client/1.0
Host: 192.168.11.7
Accept-Encoding: identity
Connection: keep-alive
Content-Length: 0
Authorization: Digest username="maker", realm="Printer API", nonce="62c64da700360138", uri="/api/v1/status", algorithm=MD5, response="181d7a4ebc44518e36d6742dcefb48d6"

HTTP/1.1 408 Request Timeout

I am seeing a timeout from the other end, but I suspect that this is caused by something else - I could only get the patch to work on the master branch, and I previously did the test originally on v4.3 through platformio, and on v5.1 direct with esp-idf.

I'll manually apply the patch to v4.3 and confirm that that works fine ...

@espressif-bot espressif-bot added Status: Done Issue is done internally Resolution: NA Issue resolution is unavailable and removed Status: In Progress Work is in progress labels Jan 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Type: Bug bugs in IDF
Projects
None yet
Development

No branches or pull requests

3 participants