wolfSSL
[Bug]: ECHConfig decoding doesn't seem to consider ``maximum_name_length`` field
Contact Details
github is fine
Version
cloned from master last week
Description
Looks like there's an error in the decoding of ECHConfig structures, when those contain a non-zero maximum_name_length field. The relevant bit of spec here is:
struct {
uint8 config_id;
HpkeKemId kem_id;
HpkePublicKey public_key;
HpkeSymmetricCipherSuite cipher_suites<4..2^16-4>;
} HpkeKeyConfig;
struct {
HpkeKeyConfig key_config;
uint8 maximum_name_length;
opaque public_name<1..255>;
Extension extensions<0..2^16-1>;
} ECHConfigContents;
Problem is here where wolfssl assumes the public_name length is two octets when it's only one, preceeded by a one octet maximum_name_length that most people do set to zero (sensibly). Usefully however (for test purposes) tls-ech.dev publishes an ECHConfig with a non-zero maximum_name_length which triggers a invalid read error.
The fix to decode this correctly is pretty obvious. ('case it helps at all, I agree the existence of the maximum_name_length field is just silly;-).
Reproduction steps
If you use my ECH-enabled cURL build (HOWTO) then you can see the error if you run using valgrind:
$ $ LD_LIBRARY_PATH=lib/.libs:src/.libs:$HOME/code/wolfssl/lib valgrind --leak-check=full src/.libs/curl -vvv https://tls-ech.dev
The resulting output is below, with the valgrind errors at the end.
Relevant log output
$ LD_LIBRARY_PATH=lib/.libs:src/.libs:$HOME/code/wolfssl/lib valgrind --leak-check=full src/.libs/curl -vvv https://tls-ech.dev
==30703== Memcheck, a memory error detector
==30703== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==30703== Using Valgrind-3.19.0 and LibVEX; rerun with -h for copyright info
==30703== Command: src/.libs/curl -vvv https://tls-ech.dev
==30703==
* !!! WARNING !!!
* This is a debug build of libcurl, do not use in production.
* STATE: INIT => CONNECT handle 0x4e42d58; line 1948
* Added connection 0. The cache now contains 1 members
* STATE: CONNECT => RESOLVING handle 0x4e42d58; line 1991
* STATE: INIT => CONNECT handle 0x4e61558; line 1948
* Added connection 1. The cache now contains 2 members
* STATE: CONNECT => RESOLVING handle 0x4e61558; line 1991
* STATE: INIT => CONNECT handle 0x4e62d28; line 1948
* Found bundle for host: 0x4e6aa28 [serially]
* Added connection 2. The cache now contains 3 members
* STATE: CONNECT => RESOLVING handle 0x4e62d28; line 1991
* STATE: INIT => CONNECT handle 0x4e64558; line 1948
* Found bundle for host: 0x4e6aa28 [serially]
* Added connection 3. The cache now contains 4 members
* STATE: CONNECT => RESOLVING handle 0x4e64558; line 1991
* STATE: RESOLVING => CONNECTING handle 0x4e62d28; line 2065
* Trying [2606:4700:4700::1001]:443...
* Hostname 'one.one.one.one' was found in DNS cache
* STATE: RESOLVING => CONNECTING handle 0x4e64558; line 2065
* Trying [2606:4700:4700::1001]:443...
* Hostname 'one.one.one.one' was found in DNS cache
* STATE: RESOLVING => CONNECTING handle 0x4e61558; line 2065
* Trying [2606:4700:4700::1001]:443...
* Connected to one.one.one.one (2606:4700:4700::1001) port 443
* successfully set certificate verify locations:
* CAfile: /etc/ssl/certs/ca-certificates.crt
* CApath: none
* ALPN: curl offers http/1.1
* Didn't find Session ID in cache for host HTTPS://one.one.one.one:443
* Connected to one.one.one.one (2606:4700:4700::1001) port 443
* successfully set certificate verify locations:
* CAfile: /etc/ssl/certs/ca-certificates.crt
* CApath: none
* ALPN: curl offers http/1.1
* Didn't find Session ID in cache for host HTTPS://one.one.one.one:443
* Connected to one.one.one.one (2606:4700:4700::1001) port 443
* successfully set certificate verify locations:
* CAfile: /etc/ssl/certs/ca-certificates.crt
* CApath: none
* ALPN: curl offers http/1.1
* Didn't find Session ID in cache for host HTTPS://one.one.one.one:443
* ALPN: server accepted http/1.1
* SSL connection using TLSv1.3 / TLS13-AES128-GCM-SHA256
* Didn't find Session ID in cache for host HTTPS://one.one.one.one:443
* Added Session ID to cache for HTTPS://one.one.one.one:443 [server]
* using HTTP/1.1
* STATE: CONNECTING => PROTOCONNECT handle 0x4e62d28; line 2109
* STATE: PROTOCONNECT => DO handle 0x4e62d28; line 2139
> POST /dns-query HTTP/1.1
Host: one.one.one.one
Accept: */*
Content-Type: application/dns-message
Content-Length: 29
* STATE: DO => DID handle 0x4e62d28; line 2233
* STATE: DID => PERFORMING handle 0x4e62d28; line 2351
* ALPN: server accepted http/1.1
* SSL connection using TLSv1.3 / TLS13-AES128-GCM-SHA256
* Found Session ID in cache for host HTTPS://one.one.one.one:443
* old SSL session ID is stale, removing
* Added Session ID to cache for HTTPS://one.one.one.one:443 [server]
* using HTTP/1.1
* STATE: CONNECTING => PROTOCONNECT handle 0x4e64558; line 2109
* STATE: PROTOCONNECT => DO handle 0x4e64558; line 2139
> POST /dns-query HTTP/1.1
Host: one.one.one.one
Accept: */*
Content-Type: application/dns-message
Content-Length: 29
* STATE: DO => DID handle 0x4e64558; line 2233
* STATE: DID => PERFORMING handle 0x4e64558; line 2351
* ALPN: server accepted http/1.1
* SSL connection using TLSv1.3 / TLS13-AES128-GCM-SHA256
* Found Session ID in cache for host HTTPS://one.one.one.one:443
* old SSL session ID is stale, removing
* Added Session ID to cache for HTTPS://one.one.one.one:443 [server]
* using HTTP/1.1
* STATE: CONNECTING => PROTOCONNECT handle 0x4e61558; line 2109
* STATE: PROTOCONNECT => DO handle 0x4e61558; line 2139
> POST /dns-query HTTP/1.1
Host: one.one.one.one
Accept: */*
Content-Type: application/dns-message
Content-Length: 29
* STATE: DO => DID handle 0x4e61558; line 2233
* STATE: DID => PERFORMING handle 0x4e61558; line 2351
* HTTP 1.1 or later with persistent connection
< HTTP/1.1 200 OK
< Server: cloudflare
< Date: Sun, 24 Sep 2023 14:36:31 GMT
< Content-Type: application/dns-message
< Connection: keep-alive
< Access-Control-Allow-Origin: *
< Content-Length: 122
< CF-RAY: 80bbc298d9e156fc-DUB
<
* STATE: PERFORMING => DONE handle 0x4e62d28; line 2550
* multi_done[DONE]: status: 0 prem: 0 done: 0
* Connection #2 to host one.one.one.one left intact
* Expire cleared
* a DoH request is completed, 2 to go
* HTTP 1.1 or later with persistent connection
< HTTP/1.1 200 OK
< Server: cloudflare
< Date: Sun, 24 Sep 2023 14:36:31 GMT
< Content-Type: application/dns-message
< Connection: keep-alive
< Access-Control-Allow-Origin: *
< Content-Length: 123
< CF-RAY: 80bbc29958c756ed-DUB
<
* STATE: PERFORMING => DONE handle 0x4e64558; line 2550
* multi_done[DONE]: status: 0 prem: 0 done: 0
* Connection #3 to host one.one.one.one left intact
* Expire cleared
* a DoH request is completed, 1 to go
* HTTP 1.1 or later with persistent connection
< HTTP/1.1 200 OK
< Server: cloudflare
< Date: Sun, 24 Sep 2023 14:36:31 GMT
< Content-Type: application/dns-message
< Connection: keep-alive
< Access-Control-Allow-Origin: *
< Content-Length: 45
< CF-RAY: 80bbc299cffa9569-DUB
<
* STATE: PERFORMING => DONE handle 0x4e61558; line 2550
* multi_done[DONE]: status: 0 prem: 0 done: 0
* Connection #1 to host one.one.one.one left intact
* Expire cleared
* a DoH request is completed, 0 to go
* DoH Host name: tls-ech.dev
* TTL: 60 seconds
* DoH A: 34.138.246.121
* DoH HTTPS:
* 0001000005004B0049FE0D00452B00200020015881D41A3E2EF8F2208185DC479245D20624DDD0918A8056F2E26AF47E26280008000100010001000340127075626C69632E746C732D6563682E6465760000
* Some HTTPS RR to process
* HTTPS_RR: priority 1, target: .
* HTTPS RR: no alpns
* HTTPS RR: no_def_alpn not set
* HTTPS RR: no ipv4hints
* HTTPS RR: ECHConfigList of length 75
* 0049FE0D00452B00200020015881D41A3E2EF8F2208185DC479245D20624DDD0918A8056F2E26AF47E26280008000100010001000340127075626C69632E746C732D6563682E6465760000
* HTTPS RR: no ipv6hints
* STATE: RESOLVING => CONNECTING handle 0x4e42d58; line 2065
* Trying 34.138.246.121:443...
* Connected to tls-ech.dev (34.138.246.121) port 443
* successfully set certificate verify locations:
* CAfile: /etc/ssl/certs/ca-certificates.crt
* CApath: none
* ALPN: curl offers http/1.1
* Didn't find Session ID in cache for host HTTPS://tls-ech.dev:443
* ECH: ECHConfig from DoH HTTPS RR
==30703== Invalid read of size 1
==30703== at 0x484DAFD: memmove (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==30703== by 0x4C6D4AE: wolfSSL_SetEchConfigs (ssl.c:791)
==30703== by 0x4902547: wolfssl_connect_step1 (wolfssl.c:764)
==30703== by 0x4903862: wolfssl_connect_common (wolfssl.c:1300)
==30703== by 0x4903ABD: wolfssl_connect_nonblocking (wolfssl.c:1387)
==30703== by 0x48FD4C7: ssl_connect_nonblocking (vtls.c:375)
==30703== by 0x48FFC73: ssl_cf_connect (vtls.c:1536)
==30703== by 0x4870FDE: Curl_conn_cf_connect (cfilters.c:296)
==30703== by 0x4875BA9: cf_setup_connect (connect.c:1206)
==30703== by 0x4870FDE: Curl_conn_cf_connect (cfilters.c:296)
==30703== by 0x486B0DF: cf_hc_baller_connect (cf-https-connect.c:135)
==30703== by 0x486B86B: cf_hc_connect (cf-https-connect.c:290)
==30703== Address 0x73426db is 0 bytes after a block of size 91 alloc'd
==30703== at 0x4848A13: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==30703== by 0x48B142F: curl_dbg_calloc (memdebug.c:175)
==30703== by 0x4882A10: Curl_doh_decode_httpsrr (doh.c:1158)
==30703== by 0x48832FC: Curl_doh_is_resolved (doh.c:1320)
==30703== by 0x489CB80: Curl_resolv_check (hostip.c:1318)
==30703== by 0x48BDCEF: multi_runsingle (multi.c:2038)
==30703== by 0x48BF53B: curl_multi_perform (multi.c:2742)
==30703== by 0x48863E6: easy_transfer (easy.c:682)
==30703== by 0x488666B: easy_perform (easy.c:772)
==30703== by 0x48866D6: curl_easy_perform (easy.c:791)
==30703== by 0x131489: serial_transfers (tool_operate.c:2499)
==30703== by 0x131AA9: run_all_transfers (tool_operate.c:2690)
==30703==
==30703== Invalid read of size 1
==30703== at 0x484DAF0: memmove (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==30703== by 0x4C6D4AE: wolfSSL_SetEchConfigs (ssl.c:791)
==30703== by 0x4902547: wolfssl_connect_step1 (wolfssl.c:764)
==30703== by 0x4903862: wolfssl_connect_common (wolfssl.c:1300)
==30703== by 0x4903ABD: wolfssl_connect_nonblocking (wolfssl.c:1387)
==30703== by 0x48FD4C7: ssl_connect_nonblocking (vtls.c:375)
==30703== by 0x48FFC73: ssl_cf_connect (vtls.c:1536)
==30703== by 0x4870FDE: Curl_conn_cf_connect (cfilters.c:296)
==30703== by 0x4875BA9: cf_setup_connect (connect.c:1206)
==30703== by 0x4870FDE: Curl_conn_cf_connect (cfilters.c:296)
==30703== by 0x486B0DF: cf_hc_baller_connect (cf-https-connect.c:135)
==30703== by 0x486B86B: cf_hc_connect (cf-https-connect.c:290)
==30703== Address 0x73426dc is 1 bytes after a block of size 91 alloc'd
==30703== at 0x4848A13: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==30703== by 0x48B142F: curl_dbg_calloc (memdebug.c:175)
==30703== by 0x4882A10: Curl_doh_decode_httpsrr (doh.c:1158)
==30703== by 0x48832FC: Curl_doh_is_resolved (doh.c:1320)
==30703== by 0x489CB80: Curl_resolv_check (hostip.c:1318)
==30703== by 0x48BDCEF: multi_runsingle (multi.c:2038)
==30703== by 0x48BF53B: curl_multi_perform (multi.c:2742)
==30703== by 0x48863E6: easy_transfer (easy.c:682)
==30703== by 0x488666B: easy_perform (easy.c:772)
==30703== by 0x48866D6: curl_easy_perform (easy.c:791)
==30703== by 0x131489: serial_transfers (tool_operate.c:2499)
==30703== by 0x131AA9: run_all_transfers (tool_operate.c:2690)
==30703==
* ECH: imported ECHConfigList of length 75
* SSL_connect failed with error -313: received alert fatal error
* multi_done[CONNECTING]: status: 35 prem: 1 done: 0
* multi_done, not reusing connection=0, forbid=0, close=0, premature=1, conn_multiplex=0
* The cache now contains 3 members
* Curl_disconnect(conn #0, dead=1)
* Closing connection
==30703==
==30703== HEAP SUMMARY:
==30703== in use at exit: 0 bytes in 0 blocks
==30703== total heap usage: 37,063 allocs, 37,063 frees, 7,614,829 bytes allocated
==30703==
==30703== All heap blocks were freed -- no leaks are possible
==30703==
==30703== For lists of detected and suppressed errors, rerun with: -s
==30703== ERROR SUMMARY: 14094 errors from 2 contexts (suppressed: 0 from 0)
Hey @sftcd,
I updated my pr to handle the public name length correctly, we can discuss it but I wanted to mark it here.
Best Wishes, John Bland
Now www.facebook.com sets maximum_name_length=100, this bug causes a lot of trouble (guess what). I think this should be fixed quickly.
On 20/01/2024 02:12, Tatsuhiro Tsujikawa wrote:
Now www.facebook.com sets maximum_name_length=100, this bug causes a lot of trouble (guess what). I think this should be fixed quickly.
I don't see an HTTPS RR for www.facebook.com - where are you seeing one with an ECHConfig with a max name length? (Not that I'm involved with wolfssl, but I'd like to test myself.)
Ta, S.
I saw it when I accessed facebook with wsslclient from ngtcp2, that is QUIC client. So is it a deployment issue on facebook side? Anyway facebook sends ECH and wolfssl tries to process it.
Ah, I don't have any way to test QUIC myself. Can you see the HTTPS RR in DNS that contains the ECHConfig?
I do not see HTTPS RR too. After tracing wolfssl code, it looks like it is greasing ECH: https://github.com/wolfSSL/wolfssl/blob/d043333bee254f3fe94545920e4685114381d875/src/tls.c#L11680
I guess maybe some other bug then, but good for some wolfssl person to check. (And if there's a way to check ECH with FB sites, I'd be happy to add that to tests I regularly run.)
Hey @tatsuhiro-t,
I'm not seeing any ech retry options coming from facebook's end and when when an ECH enabled client doesn't receive any usable configs it will send a grease ECH by default which I assume is what you're seeing. If you could send me a reproducer or some logs that show facebook's ech configs I could assist in this issue.
Best Wishes, John Bland
Here is the minimum reproducer. Compiled examples/client/client on master branch, then:
$ examples/client/client -h facebook.com -p 443 -v 4 -A /etc/ssl/certs/ca-certificates.crt
...
ECH extension received
TLSX_ECH_Parse
=================================================================
==60347==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x619000013079 at pc 0x55a5ceb013e1 bp 0x7fff3d20f470 sp 0x7fff3d20ec38
#0 0x55a5ceb013e0 in __asan_memcpy (/path/to/wolfssl/examples/client/.libs/client+0xc73e0) (BuildId: 5c18d04e97abda4484654637bd4c30d12fb0bfa8)
#1 0x7f5e565de068 in wolfSSL_SetEchConfigs /path/to/wolfssl/src/ssl.c:797:9
#2 0x7f5e56715387 in TLSX_ECH_Parse /path/to/wolfssl/src/tls.c:11946:15
#3 0x7f5e5670963a in TLSX_Parse /path/to/wolfssl/src/tls.c:14905:23
#4 0x7f5e56751832 in DoTls13EncryptedExtensions /path/to/wolfssl/src/tls13.c:5598:16
#5 0x7f5e5674c031 in DoTls13HandShakeMsgType /path/to/wolfssl/src/tls13.c:12085:15
#6 0x7f5e5675bd1a in DoTls13HandShakeMsg /path/to/wolfssl/src/tls13.c:12447:15
#7 0x7f5e564cb717 in ProcessReplyEx /path/to/wolfssl/src/internal.c:21430:31
#8 0x7f5e564c58c6 in ProcessReply /path/to/wolfssl/src/internal.c:20673:12
#9 0x7f5e56759af5 in wolfSSL_connect_TLSv13 /path/to/wolfssl/src/tls13.c:12730:35
#10 0x7f5e565ea0ae in wolfSSL_connect /path/to/wolfssl/src/ssl.c:12782:32
#11 0x55a5ceb41525 in client_test /path/to/wolfssl/examples/client/client.c:3867:19
#12 0x55a5ceb4b9f4 in main /path/to/wolfssl/examples/client/client.c:4657:9
#13 0x7f5e55e456c9 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
#14 0x7f5e55e45784 in __libc_start_main csu/../csu/libc-start.c:360:3
#15 0x55a5cea67560 in _start (/path/to/wolfssl/examples/client/.libs/client+0x2d560) (BuildId: 5c18d04e97abda4484654637bd4c30d12fb0bfa8)
In addition to the incorrect parsing of length field, the current code does not check the input buffer has enough length before accessing that space which is very dangerous.