[varshaprasad96]

Summary

Add an opt-in system-OpenSSL implementation of the crypto interface merged in #3351. Embedders can enable openssl and install an OpenSsl context before crypto initialization; AWS-LC remains the application default.

This implements the backend foundation demonstrated by the PoC. Application-wide transport selection, downstream packaging, and Red Hat FIPS qualification remain follow-ups; strict FIPS requests continue to fail closed.

Related Issue

Closes #4809

Changes

• Implement OpenSSL randomness, streaming SHA-256, AES-256-GCM, owned certificate keys, and Rustls/JWT adapters behind an optional feature, without enabling vendored OpenSSL.
• Preserve encrypted envelope and key formats, distinguish cipher setup failures from authentication failures, and clear tentative plaintext on failure.
• Enforce JWT compatibility for HMAC signature lengths, EC curves/JWK coordinates, RSA key sizes, and JOSE RSA-PSS parameters.
• Test backend interoperability, malformed inputs, TLS 1.2/1.3 mutual authentication, certificate rejection, persisted CA import, and unavailable providers in a fresh process.
• Add explicit OpenSSL-only and dual-backend CI checks and an OpenSSL-only dependency guard. Document initialization, system linkage, and adapter/coverage limitations.
• Enforce named crypto dependency boundaries with cargo-deny, classify existing exceptions, and add 23 offline regression scenarios plus contributor guidance.
• Reject encrypted PEM keys without terminal prompts, including empty-passphrase keys, with a bounded subprocess regression test.

Release Notes

Embedders can opt into the system-OpenSSL backend; AWS-LC remains the application default.

Testing

• Crypto crate tests with OpenSSL only and both backends enabled, including existing AWS-LC regressions.
• Clippy for both feature configurations, all targets, with warnings denied; Rust formatting.
• OpenSSL-only dependency graph contains no AWS-LC, Ring, or vendored OpenSSL. macOS linkage inspection resolves system libssl.3.dylib and libcrypto.3.dylib.
• All tracked Cargo lockfiles validate; Markdown lint and workflow YAML parsing pass.
• All 23 dependency-policy fixtures and cargo-deny licenses/bans/sources checks pass.
• Encrypted-PEM regression fails against the original implementation and passes with the fix, including PKCS#8 and traditional EC/RSA PEM formats.
• Repository pre-commit hook passes, including workspace/all-target Clippy, the sandbox performance harness, standalone examples and E2E compilation/lint, formatting, and non-Rust lint checks.
• Linux CI and downstream RHEL validation have not run for this branch.

Local verification used macOS ARM64. Pre-commit hooks passed when the commits were created; after rebasing onto upstream ea13dfce4, OpenSSL-only and dual-backend tests, dependency-policy checks, and all tracked lockfiles were revalidated. No whole-application or cluster E2E claim is made. P-521 certificate keys remain unsupported by this backend because the rcgen descriptor is AWS-LC-gated. Protocol adapters retain infallible callbacks and provider-ownership limitations documented in the crypto README.

Checklist

• Follows Conventional Commits
• Commits are signed off (DCO)
• Crypto architecture and extension documentation updated

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[varshaprasad96]

Looking into a few more improvements, switching it to draft to hold off from reviews.

Update: PR is ready for review.

[EmilienM]

Another iteration needed: 3 major, 16 minor (inline, 2 below)

The deciding items are the unvalidated RSA public exponent in the JWT adapter (a malformed JWK makes signatures forgeable), the unchecked EC key import, and the reachable startup panic on the OpenSSL path. The rest is consistency and guardrail polish.

Outside the diff:

• Major: crates/openshell-crypto/src/tls.rs:69 and :85: with the OpenSSL backend selected, a host whose OpenSSL config leaves no usable cipher suites makes the first facade client_builder()/server_builder() call hit .expect("selected TLS provider must support the requested protocol versions"), aborting the process instead of returning a config error, across ~17 runtime call sites (gateway TLS, sandbox boundary server, vault driver). The AWS-LC default can't hit this because its cipher list is static. The README documents the assert as a tradeoff, so either confirm that's intended for this backend or add a fallible variant.
• Minor: process: #4809 is state:triage-needed rather than state:accepted, so the PR Issue Acceptance gate fails and Branch Checks, including this PR's new CI steps, have never run on any push. A maintainer needs to accept the issue before merge.

How this was reviewed

• Commit 71973605b, round 1
• Deep review, because the change touches crypto primitives and protocol adapters, CI workflows, and build/dependency tooling (crates/openshell-crypto/**, .github/workflows/branch-checks.yml, deny.toml, tasks/**)
• Passes: architecture, performance, security, scalability, rust, kernel, supply-chain, then correctness
• Independent skeptics checked each candidate finding (1 per minor candidate); 6 of 8 held up
• Repo rules read from origin/main: AGENTS.md, CLAUDE.md, CONTRIBUTING.md, docs/CONTRIBUTING.mdx
• Not covered: Linux CI and downstream RHEL validation (not run for this branch, per the PR description); Branch Checks on any push (the acceptance gate blocks /ok to test), so the new CI lanes are unexercised; local compile, clippy, and cargo-deny runs stood in

[2000krysztof]

RSA certificate-key import appears to have another validation gap: the Id::RSA arm in Key::wrap extracts the public key without enforcing a minimum key size. I tested a 1024-bit RSA key: both PEM and PKCS#8 DER imports succeed with OpenSSL, while the existing AWS-LC backend rejects both. The imported OpenSSL key can also successfully issue a certificate. Could we preserve the existing minimum RSA key size here and add rejection tests for both import formats? The JWT adapter already enforces a 2048-bit minimum, but that check does not cover certificate-key imports.

[Ygnas]

One issue remains in the RSA PKI import path: imported keys are wrapped without enforcing the established 2048–8192-bit range. SandboxCa::from_pem uses those keys to sign proxy leaf certificates, so an OpenSSL-selected deployment can accept a weaker CA key than the existing AWS-LC path. Could we enforce the same range on RSA imports and cover PEM and PKCS#8 paths with regression tests?

[politerealism]

Thanks for this. The backend is careful in the places that usually go wrong: zeroizing the tentative plaintext on open, chunking around OpenSSL's int-length limits, rejecting encrypted PEMs without a terminal prompt, and the fallible TLS builders with real error propagation. I read it against the #4809 acceptance criteria and have a few questions and suggestions.

Correctness questions

1. Setup failures at finalize time look like auth failures. In open(), every finalize error maps to CryptoError::Authentication. The acceptance criteria say provider failures should stay distinguishable from authentication failure. Setup, AAD and tag errors are classified correctly. Does the OpenSSL error stack let you tell a tag mismatch from a provider failure at finalize? If not, a short note in the docs would help.
2. RSA key validation differs between paths. The JWT path enforces 2048-8192 bits plus modulus/exponent checks, but Key::import and Key::wrap accept any RSA key and only run check_key for EC. Is that intentional, or should certificate keys get the same bounds? What does AWS-LC do on import today?

Test coverage

3. backends_preserve_ciphertext_keys_and_jwt_signatures covers Ed25519, ES256 and ES384 in both directions, but not RSA or the PS algorithms, which are the paths with custom code. A cross-backend RSA/PS256 check would be useful, since the JOSE salt length handling is the subtle part.
4. Optional: jwt_rejects_mismatched_curve_and_pss_salt covers the encode side of curve mismatch. I checked jsonwebtoken-openssl 1.0.0 and its ES verifiers already reject a wrong-curve public key (exact point length plus affine construction on the algorithm's curve), so behavior matches AWS-LC. A one-line test for the verify side would lock that in.
5. The docs say there is no supported OpenSSL-only whole-app build. Is openssl combined with tls-tonic, tls-kube or tls-sqlx expected to compile? A cargo check of those combinations, or a note, would help.

CI and linkage

6. The acceptance criteria say "without enabling vendored OpenSSL", and the otool -L check is local to macOS. The Linux job runs in the Nix devshell, so it links Nix's OpenSSL rather than a distro's. Consider an ldd assertion that libssl and libcrypto resolve dynamically so a regression to static or vendored linking fails CI. Distro validation is out of scope here, but it would be worth saying so in the README.

Scope

7. The PR is about 3,000 diff lines. The backend, the try_* builders and the PEM fix map directly to feat(crypto): add an opt-in system OpenSSL backend #4809. The cargo-deny catalog is a different kind of change: a 333-line deny.toml, a Python regression script with 23 fixtures, a sha2/hmac exception table across about a dozen crates, and edits to three skill files (plus the sha2 removal from openshell-sandbox). The guardrail looks useful, but it gets reviewed on different terms and could slow the backend down. Would you consider splitting it into its own PR? I'll defer to the maintainers on that.

Dependencies

8. rustls-openssl =0.4.1 and jsonwebtoken-openssl =1.0.0 are exact pins on small community crates. Could you note their maintenance status and the reason for the pins in the README? Anyone packaging this for a distro will need to package or vendor both.

[varshaprasad96]

RSA certificate-key import appears to have another validation gap: the Id::RSA arm in Key::wrap extracts the public key without enforcing a minimum key size. I tested a 1024-bit RSA key: both PEM and PKCS#8 DER imports succeed with OpenSSL, while the existing AWS-LC backend rejects both. The imported OpenSSL key can also successfully issue a certificate. Could we preserve the existing minimum RSA key size here and add rejection tests for both import formats? The JWT adapter already enforces a 2048-bit minimum, but that check does not cover certificate-key imports.

@2000krysztof Thanks for catching this. You’re right—the JWT validation doesn’t cover RSA certificate-key imports. I checked the current code, and this gap remains. I’ll add the 2048-bit minimum at the shared PKI import boundary, with rejection tests for 1024-bit keys in both PEM and PKCS#8 DER formats and positive coverage for valid keys.

[varshaprasad96]

One issue remains in the RSA PKI import path: imported keys are wrapped without enforcing the established 2048–8192-bit range. SandboxCa::from_pem uses those keys to sign proxy leaf certificates, so an OpenSSL-selected deployment can accept a weaker CA key than the existing AWS-LC path. Could we enforce the same range on RSA imports and cover PEM and PKCS#8 paths with regression tests?

Thanks, Ignas. Agreed, the PKI import path should enforce the full 2048–8192-bit range. I’ve added that check at the shared wrapping boundary, with regression coverage for both PEM and PKCS#8 DER imports

[varshaprasad96]

@politerealism Thanks for the review comments:

Addressed the requests:

• Documented AEAD finalization’s error-classification limitation.
• Added wrong-curve JWT verification tests.
• Verified all three optional TLS feature combinations and added CI checks.
• Added Linux dynamic-linkage checks for libssl and libcrypto.
• Documented dependency maintenance, exact pins, and distro-validation limits.

RSA import bounds and bidirectional RSA/PSS coverage were already fixed.

[varshaprasad96]

@EmilienM thanks for the detailed review. I’ve pushed fixes for the RSA exponent validation, EC key consistency checks, and fallible TLS setup, along with the minor code, test, and CI improvements. The upstream stderr limitation is documented, and the expected unused-wrapper warning is explicitly acknowledged. Local tests and checks pass; CI still needs maintainer acceptance of #4809 (it would be helpful if a maintainer can add that label).

Reg the cargo-deny rules to be split in separate PR: I had intentionally introduced it here as it was also raised in an earlier architecture discussion, and enforcing the openshell-crypto boundary is part of the intended design. Keeping it alongside the OpenSSL backend helps prevent new dependencies from bypassing that boundary.

To keep the scope bounded, further crypto migrations, policy expansion, and the remaining adapter limitations can follow in separate PRs. Does that work fine?