Skip to content

Add opt-in system crypto-policies seeding for SSL_CTX (broad approach) - #3442

Closed
WillChilds-Klein wants to merge 5 commits into
aws:mainfrom
WillChilds-Klein:feature/crypto-policies-support
Closed

Add opt-in system crypto-policies seeding for SSL_CTX (broad approach)#3442
WillChilds-Klein wants to merge 5 commits into
aws:mainfrom
WillChilds-Klein:feature/crypto-policies-support

Conversation

@WillChilds-Klein

@WillChilds-Klein WillChilds-Klein commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

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-policies framework (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 by SSL_CTX_new is 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 data pointers, same mark.

Packagers can relocate the policy file at build time with -DAWSLC_CRYPTO_POLICY_PATH; AWSLC_CRYPTO_POLICY_FILE overrides 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:

  • Broad vs. narrow. This implements the broad, automatic approach (seed on every context creation); the alternative "narrow" approach would add an explicit opt-in call per context with no implicit behavior change. Feedback on which direction AWS-LC prefers is the main purpose of this draft.
  • Security levels are dropped. The policy's @SECLEVEL token 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.
  • secp256r1 is translated. crypto-policies uses the IANA name; AWS-LC follows OpenSSL and knows the curve as P-256. Without the translation the most widely deployed group in a stock policy is the one that gets dropped.
  • Two new internal error-queue primitives. ERR_num_errors and ERR_pop_to_count let 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 a data pointer the caller is already holding and drops a mark the caller set -- neither of which SSL_CTX_new may do.
  • Version-locked methods keep their pin. TLSv1_2_method and 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.
  • FIPS. The flag is orthogonal to -DFIPS=1; it seeds preferences and does not establish FIPS posture.

Testing:

  • Unit tests cover directive parsing, security-level stripping, missing and malformed files, DTLS-versus-TLS selection, overlong values, and an end-to-end case against the real system policy that skips when absent.
  • The filtering tests run the verbatim Amazon Linux 2023 DEFAULT policy and assert the exact group and signature-algorithm lists installed, with negative controls showing the setters reject that same policy value unfiltered.
  • Separate tests pin the failure modes where a rejected directive could still mutate the context: an unsatisfiable cipher rule, an inverted version range in both TLS and DTLS wire orders, a one-sided bound that would cross the bound already set, and a version-locked method.
  • Three tests cover the caller's error queue across seeding: a queued error, a queued error behind a mark, and a data pointer the caller already holds, which is asserted to be the same address afterward.
  • Each test pins the policy path, so a host with crypto-policies installed exercises the same assertions as one without. Removing the pin fails the DTLS test on Amazon Linux 2023.
  • A new CI job runs the flag-on suite on Amazon Linux 2023 (x86_64/aarch64, gcc and clang), where the end-to-end test reads the real policy file; the bulk suite runs with seeding neutralized.

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.

@github-actions

Copy link
Copy Markdown
Contributor

🔒 Security ReviewView Report

Please review before merging.

@WillChilds-Klein
WillChilds-Klein force-pushed the feature/crypto-policies-support branch from 41feace to 1c54250 Compare September 1, 2026 20:38
@WillChilds-Klein
WillChilds-Klein force-pushed the feature/crypto-policies-support branch from 1c54250 to 6a8cad8 Compare September 1, 2026 20:54
@codecov-commenter

codecov-commenter commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 77.77778% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.13%. Comparing base (ac587fb) to head (8fdbc58).

Files with missing lines Patch % Lines
ssl/ssl_privkey.cc 0.00% 11 Missing ⚠️
crypto/err/err.c 83.33% 3 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.
@WillChilds-Klein

Copy link
Copy Markdown
Contributor Author

Superseded by a five-PR stack, to review and merge bottom-up:

  1. Add ERR_num_errors and ERR_pop_to_count #3501 -- error-queue primitives
  2. Add a reader for the system crypto-policies OpenSSL back-end #3502 -- policy file reader
  3. Seed cipher lists and version bounds from the system crypto policy #3503 -- cipher lists and version bounds
  4. Seed groups and signature algorithms from the system crypto policy #3504 -- groups and signature algorithms
  5. Keep AWS-LC's post-quantum defaults unless the policy opts out #3505 -- post-quantum defaults

The stack is not a straight split. Two changes it makes that this PR did not:

  • Groups and signature algorithms are narrowed to the algorithms AWS-LC implements, and secp256r1 is translated to the name AWS-LC knows. As written here, a stock policy value hit an unknown name and the setter discarded the whole list, so the directives had no effect.
  • AWS-LC's post-quantum defaults survive a policy that says nothing about them. The group and signature-algorithm setters replace AWS-LC's defaults rather than intersect with them, so seeding as done here downgraded every context. Turning post-quantum off now takes an explicit AWSLC.PostQuantum = off.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants