-/* $OpenBSD: ssl_tlsext.c,v 1.144 2024/03/27 10:44:17 beck Exp $ */
+/* $OpenBSD: ssl_tlsext.c,v 1.145 2024/03/27 22:27:09 beck Exp $ */
/*
* Copyright (c) 2016, 2017, 2019 Joel Sing <jsing@openbsd.org>
* Copyright (c) 2017 Doug Hogan <doug@openbsd.org>
- * Copyright (c) 2018-2019 Bob Beck <beck@openbsd.org>
+ * Copyright (c) 2018-2019, 2024 Bob Beck <beck@openbsd.org>
*
* Permission to use, copy, modify, and distribute this software for any
* purpose with or without fee is hereby granted, provided that the above
static int
tlsext_keyshare_server_process(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
{
- CBS client_shares, key_exchange;
+ const uint16_t *client_groups = NULL, *server_groups = NULL;
+ size_t client_groups_len = 0, server_groups_len = 0;
+ size_t i, j, client_groups_index;
+ int preferred_group_found = 0;
int decode_error;
- uint16_t group;
+ uint16_t group, client_preferred_group;
+ CBS client_shares, key_exchange;
+
+ /*
+ * RFC 8446 section 4.2.8:
+ *
+ * Each KeyShareEntry value MUST correspond to a group offered in the
+ * "supported_groups" extension and MUST appear in the same order.
+ * However, the values MAY be a non-contiguous subset of the
+ * "supported_groups".
+ */
+
+ if (!tlsext_extension_seen(s, TLSEXT_TYPE_supported_groups)) {
+ *alert = SSL_AD_ILLEGAL_PARAMETER;
+ return 0;
+ }
+ if (!tlsext_extension_processed(s, TLSEXT_TYPE_supported_groups)) {
+ *alert = SSL_AD_INTERNAL_ERROR;
+ return 0;
+ }
+
+ /*
+ * XXX similar to tls1_get_supported_group, but client pref
+ * only - consider deduping later.
+ */
+ /*
+ * We are now assured of at least one client group.
+ * Get the client and server group preference orders.
+ */
+ tls1_get_group_list(s, 0, &server_groups, &server_groups_len);
+ tls1_get_group_list(s, 1, &client_groups, &client_groups_len);
+
+ /*
+ * Find the group that is most preferred by the client that
+ * we also support.
+ */
+ for (i = 0; i < client_groups_len && !preferred_group_found; i++) {
+ if (!ssl_security_supported_group(s, client_groups[i]))
+ continue;
+ for (j = 0; j < server_groups_len; j++) {
+ if (server_groups[j] == client_groups[i]) {
+ client_preferred_group = client_groups[i];
+ preferred_group_found = 1;
+ break;
+ }
+ }
+ }
if (!CBS_get_u16_length_prefixed(cbs, &client_shares))
return 0;
+ client_groups_index = 0;
while (CBS_len(&client_shares) > 0) {
+ int client_sent_group;
/* Unpack client share. */
if (!CBS_get_u16(&client_shares, &group))
return 0;
/*
- * XXX - check key exchange against supported groups from client.
- * XXX - check that groups only appear once.
+ * Ensure the client share group was sent in supported groups,
+ * and was sent in the same order as supported groups. The
+ * supported groups has already been checked for duplicates.
*/
+ client_sent_group = 0;
+ while (client_groups_index < client_groups_len) {
+ if (group == client_groups[client_groups_index++]) {
+ client_sent_group = 1;
+ break;
+ }
+ }
+ if (!client_sent_group) {
+ *alert = SSL_AD_ILLEGAL_PARAMETER;
+ return 0;
+ }
/*
* Ignore this client share if we're using earlier than TLSv1.3
if (s->s3->hs.key_share != NULL)
continue;
- /* XXX - consider implementing server preference. */
- if (!tls1_check_group(s, group))
+ /*
+ * Ignore this client share if it is not for the most client
+ * preferred supported group. This avoids a potential downgrade
+ * situation where the client sends a client share for something
+ * less preferred, and we choose to to use it instead of
+ * requesting the more preferred group.
+ */
+ if (!preferred_group_found || group != client_preferred_group)
continue;
/* Decode and store the selected key share. */
-/* $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>
goto err;
}
-
failure = 0;
err:
const struct tls_extension_funcs *server_funcs;
int failure;
size_t dlen;
+ size_t idx;
int alert;
CBB cbb;
CBS cbs;
goto done;
}
- (ssl)->version = TLS1_3_VERSION;
- CBS_init(&cbs, data, dlen);
+ ssl->version = TLS1_3_VERSION;
+
+ /* 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;
projects / openbsd / commitdiff