From 9cf493d92d78491648ca4144cda6344c3c08b998 Mon Sep 17 00:00:00 2001 From: Will Childs-Klein Date: Wed, 9 Sep 2026 15:45:53 +0000 Subject: [PATCH 01/14] Seed groups and signature algorithms from the system crypto policy Both directives are narrowed to the algorithms AWS-LC implements before being applied. A stock crypto-policies value always names something AWS-LC does not have -- X448, the FFDHE groups, Ed448, RSA-PSS-PSS, the SHA-224 pairs -- and the setters reject a whole list on the first name they do not know, so applying the value as written would discard the operator's preference order entirely. crypto-policies spells the NIST P-256 curve "secp256r1", which AWS-LC does not recognize; without translating it the most widely deployed group in the list is the one that throws the list away. The setters replace AWS-LC's defaults rather than intersect with them, so a policy silent on ML-KEM and ML-DSA drops them. --- BUILDING.md | 21 +++- docs/porting/configuration-differences.md | 19 +-- ssl/crypto_policy.cc | 82 +++++++++++++ ssl/internal.h | 10 ++ ssl/ssl_crypto_policy_test.cc | 139 +++++++++++++++++++++- ssl/ssl_privkey.cc | 35 ++++++ 6 files changed, 289 insertions(+), 17 deletions(-) diff --git a/BUILDING.md b/BUILDING.md index 06a2fbcda9f..e8277ad2475 100644 --- a/BUILDING.md +++ b/BUILDING.md @@ -137,12 +137,21 @@ policy without per-application code changes. When enabled, `SSL_CTX_new` reads `/etc/crypto-policies/back-ends/opensslcnf.config` after applying AWS-LC's -built-in defaults and applies the `CipherString`, `Ciphersuites`, and -`TLS`/`DTLS` `MinProtocol`/`MaxProtocol` directives. This is best-effort: a -missing or malformed file, or a directive AWS-LC does not support, is ignored, -and consumers may still override any setting afterward. The `@SECLEVEL=N` prefix -in `CipherString` is parsed and dropped because AWS-LC does not implement -OpenSSL security levels. +built-in defaults and applies the `CipherString`, `Ciphersuites`, +`TLS`/`DTLS` `MinProtocol`/`MaxProtocol`, `Groups`, and `SignatureAlgorithms` +directives. This is best-effort: a missing or malformed file, or a directive +AWS-LC does not support, is ignored, and consumers may still override any setting +afterward. The `@SECLEVEL=N` prefix in `CipherString` is parsed and dropped +because AWS-LC does not implement OpenSSL security levels. + +`Groups` and `SignatureAlgorithms` are narrowed to the algorithms AWS-LC +implements before being applied, keeping the operator's preference order. A stock +policy value names algorithms AWS-LC does not have, such as X448 and the FFDHE +groups, and the corresponding setters reject a whole list on the first name they +do not recognize; without narrowing, the directive would have no effect at all. +Because these setters replace AWS-LC's defaults rather than intersect with them, a +policy that does not name AWS-LC's post-quantum groups and signature algorithms +removes them. A `MinProtocol` naming a version AWS-LC does not have is the exception: the floor rises to the policy's `MaxProtocol`. Ignoring the directive would leave AWS-LC's diff --git a/docs/porting/configuration-differences.md b/docs/porting/configuration-differences.md index 8e7204b87a1..d5d914c2491 100644 --- a/docs/porting/configuration-differences.md +++ b/docs/porting/configuration-differences.md @@ -45,10 +45,12 @@ defaults are applied and before the context is returned, so a consumer that subsequently calls the relevant setters overrides the seeded values. The directives applied are `CipherString`, `Ciphersuites`, `TLS.MinProtocol`, -`TLS.MaxProtocol`, `DTLS.MinProtocol`, and `DTLS.MaxProtocol`. Seeding is -best-effort: a missing or malformed file, or a directive AWS-LC does not support, -is ignored rather than fatal. A directive AWS-LC cannot satisfy is skipped and -the built-in default stands. +`TLS.MaxProtocol`, `DTLS.MinProtocol`, `DTLS.MaxProtocol`, `Groups`, and +`SignatureAlgorithms`. Seeding is best-effort: a missing or malformed file, or a +directive AWS-LC does not support, is ignored rather than fatal. A directive +AWS-LC cannot satisfy is skipped and the built-in default stands. A context +created from one of the legacy version-locked methods, such as `TLSv1_2_method`, +keeps its single pinned version and takes no protocol bounds from the policy. A `MinProtocol` naming a version AWS-LC does not have is the exception: the floor rises to the policy's `MaxProtocol`. The built-in floor of TLS 1.0 sits below any @@ -56,9 +58,12 @@ floor a policy can ask for, so skipping the directive would leave the context offering the versions the policy forbids. A `MinProtocol` older than TLS 1.0, such as `SSLv3`, keeps the built-in floor. -A context created from one of the legacy -version-locked methods, such as `TLSv1_2_method`, keeps its single pinned version -and takes no protocol bounds from the policy. See +`Groups` and `SignatureAlgorithms` are narrowed to the algorithms AWS-LC +implements, in the order the policy gives them, and a directive naming nothing +AWS-LC implements is dropped. Both correspond to setters that replace AWS-LC's +default list rather than intersect with it, so a policy silent on AWS-LC's +post-quantum groups (ML-KEM hybrids) and signature algorithms (ML-DSA) removes +them from the context. See [BUILDING.md](../../BUILDING.md) for the build flag and the `AWSLC_CRYPTO_POLICY_FILE` override. diff --git a/ssl/crypto_policy.cc b/ssl/crypto_policy.cc index 2511bb45ede..4598f5c489f 100644 --- a/ssl/crypto_policy.cc +++ b/ssl/crypto_policy.cc @@ -148,6 +148,68 @@ uint16_t CryptoPolicyProtoVersion(const char *tok, bool is_dtls) { return 0; } +// kMaxPolicyIds bounds the algorithm IDs kept from one policy directive. No +// directive can yield more than the number of groups or signature algorithms +// AWS-LC implements, which is well under this. +constexpr size_t kMaxPolicyIds = 64; + +// GroupIdFromToken sets |*out| to the AWS-LC group ID named by the +// crypto-policies token |tok|, of length |len|, and returns false if AWS-LC has +// no such group. +bool GroupIdFromToken(uint16_t *out, const char *tok, size_t len) { + // crypto-policies uses the IANA registry name for the NIST P-256 curve. + // AWS-LC follows OpenSSL, which knows it as "P-256" and "prime256v1" only, so + // without this the most widely deployed group in the list is the one that + // throws the whole list away. + static const char kSecp256r1[] = "secp256r1"; + if (len == strlen(kSecp256r1) && OPENSSL_memcmp(tok, kSecp256r1, len) == 0) { + tok = "P-256"; + len = strlen(tok); + } + return ssl_name_to_group_id(out, tok, len); +} + +// FilterPolicyIds resolves the ':'-separated tokens of |value| through |lookup| +// and writes the IDs that resolve into |out|, which holds |out_len| entries, in +// the order the policy gave them. It returns how many were written. +// +// The Groups and SignatureAlgorithms setters reject a whole list on the first +// entry they do not accept, and a stock crypto-policies value always names +// something AWS-LC does not implement: X448 and the FFDHE groups, Ed448, the +// SHA-224 pairs, and the RSA-PSS-PSS algorithms. Applying such a value as written +// therefore discards the operator's whole preference order. Dropping the +// unsupported tokens keeps the rest of it. +// +// Repeats are dropped for the same reason: two spellings of one group, such as +// "secp256r1" and "prime256v1", resolve to a single ID, and |SSL_CTX_set1_group_ids| +// rejects a list that names it twice. +size_t FilterPolicyIds(uint16_t *out, size_t out_len, const char *value, + bool (*lookup)(uint16_t *, const char *, size_t)) { + size_t out_i = 0; + for (const char *tok = value;;) { + const char *end = strchr(tok, ':'); + const size_t len = + end != nullptr ? static_cast(end - tok) : strlen(tok); + + uint16_t id; + if (len > 0 && out_i < out_len && lookup(&id, tok, len)) { + bool seen = false; + for (size_t i = 0; i < out_i; i++) { + seen = seen || out[i] == id; + } + if (!seen) { + out[out_i++] = id; + } + } + + if (end == nullptr) { + break; + } + tok = end + 1; + } + return out_i; +} + // ApplyCipherRule applies the cipher rule |rule| to |ctx|, as // |SSL_CTX_set_cipher_list| does when |config_tls13| is false and // |SSL_CTX_set_ciphersuites| when it is true, and returns false having left |ctx| @@ -299,6 +361,26 @@ void ApplyPolicyToCtx(SSL_CTX *ctx, const char *path, bool is_dtls, } ApplyPolicyVersionBounds(ctx, cfg, is_dtls, version_locked); + + // SignatureAlgorithms and Groups, each narrowed to the algorithms AWS-LC + // implements. One buffer serves both since the directives are applied in turn. + uint16_t ids[kMaxPolicyIds]; + if (cfg.sigalgs[0] != '\0') { + const size_t n = FilterPolicyIds(ids, OPENSSL_ARRAY_SIZE(ids), cfg.sigalgs, + ssl_sigalg_id_from_name); + if (n > 0) { + // Both preference lists, matching what |SSL_CTX_set1_sigalgs_list| writes. + SSL_CTX_set_signing_algorithm_prefs(ctx, ids, n); + SSL_CTX_set_verify_algorithm_prefs(ctx, ids, n); + } + } + if (cfg.groups[0] != '\0') { + const size_t n = FilterPolicyIds(ids, OPENSSL_ARRAY_SIZE(ids), cfg.groups, + GroupIdFromToken); + if (n > 0) { + SSL_CTX_set1_group_ids(ctx, ids, n); + } + } } } // namespace diff --git a/ssl/internal.h b/ssl/internal.h index 217e2d6c032..b7c03c6edc5 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -3759,6 +3759,16 @@ OPENSSL_EXPORT bool ssl_crypto_policy_parse_file(const char *path, // Marked with OPENSSL_EXPORT to make it available for unit tests. OPENSSL_EXPORT const char *ssl_crypto_policy_default_path(void); +// ssl_sigalg_id_from_name sets |*out| to the signature algorithm named by +// |name|, of length |len|. It takes either spelling |SSL_CTX_set1_sigalgs_list| +// takes, "ecdsa_secp256r1_sha256" or "ECDSA+SHA256", queues no errors, and +// returns false if AWS-LC has no such algorithm. +// +// This is for filtering a list from an outside source, which may name algorithms +// AWS-LC does not implement, down to the ones it does. The name-based setter +// rejects the whole list on the first token it does not know. +bool ssl_sigalg_id_from_name(uint16_t *out, const char *name, size_t len); + // ssl_ctx_apply_crypto_policy seeds |ctx| from the crypto-policies OpenSSL // back-end file at |path|. It is best-effort and never fails: a missing or // malformed file, or a directive AWS-LC rejects, leaves the corresponding diff --git a/ssl/ssl_crypto_policy_test.cc b/ssl/ssl_crypto_policy_test.cc index 22d7e3709dd..f1333b1d0ad 100644 --- a/ssl/ssl_crypto_policy_test.cc +++ b/ssl/ssl_crypto_policy_test.cc @@ -14,6 +14,7 @@ #include #include +#include #include #include @@ -30,10 +31,12 @@ namespace { // The Amazon Linux 2023 / Fedora DEFAULT policy, copied byte for byte from // /usr/share/crypto-policies/DEFAULT/opensslcnf.txt with a comment line added. -// Trimming it to what AWS-LC implements would defeat its purpose: the -// Ciphersuites value names a suite AWS-LC does not have, and the Groups value -// carries crypto-policies' '*' key-share marker, so a tidier fixture would not -// exercise the code that has to tolerate either. +// Trimming it to what AWS-LC implements would defeat its purpose: Ciphersuites +// names a suite AWS-LC lacks, Groups carries crypto-policies' '*' key-share +// marker, and Groups and SignatureAlgorithms both name algorithms AWS-LC does +// not implement (X448 and the FFDHE groups; Ed448, the RSA-PSS-PSS algorithms, +// the SHA-224 pairs). A tidier fixture would exercise none of the filtering +// those directives need to take effect at all. const char kDefaultPolicy[] = "# crypto-policies OpenSSL back-end (test fixture)\n" "CipherString = @SECLEVEL=2:kEECDH:kRSA:kEDH:kPSK:kDHEPSK:kECDHEPSK:" @@ -103,6 +106,15 @@ std::vector CipherNames(const SSL_CTX *ctx) { return names; } +// ToVector copies an |Array| out so gtest can print and compare it. +std::vector ToVector(const Array &in) { + return std::vector(in.begin(), in.end()); +} + +bool Contains(const Array &haystack, uint16_t needle) { + return std::find(haystack.begin(), haystack.end(), needle) != haystack.end(); +} + bool CtxHasCipherNamed(const SSL_CTX *ctx, const char *name) { const STACK_OF(SSL_CIPHER) *ciphers = SSL_CTX_get_ciphers(ctx); for (size_t i = 0; i < sk_SSL_CIPHER_num(ciphers); i++) { @@ -442,10 +454,129 @@ TEST_F(CryptoPolicyTest, FullPolicyTLS) { EXPECT_EQ(SSL_CTX_get_max_proto_version(ctx.get()), TLS1_3_VERSION); EXPECT_GT(sk_SSL_CIPHER_num(SSL_CTX_get_ciphers(ctx.get())), 0u); + // Groups and SignatureAlgorithms took effect, keeping the policy's order and + // dropping only what AWS-LC cannot do. + EXPECT_EQ(ToVector(ctx->supported_group_list), + (std::vector{SSL_GROUP_X25519, SSL_GROUP_SECP256R1, + SSL_GROUP_SECP521R1, SSL_GROUP_SECP384R1})); + const std::vector expected_sigalgs = { + SSL_SIGN_ECDSA_SECP256R1_SHA256, SSL_SIGN_ECDSA_SECP384R1_SHA384, + SSL_SIGN_ECDSA_SECP521R1_SHA512, SSL_SIGN_ED25519, + SSL_SIGN_RSA_PSS_RSAE_SHA256, SSL_SIGN_RSA_PSS_RSAE_SHA384, + SSL_SIGN_RSA_PSS_RSAE_SHA512, SSL_SIGN_RSA_PKCS1_SHA256, + SSL_SIGN_RSA_PKCS1_SHA384, SSL_SIGN_RSA_PKCS1_SHA512}; + EXPECT_EQ(ToVector(ctx->verify_sigalgs), expected_sigalgs); + EXPECT_EQ(ToVector(ctx->cert->sigalgs), expected_sigalgs); + // A valid policy leaves the error queue clean. EXPECT_EQ(ERR_peek_error(), 0u); } +// The two directives that need filtering. Without it a stock policy value is +// rejected whole and the directive silently does nothing, which is what the +// negative controls here assert about the unfiltered value. +TEST_F(CryptoPolicyTest, UnsupportedGroupsAndSigalgsAreFiltered) { + bssl::UniquePtr raw(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(raw); + EXPECT_FALSE(SSL_CTX_set1_groups_list(raw.get(), kDefaultGroups)); + EXPECT_FALSE(SSL_CTX_set1_sigalgs_list(raw.get(), kDefaultSigalgs)); + ERR_clear_error(); + + const std::string content = std::string("Groups = ") + kDefaultGroups + "\n" + + "SignatureAlgorithms = " + kDefaultSigalgs + "\n"; + TemporaryFile policy; + ASSERT_TRUE(policy.Init(content)); + + bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(ctx); + ssl_ctx_apply_crypto_policy(ctx.get(), policy.path().c_str(), + /*is_dtls=*/false, /*version_locked=*/false); + + // P-256 is present, so the crypto-policies spelling "secp256r1" was translated + // rather than dropped. + EXPECT_EQ(ToVector(ctx->supported_group_list), + (std::vector{SSL_GROUP_X25519, SSL_GROUP_SECP256R1, + SSL_GROUP_SECP521R1, SSL_GROUP_SECP384R1})); + EXPECT_EQ(ctx->verify_sigalgs.size(), 10u); + EXPECT_EQ(ERR_peek_error(), 0u); +} + +// A directive naming nothing AWS-LC implements is dropped, leaving the built-in +// defaults in force. An empty configured list is how AWS-LC spells "use the +// defaults", so this locks the contract rather than distinguishing two states. +TEST_F(CryptoPolicyTest, WhollyUnsupportedDirectivesKeepDefaults) { + const std::string content = + "Groups = X448:ffdhe2048:ffdhe3072\n" + "SignatureAlgorithms = ed448:ECDSA+SHA224:rsa_pss_pss_sha256\n"; + TemporaryFile policy; + ASSERT_TRUE(policy.Init(content)); + + bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(ctx); + ssl_ctx_apply_crypto_policy(ctx.get(), policy.path().c_str(), + /*is_dtls=*/false, /*version_locked=*/false); + + EXPECT_TRUE(ctx->supported_group_list.empty()); + EXPECT_TRUE(ctx->cert->sigalgs.empty()); + EXPECT_TRUE(ctx->verify_sigalgs.empty()); + EXPECT_EQ(ERR_peek_error(), 0u); +} + +// Two spellings of one group collapse to a single entry. |SSL_CTX_set1_group_ids| +// rejects a list naming the same ID twice, so without this the whole directive +// would be dropped. +TEST_F(CryptoPolicyTest, RepeatedGroupSpellingsCollapse) { + const std::string content = "Groups = secp256r1:prime256v1:P-256:X25519\n"; + TemporaryFile policy; + ASSERT_TRUE(policy.Init(content)); + + bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(ctx); + ssl_ctx_apply_crypto_policy(ctx.get(), policy.path().c_str(), + /*is_dtls=*/false, /*version_locked=*/false); + + EXPECT_EQ( + ToVector(ctx->supported_group_list), + (std::vector{SSL_GROUP_SECP256R1, SSL_GROUP_X25519})); + EXPECT_EQ(ERR_peek_error(), 0u); +} + +// A policy that names neither ML-KEM nor ML-DSA takes AWS-LC's post-quantum +// defaults away, because both setters replace the built-in list rather than +// intersecting with it. Every stock crypto-policies value is such a policy. +TEST_F(CryptoPolicyTest, PolicySilentOnPQDropsPQDefaults) { + TemporaryFile policy; + ASSERT_TRUE(policy.Init(kDefaultPolicy)); + + // The built-in lists are implicit: an empty configured list means the defaults + // apply, and those name the hybrid ML-KEM groups and ML-DSA. + bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(ctx); + ASSERT_TRUE(ctx->supported_group_list.empty()); + ASSERT_TRUE(ctx->cert->sigalgs.empty()); + const Span defaults = tls1_get_default_grouplist(); + EXPECT_NE(std::find(defaults.begin(), defaults.end(), + static_cast(SSL_GROUP_X25519_MLKEM768)), + defaults.end()); + + ssl_ctx_apply_crypto_policy(ctx.get(), policy.path().c_str(), + /*is_dtls=*/false, /*version_locked=*/false); + + ASSERT_FALSE(ctx->supported_group_list.empty()); + ASSERT_FALSE(ctx->cert->sigalgs.empty()); + for (uint16_t group : + {SSL_GROUP_X25519_MLKEM768, SSL_GROUP_SECP256R1_MLKEM768, + SSL_GROUP_SECP384R1_MLKEM1024}) { + EXPECT_FALSE(Contains(ctx->supported_group_list, group)) << group; + } + for (uint16_t sigalg : + {SSL_SIGN_MLDSA44, SSL_SIGN_MLDSA65, SSL_SIGN_MLDSA87}) { + EXPECT_FALSE(Contains(ctx->cert->sigalgs, sigalg)) << sigalg; + EXPECT_FALSE(Contains(ctx->verify_sigalgs, sigalg)) << sigalg; + } + EXPECT_EQ(ERR_peek_error(), 0u); +} + TEST_F(CryptoPolicyTest, SecLevelPrefixIsStripped) { const std::string content = "CipherString = @SECLEVEL=3:ECDHE-RSA-AES128-GCM-SHA256\n"; diff --git a/ssl/ssl_privkey.cc b/ssl/ssl_privkey.cc index 6d3e204e18e..97be735289c 100644 --- a/ssl/ssl_privkey.cc +++ b/ssl/ssl_privkey.cc @@ -1002,6 +1002,41 @@ static bool parse_sigalgs_list(Array *out, const char *str) { return true; } +#if defined(AWSLC_CRYPTO_POLICIES) + +// Defined out of line from the rest of |bssl| in this file because it needs +// |parse_sigalgs_list|, which sits below |BSSL_NAMESPACE_END| with the public +// setters it serves. +BSSL_NAMESPACE_BEGIN + +bool ssl_sigalg_id_from_name(uint16_t *out, const char *name, size_t len) { + // Long enough for the longest name in |kSignatureAlgorithmNames| and for any + // "PKEY+HASH" pair. |parse_sigalgs_list| rejects anything else. + char buf[2 * kMaxSignatureAlgorithmNameLen]; + if (len == 0 || len >= sizeof(buf)) { + return false; + } + OPENSSL_memcpy(buf, name, len); + buf[len] = '\0'; + + // The parser reports a token it does not know by queueing an error. Discard it + // so probing a name is not observable in the caller's error queue. + const size_t num_errors = ERR_num_errors(); + Array sigalgs; + const bool ok = parse_sigalgs_list(&sigalgs, buf); + ERR_pop_to_count(num_errors); + // A ':' would parse as several algorithms, so require exactly one. + if (!ok || sigalgs.size() != 1) { + return false; + } + *out = sigalgs[0]; + return true; +} + +BSSL_NAMESPACE_END + +#endif // AWSLC_CRYPTO_POLICIES + int SSL_CTX_set1_sigalgs_list(SSL_CTX *ctx, const char *str) { Array sigalgs; if (!parse_sigalgs_list(&sigalgs, str)) { From e120c457e6ce1289679c159b754bcc36229569cc Mon Sep 17 00:00:00 2001 From: Will Childs-Klein Date: Wed, 9 Sep 2026 21:03:50 +0000 Subject: [PATCH 02/14] Honor the OpenSSL group-list modifiers in Groups Every stock crypto-policies Groups value marks its first entry with '*', which AWS-LC cannot resolve, so the most-preferred group was dropped from every seeded context. AWS-LC picks its own key shares, so '*' and '?' need only stripping; '-' excludes the group it prefixes. --- BUILDING.md | 2 + docs/porting/configuration-differences.md | 9 +-- ssl/crypto_policy.cc | 12 ++++ ssl/ssl_crypto_policy_test.cc | 83 +++++++++++++++++++++++ 4 files changed, 102 insertions(+), 4 deletions(-) diff --git a/BUILDING.md b/BUILDING.md index e8277ad2475..5c3ba407f50 100644 --- a/BUILDING.md +++ b/BUILDING.md @@ -149,6 +149,8 @@ implements before being applied, keeping the operator's preference order. A stoc policy value names algorithms AWS-LC does not have, such as X448 and the FFDHE groups, and the corresponding setters reject a whole list on the first name they do not recognize; without narrowing, the directive would have no effect at all. +The OpenSSL group-list modifiers are honored: `*` and `?` are stripped, since +AWS-LC selects its own key shares, and `-` drops the group it prefixes. Because these setters replace AWS-LC's defaults rather than intersect with them, a policy that does not name AWS-LC's post-quantum groups and signature algorithms removes them. diff --git a/docs/porting/configuration-differences.md b/docs/porting/configuration-differences.md index d5d914c2491..db499a957d8 100644 --- a/docs/porting/configuration-differences.md +++ b/docs/porting/configuration-differences.md @@ -60,10 +60,11 @@ as `SSLv3`, keeps the built-in floor. `Groups` and `SignatureAlgorithms` are narrowed to the algorithms AWS-LC implements, in the order the policy gives them, and a directive naming nothing -AWS-LC implements is dropped. Both correspond to setters that replace AWS-LC's -default list rather than intersect with it, so a policy silent on AWS-LC's -post-quantum groups (ML-KEM hybrids) and signature algorithms (ML-DSA) removes -them from the context. See +AWS-LC implements is dropped. In `Groups`, the OpenSSL modifiers `*` and `?` are +stripped from the group they mark and `-` drops it. Both directives correspond +to setters that replace AWS-LC's default list rather than intersect with it, so a +policy silent on AWS-LC's post-quantum groups (ML-KEM hybrids) and signature +algorithms (ML-DSA) removes them from the context. See [BUILDING.md](../../BUILDING.md) for the build flag and the `AWSLC_CRYPTO_POLICY_FILE` override. diff --git a/ssl/crypto_policy.cc b/ssl/crypto_policy.cc index 4598f5c489f..8a1ce638799 100644 --- a/ssl/crypto_policy.cc +++ b/ssl/crypto_policy.cc @@ -157,6 +157,18 @@ constexpr size_t kMaxPolicyIds = 64; // crypto-policies token |tok|, of length |len|, and returns false if AWS-LC has // no such group. bool GroupIdFromToken(uint16_t *out, const char *tok, size_t len) { + // OpenSSL 3.5 group-list modifiers, which every stock policy puts on its + // first entry: '*' asks for a key share, '?' tolerates an unimplemented + // group, '-' removes one. AWS-LC chooses its own key shares and already skips + // names it cannot resolve, so the first two need only stripping; '-' must not + // put the group back into the list. + if (len > 0 && (tok[0] == '*' || tok[0] == '?')) { + tok++; + len--; + } else if (len > 0 && tok[0] == '-') { + return false; + } + // crypto-policies uses the IANA registry name for the NIST P-256 curve. // AWS-LC follows OpenSSL, which knows it as "P-256" and "prime256v1" only, so // without this the most widely deployed group in the list is the one that diff --git a/ssl/ssl_crypto_policy_test.cc b/ssl/ssl_crypto_policy_test.cc index f1333b1d0ad..83130a0580b 100644 --- a/ssl/ssl_crypto_policy_test.cc +++ b/ssl/ssl_crypto_policy_test.cc @@ -115,6 +115,27 @@ bool Contains(const Array &haystack, uint16_t needle) { return std::find(haystack.begin(), haystack.end(), needle) != haystack.end(); } +// PolicyRequests reports whether the ':'-separated |value| asks for |name|. A +// '*' or '?' modifier still asks for the group; a '-' removes it, so the token +// is compared with the prefix left on and does not match. +bool PolicyRequests(const char *value, const char *name) { + for (const char *tok = value;;) { + const char *end = strchr(tok, ':'); + size_t len = end != nullptr ? static_cast(end - tok) : strlen(tok); + if (len > 0 && (tok[0] == '*' || tok[0] == '?')) { + tok++; + len--; + } + if (len == strlen(name) && strncmp(tok, name, len) == 0) { + return true; + } + if (end == nullptr) { + return false; + } + tok = end + 1; + } +} + bool CtxHasCipherNamed(const SSL_CTX *ctx, const char *name) { const STACK_OF(SSL_CIPHER) *ciphers = SSL_CTX_get_ciphers(ctx); for (size_t i = 0; i < sk_SSL_CIPHER_num(ciphers); i++) { @@ -501,6 +522,25 @@ TEST_F(CryptoPolicyTest, UnsupportedGroupsAndSigalgsAreFiltered) { EXPECT_EQ(ERR_peek_error(), 0u); } +// The OpenSSL 3.5 group-list modifiers. '*' and '?' decorate a group that is +// still wanted, so keeping the prefix would drop it; '-' excludes one. +TEST_F(CryptoPolicyTest, GroupListModifiers) { + const std::string content = + "Groups = *X25519:?secp384r1:-secp521r1:secp256r1\n"; + TemporaryFile policy; + ASSERT_TRUE(policy.Init(content)); + + bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(ctx); + ssl_ctx_apply_crypto_policy(ctx.get(), policy.path().c_str(), + /*is_dtls=*/false, /*version_locked=*/false); + + EXPECT_EQ(ToVector(ctx->supported_group_list), + (std::vector{SSL_GROUP_X25519, SSL_GROUP_SECP384R1, + SSL_GROUP_SECP256R1})); + EXPECT_EQ(ERR_peek_error(), 0u); +} + // A directive naming nothing AWS-LC implements is dropped, leaving the built-in // defaults in force. An empty configured list is how AWS-LC spells "use the // defaults", so this locks the contract rather than distinguishing two states. @@ -1304,6 +1344,49 @@ TEST_F(CryptoPolicySystemTest, SeedsVersionBoundsAndCiphers) { EXPECT_EQ(ERR_peek_error(), 0u); } +// A fixture is written to be resolvable, so nothing above notices when +// crypto-policies spells a group in a way AWS-LC cannot read. Every group and +// signature algorithm the system policy asks for that AWS-LC implements must +// survive into the seeded context. +TEST_F(CryptoPolicySystemTest, SeedsGroupsAndSigalgs) { + static const struct { + const char *name; + uint16_t id; + } kGroups[] = { + {"X25519", SSL_GROUP_X25519}, + {"secp256r1", SSL_GROUP_SECP256R1}, + {"secp384r1", SSL_GROUP_SECP384R1}, + {"secp521r1", SSL_GROUP_SECP521R1}, + }; + static const struct { + const char *name; + uint16_t id; + } kSigalgs[] = { + {"ECDSA+SHA256", SSL_SIGN_ECDSA_SECP256R1_SHA256}, + {"ECDSA+SHA384", SSL_SIGN_ECDSA_SECP384R1_SHA384}, + {"ECDSA+SHA512", SSL_SIGN_ECDSA_SECP521R1_SHA512}, + {"ed25519", SSL_SIGN_ED25519}, + {"rsa_pss_rsae_sha256", SSL_SIGN_RSA_PSS_RSAE_SHA256}, + {"RSA+SHA256", SSL_SIGN_RSA_PKCS1_SHA256}, + }; + + bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(ctx); + + for (const auto &group : kGroups) { + if (PolicyRequests(cfg_.groups, group.name)) { + EXPECT_TRUE(Contains(ctx->supported_group_list, group.id)) << group.name; + } + } + for (const auto &sigalg : kSigalgs) { + if (PolicyRequests(cfg_.sigalgs, sigalg.name)) { + EXPECT_TRUE(Contains(ctx->verify_sigalgs, sigalg.id)) << sigalg.name; + EXPECT_TRUE(Contains(ctx->cert->sigalgs, sigalg.id)) << sigalg.name; + } + } + EXPECT_EQ(ERR_peek_error(), 0u); +} + TEST_F(CryptoPolicySystemTest, SeedsDTLSVersionBounds) { bssl::UniquePtr ctx(SSL_CTX_new(DTLS_method())); ASSERT_TRUE(ctx); From acb93619a8779357714b4994c17706ce3f75b85f Mon Sep 17 00:00:00 2001 From: Will Childs-Klein Date: Mon, 14 Sep 2026 17:29:07 +0000 Subject: [PATCH 03/14] Suppress the sigalg name probe's errors at the source The probe ran inside a seeding scope that already suppresses errors, so trimming the queue afterward did nothing there and would have lost a caller's oldest entry anywhere else. --- ssl/ssl_privkey.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ssl/ssl_privkey.cc b/ssl/ssl_privkey.cc index 97be735289c..000e6d6dda5 100644 --- a/ssl/ssl_privkey.cc +++ b/ssl/ssl_privkey.cc @@ -1019,12 +1019,12 @@ bool ssl_sigalg_id_from_name(uint16_t *out, const char *name, size_t len) { OPENSSL_memcpy(buf, name, len); buf[len] = '\0'; - // The parser reports a token it does not know by queueing an error. Discard it - // so probing a name is not observable in the caller's error queue. - const size_t num_errors = ERR_num_errors(); + // The parser reports a token it does not know by queueing an error, so probing + // a name would otherwise be visible in the caller's error queue. + ERR_suppress_errors_begin(); Array sigalgs; const bool ok = parse_sigalgs_list(&sigalgs, buf); - ERR_pop_to_count(num_errors); + ERR_suppress_errors_end(); // A ':' would parse as several algorithms, so require exactly one. if (!ok || sigalgs.size() != 1) { return false; From 16ef387fe40b9bd584a6fad080fbdceb3efe854d Mon Sep 17 00:00:00 2001 From: Will Childs-Klein Date: Mon, 14 Sep 2026 18:46:18 +0000 Subject: [PATCH 04/14] Fail the sigalg name probe when errors cannot be suppressed --- ssl/ssl_privkey.cc | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ssl/ssl_privkey.cc b/ssl/ssl_privkey.cc index 000e6d6dda5..340ff642cc8 100644 --- a/ssl/ssl_privkey.cc +++ b/ssl/ssl_privkey.cc @@ -1020,8 +1020,12 @@ bool ssl_sigalg_id_from_name(uint16_t *out, const char *name, size_t len) { buf[len] = '\0'; // The parser reports a token it does not know by queueing an error, so probing - // a name would otherwise be visible in the caller's error queue. - ERR_suppress_errors_begin(); + // a name would otherwise be visible in the caller's error queue. Fail the probe + // when the queue cannot be protected: this reports whether a name resolves, and + // an unresolvable one is what the caller is told either way. + if (!ERR_suppress_errors_begin()) { + return false; + } Array sigalgs; const bool ok = parse_sigalgs_list(&sigalgs, buf); ERR_suppress_errors_end(); From f9a6f1c3afdfb2db6651dd1f7685d4f3c8b8c79c Mon Sep 17 00:00:00 2001 From: Will Childs-Klein Date: Mon, 14 Sep 2026 18:46:18 +0000 Subject: [PATCH 05/14] Apply removal-only Groups and both sigalg lists atomically A Groups value that only removes filters to no IDs, so the setter was skipped and the group came back with the defaults. Apply the removals to the default list instead. The signing and verify preference lists are separate allocations, so the second setter can fail with the first already installed. --- docs/porting/configuration-differences.md | 3 +- ssl/crypto_policy.cc | 59 +++++++++++++++++++++-- ssl/ssl_crypto_policy_test.cc | 40 +++++++++++++++ 3 files changed, 97 insertions(+), 5 deletions(-) diff --git a/docs/porting/configuration-differences.md b/docs/porting/configuration-differences.md index db499a957d8..c2919a31c39 100644 --- a/docs/porting/configuration-differences.md +++ b/docs/porting/configuration-differences.md @@ -61,7 +61,8 @@ as `SSLv3`, keeps the built-in floor. `Groups` and `SignatureAlgorithms` are narrowed to the algorithms AWS-LC implements, in the order the policy gives them, and a directive naming nothing AWS-LC implements is dropped. In `Groups`, the OpenSSL modifiers `*` and `?` are -stripped from the group they mark and `-` drops it. Both directives correspond +stripped from the group they mark and `-` drops it; a value that only removes +groups is applied to AWS-LC's default list. Both directives correspond to setters that replace AWS-LC's default list rather than intersect with it, so a policy silent on AWS-LC's post-quantum groups (ML-KEM hybrids) and signature algorithms (ML-DSA) removes them from the context. See diff --git a/ssl/crypto_policy.cc b/ssl/crypto_policy.cc index 8a1ce638799..27ebc0e95b2 100644 --- a/ssl/crypto_policy.cc +++ b/ssl/crypto_policy.cc @@ -222,6 +222,48 @@ size_t FilterPolicyIds(uint16_t *out, size_t out_len, const char *value, return out_i; } +// PolicyRemovesGroup reports whether the Groups value |value| takes |group| out +// with the OpenSSL '-' modifier. +bool PolicyRemovesGroup(const char *value, uint16_t group) { + for (const char *tok = value;;) { + const char *end = strchr(tok, ':'); + const size_t len = + end != nullptr ? static_cast(end - tok) : strlen(tok); + uint16_t id; + if (len > 1 && tok[0] == '-' && GroupIdFromToken(&id, tok + 1, len - 1) && + id == group) { + return true; + } + if (end == nullptr) { + return false; + } + tok = end + 1; + } +} + +// GroupsFromRemovals fills |out|, which holds |out_len| entries, with AWS-LC's +// default groups less the ones the Groups value |value| removes, and returns how +// many were written, or zero if |value| removes no group AWS-LC implements. +// +// A value that only removes leaves |FilterPolicyIds| nothing to keep, so without +// this the setter is skipped and the group the operator took out comes back with +// the defaults. +size_t GroupsFromRemovals(uint16_t *out, size_t out_len, const char *value) { + bool removed_any = false; + size_t out_i = 0; + for (uint16_t group : tls1_get_default_grouplist()) { + if (PolicyRemovesGroup(value, group)) { + removed_any = true; + continue; + } + if (out_i >= out_len) { + return 0; + } + out[out_i++] = group; + } + return removed_any ? out_i : 0; +} + // ApplyCipherRule applies the cipher rule |rule| to |ctx|, as // |SSL_CTX_set_cipher_list| does when |config_tls13| is false and // |SSL_CTX_set_ciphersuites| when it is true, and returns false having left |ctx| @@ -382,13 +424,22 @@ void ApplyPolicyToCtx(SSL_CTX *ctx, const char *path, bool is_dtls, ssl_sigalg_id_from_name); if (n > 0) { // Both preference lists, matching what |SSL_CTX_set1_sigalgs_list| writes. - SSL_CTX_set_signing_algorithm_prefs(ctx, ids, n); - SSL_CTX_set_verify_algorithm_prefs(ctx, ids, n); + // Each list is a separate allocation, so the second setter can fail with + // the first already in place. Moving the signing list aside costs nothing + // and is what lets that failure keep the defaults. + Array saved_signing = std::move(ctx->cert->sigalgs); + if (!SSL_CTX_set_signing_algorithm_prefs(ctx, ids, n) || + !SSL_CTX_set_verify_algorithm_prefs(ctx, ids, n)) { + ctx->cert->sigalgs = std::move(saved_signing); + } } } if (cfg.groups[0] != '\0') { - const size_t n = FilterPolicyIds(ids, OPENSSL_ARRAY_SIZE(ids), cfg.groups, - GroupIdFromToken); + size_t n = FilterPolicyIds(ids, OPENSSL_ARRAY_SIZE(ids), cfg.groups, + GroupIdFromToken); + if (n == 0) { + n = GroupsFromRemovals(ids, OPENSSL_ARRAY_SIZE(ids), cfg.groups); + } if (n > 0) { SSL_CTX_set1_group_ids(ctx, ids, n); } diff --git a/ssl/ssl_crypto_policy_test.cc b/ssl/ssl_crypto_policy_test.cc index 83130a0580b..c1fea2c5c6d 100644 --- a/ssl/ssl_crypto_policy_test.cc +++ b/ssl/ssl_crypto_policy_test.cc @@ -541,6 +541,46 @@ TEST_F(CryptoPolicyTest, GroupListModifiers) { EXPECT_EQ(ERR_peek_error(), 0u); } +// A value that only removes names no group to keep, so the list it removes from +// is the built-in default. Dropping the directive instead would hand back the one +// group the operator asked to be rid of. +TEST_F(CryptoPolicyTest, RemovalOnlyGroupsRemoves) { + const std::string content = "Groups = -X25519MLKEM768\n"; + TemporaryFile policy; + ASSERT_TRUE(policy.Init(content)); + + bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(ctx); + ssl_ctx_apply_crypto_policy(ctx.get(), policy.path().c_str(), + /*is_dtls=*/false, /*version_locked=*/false); + + std::vector expected; + for (uint16_t group : tls1_get_default_grouplist()) { + if (group != SSL_GROUP_X25519_MLKEM768) { + expected.push_back(group); + } + } + ASSERT_FALSE(expected.empty()); + EXPECT_EQ(ToVector(ctx->supported_group_list), expected); + EXPECT_EQ(ERR_peek_error(), 0u); +} + +// A removal AWS-LC cannot resolve leaves the defaults implicit, since spelling +// them out would freeze today's list into every context the policy touches. +TEST_F(CryptoPolicyTest, RemovalOfUnknownGroupKeepsDefaults) { + const std::string content = "Groups = -ffdhe2048\n"; + TemporaryFile policy; + ASSERT_TRUE(policy.Init(content)); + + bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(ctx); + ssl_ctx_apply_crypto_policy(ctx.get(), policy.path().c_str(), + /*is_dtls=*/false, /*version_locked=*/false); + + EXPECT_TRUE(ctx->supported_group_list.empty()); + EXPECT_EQ(ERR_peek_error(), 0u); +} + // A directive naming nothing AWS-LC implements is dropped, leaving the built-in // defaults in force. An empty configured list is how AWS-LC spells "use the // defaults", so this locks the contract rather than distinguishing two states. From 5bd04e2e4d587625045b3e38d0fe9608061c1771 Mon Sep 17 00:00:00 2001 From: Will Childs-Klein Date: Mon, 14 Sep 2026 21:18:50 +0000 Subject: [PATCH 06/14] Guard the sigalg name probe with the suppression holder --- ssl/ssl_privkey.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ssl/ssl_privkey.cc b/ssl/ssl_privkey.cc index 340ff642cc8..c52ef1d53ad 100644 --- a/ssl/ssl_privkey.cc +++ b/ssl/ssl_privkey.cc @@ -1023,12 +1023,12 @@ bool ssl_sigalg_id_from_name(uint16_t *out, const char *name, size_t len) { // a name would otherwise be visible in the caller's error queue. Fail the probe // when the queue cannot be protected: this reports whether a name resolves, and // an unresolvable one is what the caller is told either way. - if (!ERR_suppress_errors_begin()) { + ScopedErrorSuppression suppress; + if (!suppress) { return false; } Array sigalgs; const bool ok = parse_sigalgs_list(&sigalgs, buf); - ERR_suppress_errors_end(); // A ':' would parse as several algorithms, so require exactly one. if (!ok || sigalgs.size() != 1) { return false; From cd20da62291780d632deb6912db77fa21f660fa1 Mon Sep 17 00:00:00 2001 From: Will Childs-Klein Date: Tue, 15 Sep 2026 20:34:52 +0000 Subject: [PATCH 07/14] Make group and sigalg assertions post-quantum agnostic Post-quantum defaults are the next change in the stack, so nothing here should assert or document what becomes of them. --- BUILDING.md | 3 - docs/porting/configuration-differences.md | 5 +- ssl/ssl_crypto_policy_test.cc | 81 ++++++++++------------- 3 files changed, 37 insertions(+), 52 deletions(-) diff --git a/BUILDING.md b/BUILDING.md index 5c3ba407f50..2b5cb13c95a 100644 --- a/BUILDING.md +++ b/BUILDING.md @@ -151,9 +151,6 @@ groups, and the corresponding setters reject a whole list on the first name they do not recognize; without narrowing, the directive would have no effect at all. The OpenSSL group-list modifiers are honored: `*` and `?` are stripped, since AWS-LC selects its own key shares, and `-` drops the group it prefixes. -Because these setters replace AWS-LC's defaults rather than intersect with them, a -policy that does not name AWS-LC's post-quantum groups and signature algorithms -removes them. A `MinProtocol` naming a version AWS-LC does not have is the exception: the floor rises to the policy's `MaxProtocol`. Ignoring the directive would leave AWS-LC's diff --git a/docs/porting/configuration-differences.md b/docs/porting/configuration-differences.md index c2919a31c39..4b6a6756ce2 100644 --- a/docs/porting/configuration-differences.md +++ b/docs/porting/configuration-differences.md @@ -62,10 +62,7 @@ as `SSLv3`, keeps the built-in floor. implements, in the order the policy gives them, and a directive naming nothing AWS-LC implements is dropped. In `Groups`, the OpenSSL modifiers `*` and `?` are stripped from the group they mark and `-` drops it; a value that only removes -groups is applied to AWS-LC's default list. Both directives correspond -to setters that replace AWS-LC's default list rather than intersect with it, so a -policy silent on AWS-LC's post-quantum groups (ML-KEM hybrids) and signature -algorithms (ML-DSA) removes them from the context. See +groups is applied to AWS-LC's default list. See [BUILDING.md](../../BUILDING.md) for the build flag and the `AWSLC_CRYPTO_POLICY_FILE` override. diff --git a/ssl/ssl_crypto_policy_test.cc b/ssl/ssl_crypto_policy_test.cc index c1fea2c5c6d..27a54e0f27b 100644 --- a/ssl/ssl_crypto_policy_test.cc +++ b/ssl/ssl_crypto_policy_test.cc @@ -115,6 +115,33 @@ bool Contains(const Array &haystack, uint16_t needle) { return std::find(haystack.begin(), haystack.end(), needle) != haystack.end(); } +// AWS-LC's post-quantum groups and signature algorithms. Spelled out here rather +// than read from the library so this file needs no new export; a PQ algorithm +// added to AWS-LC and missing from these lists shows up as an unexpected entry in +// the assertions below, which is the failure to want. +const uint16_t kPQGroups[] = { + SSL_GROUP_MLKEM512, SSL_GROUP_MLKEM768, + SSL_GROUP_MLKEM1024, SSL_GROUP_SECP256R1_MLKEM768, + SSL_GROUP_X25519_MLKEM768, SSL_GROUP_SECP384R1_MLKEM1024, +}; +const uint16_t kPQSigalgs[] = {SSL_SIGN_MLDSA44, SSL_SIGN_MLDSA65, + SSL_SIGN_MLDSA87}; + +// WithoutPQ copies |in| out, dropping the entries in |pq|, so a test can assert +// the classical part of a seeded list exactly without also fixing whether +// post-quantum entries are in it. Whether they are is a policy question this +// layer does not answer. +std::vector WithoutPQ(const Array &in, + Span pq) { + std::vector out; + for (uint16_t id : in) { + if (std::find(pq.begin(), pq.end(), id) == pq.end()) { + out.push_back(id); + } + } + return out; +} + // PolicyRequests reports whether the ':'-separated |value| asks for |name|. A // '*' or '?' modifier still asks for the group; a '-' removes it, so the token // is compared with the prefix left on and does not match. @@ -477,7 +504,7 @@ TEST_F(CryptoPolicyTest, FullPolicyTLS) { // Groups and SignatureAlgorithms took effect, keeping the policy's order and // dropping only what AWS-LC cannot do. - EXPECT_EQ(ToVector(ctx->supported_group_list), + EXPECT_EQ(WithoutPQ(ctx->supported_group_list, kPQGroups), (std::vector{SSL_GROUP_X25519, SSL_GROUP_SECP256R1, SSL_GROUP_SECP521R1, SSL_GROUP_SECP384R1})); const std::vector expected_sigalgs = { @@ -486,8 +513,8 @@ TEST_F(CryptoPolicyTest, FullPolicyTLS) { SSL_SIGN_RSA_PSS_RSAE_SHA256, SSL_SIGN_RSA_PSS_RSAE_SHA384, SSL_SIGN_RSA_PSS_RSAE_SHA512, SSL_SIGN_RSA_PKCS1_SHA256, SSL_SIGN_RSA_PKCS1_SHA384, SSL_SIGN_RSA_PKCS1_SHA512}; - EXPECT_EQ(ToVector(ctx->verify_sigalgs), expected_sigalgs); - EXPECT_EQ(ToVector(ctx->cert->sigalgs), expected_sigalgs); + EXPECT_EQ(WithoutPQ(ctx->verify_sigalgs, kPQSigalgs), expected_sigalgs); + EXPECT_EQ(WithoutPQ(ctx->cert->sigalgs, kPQSigalgs), expected_sigalgs); // A valid policy leaves the error queue clean. EXPECT_EQ(ERR_peek_error(), 0u); @@ -515,10 +542,10 @@ TEST_F(CryptoPolicyTest, UnsupportedGroupsAndSigalgsAreFiltered) { // P-256 is present, so the crypto-policies spelling "secp256r1" was translated // rather than dropped. - EXPECT_EQ(ToVector(ctx->supported_group_list), + EXPECT_EQ(WithoutPQ(ctx->supported_group_list, kPQGroups), (std::vector{SSL_GROUP_X25519, SSL_GROUP_SECP256R1, SSL_GROUP_SECP521R1, SSL_GROUP_SECP384R1})); - EXPECT_EQ(ctx->verify_sigalgs.size(), 10u); + EXPECT_EQ(WithoutPQ(ctx->verify_sigalgs, kPQSigalgs).size(), 10u); EXPECT_EQ(ERR_peek_error(), 0u); } @@ -535,7 +562,7 @@ TEST_F(CryptoPolicyTest, GroupListModifiers) { ssl_ctx_apply_crypto_policy(ctx.get(), policy.path().c_str(), /*is_dtls=*/false, /*version_locked=*/false); - EXPECT_EQ(ToVector(ctx->supported_group_list), + EXPECT_EQ(WithoutPQ(ctx->supported_group_list, kPQGroups), (std::vector{SSL_GROUP_X25519, SSL_GROUP_SECP384R1, SSL_GROUP_SECP256R1})); EXPECT_EQ(ERR_peek_error(), 0u); @@ -545,7 +572,7 @@ TEST_F(CryptoPolicyTest, GroupListModifiers) { // is the built-in default. Dropping the directive instead would hand back the one // group the operator asked to be rid of. TEST_F(CryptoPolicyTest, RemovalOnlyGroupsRemoves) { - const std::string content = "Groups = -X25519MLKEM768\n"; + const std::string content = "Groups = -X25519\n"; TemporaryFile policy; ASSERT_TRUE(policy.Init(content)); @@ -556,7 +583,7 @@ TEST_F(CryptoPolicyTest, RemovalOnlyGroupsRemoves) { std::vector expected; for (uint16_t group : tls1_get_default_grouplist()) { - if (group != SSL_GROUP_X25519_MLKEM768) { + if (group != SSL_GROUP_X25519) { expected.push_back(group); } } @@ -616,47 +643,11 @@ TEST_F(CryptoPolicyTest, RepeatedGroupSpellingsCollapse) { /*is_dtls=*/false, /*version_locked=*/false); EXPECT_EQ( - ToVector(ctx->supported_group_list), + WithoutPQ(ctx->supported_group_list, kPQGroups), (std::vector{SSL_GROUP_SECP256R1, SSL_GROUP_X25519})); EXPECT_EQ(ERR_peek_error(), 0u); } -// A policy that names neither ML-KEM nor ML-DSA takes AWS-LC's post-quantum -// defaults away, because both setters replace the built-in list rather than -// intersecting with it. Every stock crypto-policies value is such a policy. -TEST_F(CryptoPolicyTest, PolicySilentOnPQDropsPQDefaults) { - TemporaryFile policy; - ASSERT_TRUE(policy.Init(kDefaultPolicy)); - - // The built-in lists are implicit: an empty configured list means the defaults - // apply, and those name the hybrid ML-KEM groups and ML-DSA. - bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); - ASSERT_TRUE(ctx); - ASSERT_TRUE(ctx->supported_group_list.empty()); - ASSERT_TRUE(ctx->cert->sigalgs.empty()); - const Span defaults = tls1_get_default_grouplist(); - EXPECT_NE(std::find(defaults.begin(), defaults.end(), - static_cast(SSL_GROUP_X25519_MLKEM768)), - defaults.end()); - - ssl_ctx_apply_crypto_policy(ctx.get(), policy.path().c_str(), - /*is_dtls=*/false, /*version_locked=*/false); - - ASSERT_FALSE(ctx->supported_group_list.empty()); - ASSERT_FALSE(ctx->cert->sigalgs.empty()); - for (uint16_t group : - {SSL_GROUP_X25519_MLKEM768, SSL_GROUP_SECP256R1_MLKEM768, - SSL_GROUP_SECP384R1_MLKEM1024}) { - EXPECT_FALSE(Contains(ctx->supported_group_list, group)) << group; - } - for (uint16_t sigalg : - {SSL_SIGN_MLDSA44, SSL_SIGN_MLDSA65, SSL_SIGN_MLDSA87}) { - EXPECT_FALSE(Contains(ctx->cert->sigalgs, sigalg)) << sigalg; - EXPECT_FALSE(Contains(ctx->verify_sigalgs, sigalg)) << sigalg; - } - EXPECT_EQ(ERR_peek_error(), 0u); -} - TEST_F(CryptoPolicyTest, SecLevelPrefixIsStripped) { const std::string content = "CipherString = @SECLEVEL=3:ECDHE-RSA-AES128-GCM-SHA256\n"; From 2bc68ffc52f75b913c70a9698a03198d964b1f69 Mon Sep 17 00:00:00 2001 From: Will Childs-Klein Date: Wed, 9 Sep 2026 15:55:25 +0000 Subject: [PATCH 08/14] Keep AWS-LC's post-quantum defaults unless the policy opts out Every policy the crypto-policies framework ships today predates ML-KEM and ML-DSA, and the group and signature-algorithm setters replace AWS-LC's defaults rather than intersect with them, so seeding downgraded every context. AWS-LC's post-quantum algorithms are now restored after filtering when the policy names none, and a policy that names any is left alone. A hybrid comes back only when the policy keeps its classical half, and one the policy removes with '-' stays out. The framework's directives cannot say "no post-quantum", so turning it off needs a directive of AWS-LC's own, AWSLC.PostQuantum. --- BUILDING.md | 15 ++ docs/porting/configuration-differences.md | 15 +- ssl/crypto_policy.cc | 197 ++++++++++++++- ssl/internal.h | 4 + ssl/ssl_crypto_policy_test.cc | 278 ++++++++++++++++++---- 5 files changed, 451 insertions(+), 58 deletions(-) diff --git a/BUILDING.md b/BUILDING.md index 2b5cb13c95a..0e95206dfc7 100644 --- a/BUILDING.md +++ b/BUILDING.md @@ -159,6 +159,21 @@ the context would offer the versions the policy forbids. A `MinProtocol` older than TLS 1.0, such as `SSLv3`, keeps the built-in floor, which is already stricter. +AWS-LC's post-quantum algorithms survive a policy that says nothing about them. +Every policy the framework ships today predates ML-KEM and ML-DSA, and the +setters replace AWS-LC's defaults rather than intersect with them, so seeding +would otherwise downgrade every context. A policy that names any post-quantum +algorithm is taken at its word and nothing is added back. To turn post-quantum +off, add AWS-LC's own directive to the policy file: + +``` +AWSLC.PostQuantum = off +``` + +Hybrid groups come back only when the policy keeps their classical half, so +removing `secp384r1` also removes `SecP384r1MLKEM1024`. A group the policy +removes with `-` stays out. + AWS-LC reads the file once per process, as OpenSSL reads `openssl.cnf`, so a policy change takes effect only in processes started afterward. A read that fails is retried on the next `SSL_CTX_new`, so a policy file that appears later is diff --git a/docs/porting/configuration-differences.md b/docs/porting/configuration-differences.md index 4b6a6756ce2..985acb45ed6 100644 --- a/docs/porting/configuration-differences.md +++ b/docs/porting/configuration-differences.md @@ -62,8 +62,19 @@ as `SSLv3`, keeps the built-in floor. implements, in the order the policy gives them, and a directive naming nothing AWS-LC implements is dropped. In `Groups`, the OpenSSL modifiers `*` and `?` are stripped from the group they mark and `-` drops it; a value that only removes -groups is applied to AWS-LC's default list. See -[BUILDING.md](../../BUILDING.md) for the build flag and the +groups is applied to AWS-LC's default list. + +AWS-LC's post-quantum groups (the ML-KEM hybrids) and signature algorithms +(ML-DSA) are kept unless the policy speaks about post-quantum algorithms. Every +policy the `crypto-policies` framework ships today predates them, and both setters +replace AWS-LC's default list rather than intersect with it, so seeding would +otherwise strip post-quantum support from every context. A policy that names any +post-quantum algorithm is authoritative and nothing is added back; a hybrid comes +back only when the policy keeps its classical half, and one the policy removes +with `-` stays out. `AWSLC.PostQuantum = off`, a directive of AWS-LC's own, waives +the defaults entirely, since the directives the framework writes are preference +lists with no syntax for excluding an algorithm. +See [BUILDING.md](../../BUILDING.md) for the build flag and the `AWSLC_CRYPTO_POLICY_FILE` override. diff --git a/ssl/crypto_policy.cc b/ssl/crypto_policy.cc index 27ebc0e95b2..aedbe3631b0 100644 --- a/ssl/crypto_policy.cc +++ b/ssl/crypto_policy.cc @@ -153,6 +153,64 @@ uint16_t CryptoPolicyProtoVersion(const char *tok, bool is_dtls) { // AWS-LC implements, which is well under this. constexpr size_t kMaxPolicyIds = 64; +// PolicyAlias maps one spelling the crypto-policies framework uses to the +// spelling AWS-LC's lookups take. |from| is lowercase and matched without regard +// to case, which config-file tokens do not carry reliably. +struct PolicyAlias { + const char *from; + const char *to; +}; + +// crypto-policies uses the IANA registry name for the NIST P-256 curve, which +// AWS-LC knows only as "P-256" and "prime256v1", and hyphenates the ML-KEM +// hybrids. Without these the most widely deployed group in the list, and every +// post-quantum group, resolve to nothing. +const PolicyAlias kGroupAliases[] = { + {"secp256r1", "P-256"}, + {"x25519-mlkem768", "X25519MLKEM768"}, + {"secp256r1-mlkem768", "SecP256r1MLKEM768"}, + {"secp384r1-mlkem1024", "SecP384r1MLKEM1024"}, +}; + +// crypto-policies spells ML-DSA with the FIPS 204 parameter-set names. +const PolicyAlias kSigalgAliases[] = { + {"ml-dsa-44", "mldsa44"}, + {"ml-dsa-65", "mldsa65"}, + {"ml-dsa-87", "mldsa87"}, +}; + +// kPolicyMLDSASigalgs are AWS-LC's default ML-DSA signature algorithms. +const uint16_t kPolicyMLDSASigalgs[] = {SSL_SIGN_MLDSA44, SSL_SIGN_MLDSA65, + SSL_SIGN_MLDSA87}; + +bool EqualsIgnoreAsciiCase(const char *tok, size_t len, const char *lower) { + if (strlen(lower) != len) { + return false; + } + for (size_t i = 0; i < len; i++) { + char c = tok[i]; + if (c >= 'A' && c <= 'Z') { + c = static_cast(c + ('a' - 'A')); + } + if (c != lower[i]) { + return false; + } + } + return true; +} + +// ResolveAlias rewrites |*tok| and |*len| through |aliases| if one matches. +void ResolveAlias(const char **tok, size_t *len, const PolicyAlias *aliases, + size_t num_aliases) { + for (size_t i = 0; i < num_aliases; i++) { + if (EqualsIgnoreAsciiCase(*tok, *len, aliases[i].from)) { + *tok = aliases[i].to; + *len = strlen(aliases[i].to); + return; + } + } +} + // GroupIdFromToken sets |*out| to the AWS-LC group ID named by the // crypto-policies token |tok|, of length |len|, and returns false if AWS-LC has // no such group. @@ -169,18 +227,27 @@ bool GroupIdFromToken(uint16_t *out, const char *tok, size_t len) { return false; } - // crypto-policies uses the IANA registry name for the NIST P-256 curve. - // AWS-LC follows OpenSSL, which knows it as "P-256" and "prime256v1" only, so - // without this the most widely deployed group in the list is the one that - // throws the whole list away. - static const char kSecp256r1[] = "secp256r1"; - if (len == strlen(kSecp256r1) && OPENSSL_memcmp(tok, kSecp256r1, len) == 0) { - tok = "P-256"; - len = strlen(tok); - } + ResolveAlias(&tok, &len, kGroupAliases, OPENSSL_ARRAY_SIZE(kGroupAliases)); return ssl_name_to_group_id(out, tok, len); } +// SigalgIdFromToken sets |*out| to the AWS-LC signature algorithm ID named by +// the crypto-policies token |tok|, of length |len|, and returns false if AWS-LC +// has no such algorithm. +bool SigalgIdFromToken(uint16_t *out, const char *tok, size_t len) { + ResolveAlias(&tok, &len, kSigalgAliases, OPENSSL_ARRAY_SIZE(kSigalgAliases)); + return ssl_sigalg_id_from_name(out, tok, len); +} + +bool ContainsId(Span ids, uint16_t id) { + for (uint16_t candidate : ids) { + if (candidate == id) { + return true; + } + } + return false; +} + // FilterPolicyIds resolves the ':'-separated tokens of |value| through |lookup| // and writes the IDs that resolve into |out|, which holds |out_len| entries, in // the order the policy gave them. It returns how many were written. @@ -222,6 +289,32 @@ size_t FilterPolicyIds(uint16_t *out, size_t out_len, const char *value, return out_i; } +// PolicyKeepsPQDefaults reports whether |cfg| leaves AWS-LC's post-quantum +// defaults in force. Only AWS-LC's own AWSLC.PostQuantum directive waives them +// wholesale: a crypto-policies list can remove a group it names, but nothing in +// the directives the framework writes says "no post-quantum". +bool PolicyKeepsPQDefaults(const CryptoPolicyConfig &cfg) { + return !EqualsIgnoreAsciiCase(cfg.post_quantum, strlen(cfg.post_quantum), + "off"); +} + +// HybridClassicalComponent sets |*out| to the non-post-quantum half of the +// hybrid group |group| and returns false if |group| is not a hybrid. +bool HybridClassicalComponent(uint16_t *out, uint16_t group) { + for (const HybridGroup &hybrid : HybridGroups()) { + if (hybrid.group_id != group) { + continue; + } + for (uint16_t component : hybrid.component_group_ids) { + if (!ContainsId(PQGroups(), component)) { + *out = component; + return true; + } + } + } + return false; +} + // PolicyRemovesGroup reports whether the Groups value |value| takes |group| out // with the OpenSSL '-' modifier. bool PolicyRemovesGroup(const char *value, uint16_t group) { @@ -264,6 +357,78 @@ size_t GroupsFromRemovals(uint16_t *out, size_t out_len, const char *value) { return removed_any ? out_i : 0; } +// MergeDefaultPQGroups restores AWS-LC's default post-quantum groups at the front +// of |ids|, which holds |n| of |cap| entries and came from the Groups value +// |value|, and returns the new count. +// +// The group setter replaces AWS-LC's defaults rather than intersecting with them, +// so without this a policy written before ML-KEM existed silently downgrades +// every context. A policy naming any post-quantum group has an opinion about +// them and is left alone, as does one that names a group only to remove it. +// +// A hybrid whose classical half the policy dropped is not restored: an operator +// who removed P-384 did not ask for P-384 key exchange back under another name. +size_t MergeDefaultPQGroups(uint16_t *ids, size_t n, size_t cap, + const char *value) { + if (n == 0) { + return n; + } + for (size_t i = 0; i < n; i++) { + if (ContainsId(PQGroups(), ids[i])) { + return n; + } + } + + uint16_t add[kMaxPolicyIds]; + size_t num_add = 0; + for (uint16_t group : tls1_get_default_grouplist()) { + if (!ContainsId(PQGroups(), group)) { + continue; + } + uint16_t classical; + if (HybridClassicalComponent(&classical, group) && + !ContainsId(MakeConstSpan(ids, n), classical)) { + continue; + } + if (PolicyRemovesGroup(value, group)) { + continue; + } + if (num_add < OPENSSL_ARRAY_SIZE(add)) { + add[num_add++] = group; + } + } + if (num_add == 0 || n + num_add > cap) { + return n; + } + + OPENSSL_memmove(ids + num_add, ids, n * sizeof(uint16_t)); + OPENSSL_memcpy(ids, add, num_add * sizeof(uint16_t)); + return n + num_add; +} + +// MergeDefaultPQSigalgs restores AWS-LC's default ML-DSA algorithms at the end of +// |ids|, which holds |n| of |cap| entries, and returns the new count. As with +// groups, a policy naming any of them is left alone. +// +// They go last because that is where AWS-LC's own default list puts them. +size_t MergeDefaultPQSigalgs(uint16_t *ids, size_t n, size_t cap) { + if (n == 0) { + return n; + } + for (uint16_t sigalg : kPolicyMLDSASigalgs) { + if (ContainsId(MakeConstSpan(ids, n), sigalg)) { + return n; + } + } + if (n + OPENSSL_ARRAY_SIZE(kPolicyMLDSASigalgs) > cap) { + return n; + } + for (uint16_t sigalg : kPolicyMLDSASigalgs) { + ids[n++] = sigalg; + } + return n; +} + // ApplyCipherRule applies the cipher rule |rule| to |ctx|, as // |SSL_CTX_set_cipher_list| does when |config_tls13| is false and // |SSL_CTX_set_ciphersuites| when it is true, and returns false having left |ctx| @@ -419,9 +584,13 @@ void ApplyPolicyToCtx(SSL_CTX *ctx, const char *path, bool is_dtls, // SignatureAlgorithms and Groups, each narrowed to the algorithms AWS-LC // implements. One buffer serves both since the directives are applied in turn. uint16_t ids[kMaxPolicyIds]; + const bool keep_pq = PolicyKeepsPQDefaults(cfg); if (cfg.sigalgs[0] != '\0') { - const size_t n = FilterPolicyIds(ids, OPENSSL_ARRAY_SIZE(ids), cfg.sigalgs, - ssl_sigalg_id_from_name); + size_t n = FilterPolicyIds(ids, OPENSSL_ARRAY_SIZE(ids), cfg.sigalgs, + SigalgIdFromToken); + if (keep_pq) { + n = MergeDefaultPQSigalgs(ids, n, OPENSSL_ARRAY_SIZE(ids)); + } if (n > 0) { // Both preference lists, matching what |SSL_CTX_set1_sigalgs_list| writes. // Each list is a separate allocation, so the second setter can fail with @@ -440,6 +609,9 @@ void ApplyPolicyToCtx(SSL_CTX *ctx, const char *path, bool is_dtls, if (n == 0) { n = GroupsFromRemovals(ids, OPENSSL_ARRAY_SIZE(ids), cfg.groups); } + if (keep_pq) { + n = MergeDefaultPQGroups(ids, n, OPENSSL_ARRAY_SIZE(ids), cfg.groups); + } if (n > 0) { SSL_CTX_set1_group_ids(ctx, ids, n); } @@ -542,6 +714,9 @@ bool ssl_crypto_policy_parse_file(const char *path, CryptoPolicyConfig *out) { CopyPolicyValue(out->sigalgs, sizeof(out->sigalgs), val, val_len); } else if (key_is("Groups")) { CopyPolicyValue(out->groups, sizeof(out->groups), val, val_len); + } else if (key_is("AWSLC.PostQuantum")) { + CopyPolicyValue(out->post_quantum, sizeof(out->post_quantum), val, + val_len); } } diff --git a/ssl/internal.h b/ssl/internal.h index b7c03c6edc5..4984746f112 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -3723,6 +3723,10 @@ struct CryptoPolicyConfig { char tls_max[AWSLC_CRYPTO_POLICY_MAX_TOKEN + 1]; // TLS.MaxProtocol char dtls_min[AWSLC_CRYPTO_POLICY_MAX_TOKEN + 1]; // DTLS.MinProtocol char dtls_max[AWSLC_CRYPTO_POLICY_MAX_TOKEN + 1]; // DTLS.MaxProtocol + // post_quantum is AWSLC.PostQuantum, AWS-LC's own directive. "off" waives the + // post-quantum defaults a policy silent on them would otherwise keep; any + // other value, including absent, leaves them in force. + char post_quantum[AWSLC_CRYPTO_POLICY_MAX_TOKEN + 1]; // Set when the floor directive appeared, whatever became of its value. An // absent floor leaves the context's own, which sits below every floor a policy diff --git a/ssl/ssl_crypto_policy_test.cc b/ssl/ssl_crypto_policy_test.cc index 27a54e0f27b..16c71516a44 100644 --- a/ssl/ssl_crypto_policy_test.cc +++ b/ssl/ssl_crypto_policy_test.cc @@ -115,33 +115,6 @@ bool Contains(const Array &haystack, uint16_t needle) { return std::find(haystack.begin(), haystack.end(), needle) != haystack.end(); } -// AWS-LC's post-quantum groups and signature algorithms. Spelled out here rather -// than read from the library so this file needs no new export; a PQ algorithm -// added to AWS-LC and missing from these lists shows up as an unexpected entry in -// the assertions below, which is the failure to want. -const uint16_t kPQGroups[] = { - SSL_GROUP_MLKEM512, SSL_GROUP_MLKEM768, - SSL_GROUP_MLKEM1024, SSL_GROUP_SECP256R1_MLKEM768, - SSL_GROUP_X25519_MLKEM768, SSL_GROUP_SECP384R1_MLKEM1024, -}; -const uint16_t kPQSigalgs[] = {SSL_SIGN_MLDSA44, SSL_SIGN_MLDSA65, - SSL_SIGN_MLDSA87}; - -// WithoutPQ copies |in| out, dropping the entries in |pq|, so a test can assert -// the classical part of a seeded list exactly without also fixing whether -// post-quantum entries are in it. Whether they are is a policy question this -// layer does not answer. -std::vector WithoutPQ(const Array &in, - Span pq) { - std::vector out; - for (uint16_t id : in) { - if (std::find(pq.begin(), pq.end(), id) == pq.end()) { - out.push_back(id); - } - } - return out; -} - // PolicyRequests reports whether the ':'-separated |value| asks for |name|. A // '*' or '?' modifier still asks for the group; a '-' removes it, so the token // is compared with the prefix left on and does not match. @@ -258,6 +231,16 @@ TEST_F(CryptoPolicyParseTest, FullPolicy) { EXPECT_STREQ("TLSv1.3", cfg.tls_max); EXPECT_STREQ("DTLSv1.2", cfg.dtls_min); EXPECT_STREQ("DTLSv1.2", cfg.dtls_max); + EXPECT_STREQ("", cfg.post_quantum); +} + +TEST_F(CryptoPolicyParseTest, PostQuantumDirective) { + TemporaryFile file; + ASSERT_TRUE(file.Init("AWSLC.PostQuantum = off\n")); + + CryptoPolicyConfig cfg = {}; + ASSERT_TRUE(ssl_crypto_policy_parse_file(file.path().c_str(), &cfg)); + EXPECT_STREQ("off", cfg.post_quantum); } TEST_F(CryptoPolicyParseTest, MissingFileFails) { @@ -503,18 +486,25 @@ TEST_F(CryptoPolicyTest, FullPolicyTLS) { EXPECT_GT(sk_SSL_CIPHER_num(SSL_CTX_get_ciphers(ctx.get())), 0u); // Groups and SignatureAlgorithms took effect, keeping the policy's order and - // dropping only what AWS-LC cannot do. - EXPECT_EQ(WithoutPQ(ctx->supported_group_list, kPQGroups), - (std::vector{SSL_GROUP_X25519, SSL_GROUP_SECP256R1, - SSL_GROUP_SECP521R1, SSL_GROUP_SECP384R1})); + // dropping only what AWS-LC cannot do. This policy says nothing about + // post-quantum algorithms, so AWS-LC's own are kept: the hybrids ahead of the + // classical groups and ML-DSA after the classical algorithms, as in the + // built-in defaults. + EXPECT_EQ(ToVector(ctx->supported_group_list), + (std::vector{ + SSL_GROUP_X25519_MLKEM768, SSL_GROUP_SECP256R1_MLKEM768, + SSL_GROUP_SECP384R1_MLKEM1024, SSL_GROUP_X25519, + SSL_GROUP_SECP256R1, SSL_GROUP_SECP521R1, SSL_GROUP_SECP384R1})); const std::vector expected_sigalgs = { SSL_SIGN_ECDSA_SECP256R1_SHA256, SSL_SIGN_ECDSA_SECP384R1_SHA384, SSL_SIGN_ECDSA_SECP521R1_SHA512, SSL_SIGN_ED25519, SSL_SIGN_RSA_PSS_RSAE_SHA256, SSL_SIGN_RSA_PSS_RSAE_SHA384, SSL_SIGN_RSA_PSS_RSAE_SHA512, SSL_SIGN_RSA_PKCS1_SHA256, - SSL_SIGN_RSA_PKCS1_SHA384, SSL_SIGN_RSA_PKCS1_SHA512}; - EXPECT_EQ(WithoutPQ(ctx->verify_sigalgs, kPQSigalgs), expected_sigalgs); - EXPECT_EQ(WithoutPQ(ctx->cert->sigalgs, kPQSigalgs), expected_sigalgs); + SSL_SIGN_RSA_PKCS1_SHA384, SSL_SIGN_RSA_PKCS1_SHA512, + SSL_SIGN_MLDSA44, SSL_SIGN_MLDSA65, + SSL_SIGN_MLDSA87}; + EXPECT_EQ(ToVector(ctx->verify_sigalgs), expected_sigalgs); + EXPECT_EQ(ToVector(ctx->cert->sigalgs), expected_sigalgs); // A valid policy leaves the error queue clean. EXPECT_EQ(ERR_peek_error(), 0u); @@ -542,18 +532,23 @@ TEST_F(CryptoPolicyTest, UnsupportedGroupsAndSigalgsAreFiltered) { // P-256 is present, so the crypto-policies spelling "secp256r1" was translated // rather than dropped. - EXPECT_EQ(WithoutPQ(ctx->supported_group_list, kPQGroups), - (std::vector{SSL_GROUP_X25519, SSL_GROUP_SECP256R1, - SSL_GROUP_SECP521R1, SSL_GROUP_SECP384R1})); - EXPECT_EQ(WithoutPQ(ctx->verify_sigalgs, kPQSigalgs).size(), 10u); + EXPECT_EQ(ToVector(ctx->supported_group_list), + (std::vector{ + SSL_GROUP_X25519_MLKEM768, SSL_GROUP_SECP256R1_MLKEM768, + SSL_GROUP_SECP384R1_MLKEM1024, SSL_GROUP_X25519, + SSL_GROUP_SECP256R1, SSL_GROUP_SECP521R1, SSL_GROUP_SECP384R1})); + EXPECT_EQ(ctx->verify_sigalgs.size(), 13u); EXPECT_EQ(ERR_peek_error(), 0u); } // The OpenSSL 3.5 group-list modifiers. '*' and '?' decorate a group that is // still wanted, so keeping the prefix would drop it; '-' excludes one. TEST_F(CryptoPolicyTest, GroupListModifiers) { + // Post-quantum off so the assertion is the policy's own list, without the + // hybrids the defaults would prepend. const std::string content = - "Groups = *X25519:?secp384r1:-secp521r1:secp256r1\n"; + "Groups = *X25519:?secp384r1:-secp521r1:secp256r1\n" + "AWSLC.PostQuantum = off\n"; TemporaryFile policy; ASSERT_TRUE(policy.Init(content)); @@ -562,7 +557,7 @@ TEST_F(CryptoPolicyTest, GroupListModifiers) { ssl_ctx_apply_crypto_policy(ctx.get(), policy.path().c_str(), /*is_dtls=*/false, /*version_locked=*/false); - EXPECT_EQ(WithoutPQ(ctx->supported_group_list, kPQGroups), + EXPECT_EQ(ToVector(ctx->supported_group_list), (std::vector{SSL_GROUP_X25519, SSL_GROUP_SECP384R1, SSL_GROUP_SECP256R1})); EXPECT_EQ(ERR_peek_error(), 0u); @@ -609,8 +604,9 @@ TEST_F(CryptoPolicyTest, RemovalOfUnknownGroupKeepsDefaults) { } // A directive naming nothing AWS-LC implements is dropped, leaving the built-in -// defaults in force. An empty configured list is how AWS-LC spells "use the -// defaults", so this locks the contract rather than distinguishing two states. +// defaults in force. In particular no post-quantum algorithm is merged into an +// otherwise empty result, which would leave the context offering ML-DSA and +// nothing else. TEST_F(CryptoPolicyTest, WhollyUnsupportedDirectivesKeepDefaults) { const std::string content = "Groups = X448:ffdhe2048:ffdhe3072\n" @@ -642,9 +638,201 @@ TEST_F(CryptoPolicyTest, RepeatedGroupSpellingsCollapse) { ssl_ctx_apply_crypto_policy(ctx.get(), policy.path().c_str(), /*is_dtls=*/false, /*version_locked=*/false); - EXPECT_EQ( - WithoutPQ(ctx->supported_group_list, kPQGroups), - (std::vector{SSL_GROUP_SECP256R1, SSL_GROUP_X25519})); + // The P-384 hybrid is not restored: the policy dropped P-384, and restoring it + // would put P-384 key exchange back under another name. + EXPECT_EQ(ToVector(ctx->supported_group_list), + (std::vector{SSL_GROUP_X25519_MLKEM768, + SSL_GROUP_SECP256R1_MLKEM768, + SSL_GROUP_SECP256R1, SSL_GROUP_X25519})); + EXPECT_EQ(ERR_peek_error(), 0u); +} + +// Every stock crypto-policies value predates ML-KEM and ML-DSA and so names +// neither. Since both setters replace AWS-LC's list rather than intersecting with +// it, such a policy would otherwise strip post-quantum support from every +// context that seeds from it. +TEST_F(CryptoPolicyTest, PolicySilentOnPQKeepsPQDefaults) { + TemporaryFile policy; + ASSERT_TRUE(policy.Init(kDefaultPolicy)); + + bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(ctx); + ssl_ctx_apply_crypto_policy(ctx.get(), policy.path().c_str(), + /*is_dtls=*/false, /*version_locked=*/false); + + for (uint16_t group : + {SSL_GROUP_X25519_MLKEM768, SSL_GROUP_SECP256R1_MLKEM768, + SSL_GROUP_SECP384R1_MLKEM1024}) { + EXPECT_TRUE(Contains(ctx->supported_group_list, group)) << group; + } + for (uint16_t sigalg : + {SSL_SIGN_MLDSA44, SSL_SIGN_MLDSA65, SSL_SIGN_MLDSA87}) { + EXPECT_TRUE(Contains(ctx->cert->sigalgs, sigalg)) << sigalg; + EXPECT_TRUE(Contains(ctx->verify_sigalgs, sigalg)) << sigalg; + } + EXPECT_EQ(ERR_peek_error(), 0u); +} + +// The only way to turn post-quantum off through the policy. The crypto-policies +// directives are plain preference lists with no syntax for excluding an +// algorithm, so silence cannot mean "no". +TEST_F(CryptoPolicyTest, PostQuantumOffDropsPQDefaults) { + const std::string content = + std::string(kDefaultPolicy) + "AWSLC.PostQuantum = off\n"; + TemporaryFile policy; + ASSERT_TRUE(policy.Init(content)); + + bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(ctx); + ssl_ctx_apply_crypto_policy(ctx.get(), policy.path().c_str(), + /*is_dtls=*/false, /*version_locked=*/false); + + EXPECT_EQ(ToVector(ctx->supported_group_list), + (std::vector{SSL_GROUP_X25519, SSL_GROUP_SECP256R1, + SSL_GROUP_SECP521R1, SSL_GROUP_SECP384R1})); + EXPECT_EQ(ctx->verify_sigalgs.size(), 10u); + for (uint16_t sigalg : + {SSL_SIGN_MLDSA44, SSL_SIGN_MLDSA65, SSL_SIGN_MLDSA87}) { + EXPECT_FALSE(Contains(ctx->cert->sigalgs, sigalg)) << sigalg; + } + EXPECT_EQ(ERR_peek_error(), 0u); +} + +// Any other value, including a misspelling, leaves the defaults in force. Turning +// post-quantum off is the surprising outcome, so it takes the exact spelling. +TEST_F(CryptoPolicyTest, PostQuantumOtherValuesKeepPQDefaults) { + for (const char *value : {"on", "ON", "yes", "0", "false", ""}) { + SCOPED_TRACE(value); + const std::string content = std::string(kDefaultPolicy) + + "AWSLC.PostQuantum = " + value + "\n"; + TemporaryFile policy; + ASSERT_TRUE(policy.Init(content)); + + bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(ctx); + ssl_ctx_apply_crypto_policy(ctx.get(), policy.path().c_str(), + /*is_dtls=*/false, /*version_locked=*/false); + + EXPECT_TRUE( + Contains(ctx->supported_group_list, SSL_GROUP_X25519_MLKEM768)); + EXPECT_TRUE(Contains(ctx->cert->sigalgs, SSL_SIGN_MLDSA65)); + } + EXPECT_EQ(ERR_peek_error(), 0u); +} + +// "off" is matched without regard to case, as config-file tokens do not carry it +// reliably. +TEST_F(CryptoPolicyTest, PostQuantumOffIsCaseInsensitive) { + const std::string content = + std::string(kDefaultPolicy) + "AWSLC.PostQuantum = OFF\n"; + TemporaryFile policy; + ASSERT_TRUE(policy.Init(content)); + + bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(ctx); + ssl_ctx_apply_crypto_policy(ctx.get(), policy.path().c_str(), + /*is_dtls=*/false, /*version_locked=*/false); + + EXPECT_FALSE(Contains(ctx->supported_group_list, SSL_GROUP_X25519_MLKEM768)); + EXPECT_FALSE(Contains(ctx->cert->sigalgs, SSL_SIGN_MLDSA65)); + EXPECT_EQ(ERR_peek_error(), 0u); +} + +// Only the hybrids whose classical half the policy kept come back. An operator +// who dropped a curve did not ask for it back inside a hybrid. +TEST_F(CryptoPolicyTest, HybridNeedsItsClassicalHalf) { + const std::string content = "Groups = secp384r1\n"; + TemporaryFile policy; + ASSERT_TRUE(policy.Init(content)); + + bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(ctx); + ssl_ctx_apply_crypto_policy(ctx.get(), policy.path().c_str(), + /*is_dtls=*/false, /*version_locked=*/false); + + EXPECT_EQ(ToVector(ctx->supported_group_list), + (std::vector{SSL_GROUP_SECP384R1_MLKEM1024, + SSL_GROUP_SECP384R1})); + EXPECT_EQ(ERR_peek_error(), 0u); +} + +// A policy that names one post-quantum group has an opinion about them, so the +// others are not added back. +TEST_F(CryptoPolicyTest, PolicyNamingPQGroupIsAuthoritative) { + const std::string content = + "Groups = X25519MLKEM768:X25519:secp256r1:secp384r1\n"; + TemporaryFile policy; + ASSERT_TRUE(policy.Init(content)); + + bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(ctx); + ssl_ctx_apply_crypto_policy(ctx.get(), policy.path().c_str(), + /*is_dtls=*/false, /*version_locked=*/false); + + EXPECT_EQ(ToVector(ctx->supported_group_list), + (std::vector{SSL_GROUP_X25519_MLKEM768, SSL_GROUP_X25519, + SSL_GROUP_SECP256R1, SSL_GROUP_SECP384R1})); + EXPECT_EQ(ERR_peek_error(), 0u); +} + +// Removing a hybrid with the '-' modifier is an opinion about it too, so it +// must not come back with the defaults. The other hybrids do. +TEST_F(CryptoPolicyTest, PolicyRemovingPQGroupKeepsItOut) { + const std::string content = + "Groups = X25519:secp256r1:secp384r1:-X25519MLKEM768\n"; + TemporaryFile policy; + ASSERT_TRUE(policy.Init(content)); + + bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(ctx); + ssl_ctx_apply_crypto_policy(ctx.get(), policy.path().c_str(), + /*is_dtls=*/false, /*version_locked=*/false); + + EXPECT_FALSE(Contains(ctx->supported_group_list, SSL_GROUP_X25519_MLKEM768)); + EXPECT_TRUE( + Contains(ctx->supported_group_list, SSL_GROUP_SECP256R1_MLKEM768)); + EXPECT_TRUE(Contains(ctx->supported_group_list, SSL_GROUP_X25519)); + EXPECT_EQ(ERR_peek_error(), 0u); +} + +TEST_F(CryptoPolicyTest, PolicyNamingMLDSAIsAuthoritative) { + const std::string content = "SignatureAlgorithms = mldsa65:ECDSA+SHA256\n"; + TemporaryFile policy; + ASSERT_TRUE(policy.Init(content)); + + bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(ctx); + ssl_ctx_apply_crypto_policy(ctx.get(), policy.path().c_str(), + /*is_dtls=*/false, /*version_locked=*/false); + + EXPECT_EQ(ToVector(ctx->cert->sigalgs), + (std::vector{SSL_SIGN_MLDSA65, + SSL_SIGN_ECDSA_SECP256R1_SHA256})); + EXPECT_EQ(ERR_peek_error(), 0u); +} + +// The crypto-policies framework hyphenates the post-quantum names and does not +// fix their case, so these spellings must resolve for a PQ-aware policy to be +// recognized as one at all. +TEST_F(CryptoPolicyTest, CryptoPoliciesPQSpellingsResolve) { + const std::string content = + "Groups = X25519-MLKEM768:SECP256R1-MLKEM768:secp384r1-mlkem1024\n" + "SignatureAlgorithms = ML-DSA-44:ml-dsa-65:ML-DSA-87\n"; + TemporaryFile policy; + ASSERT_TRUE(policy.Init(content)); + + bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(ctx); + ssl_ctx_apply_crypto_policy(ctx.get(), policy.path().c_str(), + /*is_dtls=*/false, /*version_locked=*/false); + + EXPECT_EQ(ToVector(ctx->supported_group_list), + (std::vector{SSL_GROUP_X25519_MLKEM768, + SSL_GROUP_SECP256R1_MLKEM768, + SSL_GROUP_SECP384R1_MLKEM1024})); + EXPECT_EQ(ToVector(ctx->cert->sigalgs), + (std::vector{SSL_SIGN_MLDSA44, SSL_SIGN_MLDSA65, + SSL_SIGN_MLDSA87})); EXPECT_EQ(ERR_peek_error(), 0u); } From 554797c75015654a489fd56490f6a267e5876526 Mon Sep 17 00:00:00 2001 From: Will Childs-Klein Date: Mon, 14 Sep 2026 18:50:39 +0000 Subject: [PATCH 09/14] Pin the removal-only Groups interaction with post-quantum off --- ssl/ssl_crypto_policy_test.cc | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/ssl/ssl_crypto_policy_test.cc b/ssl/ssl_crypto_policy_test.cc index 16c71516a44..112f45878bb 100644 --- a/ssl/ssl_crypto_policy_test.cc +++ b/ssl/ssl_crypto_policy_test.cc @@ -720,6 +720,27 @@ TEST_F(CryptoPolicyTest, PostQuantumOtherValuesKeepPQDefaults) { EXPECT_EQ(ERR_peek_error(), 0u); } +// Turning post-quantum off stops the merge; it takes nothing out. A value that +// only removes is applied to the default list either way, so the post-quantum +// groups it says nothing about stay, as they would with no Groups directive. +TEST_F(CryptoPolicyTest, PostQuantumOffKeepsRemovalOnlyGroupsWhole) { + const std::string content = + "AWSLC.PostQuantum = off\n" + "Groups = -SecP256r1MLKEM768\n"; + TemporaryFile policy; + ASSERT_TRUE(policy.Init(content)); + + bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(ctx); + ssl_ctx_apply_crypto_policy(ctx.get(), policy.path().c_str(), + /*is_dtls=*/false, /*version_locked=*/false); + + EXPECT_FALSE( + Contains(ctx->supported_group_list, SSL_GROUP_SECP256R1_MLKEM768)); + EXPECT_TRUE(Contains(ctx->supported_group_list, SSL_GROUP_X25519_MLKEM768)); + EXPECT_EQ(ERR_peek_error(), 0u); +} + // "off" is matched without regard to case, as config-file tokens do not carry it // reliably. TEST_F(CryptoPolicyTest, PostQuantumOffIsCaseInsensitive) { From 1076c8a2669c33f109bbea0a18b3bf629f1d5eed Mon Sep 17 00:00:00 2001 From: Will Childs-Klein Date: Tue, 15 Sep 2026 20:57:37 +0000 Subject: [PATCH 10/14] Parse the whole OpenSSL 3.5 list syntax in Groups and sigalgs Amazon Linux 2023's PQ subpolicy stacks both modifiers on its first group and separates preference tuples with '/', so the groups the operator prefers first were dropped, and its '?'-prefixed ML-DSA entries moved to the end of the signature list. --- ssl/crypto_policy.cc | 60 ++++++++++++------- ssl/ssl_crypto_policy_test.cc | 108 +++++++++++++++++++++++++++++++--- 2 files changed, 138 insertions(+), 30 deletions(-) diff --git a/ssl/crypto_policy.cc b/ssl/crypto_policy.cc index aedbe3631b0..62828cd24b1 100644 --- a/ssl/crypto_policy.cc +++ b/ssl/crypto_policy.cc @@ -153,6 +153,13 @@ uint16_t CryptoPolicyProtoVersion(const char *tok, bool is_dtls) { // AWS-LC implements, which is well under this. constexpr size_t kMaxPolicyIds = 64; +// kPolicyListSeparators end a token in a Groups or SignatureAlgorithms value. +// ':' separates entries, and '/' separates the tuples OpenSSL 3.5 draws its key +// shares from. AWS-LC keeps one flat preference list and chooses its own key +// shares, so a tuple boundary is just another entry boundary; treating it as +// part of a name instead loses the entries on both sides of it. +constexpr char kPolicyListSeparators[] = ":/"; + // PolicyAlias maps one spelling the crypto-policies framework uses to the // spelling AWS-LC's lookups take. |from| is lowercase and matched without regard // to case, which config-file tokens do not carry reliably. @@ -211,19 +218,28 @@ void ResolveAlias(const char **tok, size_t *len, const PolicyAlias *aliases, } } +// StripListModifiers advances |*tok| past the OpenSSL 3.5 list modifiers and +// returns false if they remove the entry. +// +// '*' asks for a key share and '?' tolerates an unimplemented algorithm; AWS-LC +// chooses its own key shares and already skips names it cannot resolve, so both +// need only stripping. A policy stacks them -- Amazon Linux 2023's PQ subpolicy +// leads its group list with "*?X25519MLKEM768" -- so one pass per prefix +// character is not enough. '-' removes the entry, and resolving it here would +// put it back in the list. +bool StripListModifiers(const char **tok, size_t *len) { + while (*len > 0 && (**tok == '*' || **tok == '?')) { + (*tok)++; + (*len)--; + } + return *len == 0 || **tok != '-'; +} + // GroupIdFromToken sets |*out| to the AWS-LC group ID named by the // crypto-policies token |tok|, of length |len|, and returns false if AWS-LC has // no such group. bool GroupIdFromToken(uint16_t *out, const char *tok, size_t len) { - // OpenSSL 3.5 group-list modifiers, which every stock policy puts on its - // first entry: '*' asks for a key share, '?' tolerates an unimplemented - // group, '-' removes one. AWS-LC chooses its own key shares and already skips - // names it cannot resolve, so the first two need only stripping; '-' must not - // put the group back into the list. - if (len > 0 && (tok[0] == '*' || tok[0] == '?')) { - tok++; - len--; - } else if (len > 0 && tok[0] == '-') { + if (!StripListModifiers(&tok, &len)) { return false; } @@ -235,6 +251,10 @@ bool GroupIdFromToken(uint16_t *out, const char *tok, size_t len) { // the crypto-policies token |tok|, of length |len|, and returns false if AWS-LC // has no such algorithm. bool SigalgIdFromToken(uint16_t *out, const char *tok, size_t len) { + if (!StripListModifiers(&tok, &len)) { + return false; + } + ResolveAlias(&tok, &len, kSigalgAliases, OPENSSL_ARRAY_SIZE(kSigalgAliases)); return ssl_sigalg_id_from_name(out, tok, len); } @@ -248,9 +268,9 @@ bool ContainsId(Span ids, uint16_t id) { return false; } -// FilterPolicyIds resolves the ':'-separated tokens of |value| through |lookup| -// and writes the IDs that resolve into |out|, which holds |out_len| entries, in -// the order the policy gave them. It returns how many were written. +// FilterPolicyIds resolves the tokens of |value| through |lookup| and writes the +// IDs that resolve into |out|, which holds |out_len| entries, in the order the +// policy gave them. It returns how many were written. // // The Groups and SignatureAlgorithms setters reject a whole list on the first // entry they do not accept, and a stock crypto-policies value always names @@ -266,9 +286,7 @@ size_t FilterPolicyIds(uint16_t *out, size_t out_len, const char *value, bool (*lookup)(uint16_t *, const char *, size_t)) { size_t out_i = 0; for (const char *tok = value;;) { - const char *end = strchr(tok, ':'); - const size_t len = - end != nullptr ? static_cast(end - tok) : strlen(tok); + const size_t len = strcspn(tok, kPolicyListSeparators); uint16_t id; if (len > 0 && out_i < out_len && lookup(&id, tok, len)) { @@ -281,10 +299,10 @@ size_t FilterPolicyIds(uint16_t *out, size_t out_len, const char *value, } } - if (end == nullptr) { + if (tok[len] == '\0') { break; } - tok = end + 1; + tok += len + 1; } return out_i; } @@ -319,18 +337,16 @@ bool HybridClassicalComponent(uint16_t *out, uint16_t group) { // with the OpenSSL '-' modifier. bool PolicyRemovesGroup(const char *value, uint16_t group) { for (const char *tok = value;;) { - const char *end = strchr(tok, ':'); - const size_t len = - end != nullptr ? static_cast(end - tok) : strlen(tok); + const size_t len = strcspn(tok, kPolicyListSeparators); uint16_t id; if (len > 1 && tok[0] == '-' && GroupIdFromToken(&id, tok + 1, len - 1) && id == group) { return true; } - if (end == nullptr) { + if (tok[len] == '\0') { return false; } - tok = end + 1; + tok += len + 1; } } diff --git a/ssl/ssl_crypto_policy_test.cc b/ssl/ssl_crypto_policy_test.cc index 112f45878bb..868f0e0e9c0 100644 --- a/ssl/ssl_crypto_policy_test.cc +++ b/ssl/ssl_crypto_policy_test.cc @@ -72,6 +72,20 @@ const char kDefaultGroups[] = "*X25519:secp256r1:X448:secp521r1:secp384r1:ffdhe2048:ffdhe3072:ffdhe4096:" "ffdhe6144:ffdhe8192"; +// The two list directives of Amazon Linux 2023's DEFAULT:PQ, verbatim. It is the +// stock policy that exercises the whole OpenSSL 3.5 list syntax: two modifiers on +// one group, a '/' between preference tuples, modifiers on signature algorithms, +// and each post-quantum algorithm spelled twice. +const char kPQSubpolicySigalgs[] = + "?mldsa44:?mldsa65:?mldsa87:ECDSA+SHA256:ECDSA+SHA384:ECDSA+SHA512:ed25519:" + "ed448:rsa_pss_pss_sha256:rsa_pss_pss_sha384:rsa_pss_pss_sha512:" + "rsa_pss_rsae_sha256:rsa_pss_rsae_sha384:rsa_pss_rsae_sha512:RSA+SHA256:" + "RSA+SHA384:RSA+SHA512:ECDSA+SHA224:RSA+SHA224"; +const char kPQSubpolicyGroups[] = + "*?X25519MLKEM768:?x25519_mlkem768:?SecP256r1MLKEM768:?p256_mlkem768:" + "?SecP384r1MLKEM1024:?p384_mlkem1024/*X25519:secp256r1:X448:secp521r1:" + "secp384r1:ffdhe2048:ffdhe3072:ffdhe4096:ffdhe6144:ffdhe8192"; + // A path no policy file will ever occupy, used both to make seeding a no-op and // as the subject of MissingFileIsIgnored. const char kNoSuchPath[] = "/nonexistent/aws-lc/crypto-policy/does-not-exist"; @@ -115,24 +129,24 @@ bool Contains(const Array &haystack, uint16_t needle) { return std::find(haystack.begin(), haystack.end(), needle) != haystack.end(); } -// PolicyRequests reports whether the ':'-separated |value| asks for |name|. A -// '*' or '?' modifier still asks for the group; a '-' removes it, so the token -// is compared with the prefix left on and does not match. +// PolicyRequests reports whether |value|, whose entries are separated by ':' or +// by the tuple boundary '/', asks for |name|. Any number of '*' and '?' modifiers +// still ask for the group; a '-' removes it, so the token is compared with the +// prefix left on and does not match. bool PolicyRequests(const char *value, const char *name) { for (const char *tok = value;;) { - const char *end = strchr(tok, ':'); - size_t len = end != nullptr ? static_cast(end - tok) : strlen(tok); - if (len > 0 && (tok[0] == '*' || tok[0] == '?')) { + size_t len = strcspn(tok, ":/"); + while (len > 0 && (tok[0] == '*' || tok[0] == '?')) { tok++; len--; } if (len == strlen(name) && strncmp(tok, name, len) == 0) { return true; } - if (end == nullptr) { + if (tok[len] == '\0') { return false; } - tok = end + 1; + tok += len + 1; } } @@ -563,6 +577,84 @@ TEST_F(CryptoPolicyTest, GroupListModifiers) { EXPECT_EQ(ERR_peek_error(), 0u); } +// A group can carry both modifiers at once, and the entry after a tuple boundary +// is an entry like any other. Stripping one prefix or splitting on ':' alone +// drops the group the policy asked for first. +TEST_F(CryptoPolicyTest, StackedModifiersAndTupleSeparator) { + // Post-quantum off so the assertion is the policy's own list, without the + // hybrids the defaults would prepend. + const std::string content = + "Groups = *?secp384r1:?secp521r1/*X25519:secp256r1\n" + "AWSLC.PostQuantum = off\n"; + TemporaryFile policy; + ASSERT_TRUE(policy.Init(content)); + + bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(ctx); + ssl_ctx_apply_crypto_policy(ctx.get(), policy.path().c_str(), + /*is_dtls=*/false, /*version_locked=*/false); + + EXPECT_EQ(ToVector(ctx->supported_group_list), + (std::vector{SSL_GROUP_SECP384R1, SSL_GROUP_SECP521R1, + SSL_GROUP_X25519, SSL_GROUP_SECP256R1})); + EXPECT_EQ(ERR_peek_error(), 0u); +} + +// Signature algorithms carry the '?' modifier too. Left on, it drops the +// algorithm, and for ML-DSA the merge then appends what the policy asked for +// first, inverting the operator's order. +TEST_F(CryptoPolicyTest, SigalgListModifiers) { + const std::string content = + "SignatureAlgorithms = ?mldsa44:ECDSA+SHA256:?ed25519\n"; + TemporaryFile policy; + ASSERT_TRUE(policy.Init(content)); + + bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(ctx); + ssl_ctx_apply_crypto_policy(ctx.get(), policy.path().c_str(), + /*is_dtls=*/false, /*version_locked=*/false); + + const std::vector expected = {SSL_SIGN_MLDSA44, + SSL_SIGN_ECDSA_SECP256R1_SHA256, + SSL_SIGN_ED25519}; + EXPECT_EQ(ToVector(ctx->cert->sigalgs), expected); + EXPECT_EQ(ToVector(ctx->verify_sigalgs), expected); + EXPECT_EQ(ERR_peek_error(), 0u); +} + +// Amazon Linux 2023's DEFAULT:PQ as shipped. The policy names post-quantum +// algorithms, so it is authoritative over them: the hybrids and ML-DSA appear +// where it put them, not where the merge would. +TEST_F(CryptoPolicyTest, PQSubpolicyGroupsAndSigalgs) { + const std::string content = std::string("Groups = ") + kPQSubpolicyGroups + + "\nSignatureAlgorithms = " + kPQSubpolicySigalgs + + "\n"; + TemporaryFile policy; + ASSERT_TRUE(policy.Init(content)); + + bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(ctx); + ssl_ctx_apply_crypto_policy(ctx.get(), policy.path().c_str(), + /*is_dtls=*/false, /*version_locked=*/false); + + EXPECT_EQ(ToVector(ctx->supported_group_list), + (std::vector{ + SSL_GROUP_X25519_MLKEM768, SSL_GROUP_SECP256R1_MLKEM768, + SSL_GROUP_SECP384R1_MLKEM1024, SSL_GROUP_X25519, + SSL_GROUP_SECP256R1, SSL_GROUP_SECP521R1, SSL_GROUP_SECP384R1})); + const std::vector expected_sigalgs = { + SSL_SIGN_MLDSA44, SSL_SIGN_MLDSA65, + SSL_SIGN_MLDSA87, SSL_SIGN_ECDSA_SECP256R1_SHA256, + SSL_SIGN_ECDSA_SECP384R1_SHA384, SSL_SIGN_ECDSA_SECP521R1_SHA512, + SSL_SIGN_ED25519, SSL_SIGN_RSA_PSS_RSAE_SHA256, + SSL_SIGN_RSA_PSS_RSAE_SHA384, SSL_SIGN_RSA_PSS_RSAE_SHA512, + SSL_SIGN_RSA_PKCS1_SHA256, SSL_SIGN_RSA_PKCS1_SHA384, + SSL_SIGN_RSA_PKCS1_SHA512}; + EXPECT_EQ(ToVector(ctx->cert->sigalgs), expected_sigalgs); + EXPECT_EQ(ToVector(ctx->verify_sigalgs), expected_sigalgs); + EXPECT_EQ(ERR_peek_error(), 0u); +} + // A value that only removes names no group to keep, so the list it removes from // is the built-in default. Dropping the directive instead would hand back the one // group the operator asked to be rid of. From e2a7a3f3898de9340e0fe363e7b94d4d46865399 Mon Sep 17 00:00:00 2001 From: Will Childs-Klein Date: Mon, 21 Sep 2026 20:15:28 +0000 Subject: [PATCH 11/14] Honor a Groups removal alongside the groups the value keeps A '-' entry now applies whether or not the same value names groups to keep, and takes the ML-KEM hybrid over the removed curve with it. A value that leaves no group at all is dropped, since an empty configured list is how AWS-LC asks for its defaults. --- BUILDING.md | 10 +-- docs/porting/configuration-differences.md | 21 +++--- ssl/crypto_policy.cc | 79 ++++++++++++++++------- ssl/ssl_crypto_policy_test.cc | 41 +++++++++++- 4 files changed, 113 insertions(+), 38 deletions(-) diff --git a/BUILDING.md b/BUILDING.md index 0e95206dfc7..b61a9bd6b2f 100644 --- a/BUILDING.md +++ b/BUILDING.md @@ -150,7 +150,9 @@ policy value names algorithms AWS-LC does not have, such as X448 and the FFDHE groups, and the corresponding setters reject a whole list on the first name they do not recognize; without narrowing, the directive would have no effect at all. The OpenSSL group-list modifiers are honored: `*` and `?` are stripped, since -AWS-LC selects its own key shares, and `-` drops the group it prefixes. +AWS-LC selects its own key shares, and `-` drops the group it prefixes whatever +else the value names. A value that leaves no group at all is not applied, since +AWS-LC reads an empty group list as a request for its defaults. A `MinProtocol` naming a version AWS-LC does not have is the exception: the floor rises to the policy's `MaxProtocol`. Ignoring the directive would leave AWS-LC's @@ -170,9 +172,9 @@ off, add AWS-LC's own directive to the policy file: AWSLC.PostQuantum = off ``` -Hybrid groups come back only when the policy keeps their classical half, so -removing `secp384r1` also removes `SecP384r1MLKEM1024`. A group the policy -removes with `-` stays out. +A hybrid group needs its classical half, so removing `secp384r1` also removes +`SecP384r1MLKEM1024`, whether the policy names the hybrid or not. A group the +policy removes with `-` stays out. AWS-LC reads the file once per process, as OpenSSL reads `openssl.cnf`, so a policy change takes effect only in processes started afterward. A read that fails diff --git a/docs/porting/configuration-differences.md b/docs/porting/configuration-differences.md index 985acb45ed6..91d65f1bf2f 100644 --- a/docs/porting/configuration-differences.md +++ b/docs/porting/configuration-differences.md @@ -60,20 +60,25 @@ as `SSLv3`, keeps the built-in floor. `Groups` and `SignatureAlgorithms` are narrowed to the algorithms AWS-LC implements, in the order the policy gives them, and a directive naming nothing -AWS-LC implements is dropped. In `Groups`, the OpenSSL modifiers `*` and `?` are -stripped from the group they mark and `-` drops it; a value that only removes -groups is applied to AWS-LC's default list. +AWS-LC implements is dropped. + +In `Groups`, the OpenSSL modifiers `*` and `?` are stripped from the group they +mark, and `-` drops the group it marks whatever else the value names. A value that +only removes groups is applied to AWS-LC's default list. A value that leaves no +group at all is not applied, since AWS-LC reads an empty group list as a request +for its defaults. AWS-LC's post-quantum groups (the ML-KEM hybrids) and signature algorithms (ML-DSA) are kept unless the policy speaks about post-quantum algorithms. Every policy the `crypto-policies` framework ships today predates them, and both setters replace AWS-LC's default list rather than intersect with it, so seeding would otherwise strip post-quantum support from every context. A policy that names any -post-quantum algorithm is authoritative and nothing is added back; a hybrid comes -back only when the policy keeps its classical half, and one the policy removes -with `-` stays out. `AWSLC.PostQuantum = off`, a directive of AWS-LC's own, waives -the defaults entirely, since the directives the framework writes are preference -lists with no syntax for excluding an algorithm. +post-quantum algorithm is authoritative and nothing is added back. A hybrid needs +its classical half, so removing that half removes the hybrid whether the policy +names it or not, and one the policy removes with `-` stays out. +`AWSLC.PostQuantum = off`, a directive of AWS-LC's own, waives the defaults +entirely, since the directives the framework writes are preference lists with no +syntax for excluding an algorithm. See [BUILDING.md](../../BUILDING.md) for the build flag and the `AWSLC_CRYPTO_POLICY_FILE` override. diff --git a/ssl/crypto_policy.cc b/ssl/crypto_policy.cc index 62828cd24b1..3963d3f773a 100644 --- a/ssl/crypto_policy.cc +++ b/ssl/crypto_policy.cc @@ -333,9 +333,9 @@ bool HybridClassicalComponent(uint16_t *out, uint16_t group) { return false; } -// PolicyRemovesGroup reports whether the Groups value |value| takes |group| out -// with the OpenSSL '-' modifier. -bool PolicyRemovesGroup(const char *value, uint16_t group) { +// ValueNamesRemoval reports whether the Groups value |value| names |group| with +// the OpenSSL '-' modifier. +bool ValueNamesRemoval(const char *value, uint16_t group) { for (const char *tok = value;;) { const size_t len = strcspn(tok, kPolicyListSeparators); uint16_t id; @@ -350,27 +350,60 @@ bool PolicyRemovesGroup(const char *value, uint16_t group) { } } -// GroupsFromRemovals fills |out|, which holds |out_len| entries, with AWS-LC's -// default groups less the ones the Groups value |value| removes, and returns how -// many were written, or zero if |value| removes no group AWS-LC implements. +// PolicyRemovesGroup reports whether the Groups value |value| takes |group| out. // -// A value that only removes leaves |FilterPolicyIds| nothing to keep, so without -// this the setter is skipped and the group the operator took out comes back with -// the defaults. -size_t GroupsFromRemovals(uint16_t *out, size_t out_len, const char *value) { - bool removed_any = false; - size_t out_i = 0; - for (uint16_t group : tls1_get_default_grouplist()) { - if (PolicyRemovesGroup(value, group)) { - removed_any = true; - continue; +// A hybrid goes out with its classical half. Removing X25519 forbids X25519 key +// exchange, and X25519MLKEM768 still performs it on the wire, so leaving the +// hybrid in place would make the removal cosmetic. +bool PolicyRemovesGroup(const char *value, uint16_t group) { + uint16_t classical; + return ValueNamesRemoval(value, group) || + (HybridClassicalComponent(&classical, group) && + ValueNamesRemoval(value, classical)); +} + +// DropRemovedGroups compacts |ids|, which holds |n| entries, down to the ones the +// Groups value |value| does not remove, and returns how many are left. +size_t DropRemovedGroups(uint16_t *ids, size_t n, const char *value) { + size_t kept = 0; + for (size_t i = 0; i < n; i++) { + if (!PolicyRemovesGroup(value, ids[i])) { + ids[kept++] = ids[i]; } - if (out_i >= out_len) { - return 0; + } + return kept; +} + +// PolicyGroupIds fills |out|, which holds |out_len| entries, with the groups the +// Groups value |value| leaves in force and returns how many were written, or zero +// to leave AWS-LC's defaults implicit. +// +// The groups |value| names are its preference order, and its '-' entries come out +// of that order. A value that only removes is applied to the default list, since +// skipping the directive would hand back the group the operator took out. +// +// A value that removes every group also gets zero: an empty configured list is how +// AWS-LC spells "use the defaults", so a policy leaving no group at all is one this +// layer cannot express and does not apply. +size_t PolicyGroupIds(uint16_t *out, size_t out_len, const char *value) { + size_t n = FilterPolicyIds(out, out_len, value, GroupIdFromToken); + const bool named_any = n != 0; + if (!named_any) { + for (uint16_t group : tls1_get_default_grouplist()) { + if (n >= out_len) { + return 0; + } + out[n++] = group; } - out[out_i++] = group; } - return removed_any ? out_i : 0; + + const size_t kept = DropRemovedGroups(out, n, value); + // A value that removed nothing from the defaults asked for nothing this layer + // can act on, so the defaults stay implicit rather than frozen into |ctx|. + if (kept == 0 || (!named_any && kept == n)) { + return 0; + } + return kept; } // MergeDefaultPQGroups restores AWS-LC's default post-quantum groups at the front @@ -620,11 +653,7 @@ void ApplyPolicyToCtx(SSL_CTX *ctx, const char *path, bool is_dtls, } } if (cfg.groups[0] != '\0') { - size_t n = FilterPolicyIds(ids, OPENSSL_ARRAY_SIZE(ids), cfg.groups, - GroupIdFromToken); - if (n == 0) { - n = GroupsFromRemovals(ids, OPENSSL_ARRAY_SIZE(ids), cfg.groups); - } + size_t n = PolicyGroupIds(ids, OPENSSL_ARRAY_SIZE(ids), cfg.groups); if (keep_pq) { n = MergeDefaultPQGroups(ids, n, OPENSSL_ARRAY_SIZE(ids), cfg.groups); } diff --git a/ssl/ssl_crypto_policy_test.cc b/ssl/ssl_crypto_policy_test.cc index 868f0e0e9c0..690890184fb 100644 --- a/ssl/ssl_crypto_policy_test.cc +++ b/ssl/ssl_crypto_policy_test.cc @@ -658,6 +658,9 @@ TEST_F(CryptoPolicyTest, PQSubpolicyGroupsAndSigalgs) { // A value that only removes names no group to keep, so the list it removes from // is the built-in default. Dropping the directive instead would hand back the one // group the operator asked to be rid of. +// +// X25519MLKEM768 goes with X25519 because it performs X25519 key exchange, which +// the operator just forbade. TEST_F(CryptoPolicyTest, RemovalOnlyGroupsRemoves) { const std::string content = "Groups = -X25519\n"; TemporaryFile policy; @@ -670,7 +673,7 @@ TEST_F(CryptoPolicyTest, RemovalOnlyGroupsRemoves) { std::vector expected; for (uint16_t group : tls1_get_default_grouplist()) { - if (group != SSL_GROUP_X25519) { + if (group != SSL_GROUP_X25519 && group != SSL_GROUP_X25519_MLKEM768) { expected.push_back(group); } } @@ -679,6 +682,42 @@ TEST_F(CryptoPolicyTest, RemovalOnlyGroupsRemoves) { EXPECT_EQ(ERR_peek_error(), 0u); } +// A removal is honored alongside the groups the same value keeps. Reading it only +// when nothing else resolves would make the removal depend on the rest of the +// list, which is not what the '-' modifier says. +TEST_F(CryptoPolicyTest, RemovalAmongNamedGroupsApplies) { + const std::string content = "Groups = X25519:secp256r1:-secp256r1\n"; + TemporaryFile policy; + ASSERT_TRUE(policy.Init(content)); + + bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(ctx); + ssl_ctx_apply_crypto_policy(ctx.get(), policy.path().c_str(), + /*is_dtls=*/false, /*version_locked=*/false); + + // Only the hybrid over X25519 is merged back, since P-256 is out. + EXPECT_EQ(ToVector(ctx->supported_group_list), + (std::vector{SSL_GROUP_X25519_MLKEM768, SSL_GROUP_X25519})); + EXPECT_EQ(ERR_peek_error(), 0u); +} + +// Removing every classical curve removes every hybrid with it, leaving no group at +// all. AWS-LC reads an empty configured list as "use the defaults", so such a +// policy cannot be expressed and is left unapplied rather than half-applied. +TEST_F(CryptoPolicyTest, RemovalOfEveryGroupKeepsDefaults) { + const std::string content = "Groups = -X25519:-secp256r1:-secp384r1\n"; + TemporaryFile policy; + ASSERT_TRUE(policy.Init(content)); + + bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(ctx); + ssl_ctx_apply_crypto_policy(ctx.get(), policy.path().c_str(), + /*is_dtls=*/false, /*version_locked=*/false); + + EXPECT_TRUE(ctx->supported_group_list.empty()); + EXPECT_EQ(ERR_peek_error(), 0u); +} + // A removal AWS-LC cannot resolve leaves the defaults implicit, since spelling // them out would freeze today's list into every context the policy touches. TEST_F(CryptoPolicyTest, RemovalOfUnknownGroupKeepsDefaults) { From 6b582c59a033a3edda84d16a1b60507f79290101 Mon Sep 17 00:00:00 2001 From: Will Childs-Klein Date: Mon, 21 Sep 2026 20:23:24 +0000 Subject: [PATCH 12/14] Check the system policy test against the library's own resolver The test scanned the policy value with a parser written in the test file, so a spelling the seeding resolver drops was also one the test never asked about. It now resolves the value through the seeding path and requires every ID that comes back. --- ssl/crypto_policy.cc | 11 ++++++ ssl/internal.h | 15 ++++++++ ssl/libssl.map | 2 ++ ssl/libssl.txt | 2 ++ ssl/ssl_crypto_policy_test.cc | 67 ++++++++++------------------------- 5 files changed, 48 insertions(+), 49 deletions(-) diff --git a/ssl/crypto_policy.cc b/ssl/crypto_policy.cc index 3963d3f773a..a92037e2f0c 100644 --- a/ssl/crypto_policy.cc +++ b/ssl/crypto_policy.cc @@ -665,6 +665,17 @@ void ApplyPolicyToCtx(SSL_CTX *ctx, const char *path, bool is_dtls, } // namespace +size_t ssl_crypto_policy_named_group_ids(uint16_t *out, size_t max_out, + const char *value) { + const size_t n = FilterPolicyIds(out, max_out, value, GroupIdFromToken); + return DropRemovedGroups(out, n, value); +} + +size_t ssl_crypto_policy_named_sigalg_ids(uint16_t *out, size_t max_out, + const char *value) { + return FilterPolicyIds(out, max_out, value, SigalgIdFromToken); +} + bool ssl_crypto_policy_parse_file(const char *path, CryptoPolicyConfig *out) { if (path == nullptr || out == nullptr) { return false; diff --git a/ssl/internal.h b/ssl/internal.h index 4984746f112..700dde6680c 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -3773,6 +3773,21 @@ OPENSSL_EXPORT const char *ssl_crypto_policy_default_path(void); // rejects the whole list on the first token it does not know. bool ssl_sigalg_id_from_name(uint16_t *out, const char *name, size_t len); +// ssl_crypto_policy_named_group_ids and ssl_crypto_policy_named_sigalg_ids +// resolve the tokens of a Groups or SignatureAlgorithms value the way seeding +// does, writing the IDs the value asks for and AWS-LC implements to |out|, which +// holds |max_out| entries, and returning how many were written. A group the value +// removes is not one it asks for. +// +// Marked with OPENSSL_EXPORT to make it available for unit tests, which read the +// system policy file with it rather than assuming how it spells an algorithm. +OPENSSL_EXPORT size_t ssl_crypto_policy_named_group_ids(uint16_t *out, + size_t max_out, + const char *value); +OPENSSL_EXPORT size_t ssl_crypto_policy_named_sigalg_ids(uint16_t *out, + size_t max_out, + const char *value); + // ssl_ctx_apply_crypto_policy seeds |ctx| from the crypto-policies OpenSSL // back-end file at |path|. It is best-effort and never fails: a missing or // malformed file, or a directive AWS-LC rejects, leaves the corresponding diff --git a/ssl/libssl.map b/ssl/libssl.map index 485e6cceb8a..14b05c2e290 100644 --- a/ssl/libssl.map +++ b/ssl/libssl.map @@ -638,6 +638,8 @@ AWS_LC_1.0 { bssl::ssl_cert_check_key_usage*; bssl::ssl_client_hello_init*; bssl::ssl_crypto_policy_default_path*; + bssl::ssl_crypto_policy_named_group_ids*; + bssl::ssl_crypto_policy_named_sigalg_ids*; bssl::ssl_crypto_policy_parse_file*; bssl::ssl_ctx_apply_crypto_policy*; bssl::ssl_decode_client_hello_inner*; diff --git a/ssl/libssl.txt b/ssl/libssl.txt index 3f3226de3b1..074d57d9451 100644 --- a/ssl/libssl.txt +++ b/ssl/libssl.txt @@ -628,6 +628,8 @@ bssl::SSL_set_handoff_mode AWS_LC_1.0 PUBLIC_CXX bssl::ssl_cert_check_key_usage AWS_LC_1.0 PRIVATE_CXX bssl::ssl_client_hello_init AWS_LC_1.0 PRIVATE_CXX bssl::ssl_crypto_policy_default_path AWS_LC_1.0 PRIVATE_CXX +bssl::ssl_crypto_policy_named_group_ids AWS_LC_1.0 PRIVATE_CXX +bssl::ssl_crypto_policy_named_sigalg_ids AWS_LC_1.0 PRIVATE_CXX bssl::ssl_crypto_policy_parse_file AWS_LC_1.0 PRIVATE_CXX bssl::ssl_ctx_apply_crypto_policy AWS_LC_1.0 PRIVATE_CXX bssl::ssl_decode_client_hello_inner AWS_LC_1.0 PRIVATE_CXX diff --git a/ssl/ssl_crypto_policy_test.cc b/ssl/ssl_crypto_policy_test.cc index 690890184fb..1455df4ff7d 100644 --- a/ssl/ssl_crypto_policy_test.cc +++ b/ssl/ssl_crypto_policy_test.cc @@ -129,27 +129,6 @@ bool Contains(const Array &haystack, uint16_t needle) { return std::find(haystack.begin(), haystack.end(), needle) != haystack.end(); } -// PolicyRequests reports whether |value|, whose entries are separated by ':' or -// by the tuple boundary '/', asks for |name|. Any number of '*' and '?' modifiers -// still ask for the group; a '-' removes it, so the token is compared with the -// prefix left on and does not match. -bool PolicyRequests(const char *value, const char *name) { - for (const char *tok = value;;) { - size_t len = strcspn(tok, ":/"); - while (len > 0 && (tok[0] == '*' || tok[0] == '?')) { - tok++; - len--; - } - if (len == strlen(name) && strncmp(tok, name, len) == 0) { - return true; - } - if (tok[len] == '\0') { - return false; - } - tok += len + 1; - } -} - bool CtxHasCipherNamed(const SSL_CTX *ctx, const char *name) { const STACK_OF(SSL_CIPHER) *ciphers = SSL_CTX_get_ciphers(ctx); for (size_t i = 0; i < sk_SSL_CIPHER_num(ciphers); i++) { @@ -1719,40 +1698,30 @@ TEST_F(CryptoPolicySystemTest, SeedsVersionBoundsAndCiphers) { // crypto-policies spells a group in a way AWS-LC cannot read. Every group and // signature algorithm the system policy asks for that AWS-LC implements must // survive into the seeded context. +// +// Which entries those are comes from the library's own resolver. A scan written +// here would carry this file's idea of the syntax, and so would fall silent on +// exactly the spellings a parser bug loses. TEST_F(CryptoPolicySystemTest, SeedsGroupsAndSigalgs) { - static const struct { - const char *name; - uint16_t id; - } kGroups[] = { - {"X25519", SSL_GROUP_X25519}, - {"secp256r1", SSL_GROUP_SECP256R1}, - {"secp384r1", SSL_GROUP_SECP384R1}, - {"secp521r1", SSL_GROUP_SECP521R1}, - }; - static const struct { - const char *name; - uint16_t id; - } kSigalgs[] = { - {"ECDSA+SHA256", SSL_SIGN_ECDSA_SECP256R1_SHA256}, - {"ECDSA+SHA384", SSL_SIGN_ECDSA_SECP384R1_SHA384}, - {"ECDSA+SHA512", SSL_SIGN_ECDSA_SECP521R1_SHA512}, - {"ed25519", SSL_SIGN_ED25519}, - {"rsa_pss_rsae_sha256", SSL_SIGN_RSA_PSS_RSAE_SHA256}, - {"RSA+SHA256", SSL_SIGN_RSA_PKCS1_SHA256}, - }; - bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); ASSERT_TRUE(ctx); - for (const auto &group : kGroups) { - if (PolicyRequests(cfg_.groups, group.name)) { - EXPECT_TRUE(Contains(ctx->supported_group_list, group.id)) << group.name; + uint16_t ids[64]; + if (cfg_.groups[0] != '\0') { + const size_t num_groups = ssl_crypto_policy_named_group_ids( + ids, OPENSSL_ARRAY_SIZE(ids), cfg_.groups); + EXPECT_GT(num_groups, 0u) << cfg_.groups; + for (size_t i = 0; i < num_groups; i++) { + EXPECT_TRUE(Contains(ctx->supported_group_list, ids[i])) << ids[i]; } } - for (const auto &sigalg : kSigalgs) { - if (PolicyRequests(cfg_.sigalgs, sigalg.name)) { - EXPECT_TRUE(Contains(ctx->verify_sigalgs, sigalg.id)) << sigalg.name; - EXPECT_TRUE(Contains(ctx->cert->sigalgs, sigalg.id)) << sigalg.name; + if (cfg_.sigalgs[0] != '\0') { + const size_t num_sigalgs = ssl_crypto_policy_named_sigalg_ids( + ids, OPENSSL_ARRAY_SIZE(ids), cfg_.sigalgs); + EXPECT_GT(num_sigalgs, 0u) << cfg_.sigalgs; + for (size_t i = 0; i < num_sigalgs; i++) { + EXPECT_TRUE(Contains(ctx->verify_sigalgs, ids[i])) << ids[i]; + EXPECT_TRUE(Contains(ctx->cert->sigalgs, ids[i])) << ids[i]; } } EXPECT_EQ(ERR_peek_error(), 0u); From febf8ceb7c526362b1d2bd5f61aa5ec374da3dba Mon Sep 17 00:00:00 2001 From: Will Childs-Klein Date: Tue, 15 Sep 2026 21:02:12 +0000 Subject: [PATCH 13/14] Document how policy seeding compares with Fedora and OpenSSL --- BUILDING.md | 25 +++-- docs/crypto-policies.md | 112 ++++++++++++++++++++++ docs/porting/configuration-differences.md | 38 ++++---- 3 files changed, 147 insertions(+), 28 deletions(-) create mode 100644 docs/crypto-policies.md diff --git a/BUILDING.md b/BUILDING.md index b61a9bd6b2f..26d0c84c773 100644 --- a/BUILDING.md +++ b/BUILDING.md @@ -149,10 +149,15 @@ implements before being applied, keeping the operator's preference order. A stoc policy value names algorithms AWS-LC does not have, such as X448 and the FFDHE groups, and the corresponding setters reject a whole list on the first name they do not recognize; without narrowing, the directive would have no effect at all. -The OpenSSL group-list modifiers are honored: `*` and `?` are stripped, since -AWS-LC selects its own key shares, and `-` drops the group it prefixes whatever -else the value names. A value that leaves no group at all is not applied, since -AWS-LC reads an empty group list as a request for its defaults. +The OpenSSL 3.5 list modifiers are honored: `*` and `?` are stripped, since AWS-LC +selects its own key shares and tolerates every name, `-` drops the entry it +prefixes whatever else the value names, and `/` bounds an entry like `:` does, +since AWS-LC keeps one flat preference list. +[docs/crypto-policies.md](docs/crypto-policies.md) sets this against what OpenSSL +does with the same file. + +A `Groups` value that leaves no group at all is not applied, since AWS-LC reads an +empty group list as a request for its defaults. A `MinProtocol` naming a version AWS-LC does not have is the exception: the floor rises to the policy's `MaxProtocol`. Ignoring the directive would leave AWS-LC's @@ -161,12 +166,12 @@ the context would offer the versions the policy forbids. A `MinProtocol` older than TLS 1.0, such as `SSLv3`, keeps the built-in floor, which is already stricter. -AWS-LC's post-quantum algorithms survive a policy that says nothing about them. -Every policy the framework ships today predates ML-KEM and ML-DSA, and the -setters replace AWS-LC's defaults rather than intersect with them, so seeding -would otherwise downgrade every context. A policy that names any post-quantum -algorithm is taken at its word and nothing is added back. To turn post-quantum -off, add AWS-LC's own directive to the policy file: +AWS-LC's post-quantum algorithms survive a policy that says nothing about them, +which includes Amazon Linux 2023's `DEFAULT`. The setters replace AWS-LC's +defaults rather than intersect with them, so seeding would otherwise downgrade +every context. A policy that names any post-quantum algorithm is taken at its +word and nothing is added back. To turn post-quantum off, add AWS-LC's own +directive to the policy file: ``` AWSLC.PostQuantum = off diff --git a/docs/crypto-policies.md b/docs/crypto-policies.md new file mode 100644 index 00000000000..e06d87bfd01 --- /dev/null +++ b/docs/crypto-policies.md @@ -0,0 +1,112 @@ +# System Crypto Policies + +Built with `-DENABLE_CRYPTO_POLICIES=ON`, AWS-LC seeds every new `SSL_CTX` from the +system-wide `crypto-policies` policy. This document compares that behavior with +what the framework expects of a back-end and with what OpenSSL does with the same +file. See [BUILDING.md](../BUILDING.md) for the build flag, the path overrides, and +AWS-LC's own `AWSLC.PostQuantum` directive. + +## The framework + +`crypto-policies` is a Fedora project ([documentation][fedora-docs], +[source][upstream]) shipped by Fedora, RHEL, and Amazon Linux 2023. An +administrator selects one policy, optionally with subpolicies +(`update-crypto-policies --set DEFAULT:PQ`), and the framework renders it into one +configuration file per consumer under `/etc/crypto-policies/back-ends/`. Amazon +Linux 2023 ships fourteen, covering BIND, GnuTLS, Java, Kerberos, Libreswan, +libssh, NSS, OpenSSH, OpenSSL, and RPM. + +AWS-LC reads exactly one of them: `opensslcnf.config`, the OpenSSL +configuration-file back-end, which is a symlink into +`/usr/share/crypto-policies//`. The `openssl.config` and +`openssl_fips.config` back-ends drive the `openssl` command-line tool; AWS-LC +ignores those along with every back-end belonging to another library. + +## Directives + +| Directive | AWS-LC | +| --- | --- | +| `CipherString` | applied as the TLS 1.2 and below cipher list; an `@SECLEVEL=N` prefix is parsed and dropped | +| `Ciphersuites` | applied as the TLS 1.3 cipher suites | +| `TLS.MinProtocol`, `TLS.MaxProtocol` | applied as the version bounds of a TLS context | +| `DTLS.MinProtocol`, `DTLS.MaxProtocol` | applied as the version bounds of a DTLS context | +| `Groups` | filtered to the groups AWS-LC implements, in the policy's order | +| `SignatureAlgorithms` | filtered to the algorithms AWS-LC implements, in the policy's order | +| anything else | ignored | + +Seeding cannot fail an application's startup. A missing or unreadable file, an +unparsable line, a value longer than AWS-LC's buffers, and a directive AWS-LC +cannot satisfy all leave the built-in default in force. A context created from a +version-locked method such as `TLSv1_2_method` keeps its pinned version and takes +no bounds from the policy. + +## List syntax + +`Groups` and `SignatureAlgorithms` carry OpenSSL 3.5 list syntax, and a stock +policy uses all of it. Amazon Linux 2023 renders `DEFAULT:PQ` with two modifiers on +its first group and a tuple boundary in the middle of the list: + +``` +Groups = *?X25519MLKEM768:?x25519_mlkem768:?SecP256r1MLKEM768:?p256_mlkem768:?SecP384r1MLKEM1024:?p384_mlkem1024/*X25519:secp256r1:X448:secp521r1:secp384r1:ffdhe2048:ffdhe3072:ffdhe4096:ffdhe6144:ffdhe8192 +``` + +| Syntax | OpenSSL | AWS-LC | +| --- | --- | --- | +| `*` | send a key share for this group | stripped; AWS-LC selects its own key shares | +| `?` | tolerate a name the library does not implement | stripped; AWS-LC tolerates every name | +| `-` | remove the entry | removes it from AWS-LC's default list | +| `/` | separate preference tuples | another entry boundary; AWS-LC keeps one flat preference list | +| a name the library does not implement | rejects the whole list unless the name carries `?` | dropped, keeping the rest of the list | + +The last row is why the lists are filtered rather than applied verbatim: every +stock policy names something AWS-LC does not implement -- X448 and the FFDHE +groups, Ed448, the RSA-PSS-PSS algorithms, the SHA-224 pairs -- and AWS-LC's +setters, like OpenSSL's, reject a list on the first name they do not recognize. +Applying such a value as written would silently discard the operator's whole +preference order. + +## Against OpenSSL on the same host + +| | OpenSSL 3.5 on Amazon Linux 2023 | AWS-LC with `-DENABLE_CRYPTO_POLICIES=ON` | +| --- | --- | --- | +| Reaches the policy through | an `.include` of the back-end file from `/etc/pki/tls/openssl.cnf` | opening the back-end file directly | +| Applies it | once, when the library reads its configuration | in each `SSL_CTX_new` | +| The rest of `openssl.cnf` | honored | ignored; AWS-LC reads no other system configuration | +| `@SECLEVEL=N` | enforced | parsed and dropped; AWS-LC has no security levels | +| Post-quantum under a policy silent about it | none offered | AWS-LC's own defaults stay | + +## Post-quantum + +Upstream Fedora ships a `NO-PQ` subpolicy, which removes every ML-KEM group and +ML-DSA algorithm, and no `PQ` one: post-quantum key exchange is in its `DEFAULT` +and an operator opts out of it. Amazon Linux 2023 and RHEL 9 invert that. Their +`DEFAULT` says nothing about post-quantum algorithms and an opt-in `PQ` subpolicy +adds them. + +AWS-LC's group and signature-algorithm setters replace its defaults rather than +intersect with them, so a policy silent about post-quantum algorithms would strip +ML-KEM and ML-DSA from every context. AWS-LC keeps its own when the policy names +none, and stands aside when the policy names any: under `DEFAULT:PQ` the operator's +order wins, ML-KEM hybrids first among the groups and ML-DSA ahead of the classical +signature algorithms. + +The `PQ` module also names `MLKEM1024-X448` and the composite ML-DSA signatures, +which AWS-LC does not implement. Neither reaches the OpenSSL back-end file, since +OpenSSL 3.5 does not implement them either, so on Amazon Linux 2023 the rendered +policy names only post-quantum algorithms AWS-LC has. + +## Amazon Linux 2023 + +Amazon Linux 2023 ships four policies -- `DEFAULT`, `FIPS`, `FUTURE`, `LEGACY` -- +and eight subpolicy modules, `PQ` among them. All four policies set +`TLS.MinProtocol = TLSv1.2` and `TLS.MaxProtocol = TLSv1.3`, so seeding raises a +context's floor from AWS-LC's TLS 1.0 and leaves its ceiling where it was. They +differ in cipher string, in security level, and in groups: `FIPS` names neither +X25519 nor Ed25519, and `FUTURE` drops `ffdhe2048` and requires `@SECLEVEL=3`, +which AWS-LC ignores. + +The `FIPS` policy narrows what a context negotiates and nothing more. A +FIPS-validated build of AWS-LC is a separate matter, selected with `-DFIPS=1`. + +[fedora-docs]: https://docs.fedoraproject.org/en-US/security/cryptography/policies/ +[upstream]: https://gitlab.com/redhat-crypto/fedora-crypto-policies diff --git a/docs/porting/configuration-differences.md b/docs/porting/configuration-differences.md index 91d65f1bf2f..3221d5308b9 100644 --- a/docs/porting/configuration-differences.md +++ b/docs/porting/configuration-differences.md @@ -60,27 +60,29 @@ as `SSLv3`, keeps the built-in floor. `Groups` and `SignatureAlgorithms` are narrowed to the algorithms AWS-LC implements, in the order the policy gives them, and a directive naming nothing -AWS-LC implements is dropped. - -In `Groups`, the OpenSSL modifiers `*` and `?` are stripped from the group they -mark, and `-` drops the group it marks whatever else the value names. A value that -only removes groups is applied to AWS-LC's default list. A value that leaves no -group at all is not applied, since AWS-LC reads an empty group list as a request -for its defaults. +AWS-LC implements is dropped. The OpenSSL 3.5 list modifiers `*` and `?` are +stripped from the entry they mark, `-` drops the entry it marks whatever else the +value names, and `/` bounds an entry as `:` does. A `Groups` value that only +removes groups is applied to AWS-LC's default list. A value that leaves no group at +all is not applied, since AWS-LC reads an empty group list as a request for its +defaults. AWS-LC's post-quantum groups (the ML-KEM hybrids) and signature algorithms -(ML-DSA) are kept unless the policy speaks about post-quantum algorithms. Every -policy the `crypto-policies` framework ships today predates them, and both setters -replace AWS-LC's default list rather than intersect with it, so seeding would -otherwise strip post-quantum support from every context. A policy that names any -post-quantum algorithm is authoritative and nothing is added back. A hybrid needs -its classical half, so removing that half removes the hybrid whether the policy -names it or not, and one the policy removes with `-` stays out. -`AWSLC.PostQuantum = off`, a directive of AWS-LC's own, waives the defaults -entirely, since the directives the framework writes are preference lists with no -syntax for excluding an algorithm. +(ML-DSA) are kept unless the policy speaks about post-quantum algorithms, which +Amazon Linux 2023's `DEFAULT` does not. Both setters replace AWS-LC's default list +rather than intersect with it, so seeding would otherwise strip post-quantum +support from every context. A policy that names any post-quantum algorithm is +authoritative and nothing is added back. A hybrid needs its classical half, so +removing that half removes the hybrid whether the policy names it or not, and one +the policy removes with `-` stays out. `AWSLC.PostQuantum = off`, a directive of +AWS-LC's own, waives the defaults entirely, since the directives the framework +writes are preference lists with no syntax for excluding an algorithm. + + See [BUILDING.md](../../BUILDING.md) for the build flag and the -`AWSLC_CRYPTO_POLICY_FILE` override. +`AWSLC_CRYPTO_POLICY_FILE` override, and +[docs/crypto-policies.md](../crypto-policies.md) for how this compares with the +`crypto-policies` framework's own expectations and with OpenSSL on the same host. Date: Thu, 17 Sep 2026 16:09:48 +0000 Subject: [PATCH 14/14] Document the raised floor in the crypto-policies comparison --- docs/crypto-policies.md | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/docs/crypto-policies.md b/docs/crypto-policies.md index e06d87bfd01..214e3bcbbd5 100644 --- a/docs/crypto-policies.md +++ b/docs/crypto-policies.md @@ -28,8 +28,8 @@ ignores those along with every back-end belonging to another library. | --- | --- | | `CipherString` | applied as the TLS 1.2 and below cipher list; an `@SECLEVEL=N` prefix is parsed and dropped | | `Ciphersuites` | applied as the TLS 1.3 cipher suites | -| `TLS.MinProtocol`, `TLS.MaxProtocol` | applied as the version bounds of a TLS context | -| `DTLS.MinProtocol`, `DTLS.MaxProtocol` | applied as the version bounds of a DTLS context | +| `TLS.MinProtocol`, `TLS.MaxProtocol` | applied as the version bounds of a TLS context; a `MinProtocol` AWS-LC cannot resolve raises the floor to `MaxProtocol` | +| `DTLS.MinProtocol`, `DTLS.MaxProtocol` | applied as the version bounds of a DTLS context, under the same rule | | `Groups` | filtered to the groups AWS-LC implements, in the policy's order | | `SignatureAlgorithms` | filtered to the algorithms AWS-LC implements, in the policy's order | | anything else | ignored | @@ -40,6 +40,12 @@ cannot satisfy all leave the built-in default in force. A context created from a version-locked method such as `TLSv1_2_method` keeps its pinned version and takes no bounds from the policy. +A `MinProtocol` is the one directive that does not simply fall back. AWS-LC's +built-in floor of TLS 1.0 sits below any floor a policy can ask for, so leaving it +would offer the versions the policy forbids. A `MinProtocol` AWS-LC cannot resolve +therefore raises the floor to `MaxProtocol`, and one naming a protocol older than +TLS 1.0 keeps the built-in floor, which is already stricter. + ## List syntax `Groups` and `SignatureAlgorithms` carry OpenSSL 3.5 list syntax, and a stock @@ -54,7 +60,7 @@ Groups = *?X25519MLKEM768:?x25519_mlkem768:?SecP256r1MLKEM768:?p256_mlkem768:?Se | --- | --- | --- | | `*` | send a key share for this group | stripped; AWS-LC selects its own key shares | | `?` | tolerate a name the library does not implement | stripped; AWS-LC tolerates every name | -| `-` | remove the entry | removes it from AWS-LC's default list | +| `-` | remove the entry | removes the entry, and any ML-KEM hybrid over it | | `/` | separate preference tuples | another entry boundary; AWS-LC keeps one flat preference list | | a name the library does not implement | rejects the whole list unless the name carries `?` | dropped, keeping the rest of the list |