WPA Enterprise fails when server sends TLS message with multiple records (IDFGH-5702)

NorbertoNorbs commented 3 years ago

Development Kit: none
Module or chip used: ESP32-WROOM-32
IDF version: v4.3
Build System: CMake
Compiler version: xtensa-esp32-elf-gcc.exe (crosstool-NG crosstool-ng-1.22.0-80-g6c4433a5) 5.2.0
Operating System: Windows
Using an IDE: Eclipse with ESP plugin
Power Supply: USB

Problem Description

When connecting to a WPA Enterprise network that authenticates using NPS running in Windows server 2016 the association fails after the ESP receives and parses the ServerHello message, the windows NPS log shows error 262 'The supplied message is incomplete. The signature was not verified.'.

I'm using PEAP/MSCHAPv2 but as far as I can tell this problem can happen in other environments/authentication methods too, since this appears to be a issue in the TLS message exchange (I found a fix that I'll describe further below).

What I found is that when the server sends a single TLS message with multiple records the ESP only process the first one.

In my case the server is sending the ServerHello and the Certificate in the same message, the ESP parses the ServeHello but fails to read the certificate and reply with a ack (The message 'application data is null, adding one byte for ack' is shown in the console) but the server ends the connection since is already sent what it was supposed to.

I narrowed the problem to the function _tls_connectionhandshake in .\components\wpa_supplicant\src\crypto\tls_mbedtls.c:

struct wpabuf * tls_connection_handshake(void *tls_ctx,
                     struct tls_connection *conn,
                     const struct wpabuf *in_data,
                     struct wpabuf **appl_data)
{
    tls_context_t *tls = conn->tls;
    int ret = 0;

    /* data freed by sender */
    conn->tls_io_data.out_data = NULL;
    if (wpabuf_len(in_data)) {
        conn->tls_io_data.in_data = wpabuf_dup(in_data);
    }
    ret = mbedtls_ssl_handshake_step(&tls->ssl);          <- The message with ServerHello is read from input here
    if (ret < 0) {
        wpa_printf(MSG_ERROR, "%s:%d", __func__, __LINE__);
        goto end;
    }

    /* Multiple reads */
    while (conn->tls_io_data.in_data) {                <- Upon reaching here 'conn->tls_io_data.in_data' is null, but there are still records to be processed in the last message received
        ret = mbedtls_ssl_handshake_step(&tls->ssl);
        if (ret < 0)
            break;
    } 

.....

I fixed the problem by adding the condition of 'is there more records to be read?' in the while loop:

...
    /* Multiple reads */
    while (conn->tls_io_data.in_data || tls->ssl.in_msglen>tls->ssl.in_hslen) {
        ret = mbedtls_ssl_handshake_step(&tls->ssl);
        if (ret < 0)
            break;
    }

...

I don't know if this is the best solution, but it may help others while the developers check this issue (I was in a tight spot with a costumer so I spent some long hours trying to debug this, I'm no wpa supplicant specialist so may be there's more to it, but at least it's working for now).

How to reproduce

I'm using the Windows NPS as the RADIUS in a fresh installed Windows 2016 server with a self-signed certificate, my route is a simple TP-Link TL-WR940N that supports WPA Enterprise (I have the same problem with a costumer that uses a Cisco router). To validate the setup I connected to this network using Ubuntu in a notebook. The wpa_enterprise idf example was used, I set the SSID, ID, Username and password fields, 'validate server' is disabled.

Debug Logs

I'm attaching 2 debug logs as a reference: The first is the error I get using the idf as is, and the other connecting with success after applying the fix.

esp32 wpa error.txt esp32 wpa success.txt

NorbertoNorbs commented 3 years ago

I just confirmed that https://github.com/espressif/esp-idf/commit/e8360fe0756ec592cbd5f4ff4d36946a22561d8f solved the issue. Thanks.

Alvin1Zhang commented 3 years ago

@NorbertoNorbs Thanks for sharing the updates.

espressif / esp-idf