On Tue, Mar 28, 2023 at 10:17:17AM +0000, Gerhard Roth wrote:
> I stumbled upon a problem that xfreerdp couldn't connect to Windows 11
> servers with NLA and TLS 1.3. This can also be reproduced with
> 
>       # openssl -tls1_3 -connect <host>:3389
> 
> Here openssl will fail with a "tlsv1 alert internal error" instead
> of blocking in "read R BLOCK".

We should figure out where that internal error alert comes from. Is it
sent by the server or is it issued by the client?

I suggest you run

$ openssl s_client -tls1_3 -tlsextdebug -msg -connect <host>:3389

without your diff and share the output. Feel free to take this off-list,
but please keep jsing in Cc.

> So I played around with the advertized TLS extensions in our ClientHello
> message an found that Windows is pleased if we add a
> psk_key_exchange_modes extension.
> 
> That aligns with RFC 8446, Sect. 4.2.9. "Pre-Shared Key Extension Modes":
> 
>       In order to use PSKs, clients MUST also send a
>       "psk_key_exchange_modes" extension.
> 
>       ...
> 
>       A client MUST provide a "psk_key_exchange_modes" extension if it
>       offers a "pre_shared_key" extension.  If clients offer
>       "pre_shared_key" without a "psk_key_exchange_modes" extension,
>       servers MUST abort the handshake.

This does not seem right.

The code you modify is about the "key_share" extension (section 4.2.8).
The quote is about the "pre_shared_key" extension (section 4.2.11).

We never send the latter (tlsext_psk_client_needs() always returns 0),
so no, we should not send this extension.

> My patch adds this extension depening on the offer of a "key_share"
> extension. But maybe this should be done unconditionally?

Do you mean removing "s->s3->hs.tls13.use_psk_dhe_ke &&" from
tlsext_psk_kex_modes_client_needs()? That would be equivalent to your
diff since we always send "key_share" for TLSv1.3 clients.

> 
> 
> Gerhard
> 
> 
> Index: lib/libssl/ssl_tlsext.c
> ===================================================================
> RCS file: /cvs/src/lib/libssl/ssl_tlsext.c,v
> retrieving revision 1.131
> diff -u -p -r1.131 ssl_tlsext.c
> --- lib/libssl/ssl_tlsext.c   26 Nov 2022 16:08:56 -0000      1.131
> +++ lib/libssl/ssl_tlsext.c   27 Mar 2023 08:51:01 -0000
> @@ -1434,6 +1434,8 @@ tlsext_keyshare_client_build(SSL *s, uin
>       if (!tls_key_share_public(s->s3->hs.key_share, &key_exchange))
>               return 0;
>  
> +     s->s3->hs.tls13.use_psk_dhe_ke = 1;
> +
>       if (!CBB_flush(cbb))
>               return 0;
>  
> 


Reply via email to