diff options
author | tb <> | 2024-04-16 17:46:30 +0000 |
---|---|---|
committer | tb <> | 2024-04-16 17:46:30 +0000 |
commit | 9f458210467d28817b342e95a6ee616b6ed5b411 (patch) | |
tree | ec214c3193a7a2dde770fe26928836b23e1bb051 | |
parent | 20b733dad0a708b937ef54aa3435a51f6ad6d75c (diff) | |
download | openbsd-9f458210467d28817b342e95a6ee616b6ed5b411.tar.gz openbsd-9f458210467d28817b342e95a6ee616b6ed5b411.tar.bz2 openbsd-9f458210467d28817b342e95a6ee616b6ed5b411.zip |
Fix key share negotiation in HRR case
In the ClientHello retrying the handshake after a HelloRetryRequest, the
client must send a single key share matching the group selected by the
server in the HRR. This is not necessarily the mutually preferred group.
Incorrect logic added in ssl_tlsect.c r1.134 would potentially reject
such a key share because of that.
Instead, add logic to ensure on the server side that there is a single
share matching the group we selected in the HRR.
Fixes a regress test in p5-IO-Socket-SSL where server is configured
with P-521:P-384 and the client with P-256:P-384:P-521. Since the
client sends an initial P-256 key share, a HRR is triggered which
the faulty logic rejected because it was not the mutually preferred
P-384 but rather matching the server-selected P-521.
This will need some deduplication in subsequent commits. We may also
want to consider honoring the mutual preference and request a key
accordingly in the HRR.
reported by bluhm, fix suggested by jsing
ok beck jsing
-rw-r--r-- | src/lib/libssl/ssl_tlsext.c | 41 |
1 files changed, 40 insertions, 1 deletions
diff --git a/src/lib/libssl/ssl_tlsext.c b/src/lib/libssl/ssl_tlsext.c index 6d8f51833b..64fa52e20c 100644 --- a/src/lib/libssl/ssl_tlsext.c +++ b/src/lib/libssl/ssl_tlsext.c | |||
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: ssl_tlsext.c,v 1.148 2024/04/04 08:02:21 tb Exp $ */ | 1 | /* $OpenBSD: ssl_tlsext.c,v 1.149 2024/04/16 17:46:30 tb Exp $ */ |
2 | /* | 2 | /* |
3 | * Copyright (c) 2016, 2017, 2019 Joel Sing <jsing@openbsd.org> | 3 | * Copyright (c) 2016, 2017, 2019 Joel Sing <jsing@openbsd.org> |
4 | * Copyright (c) 2017 Doug Hogan <doug@openbsd.org> | 4 | * Copyright (c) 2017 Doug Hogan <doug@openbsd.org> |
@@ -1493,6 +1493,45 @@ tlsext_keyshare_server_process(SSL *s, uint16_t msg_type, CBS *cbs, int *alert) | |||
1493 | return 0; | 1493 | return 0; |
1494 | } | 1494 | } |
1495 | 1495 | ||
1496 | if (s->s3->hs.tls13.hrr) { | ||
1497 | if (!CBS_get_u16_length_prefixed(cbs, &client_shares)) | ||
1498 | return 0; | ||
1499 | |||
1500 | /* Unpack client share. */ | ||
1501 | if (!CBS_get_u16(&client_shares, &group)) | ||
1502 | return 0; | ||
1503 | if (!CBS_get_u16_length_prefixed(&client_shares, &key_exchange)) | ||
1504 | return 0; | ||
1505 | |||
1506 | /* There should only be one share. */ | ||
1507 | if (CBS_len(&client_shares) != 0) | ||
1508 | return 0; | ||
1509 | |||
1510 | if (group != s->s3->hs.tls13.server_group) { | ||
1511 | *alert = SSL_AD_ILLEGAL_PARAMETER; | ||
1512 | return 0; | ||
1513 | } | ||
1514 | |||
1515 | if (s->s3->hs.key_share != NULL) { | ||
1516 | *alert = SSL_AD_INTERNAL_ERROR; | ||
1517 | return 0; | ||
1518 | } | ||
1519 | |||
1520 | /* Decode and store the selected key share. */ | ||
1521 | if ((s->s3->hs.key_share = tls_key_share_new(group)) == NULL) { | ||
1522 | *alert = SSL_AD_INTERNAL_ERROR; | ||
1523 | return 0; | ||
1524 | } | ||
1525 | if (!tls_key_share_peer_public(s->s3->hs.key_share, | ||
1526 | &key_exchange, &decode_error, NULL)) { | ||
1527 | if (!decode_error) | ||
1528 | *alert = SSL_AD_INTERNAL_ERROR; | ||
1529 | return 0; | ||
1530 | } | ||
1531 | |||
1532 | return 1; | ||
1533 | } | ||
1534 | |||
1496 | /* | 1535 | /* |
1497 | * XXX similar to tls1_get_supported_group, but client pref | 1536 | * XXX similar to tls1_get_supported_group, but client pref |
1498 | * only - consider deduping later. | 1537 | * only - consider deduping later. |