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@
author: beck <> 2024-03-27 22:27:09 +0000
committer: beck <> 2024-03-27 22:27:09 +0000
commit: 298923a130069a65297bca47c69a28e596f74798 (patch)
tree: 68bd1157c64daf7ea2cfbb0c6ca2d70bba0a9769 /src/regress
parent: feced734cd9525f3d3e8b5db422209833f23147a (diff)
download: openbsd-298923a130069a65297bca47c69a28e596f74798.tar.gz
openbsd-298923a130069a65297bca47c69a28e596f74798.tar.bz2
openbsd-298923a130069a65297bca47c69a28e596f74798.zip
1 files changed, 96 insertions, 5 deletions
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 @@
-/* $OpenBSD: tlsexttest.c,v 1.86 2024/03/26 02:43:56 beck Exp $ */
+/* $OpenBSD: tlsexttest.c,v 1.87 2024/03/27 22:27:09 beck Exp $ */
 /*
 * Copyright (c) 2017 Joel Sing <jsing@openbsd.org>
 * Copyright (c) 2017 Doug Hogan <doug@openbsd.org>
@@ -1109,7 +1109,6 @@ test_tlsext_ecpf_client(void)
                goto err;
        }
        failure = 0;
 err:
@@ -3648,6 +3647,7 @@ test_tlsext_keyshare_client(void)
        const struct tls_extension_funcs *server_funcs;
        int failure;
        size_t dlen;
+        size_t idx;
        int alert;
        CBB cbb;
        CBS cbs;
@@ -3701,18 +3701,109 @@ test_tlsext_keyshare_client(void)
                goto done;
        }
-        (ssl)->version = TLS1_3_VERSION;
+        ssl->version = TLS1_3_VERSION;
-        CBS_init(&cbs, data, dlen);
+        /* Fake up the ssl enough so the key share can process */
+        ssl->session = SSL_SESSION_new();
+        ssl->s3 = calloc(1, sizeof(*ssl->s3));
+        ssl->session->tlsext_supportedgroups = calloc(4,
+            sizeof(unsigned short));
+        if (ssl->session == NULL || ssl->s3 == NULL ||
+            ssl->session->tlsext_supportedgroups == NULL) {
+                FAIL("malloc");
+                goto done;
+        }
+        ssl->session->tlsext_supportedgroups[0] = 29;
+        ssl->session->tlsext_supportedgroups[1] = 23;
+        ssl->session->tlsext_supportedgroups[2] = 24;
+        ssl->session->tlsext_supportedgroups[3] = 25;
+        ssl->session->tlsext_supportedgroups_length = 4;
+        tls_extension_find(TLSEXT_TYPE_supported_groups, &idx);
+        ssl->s3->hs.extensions_processed |= (1 << idx);
+        ssl->s3->hs.extensions_seen |= (1 << idx);
+        ssl->s3->hs.our_max_tls_version = TLS1_3_VERSION;
+        /*
+         * We should select the key share for group 29, when group 29
+         * is the most preferred group
+         */
+        CBS_init(&cbs, data, dlen);
        if (!server_funcs->process(ssl, SSL_TLSEXT_MSG_CH, &cbs, &alert)) {
-                FAIL("failed to parse client keyshare\n");
+                FAIL("failed to process client keyshare\n");
+                goto done;
+        }
+        if (CBS_len(&cbs) != 0) {
+                FAIL("extension data remaining\n");
+                goto done;
+        }
+        if (ssl->s3->hs.key_share == NULL) {
+                FAIL("Did not select a key share");
                goto done;
        }
+        /*
+         * Pretend the client did not send the supported groups extension. We
+         * should fail to process.
+         */
+        ssl->s3->hs.extensions_seen = 0;
+        free(ssl->s3->hs.key_share);
+        ssl->s3->hs.key_share = NULL;
+        CBS_init(&cbs, data, dlen);
+        if (server_funcs->process(ssl, SSL_TLSEXT_MSG_CH, &cbs, &alert)) {
+                FAIL("Processed key share when supported groups not provided");
+                goto done;
+        }
+        ssl->s3->hs.extensions_seen |= (1 << idx);
+        /*
+         * Pretend supported groups did not get processed. We should fail to
+         * process
+         */
+        ssl->s3->hs.extensions_processed = 0;
+        ssl->s3->hs.key_share = NULL;
+        CBS_init(&cbs, data, dlen);
+        if (server_funcs->process(ssl, SSL_TLSEXT_MSG_CH, &cbs, &alert)) {
+                FAIL("Processed key share when supported groups unprocesed");
+                goto done;
+        }
+        ssl->s3->hs.extensions_processed |= (1 << idx);
+        /*
+         * Remove group 29 by making it 0xbeef, meaning 29 has not been sent in
+         * supported groups. This should fail to process.
+         */
+        ssl->session->tlsext_supportedgroups[0] = 0xbeef;
+        ssl->s3->hs.key_share = NULL;
+        CBS_init(&cbs, data, dlen);
+        if (server_funcs->process(ssl, SSL_TLSEXT_MSG_CH, &cbs, &alert)) {
+                FAIL("Processed key share with invalid group!");
+                goto done;
+        }
+        ssl->session->tlsext_supportedgroups[0] = 29;
+        /*
+         * Make 29 least preferred, while server supports both 29 and 25.
+         * Client key share is for 29 but it prefers 25. We should successfully
+         * process, but should not select this key share.
+         */
+        ssl->session->tlsext_supportedgroups[0] = 25;
+        ssl->session->tlsext_supportedgroups[3] = 29;
+        ssl->s3->hs.key_share = NULL;
+        CBS_init(&cbs, data, dlen);
+        if (!server_funcs->process(ssl, SSL_TLSEXT_MSG_CH, &cbs, &alert)) {
+                FAIL("failed to process client keyshare\n");
+                goto done;
+        }
        if (CBS_len(&cbs) != 0) {
                FAIL("extension data remaining\n");
                goto done;
        }
+        if (ssl->s3->hs.key_share != NULL) {
+                FAIL("Selected a key share when I should not have!");
+                goto done;
+        }
+        ssl->session->tlsext_supportedgroups[0] = 29;
+        ssl->session->tlsext_supportedgroups[3] = 25;
        failure = 0;
author	beck <>	2024-03-27 22:27:09 +0000
committer	beck <>	2024-03-27 22:27:09 +0000
commit	298923a130069a65297bca47c69a28e596f74798 (patch)
tree	68bd1157c64daf7ea2cfbb0c6ca2d70bba0a9769 /src/regress
parent	feced734cd9525f3d3e8b5db422209833f23147a (diff)
download	openbsd-298923a130069a65297bca47c69a28e596f74798.tar.gz openbsd-298923a130069a65297bca47c69a28e596f74798.tar.bz2 openbsd-298923a130069a65297bca47c69a28e596f74798.zip

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