From 664854ba7567b1c33d9cfe4e02d77297010a7184 Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Fri, 8 Nov 2024 13:05:00 +0100 Subject: [PATCH 1/4] tls-supported-groups SvcParam support From https://datatracker.ietf.org/doc/draft-ietf-tls-key-share-prediction/01/ Also: - draft-ietf-ohai-svcb-config became RFC 9540 for the ohttp SvcParam - added tests for ohttp --- include/zone.h | 4 ++- src/generic/parser.h | 2 +- src/generic/svcb.h | 55 ++++++++++++++++++++++++++++- tests/svcb.c | 83 +++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 140 insertions(+), 4 deletions(-) diff --git a/include/zone.h b/include/zone.h index bb564ebd..0d5e84f7 100644 --- a/include/zone.h +++ b/include/zone.h @@ -221,8 +221,10 @@ extern "C" { #define ZONE_SVC_PARAM_KEY_IPV6HINT (6u) /** URI template in relative form @rfc{9461} */ #define ZONE_SVC_PARAM_KEY_DOHPATH (7u) -/** Target is an Oblivious HTTP service @draft{ohai,svcb-config} */ +/** Target is an Oblivious HTTP service @rfc{9540} */ #define ZONE_SVC_PARAM_KEY_OHTTP (8u) +/** Supported groups in TLS @draft{ietf, tls-key-share-prediction} */ +#define ZONE_SVC_PARAM_KEY_TLS_SUPPORTED_GROUPS (9u) /** Reserved ("invalid key") @rfc{9460} */ #define ZONE_SVC_PARAM_KEY_INVALID_KEY (65535u) /** @} */ diff --git a/src/generic/parser.h b/src/generic/parser.h index f1823de1..4d6dd027 100644 --- a/src/generic/parser.h +++ b/src/generic/parser.h @@ -49,7 +49,7 @@ struct string { typedef struct mnemonic mnemonic_t; struct mnemonic { struct { - char data[16]; + char data[24]; size_t length; } key; uint32_t value; diff --git a/src/generic/svcb.h b/src/generic/svcb.h index 0011bef9..5eee0406 100644 --- a/src/generic/svcb.h +++ b/src/generic/svcb.h @@ -366,6 +366,51 @@ static int32_t parse_unknown( return 0; } +nonnull_all +static int32_t parse_tls_supported_groups( + parser_t *parser, + const type_info_t *type, + const rdata_info_t *field, + uint16_t key, + const svc_param_info_t *param, + rdata_t *rdata, + const token_t *token) +{ + const char *t = token->data, *te = token->data + token->length; + const uint8_t *rdata_start = rdata->octets; + + (void)field; + (void)key; + (void)param; + + do { + uint64_t number = 0; + for (;; t++) { + const uint64_t digit = (uint8_t)*t - '0'; + if (digit > 9) + break; + number = number * 10 + digit; + } + + uint16_t group = (uint16_t)number; + group = htobe16(group); + memcpy(rdata->octets, &group, 2); + rdata->octets += 2; + if (number > 65535) + SYNTAX_ERROR(parser, "Invalid tls-supported-group in %s", NAME(type)); + + const uint8_t *g; + for (g = rdata_start; g < rdata->octets - 2; g += 2) { + if (memcmp(g, &group, 2) == 0) + SEMANTIC_ERROR(parser, "Duplicate group in tls-supported-groups in %s", NAME(type)); + } + } while (t < te && *t++ == ','); + + if (t != te || rdata->octets > rdata->limit) + SYNTAX_ERROR(parser, "Invalid tls-supported-groups in %s", NAME(type)); + return 0; +} + nonnull_all static int32_t parse_mandatory_lax( parser_t *parser, @@ -421,7 +466,13 @@ static const svc_param_info_t svc_params[] = { // If the "alpn" SvcParam indicates support for HTTP, "dohpath" MUST be // present. SVC_PARAM("dohpath", 7u, MANDATORY_VALUE, parse_dohpath, parse_dohpath), - SVC_PARAM("ohttp", 8u, 0u, 0, 0), + // RFC9540 section 4. + // Both the presentation and wire-format values for the "ohttp" parameter + // MUST be empty. + SVC_PARAM("ohttp", 8u, NO_VALUE, parse_unknown, parse_unknown), + // draft-ietf-tls-key-share-prediction-01 section 3.1 + SVC_PARAM("tls-supported-groups", 9u, MANDATORY_VALUE, + parse_tls_supported_groups, parse_tls_supported_groups), }; static const svc_param_info_t unknown_svc_param = @@ -485,6 +536,8 @@ static really_inline size_t scan_svc_param( return (void)(*param = &svc_params[(*key = ZONE_SVC_PARAM_KEY_DOHPATH)]), 7; else if (memcmp(data, "ohttp", 5) == 0) return (void)(*param = &svc_params[(*key = ZONE_SVC_PARAM_KEY_OHTTP)]), 5; + else if (memcmp(data, "tls-supported-groups", 20) == 0) + return (void)(*param = &svc_params[(*key = ZONE_SVC_PARAM_KEY_TLS_SUPPORTED_GROUPS)]), 20; else if (memcmp(data, "key", 3) == 0) return scan_unknown_svc_param_key(data, key, param); else diff --git a/tests/svcb.c b/tests/svcb.c index f00e5751..21b7dcd3 100644 --- a/tests/svcb.c +++ b/tests/svcb.c @@ -690,6 +690,80 @@ static const rdata_t nsd_s09_rdata = 0x00, 0x01, 0x00, 0x03, 2, 'h', '2', 0x00, 0x07, 0x00, 0x12, '/', 'd', 'n', 's', '-', 'q', 'u', 'e', 'r', 'y', '{', 0xc3, 0xa9, '?', 'd', 'n', 's', '}'); +// From RFC9540 Section 4.1 +static const char ohttp_s1_text[] = + PAD("ohttp-s1 HTTPS 1 . ( alpn=h2 ohttp )"); +static const rdata_t ohttp_s1_rdata = + RDATA( + 0x00, 0x01, // priority + 0x00, // target + 0x00, 0x01, 0x00, 0x03, 2, 'h', '2', // alpn=h2 + 0x00, 0x08, 0x00, 0x00 // ohttp + ); + +static const char ohttp_s2_text[] = + PAD("ohttp-s2 HTTPS 1 . ( mandatory=ohttp ohttp )"); +static const rdata_t ohttp_s2_rdata = + RDATA( + 0x00, 0x01, // priority + 0x00, // target + 0x00, 0x00, 0x00, 0x02, 0x00, 0x08, // mandatory=ohttp + 0x00, 0x08, 0x00, 0x00 // ohttp + ); + +// From RFC9540 Section 4.2.1 +static const char ohttp_s3_text[] = + PAD("ohttp-s3 SVCB 1 doh.example.net. ( alpn=h2 dohpath=/dns-query{?dns} ohttp )"); +static const rdata_t ohttp_s3_rdata = + RDATA( + 0x00, 0x01, // priority + 3, 'd', 'o', 'h', 7, 'e', 'x', 'a', 'm', 'p', 'l', 'e', 3, 'n', 'e', 't', 0, // target + 0x00, 0x01, 0x00, 0x03, 2, 'h', '2', // alpn=h2 + 0x00, 0x07, 0x00, 0x10, '/', 'd', 'n', 's', '-', 'q', 'u', 'e', 'r', 'y', '{', '?', 'd', 'n', 's', '}', + 0x00, 0x08, 0x00, 0x00 // ohttp + ); + +// From RFC9540 Section 4: +// Both the presentation and wire-format values for the "ohttp" parameter +// MUST be empty. +static const char ohttp_f1_text[] = + PAD("ohttp-f1 HTTPS 1 . ( alpn=h2 ohttp=hopsa )"); + +#if 0 +// wire-format non-empty value for the "ohttp" parameter does not fail yet +static const char ohttp_f2_generic_text[] = + PAD("ohttp-f2 SVCB \\# 19 (\n" + "00 01 ; priority\n" + "00 ; target\n" + "00 01 00 03 02 68 32 ; alpn=h2\n" + "00 08 00 05 68 6f 70 73 61 ; ohttp=hopsa\n" + ")"); +#endif + +// From draft-ietf-tls-key-share-prediction-01 Section 3.1 +static const char tsg_s1_text[] = + PAD("tsg-s1 7200 IN SVCB 3 server.example.net. (\n" + "port=\"8004\" tls-supported-groups=29,23 )"); +static const rdata_t tsg_s1_rdata = + RDATA( + 0x00, 0x03, // priority + 6, 's', 'e', 'r', 'v', 'e', 'r', 7, 'e', 'x', 'a', 'm', 'p', 'l', 'e', 3, 'n', 'e', 't', 0, // target + 0x00, 0x03, 0x00, 0x02, 0x1f, 0x44, // port="8004" + 0x00, 0x09, 0x00, 0x04, 0x00, 0x1d, 0x00, 0x17 // tls-supported-groups=29,23 + ); + +// From draft-ietf-tls-key-share-prediction-01 Section 3.1: +// An empty list of values is invalid +static const char tsg_f1_text[] = + PAD("tsg-f1 7200 IN SVCB 3 server.example.net. (\n" + "port=\"8004\" tls-supported-groups )"); + +// From draft-ietf-tls-key-share-prediction-01 Section 3.1: +// An list containing duplicates is invalid +static const char tsg_f2_text[] = + PAD("tsg-f2 7200 IN SVCB 3 server.example.net. (\n" + "port=\"8004\" tls-supported-groups=29,23,29 )"); + // FIXME: make a test that verifies correct behavior for no-default-alpn="some value" typedef struct test test_t; @@ -777,7 +851,14 @@ static const test_t tests[] = { { true, ZONE_TYPE_HTTPS, 0, nsd_s05_https_text, &nsd_s05_https_secondary_rdata }, { false, ZONE_TYPE_HTTPS, 0, nsd_s06_text, &nsd_s06_rdata }, { false, ZONE_TYPE_HTTPS, 0, nsd_s08_text, &nsd_s08_rdata }, - { false, ZONE_TYPE_HTTPS, 0, nsd_s09_text, &nsd_s09_rdata } + { false, ZONE_TYPE_HTTPS, 0, nsd_s09_text, &nsd_s09_rdata }, + { false, ZONE_TYPE_HTTPS, 0, ohttp_s1_text, &ohttp_s1_rdata }, + { false, ZONE_TYPE_HTTPS, 0, ohttp_s2_text, &ohttp_s2_rdata }, + { false, ZONE_TYPE_SVCB, 0, ohttp_s3_text, &ohttp_s3_rdata }, + { false, ZONE_TYPE_HTTPS, ZONE_SEMANTIC_ERROR, ohttp_f1_text, NULL }, + { false, ZONE_TYPE_SVCB, 0, tsg_s1_text, &tsg_s1_rdata }, + { false, ZONE_TYPE_SVCB, ZONE_SEMANTIC_ERROR, tsg_f1_text, NULL }, + { false, ZONE_TYPE_SVCB, ZONE_SEMANTIC_ERROR, tsg_f2_text, NULL } }; static int32_t add_rr( From c1acc4caefea86f34ad3273afb2b5db29e9ab389 Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Fri, 8 Nov 2024 14:09:59 +0100 Subject: [PATCH 2/4] Address review feedback from @wcawijngaards --- src/generic/svcb.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/generic/svcb.h b/src/generic/svcb.h index 5eee0406..e226931b 100644 --- a/src/generic/svcb.h +++ b/src/generic/svcb.h @@ -383,7 +383,7 @@ static int32_t parse_tls_supported_groups( (void)key; (void)param; - do { + while (t < te && rdata->octets+2 <= rdata->limit) { uint64_t number = 0; for (;; t++) { const uint64_t digit = (uint8_t)*t - '0'; @@ -404,7 +404,9 @@ static int32_t parse_tls_supported_groups( if (memcmp(g, &group, 2) == 0) SEMANTIC_ERROR(parser, "Duplicate group in tls-supported-groups in %s", NAME(type)); } - } while (t < te && *t++ == ','); + if (*t++ != ',') + break; + } if (t != te || rdata->octets > rdata->limit) SYNTAX_ERROR(parser, "Invalid tls-supported-groups in %s", NAME(type)); From 0a7201e77ad62c553124e95c9267c08fa9040ac7 Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Fri, 8 Nov 2024 14:16:36 +0100 Subject: [PATCH 3/4] Off-by-one error in tls-supported-groups parsing --- src/generic/svcb.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/generic/svcb.h b/src/generic/svcb.h index e226931b..ec40aee7 100644 --- a/src/generic/svcb.h +++ b/src/generic/svcb.h @@ -404,8 +404,10 @@ static int32_t parse_tls_supported_groups( if (memcmp(g, &group, 2) == 0) SEMANTIC_ERROR(parser, "Duplicate group in tls-supported-groups in %s", NAME(type)); } - if (*t++ != ',') + if (*t != ',') break; + else + t++; } if (t != te || rdata->octets > rdata->limit) From 654de1e03f340d9e570d85fec72105ed796d9009 Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Tue, 12 Nov 2024 14:37:32 +0100 Subject: [PATCH 4/4] Remove mnemonic usage of svc_param_info So SIMD cmpeq on mnemonics keeps working as intended --- src/generic/parser.h | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/generic/parser.h b/src/generic/parser.h index 4d6dd027..323f4aa4 100644 --- a/src/generic/parser.h +++ b/src/generic/parser.h @@ -49,7 +49,7 @@ struct string { typedef struct mnemonic mnemonic_t; struct mnemonic { struct { - char data[24]; + char data[16]; /* MUST be 16 because of usage with SIMD cmpeq */ size_t length; } key; uint32_t value; @@ -78,7 +78,13 @@ typedef int32_t (*parse_svc_param_t)( const token_t *); struct svc_param_info { - mnemonic_t name; + struct { + struct { + char data[24]; + size_t length; + } key; + uint32_t value; + } name; uint32_t has_value; parse_svc_param_t parse, parse_lax; };