Add opt-in system crypto-policies seeding for SSL_CTX (broad approach) - #3442
Closed
WillChilds-Klein wants to merge 5 commits into
Closed
Add opt-in system crypto-policies seeding for SSL_CTX (broad approach)#3442WillChilds-Klein wants to merge 5 commits into
WillChilds-Klein wants to merge 5 commits into
Conversation
WillChilds-Klein
temporarily deployed
to
auto-approve
August 21, 2026 16:27 — with
GitHub Actions
Inactive
WillChilds-Klein
temporarily deployed
to
auto-approve
August 21, 2026 16:27 — with
GitHub Actions
Inactive
WillChilds-Klein
temporarily deployed
to
auto-approve
August 21, 2026 16:27 — with
GitHub Actions
Inactive
WillChilds-Klein
temporarily deployed
to
auto-approve
August 21, 2026 16:27 — with
GitHub Actions
Inactive
WillChilds-Klein
temporarily deployed
to
auto-approve
August 21, 2026 16:27 — with
GitHub Actions
Inactive
WillChilds-Klein
temporarily deployed
to
auto-approve
August 21, 2026 16:27 — with
GitHub Actions
Inactive
WillChilds-Klein
temporarily deployed
to
auto-approve
August 21, 2026 16:27 — with
GitHub Actions
Inactive
Contributor
|
🔒 Security Review — View Report Please review before merging. |
WillChilds-Klein
force-pushed
the
feature/crypto-policies-support
branch
from
September 1, 2026 20:38
41feace to
1c54250
Compare
WillChilds-Klein
force-pushed
the
feature/crypto-policies-support
branch
from
September 1, 2026 20:54
1c54250 to
6a8cad8
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3442 +/- ##
==========================================
+ Coverage 78.11% 78.13% +0.01%
==========================================
Files 700 700
Lines 125599 125663 +64
Branches 17372 17382 +10
==========================================
+ Hits 98117 98188 +71
+ Misses 26611 26604 -7
Partials 871 871 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Address five correctness and security findings in the opt-in seeding path: - Skip the policy's protocol bounds for version-locked SSL_METHODs. The public setters validate against the protocol method's whole range, not the pin SSL_CTX_new installs, so a policy could widen a context the caller deliberately restricted to one version. - Evaluate a cipher rule into a throwaway list before applying it. ssl_create_cipher_list installs its result before reporting failure and the TLS 1.3 merge is then skipped, so ignoring the return value left the context with no ciphers rather than the built-in defaults. - Drop std::string from libssl. STYLE.md forbids depending on the C++ runtime there, and a throwing allocator has no business inside a C entry point. Directive values now live in fixed buffers, and a value too long to hold is dropped rather than truncated. - Preserve the caller's error queue. ERR_clear_error discarded entries queued before SSL_CTX_new was ever called; mark and pop instead. - Ignore the AWSLC_CRYPTO_POLICY_FILE override when the real and effective ids differ, so the environment cannot pick the TLS policy a set-uid or set-gid process runs under.
Three fixes from review of the crypto-policies seeding path. Seeding used ERR_set_mark/ERR_pop_to_mark to hide its own errors, but a mark is a single flag on one queue entry rather than a stack, so a caller that bracketed SSL_CTX_new in its own mark had that mark cleared and lost its errors to the next ERR_pop_to_mark. Save and restore the queue instead. That in turn needs err_copy to carry the mark bit, or the same loss happens through any save/restore. The version-bound setters validate each bound against the protocol method's whole range and never against each other, so a policy whose floor sat above its ceiling was accepted and left every later handshake failing with NO_SUPPORTED_VERSIONS_ENABLED. Resolve both bounds first and apply them only if the range is non-empty, comparing protocol versions rather than wire versions since DTLS wire values run backwards. The tests seeded SSL_CTX objects from whatever policy the host had installed, so assertions about built-in defaults held only on machines without crypto-policies -- everywhere except the platforms this feature targets. Pin the policy path per test.
…queue Narrow the SignatureAlgorithms and Groups directives to the algorithms AWS-LC implements before applying them, keeping the order the policy gave them. A stock crypto-policies value names algorithms AWS-LC lacks, and the setters reject a whole list on the first name they do not recognize, so applying such a value as written discarded the operator's entire preference order. Translate secp256r1, which AWS-LC knows only as P-256. Add ERR_num_errors and ERR_pop_to_count and use them to drop the errors seeding queues. Saving and restoring the queue instead rebuilt every surviving entry, which dangles a data pointer the caller already holds and drops a mark the caller set.
This was referenced Sep 9, 2026
Contributor
Author
|
Superseded by a five-PR stack, to review and merge bottom-up:
The stack is not a straight split. Two changes it makes that this PR did not:
|
WillChilds-Klein
added a commit
that referenced
this pull request
Sep 11, 2026
Stack, split out of #3442, to merge bottom-up: 1. **#3501 -- error-queue primitives (this PR)** 2. #3502 -- policy file reader 3. #3503 -- cipher lists and version bounds 4. #3504 -- groups and signature algorithms 5. #3505 -- post-quantum defaults ## Description - Adds `ERR_num_errors` and `ERR_pop_to_count`, so code that calls into libcrypto on a caller's behalf can drop the errors it raised and leave the queue it was handed untouched. - The existing mark APIs cannot do this. `ERR_set_mark` needs an entry to mark, so it is a no-op on an empty queue, and popping to a mark consumes one the caller had already set. - `ERR_clear_error` and `ERR_restore_state` rebuild the queue, which dangles the data pointer the caller got from its last `ERR_get_error_line_data`. - A count is a position rather than a mark, so it nests inside a caller's mark without disturbing it. ## Testing / verification - New error-queue tests cover popping back to a recorded count, a count taken from an empty queue, and a count at or above the queue's length. - One case fills the ring past its capacity to confirm a stale count leaves the caller's errors alone. - One case wraps a nested call in the caller's own mark and an error carrying a data string, then checks the mark still pops and the string is intact. - One case pins that a mark does not survive `ERR_save_state`/`ERR_restore_state`, since a snapshot can be restored many times and would re-arm a mark nobody set. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issues:
No existing issue. This is a draft to solicit feedback on approach before investing further.
Description of changes:
AWS-LC deliberately ignores system configuration today. On distributions with the
crypto-policiesframework (notably Amazon Linux 2023 and Fedora, which share it), an operator selects a system-wide policy that is rendered to an OpenSSL back-end file describing the OS TLS posture. This PR lets AWS-LC optionally honor that policy so applications inherit it without code changes.This is the "broad" approach: behind a new off-by-default CMake flag (
-DENABLE_CRYPTO_POLICIES), every context created bySSL_CTX_newis seeded from the system policy after AWS-LC's built-in defaults, and consumers can still override afterward. Cipher, protocol-version, signature-algorithm and group directives are applied.The signature-algorithm and group lists are narrowed to the algorithms AWS-LC implements before being applied, in the order the policy gave them. A stock policy names algorithms AWS-LC lacks, and the setters reject a whole list on the first name they do not recognize, so applying a policy as written would throw away the operator's preference order entirely.
Seeding is best-effort and never fatal. A missing or malformed file, an unsupported directive, a cipher rule AWS-LC cannot satisfy, or a version range whose floor sits above its ceiling all leave the built-in default in place. Errors raised along the way are dropped, and the caller's own error queue comes back byte-identical: same entries, same
datapointers, same mark.Packagers can relocate the policy file at build time with
-DAWSLC_CRYPTO_POLICY_PATH;AWSLC_CRYPTO_POLICY_FILEoverrides it at run time, and is ignored when the real and effective user or group differ so an unprivileged caller cannot pick the policy a set-uid process runs under.Call-outs:
@SECLEVELtoken is parsed and ignored because AWS-LC has no security levels, so only the cipher list is applied and the level's key-size/hash intent is not enforced.secp256r1is translated.crypto-policiesuses the IANA name; AWS-LC follows OpenSSL and knows the curve asP-256. Without the translation the most widely deployed group in a stock policy is the one that gets dropped.ERR_num_errorsandERR_pop_to_countlet seeding discard its own errors by cutting the queue back to the length it had. Saving and restoring the queue instead rebuilds every surviving entry, which dangles adatapointer the caller is already holding and drops a mark the caller set -- neither of whichSSL_CTX_newmay do.TLSv1_2_methodand friends skip the policy's version directives, since the public setters check a bound against the method's whole range and would otherwise widen a pin the caller chose. A policy that tightens such a context is skipped along with one that loosens it.-DFIPS=1; it seeds preferences and does not establish FIPS posture.Testing:
DEFAULTpolicy and assert the exact group and signature-algorithm lists installed, with negative controls showing the setters reject that same policy value unfiltered.datapointer the caller already holds, which is asserted to be the same address afterward.crypto-policiesinstalled exercises the same assertions as one without. Removing the pin fails the DTLS test on Amazon Linux 2023.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.