Skip to content

Commit

Permalink
Fix Server Name Indication extension configuration
Browse files Browse the repository at this point in the history
The [Server Name Indication][wiki-sni] (SNI) [TLS
extension][rfc-5246-extensions] has become a de-facto client-side
requirement for TLS connections; most servers will abort connections
lacking the SNI extension.

Whilst Mbed TLS provides client-side support for SNI (via the
[MBEDTLS_SSL_SERVER_NAME_INDICATION][mbedtls-sni] option), the [ALTCP
compatible Mbed TLS port][lwip-altcp-tls] supplied with lwIP does not
provide an interface to correctly configure SNI. This is a [known
missing feature][gh-lwip-pr] of lwIP's ALTCP interface.

This commit adds a workaround to correctly configure the SNI extension.
This is achieved via underlying Mbed TLS interfaces, thereby avoiding
the need to patch lwIP.

Fixes #1

[wiki-sni]: https://en.wikipedia.org/wiki/Server_Name_Indication
[rfc-5246-extensions]: https://datatracker.ietf.org/doc/html/rfc5246#section-7.4.1.4
[mbedtls-sni]: https://github.com/Mbed-TLS/mbedtls/blob/mbedtls-2.28/include/mbedtls/config.h#L2134
[lwip-altcp-tls]: https://github.com/lwip-tcpip/lwip/tree/master/src/apps/altcp_tls
[gh-lwip-pr]: lwip-tcpip/lwip@c53c9d0
  • Loading branch information
marceloalcocer committed Dec 1, 2024
1 parent fee46f1 commit 922ef25
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 2 deletions.
13 changes: 12 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,24 @@ target_include_directories(
#
PRIVATE ${CMAKE_CURRENT_LIST_DIR}

# Mbed TLS port includes
#
# Internal headers for the Mbed TLS port provided with lwIP. They are not
# typically in the pre-processor search path, but are required when using
# underlying Mbed TLS interfaces not exposed by the Mbed TLS port (more
# specifically, casting for an `mbedtls_ssl_set_hostname` call for Server
# Name Indication TLS extension configuration).
#
# See the `connect_to_host` function in `picohttps.c` for details.
#
${PICO_LWIP_PATH}/src/apps/altcp_tls/

)

# Configure linker
#
# To link (statically) to additional libraries files (i.e. external to Pico SDK)
#
#
# https://cmake.org/cmake/help/latest/command/target_link_libraries.html
#
target_link_libraries(
Expand Down
45 changes: 44 additions & 1 deletion picohttps.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,16 @@
// lwIP
#include "lwip/dns.h" // Hostname resolution
#include "lwip/altcp_tls.h" // TCP + TLS (+ HTTP == HTTPS)
#include "altcp_tls_mbedtls_structs.h"
#include "lwip/prot/iana.h" // HTTPS port number

// Mbed TLS
#include "mbedtls/ssl.h" // Server Name Indication TLS extension

// Pico HTTPS request example
#include "picohttps.h" // Options, macros, forward declarations



/* Main ***********************************************************************/

void main(void){
Expand Down Expand Up @@ -226,6 +229,46 @@ bool connect_to_host(ip_addr_t* ipaddr, struct altcp_pcb** pcb){
return false;
}

// Configure hostname for Server Name Indication extension
//
// Many servers nowadays require clients to support the [Server Name
// Indication[wiki-sni] (SNI) TLS extension. In this extension, the
// hostname is included in the in the ClientHello section of the TLS
// handshake.
//
// Mbed TLS provides client-side support for SNI extension
// (`MBEDTLS_SSL_SERVER_NAME_INDICATION` option), but requires the
// hostname in order to do so. Unfortunately, the Mbed TLS port supplied
// with lwIP (ALTCP TLS) does not currently provide an interface to pass
// the hostname to Mbed TLS. This is a [known issue in lwIP][gh-lwip-pr].
//
// As a workaround, the hostname can instead be set using the underlying
// Mbed TLS interface (viz. `mbedtls_ssl_set_hostname` function). This is
// somewhat inelegant as it tightly couples our application code to the
// underlying TLS library (viz. Mbed TLS). Given that the Pico SDK already
// tightly couples us to lwIP though, and that any fix is unlikely to be
// backported to the lwIP version in the Pico SDK, this doesn't feel like
// too much of a crime…
//
// [wiki-sni]: https://en.wikipedia.org/wiki/Server_Name_Indication
// [gh-lwip-pr]: https://github.com/lwip-tcpip/lwip/pull/47/commits/c53c9d02036be24a461d2998053a52991e65b78e
//
cyw43_arch_lwip_begin();
mbedtls_err_t mbedtls_err = mbedtls_ssl_set_hostname(
&(
(
(altcp_mbedtls_state_t*)((*pcb)->state)
)->ssl_context
),
PICOHTTPS_HOSTNAME
);
cyw43_arch_lwip_end();
if(mbedtls_err){
altcp_free_pcb(*pcb);
altcp_free_config(config);
return false;
}

// Configure common argument for connection callbacks
//
// N.b. callback argument must be in scope in callbacks. As callbacks may
Expand Down
6 changes: 6 additions & 0 deletions picohttps.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,12 @@
//
typedef err_t lwip_err_t;

// Mbed TLS errors
//
// typedef here to make source of error code more explicit
//
typedef int mbedtls_err_t;

// TCP connection callback argument
//
// All callbacks associated with lwIP TCP (+ TLS) connections can be passed a
Expand Down

0 comments on commit 922ef25

Please sign in to comment.