summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorbeck <>2024-03-27 22:27:09 +0000
committerbeck <>2024-03-27 22:27:09 +0000
commit298923a130069a65297bca47c69a28e596f74798 (patch)
tree68bd1157c64daf7ea2cfbb0c6ca2d70bba0a9769
parentfeced734cd9525f3d3e8b5db422209833f23147a (diff)
downloadopenbsd-298923a130069a65297bca47c69a28e596f74798.tar.gz
openbsd-298923a130069a65297bca47c69a28e596f74798.tar.bz2
openbsd-298923a130069a65297bca47c69a28e596f74798.zip
Fix up server processing of key shares.
Ensure that the client can not provide a duplicate key share for any group, or send more key shares than groups they support. Ensure that the key shares must be provided in the same order as the client preference order specified in supported_groups. Ensure we only will choose to use a key share that is for the most preferred group by the client that we also support, to avoid the client being downgraded by sending a less preferred key share. If we do not end up with a key share for the most preferred mutually supported group, will then do a hello retry request selecting that group. Add regress for this to regress/tlsext/tlsexttest.c ok jsing@
Diffstat
-rw-r--r--src/lib/libssl/ssl_tlsext.c85
-rw-r--r--src/regress/lib/libssl/tlsext/tlsexttest.c101
2 files changed, 173 insertions, 13 deletions
diff --git a/src/lib/libssl/ssl_tlsext.c b/src/lib/libssl/ssl_tlsext.c
index e4ba549814..14cf6fce84 100644
--- a/src/lib/libssl/ssl_tlsext.c
+++ b/src/lib/libssl/ssl_tlsext.c
@@ -1,8 +1,8 @@
1/* $OpenBSD: ssl_tlsext.c,v 1.144 2024/03/27 10:44:17 beck Exp $ */ 1/* $OpenBSD: ssl_tlsext.c,v 1.145 2024/03/27 22:27:09 beck 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>
5 * Copyright (c) 2018-2019 Bob Beck <beck@openbsd.org> 5 * Copyright (c) 2018-2019, 2024 Bob Beck <beck@openbsd.org>
6 * 6 *
7 * Permission to use, copy, modify, and distribute this software for any 7 * Permission to use, copy, modify, and distribute this software for any
8 * purpose with or without fee is hereby granted, provided that the above 8 * purpose with or without fee is hereby granted, provided that the above
@@ -1464,14 +1464,65 @@ tlsext_keyshare_client_build(SSL *s, uint16_t msg_type, CBB *cbb)
1464static int 1464static int
1465tlsext_keyshare_server_process(SSL *s, uint16_t msg_type, CBS *cbs, int *alert) 1465tlsext_keyshare_server_process(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
1466{ 1466{
1467 CBS client_shares, key_exchange; 1467 const uint16_t *client_groups = NULL, *server_groups = NULL;
1468 size_t client_groups_len = 0, server_groups_len = 0;
1469 size_t i, j, client_groups_index;
1470 int preferred_group_found = 0;
1468 int decode_error; 1471 int decode_error;
1469 uint16_t group; 1472 uint16_t group, client_preferred_group;
1473 CBS client_shares, key_exchange;
1474
1475 /*
1476 * RFC 8446 section 4.2.8:
1477 *
1478 * Each KeyShareEntry value MUST correspond to a group offered in the
1479 * "supported_groups" extension and MUST appear in the same order.
1480 * However, the values MAY be a non-contiguous subset of the
1481 * "supported_groups".
1482 */
1483
1484 if (!tlsext_extension_seen(s, TLSEXT_TYPE_supported_groups)) {
1485 *alert = SSL_AD_ILLEGAL_PARAMETER;
1486 return 0;
1487 }
1488 if (!tlsext_extension_processed(s, TLSEXT_TYPE_supported_groups)) {
1489 *alert = SSL_AD_INTERNAL_ERROR;
1490 return 0;
1491 }
1492
1493 /*
1494 * XXX similar to tls1_get_supported_group, but client pref
1495 * only - consider deduping later.
1496 */
1497 /*
1498 * We are now assured of at least one client group.
1499 * Get the client and server group preference orders.
1500 */
1501 tls1_get_group_list(s, 0, &server_groups, &server_groups_len);
1502 tls1_get_group_list(s, 1, &client_groups, &client_groups_len);
1503
1504 /*
1505 * Find the group that is most preferred by the client that
1506 * we also support.
1507 */
1508 for (i = 0; i < client_groups_len && !preferred_group_found; i++) {
1509 if (!ssl_security_supported_group(s, client_groups[i]))
1510 continue;
1511 for (j = 0; j < server_groups_len; j++) {
1512 if (server_groups[j] == client_groups[i]) {
1513 client_preferred_group = client_groups[i];
1514 preferred_group_found = 1;
1515 break;
1516 }
1517 }
1518 }
1470 1519
1471 if (!CBS_get_u16_length_prefixed(cbs, &client_shares)) 1520 if (!CBS_get_u16_length_prefixed(cbs, &client_shares))
1472 return 0; 1521 return 0;
1473 1522
1523 client_groups_index = 0;
1474 while (CBS_len(&client_shares) > 0) { 1524 while (CBS_len(&client_shares) > 0) {
1525 int client_sent_group;
1475 1526
1476 /* Unpack client share. */ 1527 /* Unpack client share. */
1477 if (!CBS_get_u16(&client_shares, &group)) 1528 if (!CBS_get_u16(&client_shares, &group))
@@ -1480,9 +1531,21 @@ tlsext_keyshare_server_process(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
1480 return 0; 1531 return 0;
1481 1532
1482 /* 1533 /*
1483 * XXX - check key exchange against supported groups from client. 1534 * Ensure the client share group was sent in supported groups,
1484 * XXX - check that groups only appear once. 1535 * and was sent in the same order as supported groups. The
1536 * supported groups has already been checked for duplicates.
1485 */ 1537 */
1538 client_sent_group = 0;
1539 while (client_groups_index < client_groups_len) {
1540 if (group == client_groups[client_groups_index++]) {
1541 client_sent_group = 1;
1542 break;
1543 }
1544 }
1545 if (!client_sent_group) {
1546 *alert = SSL_AD_ILLEGAL_PARAMETER;
1547 return 0;
1548 }
1486 1549
1487 /* 1550 /*
1488 * Ignore this client share if we're using earlier than TLSv1.3 1551 * Ignore this client share if we're using earlier than TLSv1.3
@@ -1493,8 +1556,14 @@ tlsext_keyshare_server_process(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
1493 if (s->s3->hs.key_share != NULL) 1556 if (s->s3->hs.key_share != NULL)
1494 continue; 1557 continue;
1495 1558
1496 /* XXX - consider implementing server preference. */ 1559 /*
1497 if (!tls1_check_group(s, group)) 1560 * Ignore this client share if it is not for the most client
1561 * preferred supported group. This avoids a potential downgrade
1562 * situation where the client sends a client share for something
1563 * less preferred, and we choose to to use it instead of
1564 * requesting the more preferred group.
1565 */
1566 if (!preferred_group_found || group != client_preferred_group)
1498 continue; 1567 continue;
1499 1568
1500 /* Decode and store the selected key share. */ 1569 /* Decode and store the selected key share. */
diff --git a/src/regress/lib/libssl/tlsext/tlsexttest.c b/src/regress/lib/libssl/tlsext/tlsexttest.c
index c7983833dc..e61722542e 100644
--- a/src/regress/lib/libssl/tlsext/tlsexttest.c
+++ b/src/regress/lib/libssl/tlsext/tlsexttest.c
@@ -1,4 +1,4 @@
1/* $OpenBSD: tlsexttest.c,v 1.86 2024/03/26 02:43:56 beck Exp $ */ 1/* $OpenBSD: tlsexttest.c,v 1.87 2024/03/27 22:27:09 beck Exp $ */
2/* 2/*
3 * Copyright (c) 2017 Joel Sing <jsing@openbsd.org> 3 * Copyright (c) 2017 Joel Sing <jsing@openbsd.org>
4 * Copyright (c) 2017 Doug Hogan <doug@openbsd.org> 4 * Copyright (c) 2017 Doug Hogan <doug@openbsd.org>
@@ -1109,7 +1109,6 @@ test_tlsext_ecpf_client(void)
1109 goto err; 1109 goto err;
1110 } 1110 }
1111 1111
1112
1113 failure = 0; 1112 failure = 0;
1114 1113
1115 err: 1114 err:
@@ -3648,6 +3647,7 @@ test_tlsext_keyshare_client(void)
3648 const struct tls_extension_funcs *server_funcs; 3647 const struct tls_extension_funcs *server_funcs;
3649 int failure; 3648 int failure;
3650 size_t dlen; 3649 size_t dlen;
3650 size_t idx;
3651 int alert; 3651 int alert;
3652 CBB cbb; 3652 CBB cbb;
3653 CBS cbs; 3653 CBS cbs;
@@ -3701,18 +3701,109 @@ test_tlsext_keyshare_client(void)
3701 goto done; 3701 goto done;
3702 } 3702 }
3703 3703
3704 (ssl)->version = TLS1_3_VERSION; 3704 ssl->version = TLS1_3_VERSION;
3705 CBS_init(&cbs, data, dlen); 3705
3706 /* Fake up the ssl enough so the key share can process */
3707 ssl->session = SSL_SESSION_new();
3708 ssl->s3 = calloc(1, sizeof(*ssl->s3));
3709 ssl->session->tlsext_supportedgroups = calloc(4,
3710 sizeof(unsigned short));
3711 if (ssl->session == NULL || ssl->s3 == NULL ||
3712 ssl->session->tlsext_supportedgroups == NULL) {
3713 FAIL("malloc");
3714 goto done;
3715 }
3716 ssl->session->tlsext_supportedgroups[0] = 29;
3717 ssl->session->tlsext_supportedgroups[1] = 23;
3718 ssl->session->tlsext_supportedgroups[2] = 24;
3719 ssl->session->tlsext_supportedgroups[3] = 25;
3720 ssl->session->tlsext_supportedgroups_length = 4;
3721 tls_extension_find(TLSEXT_TYPE_supported_groups, &idx);
3722 ssl->s3->hs.extensions_processed |= (1 << idx);
3723 ssl->s3->hs.extensions_seen |= (1 << idx);
3724 ssl->s3->hs.our_max_tls_version = TLS1_3_VERSION;
3706 3725
3726 /*
3727 * We should select the key share for group 29, when group 29
3728 * is the most preferred group
3729 */
3730 CBS_init(&cbs, data, dlen);
3707 if (!server_funcs->process(ssl, SSL_TLSEXT_MSG_CH, &cbs, &alert)) { 3731 if (!server_funcs->process(ssl, SSL_TLSEXT_MSG_CH, &cbs, &alert)) {
3708 FAIL("failed to parse client keyshare\n"); 3732 FAIL("failed to process client keyshare\n");
3733 goto done;
3734 }
3735 if (CBS_len(&cbs) != 0) {
3736 FAIL("extension data remaining\n");
3737 goto done;
3738 }
3739 if (ssl->s3->hs.key_share == NULL) {
3740 FAIL("Did not select a key share");
3709 goto done; 3741 goto done;
3710 } 3742 }
3711 3743
3744 /*
3745 * Pretend the client did not send the supported groups extension. We
3746 * should fail to process.
3747 */
3748 ssl->s3->hs.extensions_seen = 0;
3749 free(ssl->s3->hs.key_share);
3750 ssl->s3->hs.key_share = NULL;
3751 CBS_init(&cbs, data, dlen);
3752 if (server_funcs->process(ssl, SSL_TLSEXT_MSG_CH, &cbs, &alert)) {
3753 FAIL("Processed key share when supported groups not provided");
3754 goto done;
3755 }
3756 ssl->s3->hs.extensions_seen |= (1 << idx);
3757
3758 /*
3759 * Pretend supported groups did not get processed. We should fail to
3760 * process
3761 */
3762 ssl->s3->hs.extensions_processed = 0;
3763 ssl->s3->hs.key_share = NULL;
3764 CBS_init(&cbs, data, dlen);
3765 if (server_funcs->process(ssl, SSL_TLSEXT_MSG_CH, &cbs, &alert)) {
3766 FAIL("Processed key share when supported groups unprocesed");
3767 goto done;
3768 }
3769 ssl->s3->hs.extensions_processed |= (1 << idx);
3770
3771 /*
3772 * Remove group 29 by making it 0xbeef, meaning 29 has not been sent in
3773 * supported groups. This should fail to process.
3774 */
3775 ssl->session->tlsext_supportedgroups[0] = 0xbeef;
3776 ssl->s3->hs.key_share = NULL;
3777 CBS_init(&cbs, data, dlen);
3778 if (server_funcs->process(ssl, SSL_TLSEXT_MSG_CH, &cbs, &alert)) {
3779 FAIL("Processed key share with invalid group!");
3780 goto done;
3781 }
3782 ssl->session->tlsext_supportedgroups[0] = 29;
3783
3784 /*
3785 * Make 29 least preferred, while server supports both 29 and 25.
3786 * Client key share is for 29 but it prefers 25. We should successfully
3787 * process, but should not select this key share.
3788 */
3789 ssl->session->tlsext_supportedgroups[0] = 25;
3790 ssl->session->tlsext_supportedgroups[3] = 29;
3791 ssl->s3->hs.key_share = NULL;
3792 CBS_init(&cbs, data, dlen);
3793 if (!server_funcs->process(ssl, SSL_TLSEXT_MSG_CH, &cbs, &alert)) {
3794 FAIL("failed to process client keyshare\n");
3795 goto done;
3796 }
3712 if (CBS_len(&cbs) != 0) { 3797 if (CBS_len(&cbs) != 0) {
3713 FAIL("extension data remaining\n"); 3798 FAIL("extension data remaining\n");
3714 goto done; 3799 goto done;
3715 } 3800 }
3801 if (ssl->s3->hs.key_share != NULL) {
3802 FAIL("Selected a key share when I should not have!");
3803 goto done;
3804 }
3805 ssl->session->tlsext_supportedgroups[0] = 29;
3806 ssl->session->tlsext_supportedgroups[3] = 25;
3716 3807
3717 failure = 0; 3808 failure = 0;
3718 3809
generated by cgit v1.2.3-55-g6feb (git 2.39.1) at 2025-03-21 17:34:52 +0000