From bbe16f078258ff3a8aea64b16ee2edf7d5475d2b Mon Sep 17 00:00:00 2001 From: Varsha Prasad Narsing Date: Wed, 21 Oct 2026 15:03:41 -0700 Subject: [PATCH 1/7] feat(crypto): add an opt-in system OpenSSL backend Signed-off-by: Varsha Prasad Narsing --- .github/workflows/branch-checks.yml | 17 + Cargo.lock | 80 +++ crates/openshell-crypto/CRYPTO.md | 17 +- crates/openshell-crypto/Cargo.toml | 7 + crates/openshell-crypto/README.md | 97 ++- crates/openshell-crypto/src/aws_lc.rs | 2 +- crates/openshell-crypto/src/lib.rs | 9 +- .../openshell-crypto/src/openssl_backend.rs | 288 +++++++++ .../src/openssl_backend/jwt.rs | 250 ++++++++ .../tests/fixtures/openssl-unavailable.cnf | 11 + .../openshell-crypto/tests/openssl_backend.rs | 569 ++++++++++++++++++ crates/openshell-crypto/tests/openssl_tls.rs | 135 +++++ .../tests/openssl_unavailable.rs | 86 +++ 13 files changed, 1546 insertions(+), 22 deletions(-) create mode 100644 crates/openshell-crypto/src/openssl_backend.rs create mode 100644 crates/openshell-crypto/src/openssl_backend/jwt.rs create mode 100644 crates/openshell-crypto/tests/fixtures/openssl-unavailable.cnf create mode 100644 crates/openshell-crypto/tests/openssl_backend.rs create mode 100644 crates/openshell-crypto/tests/openssl_tls.rs create mode 100644 crates/openshell-crypto/tests/openssl_unavailable.rs diff --git a/.github/workflows/branch-checks.yml b/.github/workflows/branch-checks.yml index 218d3d2657..6fa085d836 100644 --- a/.github/workflows/branch-checks.yml +++ b/.github/workflows/branch-checks.yml @@ -320,6 +320,23 @@ jobs: fi cargo test --locked -p openshell-crypto --no-default-features --test independent_backend + - name: Verify OpenSSL backend + shell: nix develop -c bash -euo pipefail {0} + env: + OPENSSL_NO_VENDOR: "1" + OPENSSL_STATIC: "0" + run: | + cargo tree --locked -p openshell-crypto --no-default-features --features openssl --edges normal,build,dev --prefix none > /tmp/openshell-openssl-dependencies.txt + if grep -Eq '^(ring|aws-lc-rs|aws-lc-sys|aws-lc-fips-sys|openssl-src) v' /tmp/openshell-openssl-dependencies.txt; then + echo "ERROR: OpenSSL-only crypto build includes another backend or vendored OpenSSL" >&2 + cat /tmp/openshell-openssl-dependencies.txt >&2 + exit 1 + fi + cargo clippy --locked -p openshell-crypto --no-default-features --features openssl --all-targets -- -D warnings + cargo test --locked -p openshell-crypto --no-default-features --features openssl + cargo clippy --locked -p openshell-crypto --features openssl --all-targets -- -D warnings + cargo test --locked -p openshell-crypto --features openssl + - name: Test shell: nix develop -c bash -euo pipefail {0} env: diff --git a/Cargo.lock b/Cargo.lock index f36af0eb0f..7091ca6f91 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2262,6 +2262,21 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "77ce24cb58228fbb8aa041425bb1050850ac19177686ea6e0f41a70416f56fdb" +[[package]] +name = "foreign-types" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f6f339eb8adc052cd2ca78910fda869aefa38d22d5cb648e6485e4d3fc06f3b1" +dependencies = [ + "foreign-types-shared", +] + +[[package]] +name = "foreign-types-shared" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "00b0228411908ca8685dba7fc2cdd70ec9990a6e753e89b6ac91a84c40fbaf4b" + [[package]] name = "form_urlencoded" version = "1.2.2" @@ -3394,6 +3409,16 @@ dependencies = [ "simple_asn1", ] +[[package]] +name = "jsonwebtoken-openssl" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "53aec0291d5d8b37fbf826ccc23f5473536dad8dafda4161a7b6b56c2b0e0202" +dependencies = [ + "jsonwebtoken", + "openssl", +] + [[package]] name = "k8s-openapi" version = "0.24.0" @@ -4414,17 +4439,21 @@ version = "0.0.0" dependencies = [ "aws-lc-rs", "jsonwebtoken", + "jsonwebtoken-openssl", "k8s-openapi", "kube", + "openssl", "pem 3.0.6", "rcgen", "rustls", + "rustls-openssl", "serde", "serde_json", "sqlx", "thiserror 2.0.20", "tonic", "x509-parser 0.18.1", + "zeroize", ] [[package]] @@ -5319,12 +5348,49 @@ dependencies = [ "tokio", ] +[[package]] +name = "openssl" +version = "0.10.81" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "77823a27f0babb03091cb9ed9ef80af3b39dbc82f97e8fa530374b7dafd87a45" +dependencies = [ + "bitflags 2.13.2", + "cfg-if", + "foreign-types", + "libc", + "openssl-macros", + "openssl-sys", +] + +[[package]] +name = "openssl-macros" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a948666b637a0f465e8564c73e89d4dde00d72d4d473cc972f390fc3dcee7d9c" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.117", +] + [[package]] name = "openssl-probe" version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7c87def4c32ab89d880effc9e097653c8da5d6ef28e6b539d313baaacfbafcbe" +[[package]] +name = "openssl-sys" +version = "0.9.117" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b47e7e6bb2c38cd930d25a23b40fa52e068c10e85f3e03a7f5ba5aaca5713695" +dependencies = [ + "cc", + "libc", + "pkg-config", + "vcpkg", +] + [[package]] name = "opentelemetry" version = "0.32.0" @@ -6905,6 +6971,20 @@ dependencies = [ "security-framework", ] +[[package]] +name = "rustls-openssl" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d67ff7ccc9938b2b7cee4ccc7efd33da2afe904ab7bd644f008defb259f288a2" +dependencies = [ + "foreign-types", + "once_cell", + "openssl", + "openssl-sys", + "rustls", + "zeroize", +] + [[package]] name = "rustls-pemfile" version = "2.2.0" diff --git a/crates/openshell-crypto/CRYPTO.md b/crates/openshell-crypto/CRYPTO.md index 9887389ee0..ca0f26c0c9 100644 --- a/crates/openshell-crypto/CRYPTO.md +++ b/crates/openshell-crypto/CRYPTO.md @@ -2,9 +2,10 @@ OpenShell routes first-party security-sensitive cryptography through `openshell-crypto`. The workspace selects AWS-LC as the production backend. -The interface supports a future system-OpenSSL backend; the current production -implementation does not assert FIPS compliance, and strict posture requests fail -closed. +The opt-in `openssl` feature supplies a system-OpenSSL backend for explicit +context selection. Neither backend asserts deployment FIPS compliance, and strict +posture requests fail closed. Application startup, dependency-owned transports, +and downstream packaging/qualification remain separate OpenSSL integration work. ## Backend boundary @@ -37,7 +38,10 @@ Explicit context selection controls first-party TLS builders even when a dependency initialized Rustls first. Without explicit selection, those builders preserve an embedder's installed provider. JWT adapters retain jsonwebtoken's process-global provider constraints; that provider cannot be replaced safely. -Neither behavior redirects dependency-owned cryptography automatically. +Neither behavior redirects dependency-owned cryptography automatically. Enabling +OpenSSL does not replace the AWS-LC default; an embedder must install its context +explicitly. System libraries and provider configuration belong to the deployment; +the backend does not modify provider policy. ## Contributor rules and coverage @@ -77,6 +81,9 @@ establish deployment compliance. A production OpenSSL/FIPS profile requires: - Tests in the qualified target environment covering TLS/mTLS, JWT, persisted CA loading, credential encryption, and negative startup/algorithm cases. -These are follow-up requirements, not guarantees of the current backend. +These are follow-up requirements, not guarantees of the current backends. The +OpenSSL protocol adapters also have infallible callbacks and provider-ownership +limitations documented in the crate README; backend tests do not resolve those +deployment requirements. Implementation details and extension APIs live in the [crypto crate README](README.md). diff --git a/crates/openshell-crypto/Cargo.toml b/crates/openshell-crypto/Cargo.toml index 13a9537785..01c73a1b8b 100644 --- a/crates/openshell-crypto/Cargo.toml +++ b/crates/openshell-crypto/Cargo.toml @@ -15,6 +15,8 @@ repository.workspace = true # embedder must install its backend before any crypto operation. default = ["aws-lc"] aws-lc = ["dep:aws-lc-rs", "rustls/aws_lc_rs", "rcgen/aws_lc_rs", "jsonwebtoken/aws_lc_rs", "tonic?/tls-aws-lc", "kube?/aws-lc-rs", "sqlx?/tls-rustls-aws-lc-rs"] +# Makes OpenSsl available for explicit context selection; does not change defaults. +openssl = ["dep:openssl", "dep:rustls-openssl", "dep:jsonwebtoken-openssl", "dep:zeroize"] tls-tonic = ["dep:tonic"] tls-kube = ["dep:kube", "dep:k8s-openapi"] tls-sqlx = ["dep:sqlx"] @@ -26,6 +28,11 @@ k8s-openapi = { workspace = true, optional = true } sqlx = { workspace = true, optional = true } aws-lc-rs = { workspace = true, optional = true } +openssl = { version = "0.10.79", default-features = false, optional = true } +rustls-openssl = { version = "=0.4.1", default-features = false, features = ["tls12"], optional = true } +# Version 1 implements jsonwebtoken 10's provider contract (v2 requires v11). +jsonwebtoken-openssl = { version = "=1.0.0", optional = true } +zeroize = { workspace = true, optional = true } rustls = { workspace = true } rcgen = { workspace = true } jsonwebtoken = { workspace = true } diff --git a/crates/openshell-crypto/README.md b/crates/openshell-crypto/README.md index 68efe366e8..3c0355550c 100644 --- a/crates/openshell-crypto/README.md +++ b/crates/openshell-crypto/README.md @@ -3,13 +3,14 @@ See [Cryptography and FIPS](CRYPTO.md) for the architecture, contributor rules, and production OpenSSL/FIPS follow-up requirements. -`openshell-crypto` owns first-party crypto backend selection. AWS-LC is the only -production implementation. This change preserves TLS algorithms, Ed25519 gateway +`openshell-crypto` owns first-party crypto backend selection. AWS-LC remains the +application default; an optional system-OpenSSL backend supports explicit context +selection by embedders. The facade preserves TLS algorithms, Ed25519 gateway JWTs, P-256 certificate keys, native trust roots, and credential envelope formats. -The interface is intended to support a system-OpenSSL backend for regulated -deployments, related to [#900](https://github.com/NVIDIA/OpenShell/issues/900). -This stage implements AWS-LC only. An OpenSSL proof of concept will validate the -provider integration separately; this is not a FIPS build or compliance claim. +The OpenSSL implementation builds on the standalone proof of concept following +[the generic backend refactor](https://github.com/NVIDIA/OpenShell/pull/3351). +Application-wide OpenSSL selection and downstream FIPS qualification remain +separate work; enabling this backend is not a FIPS build or compliance claim. ## Boundaries @@ -60,11 +61,62 @@ A second backend must adapt these integrations and the application startup path. Disabling this crate's feature alone does not remove dependency-owned crypto from the entire product graph. -The later OpenSSL build must use system shared libraries without vendoring, -respect system provider configuration, and verify the resulting linked artifacts. -Linking and module qualification are build/deployment concerns, separate from the -Rust key and primitive contracts. The proof of concept will test those concerns -and the Rustls provider integration before a production OpenSSL backend is added. +### Selecting system OpenSSL + +An embedder enables `openssl` and disables crate defaults: + +```toml +openshell-crypto = { path = "crates/openshell-crypto", default-features = false, features = ["openssl"] } +``` + +Install the context before facade use or JWT initialization: + +```rust,ignore +use openshell_crypto::{CryptoContext, OpenSsl, install_default_context}; + +install_default_context(CryptoContext::new(Box::new(OpenSsl)))?; +``` + +The feature makes `OpenSsl` available without selecting it automatically. When +both backends are enabled, `CryptoContext::default()` still chooses AWS-LC. With +AWS-LC disabled, use before installation fails closed. Inspect Cargo's resolved +graph for other dependencies that re-enable AWS-LC. This feature adds no gateway +flag, image, or supported OpenSSL-only whole-application build. + +Use OpenSSL 3 development libraries, a C compiler, and pkg-config. The Nix shell +supplies these; Homebrew OpenSSL works on macOS. Set `OPENSSL_NO_VENDOR=1` and +`OPENSSL_STATIC=0` for shared-library builds, then inspect `otool -L` (macOS) or +`ldd` (Linux). No vendoring feature is enabled. Runtime libraries, provider +packages and configuration must be supplied and verified by the deployment. + +The backend uses OpenSSL's default library context and configuration without +loading modules or overriding property queries. Configure the process before +startup; do not unload providers or change policy during operations. Keys retain +OpenSSL resources for their lifetime. Capabilities describe implemented operations, +not runtime algorithm availability. `provider_version` stays unknown because the +library version does not identify the module that performed an operation. + +Certificate keys support P-256, P-384, Ed25519 and RSA, with PEM/PKCS#8 import and +PKCS#8 export. RSA generation uses 2048 bits. P-521 certificate keys are rejected: +rcgen 0.14 exposes that descriptor only with AWS-LC enabled. Existing P-521 users +must retain AWS-LC until this limitation is resolved. This backend uses exportable +software keys and does not add HSM or PKCS#11 loading. + +TLS uses `rustls-openssl` 0.4.1 with TLS 1.2/1.3, without its `fips`, vendored or +prefer-post-quantum features. The jsonwebtoken 10 adapter includes compatibility +guards for malformed HMAC signatures, EC algorithm/curve binding, JOSE RSA-PSS +salt lengths and fixed-width EC JWK coordinates. Claim validation stays in +jsonwebtoken. AEAD setup errors remain distinct from authentication errors; +tentative plaintext is cleared on failure and returned only after authentication. + +Protocol limitations remain: Rustls hash/HKDF callbacks and JWT JWK thumbprint +hashing cannot return backend errors; adapter helpers can panic on provider +failure. The TLS adapter also uses direct OpenSSL SHA operations that do not +establish provider-module ownership. For fallible TLS setup, use Rustls's +`builder_with_provider` with `OpenSsl.tls_provider()` and handle protocol-version +errors; facade convenience builders assert compatibility. This backend does not +promise panic-free protocol operation or a provider-qualified TLS path. These +limitations require disposition during downstream integration and qualification. ## Lifecycle and posture @@ -113,8 +165,8 @@ Coverage excludes dependency-owned SSH/russh primitives, AWS SigV4 signing, SPIFFE RustCrypto verification, and non-Rust client runtimes. AWS SDK HTTPS also retains its dependency-owned selection. Non-security content hashes and ordinary scheduling randomness are not migrated. No SSH or PQC capability is asserted by -the primitive capability report. OpenSSL, strict policy, module version discovery, -and deployment qualification belong in follow-up work. +the primitive capability report. Application-wide OpenSSL integration, strict +policy, module version discovery, and deployment qualification remain follow-ups. Operator certificate fingerprints, runtime refresh replay hashes and derived token IDs, bearer-cache fingerprints, Sandbox Protocol request digests and request-ID @@ -178,6 +230,25 @@ Ring. Inspect that boundary with `cargo tree -p openshell-crypto --no-default-features --edges normal,build,dev`. Branch Checks runs this test separately from workspace tests and rejects Ring or AWS-LC in the dependency tree, including build and test dependencies. +It also runs OpenSSL-only and dual-backend tests and Clippy, and rejects Ring, +AWS-LC and vendored OpenSSL from the OpenSSL-only crate graph. Run locally with +system OpenSSL available: + +```shell +OPENSSL_NO_VENDOR=1 OPENSSL_STATIC=0 cargo test --locked -p openshell-crypto --no-default-features --features openssl +OPENSSL_NO_VENDOR=1 OPENSSL_STATIC=0 cargo test --locked -p openshell-crypto --features openssl +OPENSSL_NO_VENDOR=1 OPENSSL_STATIC=0 cargo clippy --locked -p openshell-crypto --no-default-features --features openssl --all-targets -- -D warnings +OPENSSL_NO_VENDOR=1 OPENSSL_STATIC=0 cargo clippy --locked -p openshell-crypto --features openssl --all-targets -- -D warnings +cargo tree --locked -p openshell-crypto --no-default-features --features openssl --edges normal,build,dev +``` + +Tests cover known answers, tampering, chunked AEAD, PKI/CA import, JWT algorithms +and claims, TLS 1.2/1.3 mutual authentication and rejected peers, and AWS-LC/OpenSSL +interoperability. A subprocess uses an unsatisfiable property query to test +unavailable algorithms without modifying its parent process. Its ignored child +test is invoked by the parent automatically. These are backend contract tests, +not target RHEL qualification or product E2E tests. + Existing bootstrap, credential-store, gateway TLS/OIDC, and proxy tests exercise the migrated consumers. The cargo-deny Ring restrictions and their explicit wrapper exceptions remain in force. diff --git a/crates/openshell-crypto/src/aws_lc.rs b/crates/openshell-crypto/src/aws_lc.rs index 6a6e2a667c..6dc5900def 100644 --- a/crates/openshell-crypto/src/aws_lc.rs +++ b/crates/openshell-crypto/src/aws_lc.rs @@ -1,7 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -//! The sole production backend. Backend-specific imports belong here. +//! The default AWS-LC backend. Backend-specific imports belong here. use crate::{Capabilities, CryptoBackend, CryptoError, Digest, FipsState, ProtocolBackend}; use aws_lc_rs::{ diff --git a/crates/openshell-crypto/src/lib.rs b/crates/openshell-crypto/src/lib.rs index 31a49488e2..ddee73a963 100644 --- a/crates/openshell-crypto/src/lib.rs +++ b/crates/openshell-crypto/src/lib.rs @@ -3,15 +3,18 @@ //! Backend-neutral crypto operations and library-specific protocol adapters. //! -//! AWS-LC is the only production implementation in this first stage. This crate +//! AWS-LC is the default; an opt-in system-OpenSSL backend supports explicit selection. This crate //! does not enable FIPS mode, change algorithms, or attest dependency-owned crypto. -//! The extension target is a system-OpenSSL backend for regulated deployments -//! (). See the crate README for +//! See the crate README for //! initialization, key ownership, and dependency coverage boundaries. pub mod aead; #[cfg(feature = "aws-lc")] mod aws_lc; +#[cfg(feature = "openssl")] +mod openssl_backend; +#[cfg(feature = "openssl")] +pub use openssl_backend::OpenSsl; pub mod jwt; pub mod pki; pub mod tls; diff --git a/crates/openshell-crypto/src/openssl_backend.rs b/crates/openshell-crypto/src/openssl_backend.rs new file mode 100644 index 0000000000..e4bad5898c --- /dev/null +++ b/crates/openshell-crypto/src/openssl_backend.rs @@ -0,0 +1,288 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Opt-in OpenSSL implementation using the process's default library context. +//! Provider configuration belongs to the embedding process, not this backend. + +use crate::{ + Capabilities, CryptoBackend, CryptoError, Digest, FipsState, ProtocolBackend, aead::Sealed, pki, +}; +use openssl::{ + bn::BigNumContext, + ec::PointConversionForm, + hash::{Hasher, MessageDigest}, + nid::Nid, + pkey::{Id, PKey, Private}, + pkey_ctx::PkeyCtx, + sign::Signer, + symm::{Cipher, Crypter, Mode}, +}; +use zeroize::Zeroizing; + +mod jwt; + +/// System-OpenSSL backend for explicit [`crate::CryptoContext`] selection. +/// +/// Uses OpenSSL's default library context and configuration. It does not load +/// providers, override property queries, or attest a FIPS module. Select before +/// initializing crypto-dependent libraries; changing provider configuration while +/// operations are active is unsupported. The `openssl` feature does not change +/// the AWS-LC default or redirect dependency-owned transports. +#[derive(Debug, Default, Clone, Copy)] +pub struct OpenSsl; + +struct Sha256(Hasher); +impl Digest for Sha256 { + fn update(&mut self, bytes: &[u8]) -> Result<(), CryptoError> { + self.0.update(bytes).map_err(|_| CryptoError::Operation) + } + fn finish(mut self: Box) -> Result<[u8; 32], CryptoError> { + self.0 + .finish() + .map_err(|_| CryptoError::Operation)? + .as_ref() + .try_into() + .map_err(|_| CryptoError::Operation) + } +} + +impl CryptoBackend for OpenSsl { + fn capabilities(&self) -> Capabilities { + Capabilities { + backend: "openssl", + // The library version is not the identity/version of the provider + // module that performed an operation in the default context. + provider_version: None, + fips: FipsState::NotVerified, + primitives: &["CSPRNG", "SHA-256", "AES-256-GCM"], + key_algorithms: &["ECDSA-P256-SHA256", "ECDSA-P384-SHA384", "Ed25519", "RSA"], + } + } + fn fill_random(&self, output: &mut [u8]) -> Result<(), CryptoError> { + for chunk in output.chunks_mut(1024 * 1024) { + openssl::rand::rand_bytes(chunk).map_err(|_| CryptoError::Random)?; + } + Ok(()) + } + fn sha256_digest(&self) -> Result, CryptoError> { + Ok(Box::new(Sha256( + Hasher::new(MessageDigest::sha256()).map_err(|_| CryptoError::Operation)?, + ))) + } + fn seal(&self, key: &[u8; 32], aad: &[u8], plaintext: &[u8]) -> Result { + let mut nonce = [0; 12]; + self.fill_random(&mut nonce)?; + let mut tag = [0; 16]; + let mut crypter = Crypter::new(Cipher::aes_256_gcm(), Mode::Encrypt, key, Some(&nonce)) + .map_err(|_| CryptoError::Operation)?; + update_aad(&mut crypter, aad)?; + let mut ciphertext = vec![0; output_size(plaintext.len())?]; + let written = update_data(&mut crypter, plaintext, &mut ciphertext)?; + let final_len = crypter + .finalize(&mut ciphertext[written..]) + .map_err(|_| CryptoError::Operation)?; + ciphertext.truncate(written + final_len); + crypter + .get_tag(&mut tag) + .map_err(|_| CryptoError::Operation)?; + ciphertext.extend_from_slice(&tag); + Ok(Sealed { nonce, ciphertext }) + } + fn open( + &self, + key: &[u8; 32], + aad: &[u8], + nonce: &[u8; 12], + ciphertext: &[u8], + ) -> Result, CryptoError> { + let split = ciphertext + .len() + .checked_sub(16) + .ok_or(CryptoError::Authentication)?; + let (data, tag) = ciphertext.split_at(split); + let mut crypter = Crypter::new(Cipher::aes_256_gcm(), Mode::Decrypt, key, Some(nonce)) + .map_err(|_| CryptoError::Operation)?; + update_aad(&mut crypter, aad)?; + crypter.set_tag(tag).map_err(|_| CryptoError::Operation)?; + // EVP writes tentative plaintext before authenticating. Clear it on every + // failure path and return it only after finalization verifies the tag. + let mut plaintext = Zeroizing::new(vec![0; output_size(data.len())?]); + let written = update_data(&mut crypter, data, &mut plaintext)?; + let final_len = crypter + .finalize(&mut plaintext[written..]) + .map_err(|_| CryptoError::Authentication)?; + plaintext.truncate(written + final_len); + Ok(std::mem::take(&mut *plaintext)) + } +} + +fn output_size(len: usize) -> Result { + len.checked_add(Cipher::aes_256_gcm().block_size()) + .ok_or(CryptoError::Operation) +} + +fn update_aad(crypter: &mut Crypter, aad: &[u8]) -> Result<(), CryptoError> { + for chunk in aad.chunks(1024 * 1024) { + crypter + .aad_update(chunk) + .map_err(|_| CryptoError::Operation)?; + } + Ok(()) +} + +fn update_data( + crypter: &mut Crypter, + data: &[u8], + output: &mut [u8], +) -> Result { + let mut written = 0; + // OpenSSL's update APIs use signed-int lengths. Bound both input and output + // slices rather than allowing the Rust bindings to panic on large records. + for chunk in data.chunks(1024 * 1024) { + let end = written + output_size(chunk.len())?; + written += crypter + .update(chunk, &mut output[written..end]) + .map_err(|_| CryptoError::Operation)?; + } + Ok(written) +} + +fn key_error(_: openssl::error::ErrorStack) -> rcgen::Error { + rcgen::Error::RemoteKeyError +} + +struct Key { + inner: PKey, + algorithm: &'static rcgen::SignatureAlgorithm, + public: Vec, +} + +impl Key { + fn wrap( + inner: PKey, + algorithm: &'static rcgen::SignatureAlgorithm, + ) -> Result { + let public = match inner.id() { + Id::ED25519 => inner.raw_public_key().map_err(key_error)?, + Id::EC => { + let ec = inner.ec_key().map_err(key_error)?; + let mut ctx = BigNumContext::new().map_err(key_error)?; + ec.public_key() + .to_bytes(ec.group(), PointConversionForm::UNCOMPRESSED, &mut ctx) + .map_err(key_error)? + } + Id::RSA => inner + .rsa() + .map_err(key_error)? + .public_key_to_der_pkcs1() + .map_err(key_error)?, + _ => return Err(rcgen::Error::UnsupportedSignatureAlgorithm), + }; + pki::KeyPair::new(Box::new(Self { + inner, + algorithm, + public, + })) + } + fn import(inner: PKey) -> Result { + let algorithm = match inner.id() { + Id::ED25519 => &rcgen::PKCS_ED25519, + Id::RSA => &rcgen::PKCS_RSA_SHA256, + Id::EC => match inner.ec_key().map_err(key_error)?.group().curve_name() { + Some(Nid::X9_62_PRIME256V1) => &rcgen::PKCS_ECDSA_P256_SHA256, + Some(Nid::SECP384R1) => &rcgen::PKCS_ECDSA_P384_SHA384, + _ => return Err(rcgen::Error::UnsupportedSignatureAlgorithm), + }, + _ => return Err(rcgen::Error::UnsupportedSignatureAlgorithm), + }; + Self::wrap(inner, algorithm) + } +} + +impl pki::SigningKey for Key { + fn algorithm(&self) -> &'static rcgen::SignatureAlgorithm { + self.algorithm + } + fn public_key_raw(&self) -> &[u8] { + &self.public + } + fn sign(&self, message: &[u8]) -> Result, rcgen::Error> { + let mut signer = if self.inner.id() == Id::ED25519 { + Signer::new_without_digest(&self.inner) + } else { + let digest = if self.algorithm == &rcgen::PKCS_ECDSA_P384_SHA384 + || self.algorithm == &rcgen::PKCS_RSA_SHA384 + { + MessageDigest::sha384() + } else if self.algorithm == &rcgen::PKCS_RSA_SHA512 { + MessageDigest::sha512() + } else { + MessageDigest::sha256() + }; + Signer::new(digest, &self.inner) + } + .map_err(key_error)?; + signer.sign_oneshot_to_vec(message).map_err(key_error) + } + fn export_pkcs8_der(&self) -> Result, rcgen::Error> { + self.inner.private_key_to_pkcs8().map_err(key_error) + } +} + +impl OpenSsl { + fn generate( + algorithm: &'static rcgen::SignatureAlgorithm, + ) -> Result, rcgen::Error> { + if algorithm == &rcgen::PKCS_ED25519 { + let mut context = PkeyCtx::new_id(Id::ED25519).map_err(key_error)?; + context.keygen_init().map_err(key_error)?; + return context.keygen().map_err(key_error); + } + if [ + &rcgen::PKCS_RSA_SHA256, + &rcgen::PKCS_RSA_SHA384, + &rcgen::PKCS_RSA_SHA512, + ] + .contains(&algorithm) + { + let mut context = PkeyCtx::new_id(Id::RSA).map_err(key_error)?; + context.keygen_init().map_err(key_error)?; + context.set_rsa_keygen_bits(2048).map_err(key_error)?; + return context.keygen().map_err(key_error); + } + let curve = if algorithm == &rcgen::PKCS_ECDSA_P256_SHA256 { + Nid::X9_62_PRIME256V1 + } else if algorithm == &rcgen::PKCS_ECDSA_P384_SHA384 { + Nid::SECP384R1 + } else { + return Err(rcgen::Error::UnsupportedSignatureAlgorithm); + }; + let mut context = PkeyCtx::new_id(Id::EC).map_err(key_error)?; + context.keygen_init().map_err(key_error)?; + context + .set_ec_paramgen_curve_nid(curve) + .map_err(key_error)?; + context.keygen().map_err(key_error) + } +} + +impl ProtocolBackend for OpenSsl { + fn tls_provider(&self) -> rustls::crypto::CryptoProvider { + rustls_openssl::default_provider() + } + fn generate_keypair( + &self, + algorithm: &'static rcgen::SignatureAlgorithm, + ) -> Result { + Key::wrap(Self::generate(algorithm)?, algorithm) + } + fn import_keypair_pem(&self, pem: &str) -> Result { + Key::import(PKey::private_key_from_pem(pem.as_bytes()).map_err(key_error)?) + } + fn import_keypair_der(&self, der: &[u8]) -> Result { + Key::import(PKey::private_key_from_pkcs8(der).map_err(key_error)?) + } + fn jwt_provider(&self) -> &'static jsonwebtoken::crypto::CryptoProvider { + jwt::provider() + } +} diff --git a/crates/openshell-crypto/src/openssl_backend/jwt.rs b/crates/openshell-crypto/src/openssl_backend/jwt.rs new file mode 100644 index 0000000000..7187814314 --- /dev/null +++ b/crates/openshell-crypto/src/openssl_backend/jwt.rs @@ -0,0 +1,250 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Compatibility checks around the jsonwebtoken 10 OpenSSL adapter. + +use std::sync::OnceLock; + +use jsonwebtoken::{ + Algorithm, AlgorithmFamily, DecodingKey, DecodingKeyKind, EncodingKey, + crypto::{CryptoProvider, JwtSigner, JwtVerifier}, + errors::{ErrorKind, Result}, + jwk::EllipticCurve, + signature::{Error, Signer, Verifier}, +}; +use openssl::{ + bn::BigNum, + hash::MessageDigest, + nid::Nid, + pkey::{PKey, Private, Public}, + rsa::{Padding, Rsa}, + sign::{RsaPssSaltlen, Signer as OpenSslSigner, Verifier as OpenSslVerifier}, +}; + +pub(super) fn provider() -> &'static CryptoProvider { + static PROVIDER: OnceLock = OnceLock::new(); + PROVIDER.get_or_init(|| { + let mut provider = jsonwebtoken_openssl::DEFAULT_PROVIDER.clone(); + provider.signer_factory = signer; + provider.verifier_factory = verifier; + provider.jwk_utils.extract_ec_public_key_coordinates = ec_coordinates; + provider + }) +} + +fn check_ec_key(algorithm: Algorithm, der: &[u8]) -> Result<()> { + let expected = match algorithm { + Algorithm::ES256 => Nid::X9_62_PRIME256V1, + Algorithm::ES384 => Nid::SECP384R1, + _ => return Err(ErrorKind::InvalidAlgorithm.into()), + }; + let key = PKey::private_key_from_der(der).map_err(|_| ErrorKind::InvalidEcdsaKey)?; + let ec = key.ec_key().map_err(|_| ErrorKind::InvalidEcdsaKey)?; + if ec.group().curve_name() != Some(expected) { + return Err(ErrorKind::InvalidEcdsaKey.into()); + } + Ok(()) +} + +// Signature required by jsonwebtoken's provider factory. +#[allow(clippy::trivially_copy_pass_by_ref)] +fn signer(algorithm: &Algorithm, key: &EncodingKey) -> Result> { + if matches!(algorithm, Algorithm::ES256 | Algorithm::ES384) { + check_ec_key(*algorithm, key.inner())?; + } + if matches!( + algorithm, + Algorithm::RS256 | Algorithm::RS384 | Algorithm::RS512 + ) { + if key.family() != AlgorithmFamily::Rsa { + return Err(ErrorKind::InvalidKeyFormat.into()); + } + let rsa = + PKey::private_key_from_der(key.inner()).map_err(|_| ErrorKind::InvalidKeyFormat)?; + check_rsa_size(&rsa)?; + } + if let Some(digest) = pss_digest(*algorithm) { + if key.family() != AlgorithmFamily::Rsa { + return Err(ErrorKind::InvalidKeyFormat.into()); + } + let key = + PKey::private_key_from_der(key.inner()).map_err(|_| ErrorKind::InvalidKeyFormat)?; + check_rsa_size(&key)?; + return Ok(Box::new(PssSigner { + key, + algorithm: *algorithm, + digest, + })); + } + (jsonwebtoken_openssl::DEFAULT_PROVIDER.signer_factory)(algorithm, key) +} + +#[allow(clippy::trivially_copy_pass_by_ref)] +fn verifier(algorithm: &Algorithm, key: &DecodingKey) -> Result> { + if matches!( + algorithm, + Algorithm::RS256 | Algorithm::RS384 | Algorithm::RS512 + ) { + check_rsa_size(&rsa_public_key(key)?)?; + } + if let Some(digest) = pss_digest(*algorithm) { + let key = rsa_public_key(key)?; + check_rsa_size(&key)?; + return Ok(Box::new(PssVerifier { + key, + algorithm: *algorithm, + digest, + })); + } + let inner = (jsonwebtoken_openssl::DEFAULT_PROVIDER.verifier_factory)(algorithm, key)?; + let expected = match algorithm { + Algorithm::HS256 => 32, + Algorithm::HS384 => 48, + Algorithm::HS512 => 64, + _ => return Ok(inner), + }; + Ok(Box::new(HmacVerifier { inner, expected })) +} + +fn rsa_public_key(key: &DecodingKey) -> Result> { + if key.family() != AlgorithmFamily::Rsa { + return Err(ErrorKind::InvalidKeyFormat.into()); + } + let rsa = match key.kind() { + DecodingKeyKind::SecretOrDer(der) => Rsa::public_key_from_der_pkcs1(der), + DecodingKeyKind::RsaModulusExponent { n, e } => Rsa::from_public_components( + BigNum::from_slice(n).map_err(|_| ErrorKind::InvalidKeyFormat)?, + BigNum::from_slice(e).map_err(|_| ErrorKind::InvalidKeyFormat)?, + ), + } + .map_err(|_| ErrorKind::InvalidKeyFormat)?; + PKey::from_rsa(rsa).map_err(|_| ErrorKind::InvalidKeyFormat.into()) +} + +struct HmacVerifier { + inner: Box, + expected: usize, +} + +impl Verifier> for HmacVerifier { + fn verify(&self, message: &[u8], signature: &Vec) -> std::result::Result<(), Error> { + // The underlying constant-time comparison requires equal lengths. + // Signature length is public; reject malformed input before comparison. + if signature.len() != self.expected { + return Err(Error::new()); + } + self.inner.verify(message, signature) + } +} + +impl JwtVerifier for HmacVerifier { + fn algorithm(&self) -> Algorithm { + self.inner.algorithm() + } +} + +fn pss_digest(algorithm: Algorithm) -> Option { + match algorithm { + Algorithm::PS256 => Some(MessageDigest::sha256()), + Algorithm::PS384 => Some(MessageDigest::sha384()), + Algorithm::PS512 => Some(MessageDigest::sha512()), + _ => None, + } +} + +fn check_rsa_size(key: &PKey) -> Result<()> { + if key.id() != openssl::pkey::Id::RSA || !(2048..=8192).contains(&key.bits()) { + return Err(ErrorKind::InvalidKeyFormat.into()); + } + Ok(()) +} + +struct PssSigner { + key: PKey, + algorithm: Algorithm, + digest: MessageDigest, +} + +impl Signer> for PssSigner { + fn try_sign(&self, message: &[u8]) -> std::result::Result, Error> { + let mut signer = OpenSslSigner::new(self.digest, &self.key).map_err(Error::from_source)?; + signer + .set_rsa_padding(Padding::PKCS1_PSS) + .map_err(Error::from_source)?; + signer + .set_rsa_mgf1_md(self.digest) + .map_err(Error::from_source)?; + // JOSE requires salt length equal to the digest length, not OpenSSL's + // default maximum salt length (RFC 7518 section 3.5). + signer + .set_rsa_pss_saltlen(RsaPssSaltlen::DIGEST_LENGTH) + .map_err(Error::from_source)?; + signer + .sign_oneshot_to_vec(message) + .map_err(Error::from_source) + } +} + +impl JwtSigner for PssSigner { + fn algorithm(&self) -> Algorithm { + self.algorithm + } +} + +struct PssVerifier { + key: PKey, + algorithm: Algorithm, + digest: MessageDigest, +} + +impl Verifier> for PssVerifier { + fn verify(&self, message: &[u8], signature: &Vec) -> std::result::Result<(), Error> { + let mut verifier = + OpenSslVerifier::new(self.digest, &self.key).map_err(Error::from_source)?; + verifier + .set_rsa_padding(Padding::PKCS1_PSS) + .map_err(Error::from_source)?; + verifier + .set_rsa_mgf1_md(self.digest) + .map_err(Error::from_source)?; + verifier + .set_rsa_pss_saltlen(RsaPssSaltlen::DIGEST_LENGTH) + .map_err(Error::from_source)?; + if verifier + .verify_oneshot(signature, message) + .map_err(Error::from_source)? + { + Ok(()) + } else { + Err(Error::new()) + } + } +} + +impl JwtVerifier for PssVerifier { + fn algorithm(&self) -> Algorithm { + self.algorithm + } +} + +fn ec_coordinates(der: &[u8], algorithm: Algorithm) -> Result<(EllipticCurve, Vec, Vec)> { + check_ec_key(algorithm, der)?; + let (curve, x, y) = (jsonwebtoken_openssl::DEFAULT_PROVIDER + .jwk_utils + .extract_ec_public_key_coordinates)(der, algorithm)?; + let width: usize = if algorithm == Algorithm::ES256 { + 32 + } else { + 48 + }; + // JWK coordinates have fixed width, including leading zeros (RFC 7518 6.2.1). + let pad = |coordinate: Vec| -> Result> { + let padding = width + .checked_sub(coordinate.len()) + .ok_or(ErrorKind::InvalidEcdsaKey)?; + let mut result = vec![0; padding]; + result.extend_from_slice(&coordinate); + Ok(result) + }; + Ok((curve, pad(x)?, pad(y)?)) +} diff --git a/crates/openshell-crypto/tests/fixtures/openssl-unavailable.cnf b/crates/openshell-crypto/tests/fixtures/openssl-unavailable.cnf new file mode 100644 index 0000000000..37398c7971 --- /dev/null +++ b/crates/openshell-crypto/tests/fixtures/openssl-unavailable.cnf @@ -0,0 +1,11 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +# Negative test: an unsatisfiable property query, not a FIPS configuration. +openssl_conf = openshell_init + +[openshell_init] +alg_section = algorithm_properties + +[algorithm_properties] +default_properties = provider=openshell_nonexistent_provider diff --git a/crates/openshell-crypto/tests/openssl_backend.rs b/crates/openshell-crypto/tests/openssl_backend.rs new file mode 100644 index 0000000000..41e805a62e --- /dev/null +++ b/crates/openshell-crypto/tests/openssl_backend.rs @@ -0,0 +1,569 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +#![cfg(feature = "openssl")] + +use openshell_crypto::OpenSsl; +use openshell_crypto::{ + CryptoBackend, CryptoContext, CryptoError, ProtocolBackend, aead, jwt, pki, tls, +}; +use std::{ + io::{Read, Write}, + sync::{Arc, Once}, +}; + +fn init() { + static INIT: Once = Once::new(); + INIT.call_once(|| { + // Explicit facade selection must win even when an embedder installed + // AWS-LC as Rustls's process default first. + #[cfg(feature = "aws-lc")] + rustls::crypto::aws_lc_rs::default_provider() + .install_default() + .unwrap(); + openshell_crypto::install_default_context(CryptoContext::new(Box::new(OpenSsl))).unwrap(); + }); +} + +#[test] +fn primitives_and_posture() { + init(); + assert_eq!( + openshell_crypto::sha256(b"abc").unwrap(), + [ + 0xba, 0x78, 0x16, 0xbf, 0x8f, 0x01, 0xcf, 0xea, 0x41, 0x41, 0x40, 0xde, 0x5d, 0xae, + 0x22, 0x23, 0xb0, 0x03, 0x61, 0xa3, 0x96, 0x17, 0x7a, 0x9c, 0xb4, 0x10, 0xff, 0x61, + 0xf2, 0x00, 0x15, 0xad + ] + ); + let mut digest = openshell_crypto::sha256_digest().unwrap(); + digest.update(b"a").unwrap(); + digest.update(b"bc").unwrap(); + assert_eq!( + digest.finish().unwrap(), + openshell_crypto::sha256(b"abc").unwrap() + ); + assert_ne!( + openshell_crypto::random_bytes::<32>().unwrap(), + openshell_crypto::random_bytes::<32>().unwrap() + ); + let context = openshell_crypto::default_context(); + let caps = context.verify_posture(false).unwrap(); + assert_eq!(caps.backend, "openssl"); + assert_eq!(caps.provider_version, None); + assert_eq!(caps.fips, openshell_crypto::FipsState::NotVerified); + assert_eq!( + context.verify_posture(true), + Err(CryptoError::UnsupportedPosture) + ); +} + +#[test] +fn aead_compatibility_and_rejection() { + init(); + // NIST AES-256-GCM empty-message vector: ciphertext consists only of the tag. + let tag = [ + 0x53, 0x0f, 0x8a, 0xfb, 0xc7, 0x45, 0x36, 0xb9, 0xa9, 0x63, 0xb4, 0xf1, 0xc4, 0xcb, 0x73, + 0x8b, + ]; + assert_eq!(aead::open(&[0; 32], b"", &[0; 12], &tag).unwrap(), b""); + for plain in [b"".as_slice(), b"credential-value"] { + let sealed = aead::seal(&[7; 32], b"record-id", plain).unwrap(); + assert_eq!(sealed.ciphertext.len(), plain.len() + 16); + assert_eq!( + aead::open(&[7; 32], b"record-id", &sealed.nonce, &sealed.ciphertext).unwrap(), + plain + ); + assert_eq!( + aead::open(&[8; 32], b"record-id", &sealed.nonce, &sealed.ciphertext), + Err(CryptoError::Authentication) + ); + assert_eq!( + aead::open(&[7; 32], b"wrong-id", &sealed.nonce, &sealed.ciphertext), + Err(CryptoError::Authentication) + ); + let mut damaged = sealed.ciphertext.clone(); + damaged[0] ^= 1; + assert_eq!( + aead::open(&[7; 32], b"record-id", &sealed.nonce, &damaged), + Err(CryptoError::Authentication) + ); + let mut nonce = sealed.nonce; + nonce[0] ^= 1; + assert_eq!( + aead::open(&[7; 32], b"record-id", &nonce, &sealed.ciphertext), + Err(CryptoError::Authentication) + ); + let mut damaged_tag = sealed.ciphertext.clone(); + *damaged_tag.last_mut().unwrap() ^= 1; + assert_eq!( + aead::open(&[7; 32], b"record-id", &sealed.nonce, &damaged_tag), + Err(CryptoError::Authentication) + ); + } + assert_eq!( + aead::open(&[0; 32], b"", &[0; 12], &[0; 15]), + Err(CryptoError::Authentication) + ); +} + +#[test] +fn aead_handles_multiple_update_chunks() { + let plaintext = vec![0xab; 2 * 1024 * 1024 + 17]; + let aad = vec![0xcd; 1024 * 1024 + 1]; + let sealed = OpenSsl.seal(&[7; 32], &aad, &plaintext).unwrap(); + assert_eq!( + OpenSsl + .open(&[7; 32], &aad, &sealed.nonce, &sealed.ciphertext) + .unwrap(), + plaintext + ); + #[cfg(feature = "aws-lc")] + assert_eq!( + CryptoContext::default() + .backend() + .open(&[7; 32], &aad, &sealed.nonce, &sealed.ciphertext) + .unwrap(), + plaintext + ); +} + +#[test] +fn jwt_rejects_mismatched_curve_and_pss_salt() { + init(); + let key = OpenSsl + .generate_keypair(&rcgen::PKCS_ECDSA_P256_SHA256) + .unwrap(); + let encoding = + jsonwebtoken::EncodingKey::from_ec_pem(key.serialize_pem().unwrap().as_bytes()).unwrap(); + assert!( + jwt::encode( + &jsonwebtoken::Header::new(jsonwebtoken::Algorithm::ES384), + &serde_json::json!({"exp":4_000_000_000_u64}), + &encoding + ) + .is_err() + ); + let key = OpenSsl.generate_keypair(&rcgen::PKCS_RSA_SHA256).unwrap(); + let native = + openssl::pkey::PKey::private_key_from_pkcs8(&key.serialize_der().unwrap()).unwrap(); + let decoding = + jsonwebtoken::DecodingKey::from_rsa_pem(key.public_key_pem().as_bytes()).unwrap(); + let message = b"header.claims"; + for (algorithm, digest) in [ + ( + jsonwebtoken::Algorithm::PS256, + openssl::hash::MessageDigest::sha256(), + ), + ( + jsonwebtoken::Algorithm::PS384, + openssl::hash::MessageDigest::sha384(), + ), + ( + jsonwebtoken::Algorithm::PS512, + openssl::hash::MessageDigest::sha512(), + ), + ] { + let mut signer = openssl::sign::Signer::new(digest, &native).unwrap(); + signer + .set_rsa_padding(openssl::rsa::Padding::PKCS1_PSS) + .unwrap(); + signer + .set_rsa_pss_saltlen(openssl::sign::RsaPssSaltlen::custom(0)) + .unwrap(); + let signature = signer.sign_oneshot_to_vec(message).unwrap(); + let verifier = (OpenSsl.jwt_provider().verifier_factory)(&algorithm, &decoding).unwrap(); + assert!(verifier.verify(message, &signature).is_err()); + } +} + +#[test] +fn jwt_rejects_rsa_keys_below_the_existing_minimum() { + let rsa = openssl::rsa::Rsa::generate(1024).unwrap(); + let encoding = jsonwebtoken::EncodingKey::from_rsa_der(&rsa.private_key_to_der().unwrap()); + let decoding = jsonwebtoken::DecodingKey::from_rsa_der(&rsa.public_key_to_der_pkcs1().unwrap()); + for algorithm in [ + jsonwebtoken::Algorithm::RS256, + jsonwebtoken::Algorithm::RS384, + jsonwebtoken::Algorithm::RS512, + jsonwebtoken::Algorithm::PS256, + jsonwebtoken::Algorithm::PS384, + jsonwebtoken::Algorithm::PS512, + ] { + assert!((OpenSsl.jwt_provider().signer_factory)(&algorithm, &encoding).is_err()); + assert!((OpenSsl.jwt_provider().verifier_factory)(&algorithm, &decoding).is_err()); + } +} + +#[test] +fn jwk_coordinates_preserve_leading_zeros() { + use openssl::{ + bn::{BigNum, BigNumContext}, + ec::{EcGroup, EcKey, EcPoint, PointConversionForm}, + nid::Nid, + pkey::PKey, + }; + let group = EcGroup::from_curve_name(Nid::X9_62_PRIME256V1).unwrap(); + let mut context = BigNumContext::new().unwrap(); + // Deterministic test keys. Find a coordinate with a leading zero to exercise + // fixed-width JWK encoding; normal random-key tests rarely cover this case. + for scalar in 1..=1024 { + let private = BigNum::from_u32(scalar).unwrap(); + let mut public = EcPoint::new(&group).unwrap(); + public + .mul_generator2(&group, &private, &mut context) + .unwrap(); + let raw = public + .to_bytes(&group, PointConversionForm::UNCOMPRESSED, &mut context) + .unwrap(); + if raw[1] != 0 && raw[33] != 0 { + continue; + } + let key = + PKey::from_ec_key(EcKey::from_private_components(&group, &private, &public).unwrap()) + .unwrap(); + let der = key.private_key_to_der().unwrap(); + let (curve, x, y) = (OpenSsl + .jwt_provider() + .jwk_utils + .extract_ec_public_key_coordinates)( + &der, jsonwebtoken::Algorithm::ES256 + ) + .unwrap(); + assert_eq!(curve, jsonwebtoken::jwk::EllipticCurve::P256); + assert_eq!(x, raw[1..33]); + assert_eq!(y, raw[33..]); + assert!( + (OpenSsl + .jwt_provider() + .jwk_utils + .extract_ec_public_key_coordinates)( + &der, jsonwebtoken::Algorithm::ES384 + ) + .is_err() + ); + return; + } + panic!("deterministic fixture range must include a leading-zero coordinate"); +} + +#[test] +fn backend_owned_keys_and_certificates() { + init(); + for alg in [ + &rcgen::PKCS_ECDSA_P256_SHA256, + &rcgen::PKCS_ECDSA_P384_SHA384, + &rcgen::PKCS_ED25519, + &rcgen::PKCS_RSA_SHA256, + ] { + let key = pki::generate_keypair_for(alg).unwrap(); + let der = key.serialize_der().unwrap(); + let from_der = pki::KeyPair::from_pkcs8_der(&der).unwrap(); + let from_pem = pki::KeyPair::from_pem(&key.serialize_pem().unwrap()).unwrap(); + for imported in [&from_der, &from_pem] { + assert_eq!(key.public_key_der(), imported.public_key_der()); + let cert = pki::self_signed( + rcgen::CertificateParams::new(vec!["localhost".into()]).unwrap(), + imported, + ) + .unwrap(); + let x509 = openssl::x509::X509::from_der(cert.der()).unwrap(); + assert!(x509.verify(&x509.public_key().unwrap()).unwrap()); + } + } + assert!(pki::KeyPair::from_pkcs8_der(b"invalid key").is_err()); + assert!(pki::KeyPair::from_pem("invalid key").is_err()); +} + +#[test] +fn jwt_algorithms_and_claims_validation() { + use jsonwebtoken::{ + Algorithm, DecodingKey, EncodingKey, Header, Validation, errors::ErrorKind, + }; + init(); + let claims = serde_json::json!({"sub":"sandbox", "iss":"gateway", "aud":"sandbox", "exp":4_000_000_000_u64}); + let mut keys = vec![( + Algorithm::HS256, + EncodingKey::from_secret(b"backend-test-secret"), + DecodingKey::from_secret(b"backend-test-secret"), + )]; + for (alg, key_alg) in [ + (Algorithm::EdDSA, &rcgen::PKCS_ED25519), + (Algorithm::ES256, &rcgen::PKCS_ECDSA_P256_SHA256), + (Algorithm::RS256, &rcgen::PKCS_RSA_SHA256), + ] { + let key = pki::generate_keypair_for(key_alg).unwrap(); + let private = key.serialize_pem().unwrap(); + let public = key.public_key_pem(); + let pair = match alg { + Algorithm::EdDSA => ( + EncodingKey::from_ed_pem(private.as_bytes()).unwrap(), + DecodingKey::from_ed_pem(public.as_bytes()).unwrap(), + ), + Algorithm::ES256 => ( + EncodingKey::from_ec_pem(private.as_bytes()).unwrap(), + DecodingKey::from_ec_pem(public.as_bytes()).unwrap(), + ), + _ => ( + EncodingKey::from_rsa_pem(private.as_bytes()).unwrap(), + DecodingKey::from_rsa_pem(public.as_bytes()).unwrap(), + ), + }; + keys.push((alg, pair.0, pair.1)); + } + for (alg, private, public) in keys { + let token = jwt::encode(&Header::new(alg), &claims, &private).unwrap(); + let mut validation = Validation::new(alg); + validation.set_issuer(&["gateway"]); + validation.set_audience(&["sandbox"]); + assert_eq!( + jwt::decode::(&token, &public, &validation) + .unwrap() + .claims, + claims + ); + let mut tampered = token.clone().into_bytes(); + let signature_start = tampered.iter().rposition(|b| *b == b'.').unwrap() + 1; + tampered[signature_start] = if tampered[signature_start] == b'A' { + b'B' + } else { + b'A' + }; + assert_eq!( + jwt::decode::(&tampered, &public, &validation) + .unwrap_err() + .kind(), + &ErrorKind::InvalidSignature + ); + validation.set_issuer(&["wrong"]); + assert_eq!( + jwt::decode::(&token, &public, &validation) + .unwrap_err() + .kind(), + &ErrorKind::InvalidIssuer + ); + validation.set_issuer(&["gateway"]); + validation.set_audience(&["wrong"]); + assert_eq!( + jwt::decode::(&token, &public, &validation) + .unwrap_err() + .kind(), + &ErrorKind::InvalidAudience + ); + validation.set_audience(&["sandbox"]); + let expired = + serde_json::json!({"sub":"sandbox", "iss":"gateway", "aud":"sandbox", "exp":1}); + let token = jwt::encode(&Header::new(alg), &expired, &private).unwrap(); + assert_eq!( + jwt::decode::(&token, &public, &validation) + .unwrap_err() + .kind(), + &ErrorKind::ExpiredSignature + ); + } +} + +#[test] +fn tls13_handshake_and_application_data() { + init(); + let pki::CertifiedKey { cert, key_pair } = + pki::generate_simple_self_signed(vec!["localhost".into()]).unwrap(); + let mut roots = rustls::RootCertStore::empty(); + roots.add(cert.der().clone()).unwrap(); + let client = tls::client_builder_with_protocol_versions(&[&rustls::version::TLS13]) + .with_root_certificates(roots) + .with_no_client_auth(); + let server = tls::server_builder_with_protocol_versions(&[&rustls::version::TLS13]) + .with_no_client_auth() + .with_single_cert( + vec![cert.der().clone()], + rustls::pki_types::PrivatePkcs8KeyDer::from(key_pair.serialize_der().unwrap()).into(), + ) + .unwrap(); + let mut client = + rustls::ClientConnection::new(Arc::new(client), "localhost".try_into().unwrap()).unwrap(); + let mut server = rustls::ServerConnection::new(Arc::new(server)).unwrap(); + for _ in 0..10 { + let mut wire = Vec::new(); + client.write_tls(&mut wire).unwrap(); + server.read_tls(&mut wire.as_slice()).unwrap(); + server.process_new_packets().unwrap(); + wire.clear(); + server.write_tls(&mut wire).unwrap(); + client.read_tls(&mut wire.as_slice()).unwrap(); + client.process_new_packets().unwrap(); + if !client.is_handshaking() && !server.is_handshaking() { + break; + } + } + assert!(!client.is_handshaking() && !server.is_handshaking()); + assert_eq!( + client.protocol_version(), + Some(rustls::ProtocolVersion::TLSv1_3) + ); + assert_eq!( + client.negotiated_cipher_suite().unwrap().suite(), + rustls::CipherSuite::TLS13_AES_256_GCM_SHA384 + ); + client.writer().write_all(b"openssl transport").unwrap(); + let mut wire = Vec::new(); + client.write_tls(&mut wire).unwrap(); + server.read_tls(&mut wire.as_slice()).unwrap(); + server.process_new_packets().unwrap(); + let mut received = [0; 17]; + server.reader().read_exact(&mut received).unwrap(); + assert_eq!(&received, b"openssl transport"); +} + +#[test] +fn imported_ca_signs_with_openssl() { + init(); + let key = pki::generate_keypair().unwrap(); + let mut params = rcgen::CertificateParams::default(); + params.is_ca = rcgen::IsCa::Ca(rcgen::BasicConstraints::Unconstrained); + params.key_usages = vec![rcgen::KeyUsagePurpose::KeyCertSign]; + let ca = pki::self_signed(params, &key).unwrap(); + let pem = key.serialize_pem().unwrap(); + let issuer = pki::issuer_from_der(ca.der(), pki::KeyPair::from_pem(&pem).unwrap()).unwrap(); + let leaf_key = pki::generate_keypair().unwrap(); + let leaf = pki::signed_by_issuer( + rcgen::CertificateParams::new(vec!["localhost".into()]).unwrap(), + &leaf_key, + &issuer, + ) + .unwrap(); + let ca = openssl::x509::X509::from_der(ca.der()).unwrap(); + let leaf = openssl::x509::X509::from_der(leaf.der()).unwrap(); + assert!(leaf.verify(&ca.public_key().unwrap()).unwrap()); + assert_eq!( + leaf.issuer_name().to_der().unwrap(), + ca.subject_name().to_der().unwrap() + ); +} + +#[test] +fn malformed_hmac_signatures_are_rejected() { + init(); + for algorithm in [ + jsonwebtoken::Algorithm::HS256, + jsonwebtoken::Algorithm::HS384, + jsonwebtoken::Algorithm::HS512, + ] { + let token = jwt::encode( + &jsonwebtoken::Header::new(algorithm), + &serde_json::json!({"exp":4_000_000_000_u64}), + &jsonwebtoken::EncodingKey::from_secret(b"test-key"), + ) + .unwrap(); + let (message, _) = token.rsplit_once('.').unwrap(); + for signature in ["", "AA", &"A".repeat(100)] { + assert_eq!( + jwt::decode::( + format!("{message}.{signature}"), + &jsonwebtoken::DecodingKey::from_secret(b"test-key"), + &jsonwebtoken::Validation::new(algorithm), + ) + .unwrap_err() + .kind(), + &jsonwebtoken::errors::ErrorKind::InvalidSignature + ); + } + } +} + +#[test] +#[cfg(feature = "aws-lc")] +fn backends_preserve_ciphertext_keys_and_jwt_signatures() { + use jsonwebtoken::{Algorithm, DecodingKey, EncodingKey}; + init(); + let aws = CryptoContext::default(); + assert_eq!(aws.backend().capabilities().backend, "aws-lc"); + let openssl: &dyn ProtocolBackend = &OpenSsl; + for (producer, consumer) in [(aws.backend(), openssl), (openssl, aws.backend())] { + let sealed = producer + .seal(&[42; 32], b"record-id", b"persisted credential") + .unwrap(); + assert_eq!( + consumer + .open(&[42; 32], b"record-id", &sealed.nonce, &sealed.ciphertext) + .unwrap(), + b"persisted credential" + ); + for (key_algorithm, algorithms) in [ + (&rcgen::PKCS_ED25519, &[Algorithm::EdDSA][..]), + (&rcgen::PKCS_ECDSA_P256_SHA256, &[Algorithm::ES256][..]), + (&rcgen::PKCS_ECDSA_P384_SHA384, &[Algorithm::ES384][..]), + ] { + let key = producer.generate_keypair(key_algorithm).unwrap(); + let imported = consumer + .import_keypair_der(&key.serialize_der().unwrap()) + .unwrap(); + assert_eq!(imported.public_key_der(), key.public_key_der()); + let private = key.serialize_pem().unwrap(); + let public = key.public_key_pem(); + let (encoding, decoding) = if algorithms[0] == Algorithm::EdDSA { + ( + EncodingKey::from_ed_pem(private.as_bytes()).unwrap(), + DecodingKey::from_ed_pem(public.as_bytes()).unwrap(), + ) + } else { + ( + EncodingKey::from_ec_pem(private.as_bytes()).unwrap(), + DecodingKey::from_ec_pem(public.as_bytes()).unwrap(), + ) + }; + for algorithm in algorithms { + check_signature(producer, consumer, *algorithm, &encoding, &decoding); + } + } + let key = OpenSsl.generate_keypair(&rcgen::PKCS_RSA_SHA256).unwrap(); + let imported = consumer + .import_keypair_der(&key.serialize_der().unwrap()) + .unwrap(); + assert_eq!(imported.public_key_der(), key.public_key_der()); + let encoding = EncodingKey::from_rsa_pem(key.serialize_pem().unwrap().as_bytes()).unwrap(); + let decoding = DecodingKey::from_rsa_pem(key.public_key_pem().as_bytes()).unwrap(); + for algorithm in [ + Algorithm::RS256, + Algorithm::RS384, + Algorithm::RS512, + Algorithm::PS256, + Algorithm::PS384, + Algorithm::PS512, + ] { + check_signature(producer, consumer, algorithm, &encoding, &decoding); + } + for algorithm in [Algorithm::HS256, Algorithm::HS384, Algorithm::HS512] { + check_signature( + producer, + consumer, + algorithm, + &EncodingKey::from_secret(b"compatibility key"), + &DecodingKey::from_secret(b"compatibility key"), + ); + } + } +} + +#[cfg(feature = "aws-lc")] +fn check_signature( + producer: &dyn ProtocolBackend, + consumer: &dyn ProtocolBackend, + algorithm: jsonwebtoken::Algorithm, + encoding: &jsonwebtoken::EncodingKey, + decoding: &jsonwebtoken::DecodingKey, +) { + let signer = (producer.jwt_provider().signer_factory)(&algorithm, encoding).unwrap(); + let verifier = (consumer.jwt_provider().verifier_factory)(&algorithm, decoding).unwrap(); + let signature = signer.try_sign(b"header.claims").unwrap(); + verifier + .verify(b"header.claims", &signature) + .unwrap_or_else(|error| { + panic!( + "{} -> {} {algorithm:?}: {error}", + producer.capabilities().backend, + consumer.capabilities().backend + ) + }); + assert!(verifier.verify(b"wrong.claims", &signature).is_err()); +} diff --git a/crates/openshell-crypto/tests/openssl_tls.rs b/crates/openshell-crypto/tests/openssl_tls.rs new file mode 100644 index 0000000000..6a3960351b --- /dev/null +++ b/crates/openshell-crypto/tests/openssl_tls.rs @@ -0,0 +1,135 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +#![cfg(feature = "openssl")] + +use openshell_crypto::{CryptoContext, OpenSsl, ProtocolBackend, pki, tls}; +use std::sync::{Arc, Once}; + +fn init() { + static INIT: Once = Once::new(); + INIT.call_once(|| { + #[cfg(feature = "aws-lc")] + rustls::crypto::aws_lc_rs::default_provider() + .install_default() + .unwrap(); + openshell_crypto::install_default_context(CryptoContext::new(Box::new(OpenSsl))).unwrap(); + }); +} + +fn handshake( + client: rustls::ClientConfig, + server: rustls::ServerConfig, + host: &'static str, +) -> Result<(), rustls::Error> { + let mut client = + rustls::ClientConnection::new(Arc::new(client), host.try_into().unwrap()).unwrap(); + let mut server = rustls::ServerConnection::new(Arc::new(server)).unwrap(); + for _ in 0..10 { + let mut wire = Vec::new(); + client.write_tls(&mut wire).unwrap(); + server.read_tls(&mut wire.as_slice()).unwrap(); + server.process_new_packets()?; + wire.clear(); + server.write_tls(&mut wire).unwrap(); + client.read_tls(&mut wire.as_slice()).unwrap(); + client.process_new_packets()?; + if !client.is_handshaking() && !server.is_handshaking() { + return Ok(()); + } + } + Err(rustls::Error::General("handshake did not finish".into())) +} + +fn leaf( + ca: &pki::Certificate, + ca_key: &pki::KeyPair, + usage: rcgen::ExtendedKeyUsagePurpose, +) -> pki::CertifiedKey { + let key_pair = pki::generate_keypair().unwrap(); + let mut params = rcgen::CertificateParams::new(vec!["localhost".into()]).unwrap(); + params.extended_key_usages = vec![usage]; + params.key_usages = vec![rcgen::KeyUsagePurpose::DigitalSignature]; + let cert = pki::signed_by(params, &key_pair, ca, ca_key).unwrap(); + pki::CertifiedKey { cert, key_pair } +} + +#[test] +fn tls12_and_tls13_enforce_server_and_client_authentication() { + init(); + assert!(std::ptr::eq( + tls::configuration_provider().secure_random, + OpenSsl.tls_provider().secure_random + )); + let ca_key = pki::generate_keypair().unwrap(); + let mut params = rcgen::CertificateParams::default(); + params.is_ca = rcgen::IsCa::Ca(rcgen::BasicConstraints::Unconstrained); + params.key_usages = vec![rcgen::KeyUsagePurpose::KeyCertSign]; + let ca = pki::self_signed(params, &ca_key).unwrap(); + let server_key = leaf(&ca, &ca_key, rcgen::ExtendedKeyUsagePurpose::ServerAuth); + let client_key = leaf(&ca, &ca_key, rcgen::ExtendedKeyUsagePurpose::ClientAuth); + let mut roots = rustls::RootCertStore::empty(); + roots.add(ca.der().clone()).unwrap(); + for version in [&rustls::version::TLS12, &rustls::version::TLS13] { + let verifier = rustls::server::WebPkiClientVerifier::builder_with_provider( + Arc::new(roots.clone()), + tls::configuration_provider(), + ) + .build() + .unwrap(); + let server = tls::server_builder_with_protocol_versions(&[version]) + .with_client_cert_verifier(verifier) + .with_single_cert( + vec![server_key.cert.der().clone()], + rustls::pki_types::PrivatePkcs8KeyDer::from( + server_key.key_pair.serialize_der().unwrap(), + ) + .into(), + ) + .unwrap(); + let client = tls::client_builder_with_protocol_versions(&[version]) + .with_root_certificates(roots.clone()) + .with_client_auth_cert( + vec![client_key.cert.der().clone()], + rustls::pki_types::PrivatePkcs8KeyDer::from( + client_key.key_pair.serialize_der().unwrap(), + ) + .into(), + ) + .unwrap(); + handshake(client.clone(), server.clone(), "localhost").unwrap(); + assert!(matches!( + handshake(client, server.clone(), "wrong.example"), + Err(rustls::Error::InvalidCertificate(_)) + )); + let anonymous = tls::client_builder_with_protocol_versions(&[version]) + .with_root_certificates(roots.clone()) + .with_no_client_auth(); + assert!(matches!( + handshake(anonymous, server.clone(), "localhost"), + Err(rustls::Error::NoCertificatesPresented) + )); + let untrusted = tls::client_builder_with_protocol_versions(&[version]) + .with_root_certificates(rustls::RootCertStore::empty()) + .with_no_client_auth(); + assert!(matches!( + handshake(untrusted, server.clone(), "localhost"), + Err(rustls::Error::InvalidCertificate(_)) + )); + let stranger = pki::generate_simple_self_signed(vec!["localhost".into()]).unwrap(); + let unauthorized = tls::client_builder_with_protocol_versions(&[version]) + .with_root_certificates(roots.clone()) + .with_client_auth_cert( + vec![stranger.cert.der().clone()], + rustls::pki_types::PrivatePkcs8KeyDer::from( + stranger.key_pair.serialize_der().unwrap(), + ) + .into(), + ) + .unwrap(); + assert!(matches!( + handshake(unauthorized, server, "localhost"), + Err(rustls::Error::InvalidCertificate(_)) + )); + } +} diff --git a/crates/openshell-crypto/tests/openssl_unavailable.rs b/crates/openshell-crypto/tests/openssl_unavailable.rs new file mode 100644 index 0000000000..2d880c7fd8 --- /dev/null +++ b/crates/openshell-crypto/tests/openssl_unavailable.rs @@ -0,0 +1,86 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +#![cfg(feature = "openssl")] + +use openshell_crypto::{CryptoBackend, CryptoContext, CryptoError, OpenSsl, ProtocolBackend}; + +#[test] +fn unavailable_provider_fails_without_fallback() { + // OpenSSL reads configuration once. Exercise failure in a fresh process, + // without changing the test runner's environment or process-global state. + let result = std::process::Command::new(std::env::current_exe().unwrap()) + .args([ + "--ignored", + "--exact", + "unavailable_provider_child", + "--nocapture", + ]) + .env("OPENSHELL_TEST_UNAVAILABLE", "1") + .env( + "OPENSSL_CONF", + concat!( + env!("CARGO_MANIFEST_DIR"), + "/tests/fixtures/openssl-unavailable.cnf" + ), + ) + .output() + .unwrap(); + assert!( + result.status.success(), + "{}\n{}", + String::from_utf8_lossy(&result.stdout), + String::from_utf8_lossy(&result.stderr) + ); + assert!(String::from_utf8_lossy(&result.stdout).contains("1 passed")); +} + +#[test] +#[ignore = "run in a subprocess with deliberately unavailable OpenSSL algorithms"] +fn unavailable_provider_child() { + assert_eq!(std::env::var("OPENSHELL_TEST_UNAVAILABLE").unwrap(), "1"); + openshell_crypto::install_default_context(CryptoContext::new(Box::new(OpenSsl))).unwrap(); + assert_eq!( + openshell_crypto::sha256(b"message"), + Err(CryptoError::Operation) + ); + assert_eq!( + openshell_crypto::random_bytes::<32>(), + Err(CryptoError::Random) + ); + assert_eq!( + OpenSsl.seal(&[0; 32], b"", b"plaintext").unwrap_err(), + CryptoError::Random + ); + // A full-length tag reaches cipher initialization: unavailable AES is an + // operational error, not a corrupt stored credential. + assert_eq!( + OpenSsl.open(&[0; 32], b"", &[0; 12], &[0; 16]), + Err(CryptoError::Operation) + ); + for algorithm in [ + &rcgen::PKCS_ECDSA_P256_SHA256, + &rcgen::PKCS_ECDSA_P384_SHA384, + &rcgen::PKCS_ED25519, + &rcgen::PKCS_RSA_SHA256, + ] { + assert!(OpenSsl.generate_keypair(algorithm).is_err()); + } + assert!( + openshell_crypto::jwt::encode( + &jsonwebtoken::Header::default(), + &serde_json::json!({"exp":4_000_000_000_u64}), + &jsonwebtoken::EncodingKey::from_secret(b"test-only-key"), + ) + .is_err() + ); + // The adapter filters unavailable ciphers; use the fallible Rustls builder + // rather than the facade convenience builder, which asserts compatibility. + let provider = OpenSsl.tls_provider(); + assert!(provider.cipher_suites.is_empty()); + assert!( + rustls::ClientConfig::builder_with_provider(std::sync::Arc::new(provider)) + .with_safe_default_protocol_versions() + .is_err() + ); +} From dc92d95f66e7c19ab12da824c66b7011c0e6951f Mon Sep 17 00:00:00 2001 From: Varsha Prasad Narsing Date: Wed, 21 Oct 2026 23:18:11 -0700 Subject: [PATCH 2/7] ci(crypto): enforce crypto dependency boundaries with cargo-deny Signed-off-by: Varsha Prasad Narsing --- .agents/skills/build-from-issue/SKILL.md | 2 + .agents/skills/fix-security-issue/SKILL.md | 2 + .agents/skills/review-github-pr/SKILL.md | 2 +- .github/workflows/branch-checks.yml | 3 + AGENTS.md | 2 +- Cargo.lock | 1 - crates/openshell-crypto/CRYPTO.md | 63 ++++ crates/openshell-crypto/README.md | 14 +- crates/openshell-sandbox/Cargo.toml | 1 - deny.toml | 336 +++++++++++++++++- tasks/rust.toml | 6 + .../scripts/test-crypto-dependency-policy.py | 171 +++++++++ 12 files changed, 591 insertions(+), 12 deletions(-) create mode 100644 tasks/scripts/test-crypto-dependency-policy.py diff --git a/.agents/skills/build-from-issue/SKILL.md b/.agents/skills/build-from-issue/SKILL.md index 6e99e7e1b2..b180c63fb5 100644 --- a/.agents/skills/build-from-issue/SKILL.md +++ b/.agents/skills/build-from-issue/SKILL.md @@ -34,3 +34,5 @@ Every PR from this issue-backed workflow must have its own existing issue and us Do not apply acceptance or roadmap decisions on behalf of a maintainer. Do not introduce `agent:*` workflow labels. For first-party security-sensitive crypto, use `openshell-crypto` and extend its facade/backend contracts when needed. Follow the [crypto architecture](../../../crates/openshell-crypto/CRYPTO.md) for compatibility, failure handling, and explicit coverage exceptions. + +When changing crypto dependencies, run `mise run rust:deny:policy`, including its dependency-boundary regression fixtures. Review any `deny.toml` wrapper change against the documented exception scope; an existing allowance does not authorize new security-sensitive operations outside the facade. diff --git a/.agents/skills/fix-security-issue/SKILL.md b/.agents/skills/fix-security-issue/SKILL.md index 3391ed2690..b0b15e981b 100644 --- a/.agents/skills/fix-security-issue/SKILL.md +++ b/.agents/skills/fix-security-issue/SKILL.md @@ -18,3 +18,5 @@ Use this skill after an authorized `review-security-issue` review identifies an Begin any fix comments with `> **🔧 security-fix-agent**`. Do not change human disposition or introduce `agent:*` workflow labels. For first-party security-sensitive crypto, use `openshell-crypto` and extend its facade/backend contracts when needed. Follow the [crypto architecture](../../../crates/openshell-crypto/CRYPTO.md) for compatibility, failure handling, and explicit coverage exceptions. + +When changing crypto dependencies, run `mise run rust:deny:policy`, including its dependency-boundary regression fixtures. Review any `deny.toml` wrapper change against the documented exception scope; an existing allowance does not authorize new security-sensitive operations outside the facade. diff --git a/.agents/skills/review-github-pr/SKILL.md b/.agents/skills/review-github-pr/SKILL.md index 9813402c76..e71a3182bf 100644 --- a/.agents/skills/review-github-pr/SKILL.md +++ b/.agents/skills/review-github-pr/SKILL.md @@ -153,7 +153,7 @@ Read through the full diff (and the PR description if available). Produce a summ suggested change. - Do not fabricate concerns or claim a behavioral regression without evidence for both the prior and proposed behavior. -- **Cryptographic boundaries**: For changed first-party security-sensitive crypto, verify use of `openshell-crypto`, backend dispatch, failure propagation without fallback, and protocol/storage compatibility. Missing operations belong in the facade/backend contracts. Check dependency-owned and non-security exceptions against the repository's [crypto architecture](../../../crates/openshell-crypto/CRYPTO.md); protocol/parsing types alone are not bypasses. Report undocumented bypasses or scope expansion under **Potential Concerns**. +- **Cryptographic boundaries**: For changed first-party security-sensitive crypto, verify use of `openshell-crypto`, backend dispatch, failure propagation without fallback, and protocol/storage compatibility. Missing operations belong in the facade/backend contracts. Check dependency-owned and other explicit exceptions against the repository's [crypto architecture](../../../crates/openshell-crypto/CRYPTO.md); protocol/parsing types alone are not bypasses. Check `mise run rust:deny:policy` results and review changes to `deny.toml` wrappers; package-level allowances do not authorize new bypasses within those packages. Report undocumented bypasses or scope expansion under **Potential Concerns**. - **Agent infrastructure**: When the PR changes behavior, commands, or development workflows, use the `sync-agent-infra` maintenance map to check that related skills were updated. When it adds, removes, or renames skills or crates; changes workflow relationships or skill coverage; modifies issue or PR templates; or changes agent cross-references, apply the full consistency checklist. Report missing companion updates or drift under **Potential Concerns**. ## Step 5: Output diff --git a/.github/workflows/branch-checks.yml b/.github/workflows/branch-checks.yml index 6fa085d836..c8e9eed977 100644 --- a/.github/workflows/branch-checks.yml +++ b/.github/workflows/branch-checks.yml @@ -196,6 +196,9 @@ jobs: prepare-shell: "true" shell-installable: .#devShells.x86_64-linux.default + - name: Verify crypto dependency guardrail + run: uv run --no-project python tasks/scripts/test-crypto-dependency-policy.py + - name: Check dependencies run: cargo deny check licenses bans sources diff --git a/AGENTS.md b/AGENTS.md index d1652c2606..c31e822b66 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -237,7 +237,7 @@ When behavior, commands, or development workflows change, review the related age ## Security -- Route first-party security-sensitive cryptographic operations through `openshell-crypto`. If an operation is missing, extend the facade and backend implementation instead of adding a direct cryptographic implementation in an application crate. Preserve protocol compatibility and propagate backend failures without fallback. Protocol/parsing types and documented non-security or dependency-owned exceptions remain permitted; classify exceptions explicitly. Follow [Cryptography and FIPS](crates/openshell-crypto/CRYPTO.md). +- Route first-party security-sensitive cryptographic operations through `openshell-crypto`. If an operation is missing, extend the facade and backend implementation instead of adding a direct cryptographic implementation in an application crate. Preserve protocol compatibility and propagate backend failures without fallback. Protocol/parsing types and documented exceptions remain permitted; classify exceptions explicitly. Run `mise run rust:deny:policy` when changing crypto dependencies; review `deny.toml` exceptions rather than widening them to pass CI. Follow [Cryptography and FIPS](crates/openshell-crypto/CRYPTO.md). - Never commit secrets, API keys, or credentials. If a file looks like it contains secrets (`.env`, `credentials.json`, etc.), do not stage it. - Do not run destructive operations (force push, hard reset, database drops) without explicit human confirmation. - Scope changes to the issue at hand. Do not make unrelated changes in the same branch. diff --git a/Cargo.lock b/Cargo.lock index 7091ca6f91..73e5f53d70 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4938,7 +4938,6 @@ dependencies = [ "seccompiler", "serde", "serde_json", - "sha2 0.10.9", "socket2", "tempfile", "tokio", diff --git a/crates/openshell-crypto/CRYPTO.md b/crates/openshell-crypto/CRYPTO.md index ca0f26c0c9..2aa09d0515 100644 --- a/crates/openshell-crypto/CRYPTO.md +++ b/crates/openshell-crypto/CRYPTO.md @@ -60,6 +60,69 @@ cache fingerprints, UUID entropy, SSH, and other dependency-owned operations. Review changes against that scope. Cargo features and contributor instructions alone do not prove exclusive backend ownership. +## Dependency guardrail + +The root `deny.toml` uses cargo-deny's `bans.deny` and `wrappers` rules to reject +unapproved direct dependencies on named crypto implementations. AWS-LC and OpenSSL +are accessible through `openshell-crypto` and the explicitly listed libraries +that own their own crypto operations. Native `*-sys` crates and the OpenSSL +protocol adapters have separate rules to prevent bypassing the provider boundary. +The catalog also covers RustCrypto hashes, MACs, ciphers, and signature libraries. +Protocol/parsing types remain usable in application crates. + +Cargo Deny in Branch Checks enforces the policy with all workspace features and +no target filter, including build and development dependencies. Renaming a +dependency in `Cargo.toml` does not bypass a package-name ban. An allowed wrapper +on one path does not excuse another unapproved direct parent of the same package. +Run the policy and its offline regression fixtures with: + +```shell +mise run rust:deny:policy +``` + +Run just the regression fixtures with `mise run rust:deny:crypto-test`. They use +local stub packages to exercise cargo-deny without compiling cryptography or +downloading dependencies. They test allowed provider paths, shared-dependency +bypasses, aliases, build/dev/target dependencies, and unapproved intermediary +wrappers. The normal policy check separately validates the real dependency graph. + +### Existing direct first-party exceptions + +These are package-level allowances, not permission to add new security-sensitive +operations outside the facade. Migration must preserve protocols and stored +identities and propagate backend failures. Remove an allowance when its last use +is migrated or deleted. + +| Dependency | Allowed application crates | Classification | +| --- | --- | --- | +| `sha2` | `openshell-core` | Endpoint identities and canonical policy hashes; policy binding still requires migration. | +| `sha2` | `openshell-driver-docker`, `openshell-driver-kubernetes-secrets`, `openshell-driver-vault` | Deterministic volume and credential-storage names, not authentication. | +| `sha2` | `openshell-driver-vm`, `openshell-isolation-interface`, `openshell-supervisor-network` | Artifact integrity, outer-fence evidence, and DNS policy binding; security-sensitive migration remains required. | +| `sha2` | `openshell-gateway-interceptors` | Provider-profile snapshot revision hashes; content identity outside backend dispatch. | +| `sha2` | `openshell-supervisor-process` | Diagnostic debug-token fingerprints. | +| `sha2`, `hmac` | `openshell-server` | Naming, coordination, request matching, and tests, plus security-sensitive bearer-cache keys, launch-session derivation, policy/review bindings, credential handles, and protected mutation fingerprints. The security-sensitive uses must migrate; the HMAC exception is for `grpc::mutation_replay`'s protection against offline guessing. | +| `rsa` | `openshell-server` | Development dependency for OIDC test keys. Cargo-deny cannot restrict a wrapper exception to development dependencies. | +| `sha1` | `openshell-supervisor-network` | WebSocket accept-handshake hashing, a protocol requirement rather than cryptographic authentication. | + +The other wrappers in `deny.toml` cover dependency-owned operations: SSH, +TLS/X.509/JWT, AWS SigV4, OAuth PKCE, database protocols, archive handling, +artifact/content hashing, and implementation-to-implementation dependencies. +The existing Rustls/Ring exceptions support the Z3 downloader; wrapper rules +constrain direct parents, not an entire ancestry exclusively to Z3. Dependency +upgrades that introduce a new parent must receive review instead of automatically +adding that parent to the allowlist. + +This is an enumerated dependency guardrail, not a source scanner or a FIPS +attestation. It does not automatically recognize a new cryptographic crate, +detect hand-written cryptography, inspect calls through re-exports, or prevent +new uses inside an already allowed package. Generic entropy sources such as +`rand`, `getrandom`, and `uuid` remain outside this catalog because ordinary +scheduling and dependency-owned entropy coexist with the outstanding +security-sensitive UUID uses. Standalone Cargo workspaces require their own +policy invocation. Review source changes as well as manifests, extend the catalog +when introducing a crypto library, and document every new exception here with +its scope and migration requirement. Do not weaken the policy merely to pass CI. + ## OpenSSL and FIPS follow-up Backend capability reporting describes that backend's operations. A backend name, diff --git a/crates/openshell-crypto/README.md b/crates/openshell-crypto/README.md index 3c0355550c..ad5d205674 100644 --- a/crates/openshell-crypto/README.md +++ b/crates/openshell-crypto/README.md @@ -168,11 +168,13 @@ scheduling randomness are not migrated. No SSH or PQC capability is asserted by the primitive capability report. Application-wide OpenSSL integration, strict policy, module version discovery, and deployment qualification remain follow-ups. -Operator certificate fingerprints, runtime refresh replay hashes and derived token +Migrated paths for operator certificate fingerprints, runtime refresh replay hashes and derived token IDs, bearer-cache fingerprints, Sandbox Protocol request digests and request-ID entropy, Kubernetes/VM runtime fence entropy, VM socket-directory entropy, and rootfs staging-token entropy use -the selected backend. +the selected backend. The [dependency exception inventory](CRYPTO.md#existing-direct-first-party-exceptions) +records remaining direct implementations, including other bearer-cache and +session-identity paths. These operations retain SHA-256, UUID v4 encoding where applicable, and existing hex encodings. Backend failures propagate; callers must not substitute another implementation or continue with a placeholder identity. @@ -194,9 +196,11 @@ not a whole-application crypto coverage claim: | Debug token fingerprints | Diagnostic display only; direct hashing remains outside the contract even though the input is a credential. | | Schema compatibility tests, content-cache identities, retry jitter, test fixtures | Non-authentication uses remain unchanged. | -Cargo feature selection does not prevent a crate from adding another crypto -dependency. Review first-party security-sensitive operations against this scope; -no source-scanning boundary lint is installed. +The [cargo-deny dependency guardrail](CRYPTO.md#dependency-guardrail) rejects +unapproved direct dependencies on the catalogued crypto implementations. It +preserves explicitly classified exceptions, including security-sensitive uses +awaiting migration. Review operations inside those crates against that scope; +the dependency check does not inspect source-level usage. Durable proxy CA loading uses backend key import and `pki::issuer_from_der`. The helper uses `x509-parser` without verification features to read the subject, diff --git a/crates/openshell-sandbox/Cargo.toml b/crates/openshell-sandbox/Cargo.toml index 78414b267a..cf29bcc9e9 100644 --- a/crates/openshell-sandbox/Cargo.toml +++ b/crates/openshell-sandbox/Cargo.toml @@ -37,7 +37,6 @@ bytes = { workspace = true } hex = "0.4" ipnet = "2" rand = "0.10" -sha2 = { workspace = true } # Async runtime tokio = { workspace = true } diff --git a/deny.toml b/deny.toml index 16be75953e..6366c028b9 100644 --- a/deny.toml +++ b/deny.toml @@ -62,15 +62,345 @@ registries = [] # -- Bans ---------------------------------------------------------------------- [bans] -deny = [ - { name = "ring", wrappers = ["rustls", "rustls-webpki"], reason = "Use the selected crypto backend instead. These wrappers are allowed for z3-sys's prebuilt archive downloader." }, -] multiple-versions = "warn" wildcards = "allow" highlight = "all" workspace-default-features = "allow" external-default-features = "allow" +# Crypto implementations must stay behind the facade or an explicit wrapper. +# See crates/openshell-crypto/CRYPTO.md#dependency-guardrail for classifications. +# Wrappers permit only direct parents, not arbitrary descendants. Keep this +# catalog and its exceptions reviewed when changing cryptographic dependencies. + +[[bans.deny]] +name = "ring" +wrappers = ["rustls", "rustls-webpki"] +reason = "Use the selected crypto backend instead. These wrappers are allowed for z3-sys's prebuilt archive downloader." + +[[bans.deny]] +name = "aws-lc-rs" +reason = "Use openshell-crypto; exceptions are classified in CRYPTO.md." +wrappers = [ + "jsonwebtoken", + "openshell-crypto", + "rcgen", + "russh", + "rustls", + "rustls-webpki", +] + +[[bans.deny]] +name = "aws-lc-sys" +reason = "Native crypto bindings belong behind their provider libraries, not application crates." +wrappers = [ + "aws-lc-rs", +] + +[[bans.deny]] +name = "aws-lc-fips-sys" +# Keep this boundary even when the FIPS build feature is disabled. +reason = "Native crypto bindings belong behind their provider libraries, not application crates." +wrappers = [ + "aws-lc-rs", +] + +[[bans.deny]] +name = "openssl" +reason = "Use openshell-crypto; exceptions are classified in CRYPTO.md." +wrappers = [ + "jsonwebtoken-openssl", + "openshell-crypto", + "rustls-openssl", +] + +[[bans.deny]] +name = "openssl-sys" +reason = "Native crypto bindings belong behind their provider libraries, not application crates." +wrappers = [ + "openssl", + "rustls-openssl", +] + +[[bans.deny]] +name = "rustls-openssl" +reason = "OpenSSL protocol adapters are owned by openshell-crypto." +wrappers = [ + "openshell-crypto", +] + +[[bans.deny]] +name = "jsonwebtoken-openssl" +reason = "OpenSSL protocol adapters are owned by openshell-crypto." +wrappers = [ + "openshell-crypto", +] + +[[bans.deny]] +name = "sha1" +reason = "Use openshell-crypto; exceptions are classified in CRYPTO.md." +wrappers = [ + "axum", + "openshell-supervisor-network", + "russh", + "ssh-key", + "tungstenite", + "zip", +] + +[[bans.deny]] +name = "sha2" +reason = "Use openshell-crypto; exceptions are classified in CRYPTO.md." +wrappers = [ + "aws-sigv4", + "bcrypt-pbkdf", + "ed25519-dalek", + "jsonwebtoken", + "lzma-rust2", + "oauth2", + "oci-client", + "openshell-core", + "openshell-driver-docker", + "openshell-driver-kubernetes-secrets", + "openshell-driver-vault", + "openshell-driver-vm", + "openshell-gateway-interceptors", + "openshell-isolation-interface", + "openshell-server", + "openshell-supervisor-network", + "openshell-supervisor-process", + "p256", + "p384", + "p521", + "pageant", + "pest_meta", + "pkcs5", + "rsa", + "russh", + "scrypt", + "sqlx-core", + "sqlx-macros-core", + "sqlx-postgres", + "ssh-key", +] + +[[bans.deny]] +name = "sha3" +reason = "Use openshell-crypto; exceptions are classified in CRYPTO.md." +wrappers = [ + "ml-kem", + "russh", +] + +[[bans.deny]] +name = "hmac" +reason = "Use openshell-crypto; exceptions are classified in CRYPTO.md." +wrappers = [ + "aws-sigv4", + "hkdf", + "jsonwebtoken", + "openshell-server", + "pbkdf2", + "rfc6979", + "russh", + "sqlx-postgres", + "ssh-key", + "zip", +] + +[[bans.deny]] +name = "hkdf" +reason = "Use openshell-crypto; exceptions are classified in CRYPTO.md." +wrappers = [ + "elliptic-curve", + "sqlx-postgres", +] + +[[bans.deny]] +name = "rsa" +reason = "Use openshell-crypto; exceptions are classified in CRYPTO.md." +wrappers = [ + "jsonwebtoken", + "openshell-server", + "russh", + "ssh-key", +] + +[[bans.deny]] +name = "aes" +reason = "Use openshell-crypto; exceptions are classified in CRYPTO.md." +wrappers = [ + "aes-gcm", + "pkcs5", + "russh", + "ssh-cipher", + "zip", +] + +[[bans.deny]] +name = "aes-gcm" +reason = "Use openshell-crypto; exceptions are classified in CRYPTO.md." +wrappers = [ + "pkcs5", + "ssh-cipher", +] + +[[bans.deny]] +name = "chacha20" +reason = "Use openshell-crypto; exceptions are classified in CRYPTO.md." +wrappers = [ + "rand", + "ssh-cipher", +] + +[[bans.deny]] +name = "chacha20poly1305" +reason = "Use openshell-crypto; exceptions are classified in CRYPTO.md." + +[[bans.deny]] +name = "poly1305" +reason = "Use openshell-crypto; exceptions are classified in CRYPTO.md." +wrappers = [ + "ssh-cipher", +] + +[[bans.deny]] +name = "p256" +reason = "Use openshell-crypto; exceptions are classified in CRYPTO.md." +wrappers = [ + "jsonwebtoken", + "russh", + "ssh-key", +] + +[[bans.deny]] +name = "p384" +reason = "Use openshell-crypto; exceptions are classified in CRYPTO.md." +wrappers = [ + "jsonwebtoken", + "russh", + "ssh-key", +] + +[[bans.deny]] +name = "p521" +reason = "Use openshell-crypto; exceptions are classified in CRYPTO.md." +wrappers = [ + "russh", + "ssh-key", +] + +[[bans.deny]] +name = "ed25519-dalek" +reason = "Use openshell-crypto; exceptions are classified in CRYPTO.md." +wrappers = [ + "jsonwebtoken", + "russh", + "ssh-key", +] + +[[bans.deny]] +name = "curve25519-dalek" +reason = "Use openshell-crypto; exceptions are classified in CRYPTO.md." +wrappers = [ + "ed25519-dalek", + "russh", +] + +[[bans.deny]] +name = "blake2" +reason = "Use openshell-crypto; exceptions are classified in CRYPTO.md." +wrappers = [ + "argon2", +] + +[[bans.deny]] +name = "blake3" +reason = "Use openshell-crypto; exceptions are classified in CRYPTO.md." + +[[bans.deny]] +name = "md-5" +reason = "Use openshell-crypto; exceptions are classified in CRYPTO.md." +wrappers = [ + "sqlx-postgres", +] + +[[bans.deny]] +name = "md5" +reason = "Use openshell-crypto; exceptions are classified in CRYPTO.md." +wrappers = [ + "russh", +] + +[[bans.deny]] +name = "pbkdf2" +reason = "Use openshell-crypto; exceptions are classified in CRYPTO.md." +wrappers = [ + "bcrypt-pbkdf", + "pkcs5", + "russh", + "scrypt", + "zip", +] + +[[bans.deny]] +name = "scrypt" +reason = "Use openshell-crypto; exceptions are classified in CRYPTO.md." +wrappers = [ + "pkcs5", + "russh", +] + +[[bans.deny]] +name = "argon2" +reason = "Use openshell-crypto; exceptions are classified in CRYPTO.md." +wrappers = [ + "ssh-key", +] + +[[bans.deny]] +name = "k256" +reason = "Use openshell-crypto; exceptions are classified in CRYPTO.md." + +[[bans.deny]] +name = "secp256k1" +reason = "Use openshell-crypto; exceptions are classified in CRYPTO.md." + +# Lower-level implementations used by the dependency-owned SSH stack. +[[bans.deny]] +name = "bcrypt-pbkdf" +wrappers = ["ssh-key"] +reason = "SSH key derivation is dependency-owned." + +[[bans.deny]] +name = "des" +reason = "No DES implementation is enabled; compatibility exceptions require explicit review." + +[[bans.deny]] +name = "ecdsa" +wrappers = ["p256", "p384", "p521", "russh"] +reason = "Use the facade for signing; dependency-owned implementations remain permitted." + +[[bans.deny]] +name = "fiat-crypto" +wrappers = ["curve25519-dalek", "p384"] +reason = "Curve arithmetic belongs behind the dependency-owned implementations." + +[[bans.deny]] +name = "ml-kem" +wrappers = ["russh"] +reason = "Post-quantum SSH cryptography is dependency-owned." + +[[bans.deny]] +name = "rfc6979" +wrappers = ["ecdsa"] +reason = "Signature nonce generation belongs behind the signing implementation." + +[[bans.deny]] +name = "ssh-cipher" +wrappers = ["ssh-key"] +reason = "SSH ciphers are dependency-owned." + # -- Sources ------------------------------------------------------------------- [sources] unknown-registry = "deny" diff --git a/tasks/rust.toml b/tasks/rust.toml index 682e478fca..cb5730ba12 100644 --- a/tasks/rust.toml +++ b/tasks/rust.toml @@ -64,12 +64,18 @@ hide = true ["rust:deny"] description = "Check dependencies for all cargo-deny rules" +depends = ["rust:deny:crypto-test"] run = "cargo deny check" ["rust:deny:policy"] description = "Check dependencies for license violations, bans, and source restrictions" +depends = ["rust:deny:crypto-test"] run = "cargo deny check licenses bans sources" +["rust:deny:crypto-test"] +description = "Verify cargo-deny rejects crypto dependencies that bypass the facade" +run = "uv run --no-project python tasks/scripts/test-crypto-dependency-policy.py" + ["rust:verify:telemetry-off"] description = "Verify telemetry emission code is compiled out with --no-default-features" diff --git a/tasks/scripts/test-crypto-dependency-policy.py b/tasks/scripts/test-crypto-dependency-policy.py new file mode 100644 index 0000000000..6845a32204 --- /dev/null +++ b/tasks/scripts/test-crypto-dependency-policy.py @@ -0,0 +1,171 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Exercise the real cargo-deny policy with tiny, offline Cargo package graphs. + +No crypto code is built or downloaded. Package names and dependency edges are +enough to check that an approved wrapper cannot mask an application bypass. +""" + +import json +import subprocess +import tempfile +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +POLICY = ROOT / "deny.toml" + + +def check_graph( + directory: Path, + edges: list[tuple[str, str, str, str]], + banned: str | None = None, +) -> None: + """Each edge is (parent, package, manifest section, dependency alias).""" + packages = {name for parent, child, _, _ in edges for name in (parent, child)} + members = { + name + for name in packages + if name.startswith("openshell-") or name == "policy-test-app" + } + directory.mkdir() + (directory / "Cargo.toml").write_text( + '[workspace]\nresolver = "2"\nmembers = ' + + json.dumps(sorted(members)) + + "\nexclude = " + + json.dumps(sorted(packages - members)) + + "\n", + encoding="utf-8", + ) + for package in packages: + source = directory / package / "src" + source.mkdir(parents=True) + (source / "lib.rs").write_text("", encoding="utf-8") + manifest = ( + f'[package]\nname = "{package}"\nversion = "0.0.0"\nedition = "2021"\n' + ) + sections: dict[str, list[str]] = {} + for parent, child, section, alias in edges: + if parent == package: + sections.setdefault(section, []).append( + f'{alias} = {{ package = "{child}", path = "../{child}" }}' + ) + for section, dependencies in sections.items(): + manifest += f"\n[{section}]\n" + "\n".join(dependencies) + "\n" + (source.parent / "Cargo.toml").write_text(manifest, encoding="utf-8") + + result = subprocess.run( + [ + "cargo", + "deny", + "--offline", + "--color", + "never", + "--manifest-path", + str(directory / "Cargo.toml"), + "--config", + str(POLICY), + "check", + "bans", + ], + cwd=directory, + capture_output=True, + text=True, + check=False, + ) + output = result.stdout + result.stderr + if banned is None: + passed = result.returncode == 0 + else: + # A metadata/configuration failure must not count as a successful ban. + passed = ( + result.returncode != 0 and f"error[banned]: crate '{banned} =" in output + ) + if not passed: + raise AssertionError(f"{directory.name}: unexpected policy result\n{output}") + print(f"PASS {directory.name}") + + +def edge( + parent: str, child: str, section: str = "dependencies", alias: str = "" +) -> tuple[str, str, str, str]: + return parent, child, section, alias or child + + +def main() -> None: + facade = [ + edge("policy-test-app", "openshell-crypto"), + edge("openshell-crypto", "aws-lc-rs"), + edge("aws-lc-rs", "aws-lc-sys"), + edge("aws-lc-rs", "aws-lc-fips-sys"), + edge("openshell-crypto", "openssl"), + edge("openssl", "openssl-sys"), + edge("openshell-crypto", "rustls-openssl"), + edge("rustls-openssl", "openssl"), + edge("rustls-openssl", "openssl-sys"), + edge("openshell-crypto", "jsonwebtoken-openssl"), + edge("jsonwebtoken-openssl", "openssl"), + ] + with tempfile.TemporaryDirectory(prefix="openshell-crypto-policy-") as temp: + root = Path(temp) + check_graph(root / "facade-backends", facade) + check_graph( + root / "documented-exceptions", + [ + edge("openshell-server", "rsa", "dev-dependencies"), + edge("openshell-driver-vault", "sha2"), + edge("policy-test-app", "russh"), + edge("russh", "aes"), + edge("policy-test-app", "rustls"), + edge("rustls", "ring"), + ], + ) + # Keep the valid facade path present in every negative case. A shared + # implementation must still reject each unapproved direct parent. + for package in ( + "aws-lc-rs", + "aws-lc-sys", + "aws-lc-fips-sys", + "openssl", + "openssl-sys", + "rustls-openssl", + "jsonwebtoken-openssl", + "ring", + "sha2", + "hmac", + "aes-gcm", + "rsa", + "ed25519-dalek", + "blake3", + "ecdsa", + "ml-kem", + ): + check_graph( + root / f"direct-{package}", + [*facade, edge("policy-test-app", package)], + package, + ) + for name, section in ( + ("renamed", "dependencies"), + ("build", "build-dependencies"), + ("dev", "dev-dependencies"), + ("windows-only", "target.'cfg(windows)'.dependencies"), + ): + check_graph( + root / name, + [*facade, edge("policy-test-app", "openssl", section, "crypto_alias")], + "openssl", + ) + check_graph( + root / "unapproved-wrapper", + [ + *facade, + edge("policy-test-app", "another-crypto-wrapper"), + edge("another-crypto-wrapper", "openssl"), + ], + "openssl", + ) + + +if __name__ == "__main__": + main() From 71973605b674605c7705b901074c7e4a6309cdea Mon Sep 17 00:00:00 2001 From: Varsha Prasad Narsing Date: Wed, 21 Oct 2026 23:34:02 -0700 Subject: [PATCH 3/7] fix(crypto): reject encrypted PEM keys without prompting Signed-off-by: Varsha Prasad Narsing --- crates/openshell-crypto/README.md | 3 +- .../openshell-crypto/src/openssl_backend.rs | 13 ++- crates/openshell-crypto/tests/openssl_pem.rs | 86 +++++++++++++++++++ 3 files changed, 100 insertions(+), 2 deletions(-) create mode 100644 crates/openshell-crypto/tests/openssl_pem.rs diff --git a/crates/openshell-crypto/README.md b/crates/openshell-crypto/README.md index ad5d205674..8d9d1fd369 100644 --- a/crates/openshell-crypto/README.md +++ b/crates/openshell-crypto/README.md @@ -97,7 +97,8 @@ not runtime algorithm availability. `provider_version` stays unknown because the library version does not identify the module that performed an operation. Certificate keys support P-256, P-384, Ed25519 and RSA, with PEM/PKCS#8 import and -PKCS#8 export. RSA generation uses 2048 bits. P-521 certificate keys are rejected: +PKCS#8 export. Encrypted PEM keys, including those with an empty passphrase, are +rejected without prompting for a password. RSA generation uses 2048 bits. P-521 certificate keys are rejected: rcgen 0.14 exposes that descriptor only with AWS-LC enabled. Existing P-521 users must retain AWS-LC until this limitation is resolved. This backend uses exportable software keys and does not add HSM or PKCS#11 loading. diff --git a/crates/openshell-crypto/src/openssl_backend.rs b/crates/openshell-crypto/src/openssl_backend.rs index e4bad5898c..2941267bcb 100644 --- a/crates/openshell-crypto/src/openssl_backend.rs +++ b/crates/openshell-crypto/src/openssl_backend.rs @@ -277,7 +277,18 @@ impl ProtocolBackend for OpenSsl { Key::wrap(Self::generate(algorithm)?, algorithm) } fn import_keypair_pem(&self, pem: &str) -> Result { - Key::import(PKey::private_key_from_pem(pem.as_bytes()).map_err(key_error)?) + // The default OpenSSL callback prompts on the controlling terminal. + // This API has no passphrase contract: reject encrypted input without + // terminal I/O, including input encrypted with an empty passphrase. + let mut password_requested = false; + let key = PKey::private_key_from_pem_callback(pem.as_bytes(), |_| { + password_requested = true; + Ok(0) + }); + if password_requested { + return Err(rcgen::Error::CouldNotParseKeyPair); + } + Key::import(key.map_err(key_error)?) } fn import_keypair_der(&self, der: &[u8]) -> Result { Key::import(PKey::private_key_from_pkcs8(der).map_err(key_error)?) diff --git a/crates/openshell-crypto/tests/openssl_pem.rs b/crates/openshell-crypto/tests/openssl_pem.rs new file mode 100644 index 0000000000..acb37813b5 --- /dev/null +++ b/crates/openshell-crypto/tests/openssl_pem.rs @@ -0,0 +1,86 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +#![cfg(feature = "openssl")] + +use openshell_crypto::{OpenSsl, ProtocolBackend}; +use openssl::{ec::EcGroup, ec::EcKey, nid::Nid, pkey::PKey, rsa::Rsa, symm::Cipher}; +use std::{ + process::{Command, Stdio}, + time::{Duration, Instant}, +}; + +#[test] +fn encrypted_pem_is_rejected_without_prompting() { + // Isolate terminal I/O and bound execution: reverting to OpenSSL's default + // callback must fail the test, whether a controlling terminal exists or not. + let mut child = Command::new(std::env::current_exe().unwrap()) + .args(["--ignored", "--exact", "pem_import_child", "--nocapture"]) + .env("OPENSHELL_TEST_PEM_IMPORT", "1") + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + let deadline = Instant::now() + Duration::from_secs(10); + while child.try_wait().unwrap().is_none() { + if Instant::now() >= deadline { + child.kill().unwrap(); + child.wait().unwrap(); + panic!("PEM import did not finish without interactive input"); + } + std::thread::sleep(Duration::from_millis(20)); + } + let result = child.wait_with_output().unwrap(); + let stdout = String::from_utf8_lossy(&result.stdout); + let stderr = String::from_utf8_lossy(&result.stderr); + assert!(result.status.success(), "{stdout}\n{stderr}"); + assert!(stdout.contains("1 passed"), "{stdout}"); + assert!(stderr.is_empty(), "PEM import wrote to stderr: {stderr}"); +} + +#[test] +#[ignore = "run by the parent with an input deadline and captured diagnostics"] +fn pem_import_child() { + assert_eq!(std::env::var("OPENSHELL_TEST_PEM_IMPORT").unwrap(), "1"); + let ec = EcKey::generate(&EcGroup::from_curve_name(Nid::X9_62_PRIME256V1).unwrap()).unwrap(); + let rsa = Rsa::generate(2048).unwrap(); + let keys = [ + PKey::from_ec_key(ec.clone()).unwrap(), + PKey::from_rsa(rsa.clone()).unwrap(), + ]; + for key in &keys { + let pem = key.private_key_to_pem_pkcs8().unwrap(); + let imported = OpenSsl + .import_keypair_pem(std::str::from_utf8(&pem).unwrap()) + .unwrap(); + assert_eq!(imported.public_key_der(), key.public_key_to_der().unwrap()); + } + // Both the generic PKCS#8 and traditional PEM loaders can request passwords. + for password in [b"test-only-passphrase".as_slice(), b""] { + let mut encrypted = keys + .iter() + .map(|key| { + key.private_key_to_pem_pkcs8_passphrase(Cipher::aes_256_cbc(), password) + .unwrap() + }) + .collect::>(); + encrypted.push( + ec.private_key_to_pem_passphrase(Cipher::aes_256_cbc(), password) + .unwrap(), + ); + encrypted.push( + rsa.private_key_to_pem_passphrase(Cipher::aes_256_cbc(), password) + .unwrap(), + ); + for pem in encrypted { + assert!( + OpenSsl + .import_keypair_pem(std::str::from_utf8(&pem).unwrap()) + .is_err(), + "encrypted PEM must be rejected, including with an empty passphrase" + ); + } + } + assert!(OpenSsl.import_keypair_pem("invalid key").is_err()); +} From 63890914394bc075055d59582efe300db1d1611d Mon Sep 17 00:00:00 2001 From: Varsha Prasad Narsing Date: Thu, 22 Oct 2026 23:51:47 -0700 Subject: [PATCH 4/7] fix(crypto): validate OpenSSL keys and propagate TLS setup errors Signed-off-by: Varsha Prasad Narsing --- crates/openshell-cli/src/tls.rs | 8 +- crates/openshell-crypto/CRYPTO.md | 6 + crates/openshell-crypto/README.md | 18 ++- .../openshell-crypto/src/openssl_backend.rs | 4 + .../src/openssl_backend/jwt.rs | 23 +++- crates/openshell-crypto/src/tls.rs | 62 +++++++-- .../tests/context_selection.rs | 6 +- .../tests/openssl_key_validation.rs | 126 ++++++++++++++++++ crates/openshell-crypto/tests/openssl_tls.rs | 18 ++- .../tests/openssl_unavailable.rs | 25 ++-- .../openshell-sandbox-backend/src/runtime.rs | 10 +- .../openshell-sandbox/src/boundary_server.rs | 8 +- crates/openshell-sdk/src/transport.rs | 7 +- crates/openshell-server/src/tls.rs | 8 +- .../src/l7/tls.rs | 12 +- 15 files changed, 290 insertions(+), 51 deletions(-) create mode 100644 crates/openshell-crypto/tests/openssl_key_validation.rs diff --git a/crates/openshell-cli/src/tls.rs b/crates/openshell-cli/src/tls.rs index 6dceb395de..0b1e7b8f5c 100644 --- a/crates/openshell-cli/src/tls.rs +++ b/crates/openshell-cli/src/tls.rs @@ -205,7 +205,9 @@ pub fn build_rustls_config(materials: &TlsMaterials) -> Result Result { - let config = openshell_crypto::tls::client_builder() + let config = openshell_crypto::tls::try_client_builder() + .into_diagnostic() + .wrap_err("configure gateway TLS client")? .dangerous() .with_custom_certificate_verifier(std::sync::Arc::new(InsecureServerCertVerifier)) .with_no_client_auth(); diff --git a/crates/openshell-crypto/CRYPTO.md b/crates/openshell-crypto/CRYPTO.md index 2aa09d0515..de475e294d 100644 --- a/crates/openshell-crypto/CRYPTO.md +++ b/crates/openshell-crypto/CRYPTO.md @@ -43,6 +43,12 @@ OpenSSL does not replace the AWS-LC default; an embedder must install its contex explicitly. System libraries and provider configuration belong to the deployment; the backend does not modify provider policy. +Runtime TLS configuration uses the facade's `try_client_builder` and +`try_server_builder` APIs (including explicit protocol-version variants). A +provider with no usable cipher suites or compatible key-exchange groups produces +a configuration error without panic or fallback. The older infallible builders +are compatibility helpers, not the runtime error-handling contract. + ## Contributor rules and coverage Route new first-party security-sensitive hashing, randomness, encryption, diff --git a/crates/openshell-crypto/README.md b/crates/openshell-crypto/README.md index 8d9d1fd369..2672234da8 100644 --- a/crates/openshell-crypto/README.md +++ b/crates/openshell-crypto/README.md @@ -105,17 +105,23 @@ software keys and does not add HSM or PKCS#11 loading. TLS uses `rustls-openssl` 0.4.1 with TLS 1.2/1.3, without its `fips`, vendored or prefer-post-quantum features. The jsonwebtoken 10 adapter includes compatibility -guards for malformed HMAC signatures, EC algorithm/curve binding, JOSE RSA-PSS -salt lengths and fixed-width EC JWK coordinates. Claim validation stays in +guards for malformed HMAC signatures, EC key consistency and algorithm/curve +binding, RSA public-component bounds, JOSE RSA-PSS salt lengths and fixed-width +EC JWK coordinates. Certificate key import also checks EC key consistency. +Claim validation stays in jsonwebtoken. AEAD setup errors remain distinct from authentication errors; tentative plaintext is cleared on failure and returned only after authentication. Protocol limitations remain: Rustls hash/HKDF callbacks and JWT JWK thumbprint hashing cannot return backend errors; adapter helpers can panic on provider -failure. The TLS adapter also uses direct OpenSSL SHA operations that do not -establish provider-module ownership. For fallible TLS setup, use Rustls's -`builder_with_provider` with `OpenSsl.tls_provider()` and handle protocol-version -errors; facade convenience builders assert compatibility. This backend does not +failure. The TLS adapter also uses direct OpenSSL SHA operations and low-level +EC key generation that do not establish provider-module ownership. First-party +TLS setup uses `tls::try_client_builder()` / `tls::try_server_builder()` and their +`_with_protocol_versions` variants. These return configuration errors when the +selected provider has no usable ciphers or compatible key-exchange groups, without +switching providers or relaxing the requested protocol versions. The older +infallible builders remain for API compatibility and panic on configuration errors; +runtime consumers should use the fallible variants. This backend does not promise panic-free protocol operation or a provider-qualified TLS path. These limitations require disposition during downstream integration and qualification. diff --git a/crates/openshell-crypto/src/openssl_backend.rs b/crates/openshell-crypto/src/openssl_backend.rs index 2941267bcb..92e94ebfe2 100644 --- a/crates/openshell-crypto/src/openssl_backend.rs +++ b/crates/openshell-crypto/src/openssl_backend.rs @@ -166,6 +166,10 @@ impl Key { Id::ED25519 => inner.raw_public_key().map_err(key_error)?, Id::EC => { let ec = inner.ec_key().map_err(key_error)?; + // Decoding a key does not validate its scalar, public point, or + // their consistency. Reject invalid material before exposing it + // to certificate construction or signing. + ec.check_key().map_err(key_error)?; let mut ctx = BigNumContext::new().map_err(key_error)?; ec.public_key() .to_bytes(ec.group(), PointConversionForm::UNCOMPRESSED, &mut ctx) diff --git a/crates/openshell-crypto/src/openssl_backend/jwt.rs b/crates/openshell-crypto/src/openssl_backend/jwt.rs index 7187814314..92b011e73c 100644 --- a/crates/openshell-crypto/src/openssl_backend/jwt.rs +++ b/crates/openshell-crypto/src/openssl_backend/jwt.rs @@ -43,6 +43,7 @@ fn check_ec_key(algorithm: Algorithm, der: &[u8]) -> Result<()> { if ec.group().curve_name() != Some(expected) { return Err(ErrorKind::InvalidEcdsaKey.into()); } + ec.check_key().map_err(|_| ErrorKind::InvalidEcdsaKey)?; Ok(()) } @@ -61,7 +62,7 @@ fn signer(algorithm: &Algorithm, key: &EncodingKey) -> Result } let rsa = PKey::private_key_from_der(key.inner()).map_err(|_| ErrorKind::InvalidKeyFormat)?; - check_rsa_size(&rsa)?; + check_rsa_key(&rsa)?; } if let Some(digest) = pss_digest(*algorithm) { if key.family() != AlgorithmFamily::Rsa { @@ -69,7 +70,7 @@ fn signer(algorithm: &Algorithm, key: &EncodingKey) -> Result } let key = PKey::private_key_from_der(key.inner()).map_err(|_| ErrorKind::InvalidKeyFormat)?; - check_rsa_size(&key)?; + check_rsa_key(&key)?; return Ok(Box::new(PssSigner { key, algorithm: *algorithm, @@ -85,11 +86,11 @@ fn verifier(algorithm: &Algorithm, key: &DecodingKey) -> Result Option { } } -fn check_rsa_size(key: &PKey) -> Result<()> { +fn check_rsa_key(key: &PKey) -> Result<()> { if key.id() != openssl::pkey::Id::RSA || !(2048..=8192).contains(&key.bits()) { return Err(ErrorKind::InvalidKeyFormat.into()); } + let rsa = key.rsa().map_err(|_| ErrorKind::InvalidKeyFormat)?; + // Match AWS-LC's signature-key bounds: a positive odd modulus and an odd + // public exponent greater than one, at most 33 bits. OpenSSL's component + // constructors only assemble a key; they do not enforce these constraints. + if rsa.n().is_negative() + || !rsa.n().is_odd() + || rsa.e().is_negative() + || !rsa.e().is_odd() + || !(2..=33).contains(&rsa.e().num_bits()) + { + return Err(ErrorKind::InvalidKeyFormat.into()); + } Ok(()) } diff --git a/crates/openshell-crypto/src/tls.rs b/crates/openshell-crypto/src/tls.rs index c11f9cb6e7..1be43b5d75 100644 --- a/crates/openshell-crypto/src/tls.rs +++ b/crates/openshell-crypto/src/tls.rs @@ -53,34 +53,80 @@ pub fn configuration_provider() -> Arc { } } -/// Client builder using the same provider as key loading and custom verifiers. +/// Fallible client builder using the same provider as key loading and verifiers. +/// +/// Returns a configuration error if the selected provider has no usable cipher +/// suites or key-exchange groups. Does not change providers or protocol policy. +pub fn try_client_builder() +-> Result, rustls::Error> { + try_client_builder_with_protocol_versions(rustls::DEFAULT_VERSIONS) +} + +/// Fallible client builder preserving an explicit protocol-version policy. +pub fn try_client_builder_with_protocol_versions( + versions: &[&'static rustls::SupportedProtocolVersion], +) -> Result, rustls::Error> { + rustls::ClientConfig::builder_with_provider(configuration_provider()) + .with_protocol_versions(versions) +} + +/// Fallible server builder using the selected configuration provider. +/// +/// Returns a configuration error if the selected provider has no usable cipher +/// suites or key-exchange groups. Does not change providers or protocol policy. +pub fn try_server_builder() +-> Result, rustls::Error> { + try_server_builder_with_protocol_versions(rustls::DEFAULT_VERSIONS) +} + +/// Fallible server builder preserving an explicit protocol-version policy. +pub fn try_server_builder_with_protocol_versions( + versions: &[&'static rustls::SupportedProtocolVersion], +) -> Result, rustls::Error> { + rustls::ServerConfig::builder_with_provider(configuration_provider()) + .with_protocol_versions(versions) +} + +/// Compatibility client builder. Use [`try_client_builder`] in runtime code. +/// +/// # Panics +/// Panics if the selected provider cannot support the default protocol versions. #[must_use] pub fn client_builder() -> rustls::ConfigBuilder { client_builder_with_protocol_versions(rustls::DEFAULT_VERSIONS) } -/// Client builder preserving an explicit protocol-version policy. +/// Compatibility client builder preserving an explicit protocol-version policy. +/// Prefer [`try_client_builder_with_protocol_versions`] in runtime code. +/// +/// # Panics +/// Panics if the selected provider cannot support the requested protocol versions. #[must_use] pub fn client_builder_with_protocol_versions( versions: &[&'static rustls::SupportedProtocolVersion], ) -> rustls::ConfigBuilder { - rustls::ClientConfig::builder_with_provider(configuration_provider()) - .with_protocol_versions(versions) + try_client_builder_with_protocol_versions(versions) .expect("selected TLS provider must support the requested protocol versions") } -/// Server builder using the selected configuration provider. +/// Compatibility server builder. Use [`try_server_builder`] in runtime code. +/// +/// # Panics +/// Panics if the selected provider cannot support the default protocol versions. #[must_use] pub fn server_builder() -> rustls::ConfigBuilder { server_builder_with_protocol_versions(rustls::DEFAULT_VERSIONS) } -/// Server builder preserving an explicit protocol-version policy. +/// Compatibility server builder preserving an explicit protocol-version policy. +/// Prefer [`try_server_builder_with_protocol_versions`] in runtime code. +/// +/// # Panics +/// Panics if the selected provider cannot support the requested protocol versions. #[must_use] pub fn server_builder_with_protocol_versions( versions: &[&'static rustls::SupportedProtocolVersion], ) -> rustls::ConfigBuilder { - rustls::ServerConfig::builder_with_provider(configuration_provider()) - .with_protocol_versions(versions) + try_server_builder_with_protocol_versions(versions) .expect("selected TLS provider must support the requested protocol versions") } diff --git a/crates/openshell-crypto/tests/context_selection.rs b/crates/openshell-crypto/tests/context_selection.rs index 2a0205e61f..5ac2b754f1 100644 --- a/crates/openshell-crypto/tests/context_selection.rs +++ b/crates/openshell-crypto/tests/context_selection.rs @@ -171,10 +171,12 @@ fn selected_context_controls_application_adapters_and_cannot_be_replaced() { .unwrap(); let mut roots = rustls::RootCertStore::empty(); roots.add(cert.der().clone()).unwrap(); - let client = openshell_crypto::tls::client_builder() + let client = openshell_crypto::tls::try_client_builder() + .unwrap() .with_root_certificates(roots) .with_no_client_auth(); - let server = openshell_crypto::tls::server_builder() + let server = openshell_crypto::tls::try_server_builder() + .unwrap() .with_no_client_auth() .with_single_cert( vec![cert.der().clone()], diff --git a/crates/openshell-crypto/tests/openssl_key_validation.rs b/crates/openshell-crypto/tests/openssl_key_validation.rs new file mode 100644 index 0000000000..e20337bc83 --- /dev/null +++ b/crates/openshell-crypto/tests/openssl_key_validation.rs @@ -0,0 +1,126 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +#![cfg(feature = "openssl")] + +use jsonwebtoken::{Algorithm, DecodingKey, EncodingKey}; +use openshell_crypto::{OpenSsl, ProtocolBackend}; +use openssl::{ + bn::{BigNum, BigNumContext}, + ec::{EcGroup, EcKey}, + nid::Nid, + pkey::PKey, + rsa::Rsa, +}; + +#[test] +fn jwt_rejects_invalid_rsa_public_components() { + let provider = OpenSsl.jwt_provider(); + let valid = Rsa::generate(2048).unwrap(); + let mut oversized_exponent = BigNum::new().unwrap(); + oversized_exponent.set_bit(33).unwrap(); + oversized_exponent.add_word(1).unwrap(); + for exponent in [ + BigNum::from_u32(0).unwrap(), + BigNum::from_u32(1).unwrap(), + BigNum::from_u32(2).unwrap(), + BigNum::from_u32(65536).unwrap(), + oversized_exponent, + ] { + // Both JWK components and DER reach the adapter; parsing alone accepts + // these values and must not be mistaken for RSA public-key validation. + let key = Rsa::from_public_components(valid.n().to_owned().unwrap(), exponent).unwrap(); + let der = key.public_key_to_der_pkcs1().unwrap(); + let keys = [ + DecodingKey::from_rsa_raw_components(&key.n().to_vec(), &key.e().to_vec()), + DecodingKey::from_rsa_der(&der), + ]; + for algorithm in [ + Algorithm::RS256, + Algorithm::RS384, + Algorithm::RS512, + Algorithm::PS256, + Algorithm::PS384, + Algorithm::PS512, + ] { + for key in &keys { + assert!( + (provider.verifier_factory)(&algorithm, key).is_err(), + "invalid RSA exponent accepted for {algorithm:?}" + ); + } + } + } + + let mut even_modulus = valid.n().to_owned().unwrap(); + even_modulus.sub_word(1).unwrap(); + let invalid = DecodingKey::from_rsa_raw_components(&even_modulus.to_vec(), &valid.e().to_vec()); + for algorithm in [Algorithm::RS256, Algorithm::PS256] { + assert!((provider.verifier_factory)(&algorithm, &invalid).is_err()); + } + + // Preserve non-FIPS compatibility, including the historically valid e=3. + for exponent in [3, 65537] { + let key = DecodingKey::from_rsa_raw_components( + &valid.n().to_vec(), + &BigNum::from_u32(exponent).unwrap().to_vec(), + ); + for algorithm in [Algorithm::RS256, Algorithm::PS256] { + assert!((provider.verifier_factory)(&algorithm, &key).is_ok()); + } + } +} + +#[test] +fn pki_and_jwt_reject_inconsistent_ec_private_keys() { + let provider = OpenSsl.jwt_provider(); + for (curve, algorithm) in [ + (Nid::X9_62_PRIME256V1, Algorithm::ES256), + (Nid::SECP384R1, Algorithm::ES384), + ] { + let group = EcGroup::from_curve_name(curve).unwrap(); + let valid = EcKey::generate(&group).unwrap(); + let other = EcKey::generate(&group).unwrap(); + let mut order = BigNum::new().unwrap(); + group + .order(&mut order, &mut BigNumContext::new().unwrap()) + .unwrap(); + let invalid_keys = [ + EcKey::from_private_components(&group, valid.private_key(), other.public_key()) + .unwrap(), + EcKey::from_private_components( + &group, + &BigNum::from_u32(0).unwrap(), + valid.public_key(), + ) + .unwrap(), + EcKey::from_private_components(&group, &order, valid.public_key()).unwrap(), + ]; + for ec in invalid_keys { + assert!(ec.check_key().is_err()); + let sec1 = ec.private_key_to_der().unwrap(); + let key = PKey::from_ec_key(ec).unwrap(); + let pkcs8 = key.private_key_to_pkcs8().unwrap(); + let pem = String::from_utf8(key.private_key_to_pem_pkcs8().unwrap()).unwrap(); + // These are parseable encodings of invalid key material. + assert!(PKey::private_key_from_pkcs8(&pkcs8).is_ok()); + assert!(OpenSsl.import_keypair_der(&pkcs8).is_err()); + assert!(OpenSsl.import_keypair_pem(&pem).is_err()); + assert!( + (provider.signer_factory)(&algorithm, &EncodingKey::from_ec_der(&sec1)).is_err() + ); + assert!( + (provider.jwk_utils.extract_ec_public_key_coordinates)(&sec1, algorithm).is_err() + ); + } + + let sec1 = valid.private_key_to_der().unwrap(); + let key = PKey::from_ec_key(valid).unwrap(); + assert!( + OpenSsl + .import_keypair_der(&key.private_key_to_pkcs8().unwrap()) + .is_ok() + ); + assert!((provider.signer_factory)(&algorithm, &EncodingKey::from_ec_der(&sec1)).is_ok()); + } +} diff --git a/crates/openshell-crypto/tests/openssl_tls.rs b/crates/openshell-crypto/tests/openssl_tls.rs index 6a3960351b..57592f4190 100644 --- a/crates/openshell-crypto/tests/openssl_tls.rs +++ b/crates/openshell-crypto/tests/openssl_tls.rs @@ -57,6 +57,9 @@ fn leaf( #[test] fn tls12_and_tls13_enforce_server_and_client_authentication() { init(); + // An explicit empty protocol policy must not be replaced with defaults. + assert!(tls::try_client_builder_with_protocol_versions(&[]).is_err()); + assert!(tls::try_server_builder_with_protocol_versions(&[]).is_err()); assert!(std::ptr::eq( tls::configuration_provider().secure_random, OpenSsl.tls_provider().secure_random @@ -77,7 +80,8 @@ fn tls12_and_tls13_enforce_server_and_client_authentication() { ) .build() .unwrap(); - let server = tls::server_builder_with_protocol_versions(&[version]) + let server = tls::try_server_builder_with_protocol_versions(&[version]) + .unwrap() .with_client_cert_verifier(verifier) .with_single_cert( vec![server_key.cert.der().clone()], @@ -87,7 +91,8 @@ fn tls12_and_tls13_enforce_server_and_client_authentication() { .into(), ) .unwrap(); - let client = tls::client_builder_with_protocol_versions(&[version]) + let client = tls::try_client_builder_with_protocol_versions(&[version]) + .unwrap() .with_root_certificates(roots.clone()) .with_client_auth_cert( vec![client_key.cert.der().clone()], @@ -102,14 +107,16 @@ fn tls12_and_tls13_enforce_server_and_client_authentication() { handshake(client, server.clone(), "wrong.example"), Err(rustls::Error::InvalidCertificate(_)) )); - let anonymous = tls::client_builder_with_protocol_versions(&[version]) + let anonymous = tls::try_client_builder_with_protocol_versions(&[version]) + .unwrap() .with_root_certificates(roots.clone()) .with_no_client_auth(); assert!(matches!( handshake(anonymous, server.clone(), "localhost"), Err(rustls::Error::NoCertificatesPresented) )); - let untrusted = tls::client_builder_with_protocol_versions(&[version]) + let untrusted = tls::try_client_builder_with_protocol_versions(&[version]) + .unwrap() .with_root_certificates(rustls::RootCertStore::empty()) .with_no_client_auth(); assert!(matches!( @@ -117,7 +124,8 @@ fn tls12_and_tls13_enforce_server_and_client_authentication() { Err(rustls::Error::InvalidCertificate(_)) )); let stranger = pki::generate_simple_self_signed(vec!["localhost".into()]).unwrap(); - let unauthorized = tls::client_builder_with_protocol_versions(&[version]) + let unauthorized = tls::try_client_builder_with_protocol_versions(&[version]) + .unwrap() .with_root_certificates(roots.clone()) .with_client_auth_cert( vec![stranger.cert.der().clone()], diff --git a/crates/openshell-crypto/tests/openssl_unavailable.rs b/crates/openshell-crypto/tests/openssl_unavailable.rs index 2d880c7fd8..f04f692643 100644 --- a/crates/openshell-crypto/tests/openssl_unavailable.rs +++ b/crates/openshell-crypto/tests/openssl_unavailable.rs @@ -3,7 +3,7 @@ #![cfg(feature = "openssl")] -use openshell_crypto::{CryptoBackend, CryptoContext, CryptoError, OpenSsl, ProtocolBackend}; +use openshell_crypto::{CryptoBackend, CryptoContext, CryptoError, OpenSsl, ProtocolBackend, tls}; #[test] fn unavailable_provider_fails_without_fallback() { @@ -74,13 +74,18 @@ fn unavailable_provider_child() { ) .is_err() ); - // The adapter filters unavailable ciphers; use the fallible Rustls builder - // rather than the facade convenience builder, which asserts compatibility. - let provider = OpenSsl.tls_provider(); - assert!(provider.cipher_suites.is_empty()); - assert!( - rustls::ClientConfig::builder_with_provider(std::sync::Arc::new(provider)) - .with_safe_default_protocol_versions() - .is_err() - ); + // Exercise the same facade builders as production callers: a provider with + // no usable ciphers must return an error, not panic or fall back to AWS-LC. + assert!(OpenSsl.tls_provider().cipher_suites.is_empty()); + let errors = [ + tls::try_client_builder().unwrap_err(), + tls::try_server_builder().unwrap_err(), + tls::try_client_builder_with_protocol_versions(&[&rustls::version::TLS13]).unwrap_err(), + tls::try_server_builder_with_protocol_versions(&[&rustls::version::TLS13]).unwrap_err(), + ]; + for error in errors { + assert!( + matches!(error, rustls::Error::General(ref message) if message == "no usable cipher suites configured") + ); + } } diff --git a/crates/openshell-sandbox-backend/src/runtime.rs b/crates/openshell-sandbox-backend/src/runtime.rs index 8e54aa9832..85b3f22b2a 100644 --- a/crates/openshell-sandbox-backend/src/runtime.rs +++ b/crates/openshell-sandbox-backend/src/runtime.rs @@ -444,10 +444,12 @@ fn tls_client_config(tls: &SandboxTlsClientConfig) -> Result Result { - Ok(openshell_crypto::tls::client_builder() + Ok(openshell_crypto::tls::try_client_builder() + .map_err(|e| SdkError::invalid_config(format!("configure gateway TLS client: {e}")))? .dangerous() .with_custom_certificate_verifier(std::sync::Arc::new(InsecureServerCertVerifier)) .with_no_client_auth()) diff --git a/crates/openshell-server/src/tls.rs b/crates/openshell-server/src/tls.rs index 26bcb713ce..bf1ffd07aa 100644 --- a/crates/openshell-server/src/tls.rs +++ b/crates/openshell-server/src/tls.rs @@ -408,7 +408,9 @@ fn build_server_config( .build() .map_err(|e| Error::tls(format!("failed to build client verifier: {e}")))?; - let builder = openshell_crypto::tls::server_builder().with_client_cert_verifier(verifier); + let builder = openshell_crypto::tls::try_server_builder() + .map_err(|e| Error::tls(format!("configure gateway TLS server: {e}")))? + .with_client_cert_verifier(verifier); if let Some(resolver) = resolver { builder.with_cert_resolver(resolver) } else { @@ -417,7 +419,9 @@ fn build_server_config( .map_err(|e| Error::tls(format!("failed to create TLS config: {e}")))? } } else { - let builder = openshell_crypto::tls::server_builder().with_no_client_auth(); + let builder = openshell_crypto::tls::try_server_builder() + .map_err(|e| Error::tls(format!("configure gateway TLS server: {e}")))? + .with_no_client_auth(); if let Some(resolver) = resolver { builder.with_cert_resolver(resolver) } else { diff --git a/crates/openshell-supervisor-network/src/l7/tls.rs b/crates/openshell-supervisor-network/src/l7/tls.rs index fe24afc90d..d0a60aad93 100644 --- a/crates/openshell-supervisor-network/src/l7/tls.rs +++ b/crates/openshell-supervisor-network/src/l7/tls.rs @@ -119,7 +119,9 @@ impl SandboxCa { .wrap_err("parse proxy CA private key")? .ok_or_else(|| miette!("proxy CA private key file contains no private key"))?; let ca_der = certificates[0].clone(); - openshell_crypto::tls::server_builder() + openshell_crypto::tls::try_server_builder() + .into_diagnostic() + .wrap_err("configure proxy CA TLS validation")? .with_no_client_auth() .with_single_cert(certificates, private_key) .into_diagnostic() @@ -219,7 +221,9 @@ impl ProxyTlsState { /// Get or generate a leaf cert for the hostname and return a TLS acceptor. fn acceptor_for(&self, hostname: &str) -> Result { let leaf = self.cert_cache.get_or_generate(hostname)?; - let mut server_config = openshell_crypto::tls::server_builder() + let mut server_config = openshell_crypto::tls::try_server_builder() + .into_diagnostic() + .wrap_err("configure proxy TLS server")? .with_no_client_auth() .with_single_cert(leaf.cert_chain.clone(), leaf.private_key.clone_key()) .into_diagnostic()?; @@ -276,7 +280,9 @@ pub async fn tls_connect_upstream( /// `system_ca_bundle` is ignored because the native store already reflects all /// operator-installed trust anchors. pub fn build_upstream_client_config(system_ca_bundle: &str) -> Result> { - let mut config = openshell_crypto::tls::client_builder() + let mut config = openshell_crypto::tls::try_client_builder() + .into_diagnostic() + .wrap_err("configure upstream TLS client")? .with_root_certificates(build_upstream_root_store(system_ca_bundle)?) .with_no_client_auth(); config.alpn_protocols = vec![b"http/1.1".to_vec()]; From abc1959770c3a7a561f50ddb26af7aa2e912f899 Mon Sep 17 00:00:00 2001 From: Varsha Prasad Narsing Date: Fri, 23 Oct 2026 13:46:30 -0700 Subject: [PATCH 5/7] fix(crypto): address OpenSSL backend review findings Signed-off-by: Varsha Prasad Narsing --- .github/workflows/branch-checks.yml | 12 +- crates/openshell-crypto/CRYPTO.md | 4 + crates/openshell-crypto/README.md | 18 +- crates/openshell-crypto/src/aws_lc.rs | 1 + crates/openshell-crypto/src/lib.rs | 7 +- .../openshell-crypto/src/openssl_backend.rs | 55 ++++- .../src/openssl_backend/jwt.rs | 176 +++++++++------ crates/openshell-crypto/tests/common/mod.rs | 47 ++++ .../tests/context_selection.rs | 21 +- .../tests/jwt_rejection_parity.rs | 206 ++++++++++++++++++ .../openshell-crypto/tests/openssl_backend.rs | 67 ++---- .../tests/openssl_key_validation.rs | 78 +++++++ crates/openshell-crypto/tests/openssl_tls.rs | 42 +--- .../tests/openssl_unavailable.rs | 26 ++- deny.toml | 8 + .../scripts/test-crypto-dependency-policy.py | 78 +++++-- 16 files changed, 643 insertions(+), 203 deletions(-) create mode 100644 crates/openshell-crypto/tests/common/mod.rs create mode 100644 crates/openshell-crypto/tests/jwt_rejection_parity.rs diff --git a/.github/workflows/branch-checks.yml b/.github/workflows/branch-checks.yml index c8e9eed977..1f835025d0 100644 --- a/.github/workflows/branch-checks.yml +++ b/.github/workflows/branch-checks.yml @@ -196,11 +196,15 @@ jobs: prepare-shell: "true" shell-installable: .#devShells.x86_64-linux.default + - name: Install pinned cargo-deny + run: mise install --locked github:EmbarkStudios/cargo-deny + + # Enter mise inside the Nix shell so Nix's cargo-deny cannot shadow the pin. - name: Verify crypto dependency guardrail - run: uv run --no-project python tasks/scripts/test-crypto-dependency-policy.py + run: mise exec github:EmbarkStudios/cargo-deny -- uv run --no-project python tasks/scripts/test-crypto-dependency-policy.py - name: Check dependencies - run: cargo deny check licenses bans sources + run: mise exec github:EmbarkStudios/cargo-deny -- cargo deny check licenses bans sources rust-format: name: Rust format @@ -315,7 +319,7 @@ jobs: - name: Verify crypto backend independence shell: nix develop -c bash -euo pipefail {0} run: | - cargo tree --locked -p openshell-crypto --no-default-features --edges normal,build,dev --prefix none > /tmp/openshell-crypto-dependencies.txt + cargo tree --locked -p openshell-crypto --no-default-features --edges normal,build,dev --target all --prefix none > /tmp/openshell-crypto-dependencies.txt if grep -Eq '^(ring|aws-lc-rs|aws-lc-sys|aws-lc-fips-sys) v' /tmp/openshell-crypto-dependencies.txt; then echo "ERROR: backend-free crypto build includes Ring or AWS-LC" >&2 cat /tmp/openshell-crypto-dependencies.txt >&2 @@ -329,7 +333,7 @@ jobs: OPENSSL_NO_VENDOR: "1" OPENSSL_STATIC: "0" run: | - cargo tree --locked -p openshell-crypto --no-default-features --features openssl --edges normal,build,dev --prefix none > /tmp/openshell-openssl-dependencies.txt + cargo tree --locked -p openshell-crypto --no-default-features --features openssl --edges normal,build,dev --target all --prefix none > /tmp/openshell-openssl-dependencies.txt if grep -Eq '^(ring|aws-lc-rs|aws-lc-sys|aws-lc-fips-sys|openssl-src) v' /tmp/openshell-openssl-dependencies.txt; then echo "ERROR: OpenSSL-only crypto build includes another backend or vendored OpenSSL" >&2 cat /tmp/openshell-openssl-dependencies.txt >&2 diff --git a/crates/openshell-crypto/CRYPTO.md b/crates/openshell-crypto/CRYPTO.md index de475e294d..e42b756649 100644 --- a/crates/openshell-crypto/CRYPTO.md +++ b/crates/openshell-crypto/CRYPTO.md @@ -73,6 +73,8 @@ unapproved direct dependencies on named crypto implementations. AWS-LC and OpenS are accessible through `openshell-crypto` and the explicitly listed libraries that own their own crypto operations. Native `*-sys` crates and the OpenSSL protocol adapters have separate rules to prevent bypassing the provider boundary. +`openssl-src` is banned without wrapper exceptions: OpenSSL must come from the +system, including when enabled transitively through another dependency. The catalog also covers RustCrypto hashes, MACs, ciphers, and signature libraries. Protocol/parsing types remain usable in application crates. @@ -87,6 +89,8 @@ mise run rust:deny:policy ``` Run just the regression fixtures with `mise run rust:deny:crypto-test`. They use +the cargo-deny version pinned in `mise.toml`, as does the Branch Checks job; the +fixtures reject a mismatched tool version before checking dependency graphs. They use local stub packages to exercise cargo-deny without compiling cryptography or downloading dependencies. They test allowed provider paths, shared-dependency bypasses, aliases, build/dev/target dependencies, and unapproved intermediary diff --git a/crates/openshell-crypto/README.md b/crates/openshell-crypto/README.md index 2672234da8..5d849be5d1 100644 --- a/crates/openshell-crypto/README.md +++ b/crates/openshell-crypto/README.md @@ -107,9 +107,11 @@ TLS uses `rustls-openssl` 0.4.1 with TLS 1.2/1.3, without its `fips`, vendored o prefer-post-quantum features. The jsonwebtoken 10 adapter includes compatibility guards for malformed HMAC signatures, EC key consistency and algorithm/curve binding, RSA public-component bounds, JOSE RSA-PSS salt lengths and fixed-width -EC JWK coordinates. Certificate key import also checks EC key consistency. -Claim validation stays in -jsonwebtoken. AEAD setup errors remain distinct from authentication errors; +EC JWK coordinates. Certificate key import also checks EC key consistency and +requires RSA keys in the 2048–8192-bit range, matching AWS-LC. +RSA signing/verification, EC signing, and EC JWK extraction reuse the validated +OpenSSL key instead of parsing it again through the upstream adapter. Claim +validation stays in jsonwebtoken. AEAD setup errors remain distinct from authentication errors; tentative plaintext is cleared on failure and returned only after authentication. Protocol limitations remain: Rustls hash/HKDF callbacks and JWT JWK thumbprint @@ -125,6 +127,16 @@ runtime consumers should use the fallible variants. This backend does not promise panic-free protocol operation or a provider-qualified TLS path. These limitations require disposition during downstream integration and qualification. +The pinned `rustls-openssl` 0.4.1 verifier prints OpenSSL error stacks directly to +stderr through `std::dbg!`, including in release builds, when verification returns +an OpenSSL error. Peer-controlled malformed signatures can trigger this path; +the output bypasses application logging controls and can create repeated +diagnostic noise. Verification still fails. Upstream +[0.4.2 removes this output](https://github.com/tofay/rustls-openssl/blob/0.4.2/src/verify.rs#L347). +Upgrade and validate the dependency before deploying this TLS path; that release +also changes provider routing and FIPS reporting, so the upgrade requires more +than removing the diagnostic call. + ## Lifecycle and posture `default_context()` lazily selects AWS-LC when that feature is enabled. An embedder can call diff --git a/crates/openshell-crypto/src/aws_lc.rs b/crates/openshell-crypto/src/aws_lc.rs index 6dc5900def..b8c6222eae 100644 --- a/crates/openshell-crypto/src/aws_lc.rs +++ b/crates/openshell-crypto/src/aws_lc.rs @@ -36,6 +36,7 @@ impl CryptoBackend for AwsLc { "ECDSA-P384-SHA384", "ECDSA-P521-SHA512", "Ed25519", + "RSA", ], } } diff --git a/crates/openshell-crypto/src/lib.rs b/crates/openshell-crypto/src/lib.rs index ddee73a963..24306ed11f 100644 --- a/crates/openshell-crypto/src/lib.rs +++ b/crates/openshell-crypto/src/lib.rs @@ -49,6 +49,10 @@ pub enum FipsState { } /// Operations implemented by a backend. Protocol coverage is documented separately. +/// +/// These lists describe each backend individually, not a common set required of +/// every backend. Host provider policy can still reject a listed operation; +/// capabilities do not guarantee runtime availability or FIPS compliance. #[derive(Debug, Clone, PartialEq, Eq)] pub struct Capabilities { /// Stable backend identifier. @@ -59,7 +63,8 @@ pub struct Capabilities { pub fips: FipsState, /// Primitive operations supported by this implementation. pub primitives: &'static [&'static str], - /// Key generation algorithms available through the PKI adapter. + /// Key algorithms implemented by this backend's PKI and JWT adapters. + /// Listing an algorithm does not imply support for every operation or variant. pub key_algorithms: &'static [&'static str], } diff --git a/crates/openshell-crypto/src/openssl_backend.rs b/crates/openshell-crypto/src/openssl_backend.rs index 92e94ebfe2..88f73b5ea8 100644 --- a/crates/openshell-crypto/src/openssl_backend.rs +++ b/crates/openshell-crypto/src/openssl_backend.rs @@ -76,7 +76,11 @@ impl CryptoBackend for OpenSsl { let mut crypter = Crypter::new(Cipher::aes_256_gcm(), Mode::Encrypt, key, Some(&nonce)) .map_err(|_| CryptoError::Operation)?; update_aad(&mut crypter, aad)?; - let mut ciphertext = vec![0; output_size(plaintext.len())?]; + // Retain room for the tag after truncating the encryption output. + let capacity = output_size(plaintext.len())? + .checked_add(tag.len()) + .ok_or(CryptoError::Operation)?; + let mut ciphertext = vec![0; capacity]; let written = update_data(&mut crypter, plaintext, &mut ciphertext)?; let final_len = crypter .finalize(&mut ciphertext[written..]) @@ -175,11 +179,18 @@ impl Key { .to_bytes(ec.group(), PointConversionForm::UNCOMPRESSED, &mut ctx) .map_err(key_error)? } - Id::RSA => inner - .rsa() - .map_err(key_error)? - .public_key_to_der_pkcs1() - .map_err(key_error)?, + Id::RSA => { + // Preserve AWS-LC's 2048–8192-bit range for certificate keys on PEM + // and PKCS#8 DER imports, before exposing a signing key. + if !(2048..=8192).contains(&inner.bits()) { + return Err(rcgen::Error::CouldNotParseKeyPair); + } + inner + .rsa() + .map_err(key_error)? + .public_key_to_der_pkcs1() + .map_err(key_error)? + } _ => return Err(rcgen::Error::UnsupportedSignatureAlgorithm), }; pki::KeyPair::new(Box::new(Self { @@ -211,7 +222,7 @@ impl pki::SigningKey for Key { &self.public } fn sign(&self, message: &[u8]) -> Result, rcgen::Error> { - let mut signer = if self.inner.id() == Id::ED25519 { + let mut signer = if self.algorithm == &rcgen::PKCS_ED25519 { Signer::new_without_digest(&self.inner) } else { let digest = if self.algorithm == &rcgen::PKCS_ECDSA_P384_SHA384 @@ -220,8 +231,12 @@ impl pki::SigningKey for Key { MessageDigest::sha384() } else if self.algorithm == &rcgen::PKCS_RSA_SHA512 { MessageDigest::sha512() - } else { + } else if self.algorithm == &rcgen::PKCS_ECDSA_P256_SHA256 + || self.algorithm == &rcgen::PKCS_RSA_SHA256 + { MessageDigest::sha256() + } else { + return Err(rcgen::Error::UnsupportedSignatureAlgorithm); }; Signer::new(digest, &self.inner) } @@ -301,3 +316,27 @@ impl ProtocolBackend for OpenSsl { jwt::provider() } } + +#[cfg(all(test, feature = "aws-lc"))] +mod tests { + use super::*; + use crate::pki::SigningKey; + + #[test] + fn signing_rejects_an_unmapped_algorithm() { + // Exercise signing directly: generation/import currently reject P-521, + // but extending those paths must not silently select SHA-256 here. + let mut context = PkeyCtx::new_id(Id::EC).unwrap(); + context.keygen_init().unwrap(); + context.set_ec_paramgen_curve_nid(Nid::SECP521R1).unwrap(); + let key = Key { + inner: context.keygen().unwrap(), + algorithm: &rcgen::PKCS_ECDSA_P521_SHA512, + public: Vec::new(), + }; + assert!(matches!( + key.sign(b"certificate contents"), + Err(rcgen::Error::UnsupportedSignatureAlgorithm) + )); + } +} diff --git a/crates/openshell-crypto/src/openssl_backend/jwt.rs b/crates/openshell-crypto/src/openssl_backend/jwt.rs index 92b011e73c..4a0774950b 100644 --- a/crates/openshell-crypto/src/openssl_backend/jwt.rs +++ b/crates/openshell-crypto/src/openssl_backend/jwt.rs @@ -13,7 +13,8 @@ use jsonwebtoken::{ signature::{Error, Signer, Verifier}, }; use openssl::{ - bn::BigNum, + bn::{BigNum, BigNumContext}, + ecdsa::EcdsaSig, hash::MessageDigest, nid::Nid, pkey::{PKey, Private, Public}, @@ -32,7 +33,7 @@ pub(super) fn provider() -> &'static CryptoProvider { }) } -fn check_ec_key(algorithm: Algorithm, der: &[u8]) -> Result<()> { +fn parse_ec_key(algorithm: Algorithm, der: &[u8]) -> Result> { let expected = match algorithm { Algorithm::ES256 => Nid::X9_62_PRIME256V1, Algorithm::ES384 => Nid::SECP384R1, @@ -44,37 +45,41 @@ fn check_ec_key(algorithm: Algorithm, der: &[u8]) -> Result<()> { return Err(ErrorKind::InvalidEcdsaKey.into()); } ec.check_key().map_err(|_| ErrorKind::InvalidEcdsaKey)?; - Ok(()) + Ok(key) } // Signature required by jsonwebtoken's provider factory. #[allow(clippy::trivially_copy_pass_by_ref)] fn signer(algorithm: &Algorithm, key: &EncodingKey) -> Result> { if matches!(algorithm, Algorithm::ES256 | Algorithm::ES384) { - check_ec_key(*algorithm, key.inner())?; - } - if matches!( - algorithm, - Algorithm::RS256 | Algorithm::RS384 | Algorithm::RS512 - ) { - if key.family() != AlgorithmFamily::Rsa { + if key.family() != AlgorithmFamily::Ec { return Err(ErrorKind::InvalidKeyFormat.into()); } - let rsa = - PKey::private_key_from_der(key.inner()).map_err(|_| ErrorKind::InvalidKeyFormat)?; - check_rsa_key(&rsa)?; + let key = parse_ec_key(*algorithm, key.inner())?; + let (digest, width) = match algorithm { + Algorithm::ES256 => (MessageDigest::sha256(), 32), + Algorithm::ES384 => (MessageDigest::sha384(), 48), + _ => return Err(ErrorKind::InvalidAlgorithm.into()), + }; + return Ok(Box::new(EcSigner { + key, + algorithm: *algorithm, + digest, + width, + })); } - if let Some(digest) = pss_digest(*algorithm) { + if let Some((digest, padding)) = rsa_parameters(*algorithm) { if key.family() != AlgorithmFamily::Rsa { return Err(ErrorKind::InvalidKeyFormat.into()); } let key = PKey::private_key_from_der(key.inner()).map_err(|_| ErrorKind::InvalidKeyFormat)?; check_rsa_key(&key)?; - return Ok(Box::new(PssSigner { + return Ok(Box::new(RsaSigner { key, algorithm: *algorithm, digest, + padding, })); } (jsonwebtoken_openssl::DEFAULT_PROVIDER.signer_factory)(algorithm, key) @@ -82,19 +87,14 @@ fn signer(algorithm: &Algorithm, key: &EncodingKey) -> Result #[allow(clippy::trivially_copy_pass_by_ref)] fn verifier(algorithm: &Algorithm, key: &DecodingKey) -> Result> { - if matches!( - algorithm, - Algorithm::RS256 | Algorithm::RS384 | Algorithm::RS512 - ) { - check_rsa_key(&rsa_public_key(key)?)?; - } - if let Some(digest) = pss_digest(*algorithm) { + if let Some((digest, padding)) = rsa_parameters(*algorithm) { let key = rsa_public_key(key)?; check_rsa_key(&key)?; - return Ok(Box::new(PssVerifier { + return Ok(Box::new(RsaVerifier { key, algorithm: *algorithm, digest, + padding, })); } let inner = (jsonwebtoken_openssl::DEFAULT_PROVIDER.verifier_factory)(algorithm, key)?; @@ -144,11 +144,14 @@ impl JwtVerifier for HmacVerifier { } } -fn pss_digest(algorithm: Algorithm) -> Option { +fn rsa_parameters(algorithm: Algorithm) -> Option<(MessageDigest, Padding)> { match algorithm { - Algorithm::PS256 => Some(MessageDigest::sha256()), - Algorithm::PS384 => Some(MessageDigest::sha384()), - Algorithm::PS512 => Some(MessageDigest::sha512()), + Algorithm::RS256 => Some((MessageDigest::sha256(), Padding::PKCS1)), + Algorithm::RS384 => Some((MessageDigest::sha384(), Padding::PKCS1)), + Algorithm::RS512 => Some((MessageDigest::sha512(), Padding::PKCS1)), + Algorithm::PS256 => Some((MessageDigest::sha256(), Padding::PKCS1_PSS)), + Algorithm::PS384 => Some((MessageDigest::sha384(), Padding::PKCS1_PSS)), + Algorithm::PS512 => Some((MessageDigest::sha512(), Padding::PKCS1_PSS)), _ => None, } } @@ -172,57 +175,62 @@ fn check_rsa_key(key: &PKey) -> Result<()> { Ok(()) } -struct PssSigner { +struct RsaSigner { key: PKey, algorithm: Algorithm, digest: MessageDigest, + padding: Padding, } -impl Signer> for PssSigner { +impl Signer> for RsaSigner { fn try_sign(&self, message: &[u8]) -> std::result::Result, Error> { let mut signer = OpenSslSigner::new(self.digest, &self.key).map_err(Error::from_source)?; signer - .set_rsa_padding(Padding::PKCS1_PSS) - .map_err(Error::from_source)?; - signer - .set_rsa_mgf1_md(self.digest) - .map_err(Error::from_source)?; - // JOSE requires salt length equal to the digest length, not OpenSSL's - // default maximum salt length (RFC 7518 section 3.5). - signer - .set_rsa_pss_saltlen(RsaPssSaltlen::DIGEST_LENGTH) + .set_rsa_padding(self.padding) .map_err(Error::from_source)?; + if self.padding == Padding::PKCS1_PSS { + signer + .set_rsa_mgf1_md(self.digest) + .map_err(Error::from_source)?; + // JOSE requires salt length equal to the digest length (RFC 7518 3.5). + signer + .set_rsa_pss_saltlen(RsaPssSaltlen::DIGEST_LENGTH) + .map_err(Error::from_source)?; + } signer .sign_oneshot_to_vec(message) .map_err(Error::from_source) } } -impl JwtSigner for PssSigner { +impl JwtSigner for RsaSigner { fn algorithm(&self) -> Algorithm { self.algorithm } } -struct PssVerifier { +struct RsaVerifier { key: PKey, algorithm: Algorithm, digest: MessageDigest, + padding: Padding, } -impl Verifier> for PssVerifier { +impl Verifier> for RsaVerifier { fn verify(&self, message: &[u8], signature: &Vec) -> std::result::Result<(), Error> { let mut verifier = OpenSslVerifier::new(self.digest, &self.key).map_err(Error::from_source)?; verifier - .set_rsa_padding(Padding::PKCS1_PSS) - .map_err(Error::from_source)?; - verifier - .set_rsa_mgf1_md(self.digest) - .map_err(Error::from_source)?; - verifier - .set_rsa_pss_saltlen(RsaPssSaltlen::DIGEST_LENGTH) + .set_rsa_padding(self.padding) .map_err(Error::from_source)?; + if self.padding == Padding::PKCS1_PSS { + verifier + .set_rsa_mgf1_md(self.digest) + .map_err(Error::from_source)?; + verifier + .set_rsa_pss_saltlen(RsaPssSaltlen::DIGEST_LENGTH) + .map_err(Error::from_source)?; + } if verifier .verify_oneshot(signature, message) .map_err(Error::from_source)? @@ -234,30 +242,66 @@ impl Verifier> for PssVerifier { } } -impl JwtVerifier for PssVerifier { +impl JwtVerifier for RsaVerifier { + fn algorithm(&self) -> Algorithm { + self.algorithm + } +} + +// Keep the validated key rather than asking the upstream factory to parse it again. +struct EcSigner { + key: PKey, + algorithm: Algorithm, + digest: MessageDigest, + width: i32, +} + +impl Signer> for EcSigner { + fn try_sign(&self, message: &[u8]) -> std::result::Result, Error> { + let mut signer = OpenSslSigner::new(self.digest, &self.key).map_err(Error::from_source)?; + let der = signer + .sign_oneshot_to_vec(message) + .map_err(Error::from_source)?; + // JOSE uses fixed-width r || s, whereas OpenSSL returns an ASN.1 sequence. + let signature = EcdsaSig::from_der(&der).map_err(Error::from_source)?; + let mut raw = signature + .r() + .to_vec_padded(self.width) + .map_err(Error::from_source)?; + raw.extend( + signature + .s() + .to_vec_padded(self.width) + .map_err(Error::from_source)?, + ); + Ok(raw) + } +} + +impl JwtSigner for EcSigner { fn algorithm(&self) -> Algorithm { self.algorithm } } fn ec_coordinates(der: &[u8], algorithm: Algorithm) -> Result<(EllipticCurve, Vec, Vec)> { - check_ec_key(algorithm, der)?; - let (curve, x, y) = (jsonwebtoken_openssl::DEFAULT_PROVIDER - .jwk_utils - .extract_ec_public_key_coordinates)(der, algorithm)?; - let width: usize = if algorithm == Algorithm::ES256 { - 32 - } else { - 48 + let key = parse_ec_key(algorithm, der)?; + let ec = key.ec_key().map_err(|_| ErrorKind::InvalidEcdsaKey)?; + let (curve, width) = match algorithm { + Algorithm::ES256 => (EllipticCurve::P256, 32), + Algorithm::ES384 => (EllipticCurve::P384, 48), + _ => return Err(ErrorKind::InvalidAlgorithm.into()), }; + let mut x = BigNum::new().map_err(Error::from_source)?; + let mut y = BigNum::new().map_err(Error::from_source)?; + let mut context = BigNumContext::new().map_err(Error::from_source)?; + ec.public_key() + .affine_coordinates_gfp(ec.group(), &mut x, &mut y, &mut context) + .map_err(|_| ErrorKind::InvalidEcdsaKey)?; // JWK coordinates have fixed width, including leading zeros (RFC 7518 6.2.1). - let pad = |coordinate: Vec| -> Result> { - let padding = width - .checked_sub(coordinate.len()) - .ok_or(ErrorKind::InvalidEcdsaKey)?; - let mut result = vec![0; padding]; - result.extend_from_slice(&coordinate); - Ok(result) - }; - Ok((curve, pad(x)?, pad(y)?)) + Ok(( + curve, + x.to_vec_padded(width).map_err(Error::from_source)?, + y.to_vec_padded(width).map_err(Error::from_source)?, + )) } diff --git a/crates/openshell-crypto/tests/common/mod.rs b/crates/openshell-crypto/tests/common/mod.rs new file mode 100644 index 0000000000..68e081d971 --- /dev/null +++ b/crates/openshell-crypto/tests/common/mod.rs @@ -0,0 +1,47 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +use std::sync::Arc; + +#[cfg(feature = "openssl")] +use openshell_crypto::{CryptoContext, OpenSsl}; +#[cfg(feature = "openssl")] +use std::sync::Once; + +// The context-selection test uses only the backend-neutral handshake helper. +#[cfg(feature = "openssl")] +#[allow(dead_code)] +pub fn init_openssl() { + static INIT: Once = Once::new(); + INIT.call_once(|| { + // Explicit facade selection must override an existing Rustls default. + #[cfg(feature = "aws-lc")] + rustls::crypto::aws_lc_rs::default_provider() + .install_default() + .unwrap(); + openshell_crypto::install_default_context(CryptoContext::new(Box::new(OpenSsl))).unwrap(); + }); +} + +pub fn handshake( + client: rustls::ClientConfig, + server: rustls::ServerConfig, + host: &'static str, +) -> Result<(rustls::ClientConnection, rustls::ServerConnection), rustls::Error> { + let mut client = rustls::ClientConnection::new(Arc::new(client), host.try_into().unwrap())?; + let mut server = rustls::ServerConnection::new(Arc::new(server))?; + for _ in 0..10 { + let mut wire = Vec::new(); + client.write_tls(&mut wire).unwrap(); + server.read_tls(&mut wire.as_slice()).unwrap(); + server.process_new_packets()?; + wire.clear(); + server.write_tls(&mut wire).unwrap(); + client.read_tls(&mut wire.as_slice()).unwrap(); + client.process_new_packets()?; + if !client.is_handshaking() && !server.is_handshaking() { + return Ok((client, server)); + } + } + Err(rustls::Error::General("handshake did not finish".into())) +} diff --git a/crates/openshell-crypto/tests/context_selection.rs b/crates/openshell-crypto/tests/context_selection.rs index 5ac2b754f1..13eec2e529 100644 --- a/crates/openshell-crypto/tests/context_selection.rs +++ b/crates/openshell-crypto/tests/context_selection.rs @@ -3,6 +3,8 @@ #![cfg(feature = "aws-lc")] +mod common; + use openshell_crypto::{ Capabilities, CryptoBackend, CryptoContext, CryptoError, Digest, ProtocolBackend, aead::Sealed, }; @@ -186,24 +188,7 @@ fn selected_context_controls_application_adapters_and_cannot_be_replaced() { assert_eq!(client.crypto_provider().cipher_suites.len(), 1); assert_eq!(server.crypto_provider().cipher_suites.len(), 1); let suite = client.crypto_provider().cipher_suites[0].suite(); - let mut client = - rustls::ClientConnection::new(std::sync::Arc::new(client), "localhost".try_into().unwrap()) - .unwrap(); - let mut server = rustls::ServerConnection::new(std::sync::Arc::new(server)).unwrap(); - for _ in 0..10 { - let mut wire = Vec::new(); - client.write_tls(&mut wire).unwrap(); - server.read_tls(&mut wire.as_slice()).unwrap(); - server.process_new_packets().unwrap(); - wire.clear(); - server.write_tls(&mut wire).unwrap(); - client.read_tls(&mut wire.as_slice()).unwrap(); - client.process_new_packets().unwrap(); - if !client.is_handshaking() && !server.is_handshaking() { - break; - } - } - assert!(!client.is_handshaking() && !server.is_handshaking()); + let (client, _) = common::handshake(client, server, "localhost").unwrap(); assert_eq!(client.negotiated_cipher_suite().unwrap().suite(), suite); assert_eq!( openshell_crypto::default_context() diff --git a/crates/openshell-crypto/tests/jwt_rejection_parity.rs b/crates/openshell-crypto/tests/jwt_rejection_parity.rs new file mode 100644 index 0000000000..3516960ba6 --- /dev/null +++ b/crates/openshell-crypto/tests/jwt_rejection_parity.rs @@ -0,0 +1,206 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +#![cfg(all(feature = "aws-lc", feature = "openssl"))] + +use jsonwebtoken::{Algorithm, DecodingKey, EncodingKey, crypto::CryptoProvider}; +use openshell_crypto::{CryptoContext, OpenSsl, ProtocolBackend}; +use openssl::{hash::MessageDigest, pkey::PKey, rsa::Padding, sign::RsaPssSaltlen}; + +const MESSAGE: &[u8] = b"header.claims"; +const RSA_ALGORITHMS: [Algorithm; 6] = [ + Algorithm::RS256, + Algorithm::RS384, + Algorithm::RS512, + Algorithm::PS256, + Algorithm::PS384, + Algorithm::PS512, +]; + +fn providers() -> [(&'static str, &'static CryptoProvider); 2] { + [ + ("aws-lc", CryptoContext::default().backend().jwt_provider()), + ("openssl", OpenSsl.jwt_provider()), + ] +} + +// AWS-LC defers some checks until the operation; OpenSSL checks at construction. +// Assert rejection by each provider, not just equality (both accepting is a bug). +fn reject_signing(case: &str, algorithm: Algorithm, key: &EncodingKey) { + for (name, provider) in providers() { + let accepted = (provider.signer_factory)(&algorithm, key) + .is_ok_and(|signer| signer.try_sign(MESSAGE).is_ok()); + assert!(!accepted, "{name} accepted {case} for {algorithm:?}"); + } +} + +fn reject_verification(case: &str, algorithm: Algorithm, key: &DecodingKey, signature: &Vec) { + for (name, provider) in providers() { + let accepted = (provider.verifier_factory)(&algorithm, key) + .is_ok_and(|verifier| verifier.verify(MESSAGE, signature).is_ok()); + assert!(!accepted, "{name} accepted {case} for {algorithm:?}"); + } +} + +#[test] +fn malformed_keys_are_rejected_by_both_backends() { + for der in [&[][..], &[0x30, 0x03, 0x02, 0x01][..]] { + for algorithm in RSA_ALGORITHMS { + reject_signing( + "malformed RSA DER", + algorithm, + &EncodingKey::from_rsa_der(der), + ); + reject_verification( + "malformed RSA DER", + algorithm, + &DecodingKey::from_rsa_der(der), + &vec![0; 256], + ); + } + for algorithm in [Algorithm::ES256, Algorithm::ES384] { + reject_signing( + "malformed EC DER", + algorithm, + &EncodingKey::from_ec_der(der), + ); + reject_verification( + "malformed EC public key", + algorithm, + &DecodingKey::from_ec_der(der), + &vec![0; 64], + ); + } + reject_signing( + "malformed Ed25519 DER", + Algorithm::EdDSA, + &EncodingKey::from_ed_der(der), + ); + reject_verification( + "malformed Ed25519 public key", + Algorithm::EdDSA, + &DecodingKey::from_ed_der(der), + &vec![0; 64], + ); + } + + let rsa = openssl::rsa::Rsa::generate(1024).unwrap(); + let encoding = EncodingKey::from_rsa_der(&rsa.private_key_to_der().unwrap()); + let decoding = DecodingKey::from_rsa_der(&rsa.public_key_to_der_pkcs1().unwrap()); + // Use a real signature so failure cannot be attributed only to garbage bytes. + let native = PKey::from_rsa(rsa).unwrap(); + for (algorithm, digest) in [ + (Algorithm::RS256, MessageDigest::sha256()), + (Algorithm::RS384, MessageDigest::sha384()), + (Algorithm::RS512, MessageDigest::sha512()), + ] { + let mut signer = openssl::sign::Signer::new(digest, &native).unwrap(); + let signature = signer.sign_oneshot_to_vec(MESSAGE).unwrap(); + reject_verification("undersized RSA key", algorithm, &decoding, &signature); + } + for algorithm in RSA_ALGORITHMS { + reject_signing("undersized RSA key", algorithm, &encoding); + } + + for (curve, wrong_algorithm) in [ + (&rcgen::PKCS_ECDSA_P256_SHA256, Algorithm::ES384), + (&rcgen::PKCS_ECDSA_P384_SHA384, Algorithm::ES256), + ] { + let key = OpenSsl.generate_keypair(curve).unwrap(); + let encoding = EncodingKey::from_ec_pem(key.serialize_pem().unwrap().as_bytes()).unwrap(); + reject_signing("mismatched EC curve", wrong_algorithm, &encoding); + } +} + +#[test] +fn malformed_signatures_are_rejected_by_both_backends() { + for (key_algorithm, algorithms) in [ + (&rcgen::PKCS_RSA_SHA256, &RSA_ALGORITHMS[..]), + (&rcgen::PKCS_ECDSA_P256_SHA256, &[Algorithm::ES256][..]), + (&rcgen::PKCS_ECDSA_P384_SHA384, &[Algorithm::ES384][..]), + (&rcgen::PKCS_ED25519, &[Algorithm::EdDSA][..]), + ] { + let key = OpenSsl.generate_keypair(key_algorithm).unwrap(); + let private = key.serialize_pem().unwrap(); + let public = key.public_key_pem(); + let (encoding, decoding) = match algorithms[0] { + Algorithm::RS256 => ( + EncodingKey::from_rsa_pem(private.as_bytes()).unwrap(), + DecodingKey::from_rsa_pem(public.as_bytes()).unwrap(), + ), + Algorithm::EdDSA => ( + EncodingKey::from_ed_pem(private.as_bytes()).unwrap(), + DecodingKey::from_ed_pem(public.as_bytes()).unwrap(), + ), + _ => ( + EncodingKey::from_ec_pem(private.as_bytes()).unwrap(), + DecodingKey::from_ec_pem(public.as_bytes()).unwrap(), + ), + }; + for &algorithm in algorithms { + check_signatures(algorithm, &encoding, &decoding); + } + } + for algorithm in [Algorithm::HS256, Algorithm::HS384, Algorithm::HS512] { + check_signatures( + algorithm, + &EncodingKey::from_secret(b"parity-test-key"), + &DecodingKey::from_secret(b"parity-test-key"), + ); + } +} + +fn check_signatures(algorithm: Algorithm, encoding: &EncodingKey, decoding: &DecodingKey) { + for (producer, provider) in providers() { + let signer = (provider.signer_factory)(&algorithm, encoding).unwrap(); + let signature = signer.try_sign(MESSAGE).unwrap(); + // Positive controls ensure the fixtures work with both providers. + for (consumer, provider) in providers() { + let verifier = (provider.verifier_factory)(&algorithm, decoding).unwrap(); + assert!( + verifier.verify(MESSAGE, &signature).is_ok(), + "{producer} -> {consumer}: {algorithm:?}" + ); + assert!(verifier.verify(b"changed.claims", &signature).is_err()); + } + let mut extended = signature.clone(); + extended.push(0); + let mut tampered = signature.clone(); + tampered[0] ^= 1; + for (case, invalid) in [ + ("empty signature", vec![]), + ( + "truncated signature", + signature[..signature.len() - 1].to_vec(), + ), + ("extended signature", extended), + ("tampered signature", tampered), + ] { + reject_verification(case, algorithm, decoding, &invalid); + } + } +} + +#[test] +fn non_jose_pss_salt_is_rejected_by_both_backends() { + let key = OpenSsl.generate_keypair(&rcgen::PKCS_RSA_SHA256).unwrap(); + let native = PKey::private_key_from_pkcs8(&key.serialize_der().unwrap()).unwrap(); + let decoding = DecodingKey::from_rsa_pem(key.public_key_pem().as_bytes()).unwrap(); + for (algorithm, digest) in [ + (Algorithm::PS256, MessageDigest::sha256()), + (Algorithm::PS384, MessageDigest::sha384()), + (Algorithm::PS512, MessageDigest::sha512()), + ] { + let digest_len = i32::try_from(digest.size()).unwrap(); + for salt_len in [0, digest_len - 1, digest_len + 1] { + let mut signer = openssl::sign::Signer::new(digest, &native).unwrap(); + signer.set_rsa_padding(Padding::PKCS1_PSS).unwrap(); + signer.set_rsa_mgf1_md(digest).unwrap(); + signer + .set_rsa_pss_saltlen(RsaPssSaltlen::custom(salt_len)) + .unwrap(); + let signature = signer.sign_oneshot_to_vec(MESSAGE).unwrap(); + reject_verification("non-JOSE PSS salt length", algorithm, &decoding, &signature); + } + } +} diff --git a/crates/openshell-crypto/tests/openssl_backend.rs b/crates/openshell-crypto/tests/openssl_backend.rs index 41e805a62e..8ef848e9c8 100644 --- a/crates/openshell-crypto/tests/openssl_backend.rs +++ b/crates/openshell-crypto/tests/openssl_backend.rs @@ -3,31 +3,18 @@ #![cfg(feature = "openssl")] -use openshell_crypto::OpenSsl; -use openshell_crypto::{ - CryptoBackend, CryptoContext, CryptoError, ProtocolBackend, aead, jwt, pki, tls, -}; -use std::{ - io::{Read, Write}, - sync::{Arc, Once}, -}; +mod common; -fn init() { - static INIT: Once = Once::new(); - INIT.call_once(|| { - // Explicit facade selection must win even when an embedder installed - // AWS-LC as Rustls's process default first. - #[cfg(feature = "aws-lc")] - rustls::crypto::aws_lc_rs::default_provider() - .install_default() - .unwrap(); - openshell_crypto::install_default_context(CryptoContext::new(Box::new(OpenSsl))).unwrap(); - }); -} +use common::{handshake, init_openssl}; +#[cfg(feature = "aws-lc")] +use openshell_crypto::CryptoContext; +use openshell_crypto::OpenSsl; +use openshell_crypto::{CryptoBackend, CryptoError, ProtocolBackend, aead, jwt, pki, tls}; +use std::io::{Read, Write}; #[test] fn primitives_and_posture() { - init(); + init_openssl(); assert_eq!( openshell_crypto::sha256(b"abc").unwrap(), [ @@ -60,7 +47,7 @@ fn primitives_and_posture() { #[test] fn aead_compatibility_and_rejection() { - init(); + init_openssl(); // NIST AES-256-GCM empty-message vector: ciphertext consists only of the tag. let tag = [ 0x53, 0x0f, 0x8a, 0xfb, 0xc7, 0x45, 0x36, 0xb9, 0xa9, 0x63, 0xb4, 0xf1, 0xc4, 0xcb, 0x73, @@ -130,7 +117,7 @@ fn aead_handles_multiple_update_chunks() { #[test] fn jwt_rejects_mismatched_curve_and_pss_salt() { - init(); + init_openssl(); let key = OpenSsl .generate_keypair(&rcgen::PKCS_ECDSA_P256_SHA256) .unwrap(); @@ -249,18 +236,20 @@ fn jwk_coordinates_preserve_leading_zeros() { #[test] fn backend_owned_keys_and_certificates() { - init(); + init_openssl(); for alg in [ &rcgen::PKCS_ECDSA_P256_SHA256, &rcgen::PKCS_ECDSA_P384_SHA384, &rcgen::PKCS_ED25519, &rcgen::PKCS_RSA_SHA256, + &rcgen::PKCS_RSA_SHA384, + &rcgen::PKCS_RSA_SHA512, ] { let key = pki::generate_keypair_for(alg).unwrap(); let der = key.serialize_der().unwrap(); let from_der = pki::KeyPair::from_pkcs8_der(&der).unwrap(); let from_pem = pki::KeyPair::from_pem(&key.serialize_pem().unwrap()).unwrap(); - for imported in [&from_der, &from_pem] { + for imported in [&key, &from_der, &from_pem] { assert_eq!(key.public_key_der(), imported.public_key_der()); let cert = pki::self_signed( rcgen::CertificateParams::new(vec!["localhost".into()]).unwrap(), @@ -280,7 +269,7 @@ fn jwt_algorithms_and_claims_validation() { use jsonwebtoken::{ Algorithm, DecodingKey, EncodingKey, Header, Validation, errors::ErrorKind, }; - init(); + init_openssl(); let claims = serde_json::json!({"sub":"sandbox", "iss":"gateway", "aud":"sandbox", "exp":4_000_000_000_u64}); let mut keys = vec![( Algorithm::HS256, @@ -365,7 +354,7 @@ fn jwt_algorithms_and_claims_validation() { #[test] fn tls13_handshake_and_application_data() { - init(); + init_openssl(); let pki::CertifiedKey { cert, key_pair } = pki::generate_simple_self_signed(vec!["localhost".into()]).unwrap(); let mut roots = rustls::RootCertStore::empty(); @@ -380,23 +369,7 @@ fn tls13_handshake_and_application_data() { rustls::pki_types::PrivatePkcs8KeyDer::from(key_pair.serialize_der().unwrap()).into(), ) .unwrap(); - let mut client = - rustls::ClientConnection::new(Arc::new(client), "localhost".try_into().unwrap()).unwrap(); - let mut server = rustls::ServerConnection::new(Arc::new(server)).unwrap(); - for _ in 0..10 { - let mut wire = Vec::new(); - client.write_tls(&mut wire).unwrap(); - server.read_tls(&mut wire.as_slice()).unwrap(); - server.process_new_packets().unwrap(); - wire.clear(); - server.write_tls(&mut wire).unwrap(); - client.read_tls(&mut wire.as_slice()).unwrap(); - client.process_new_packets().unwrap(); - if !client.is_handshaking() && !server.is_handshaking() { - break; - } - } - assert!(!client.is_handshaking() && !server.is_handshaking()); + let (mut client, mut server) = handshake(client, server, "localhost").unwrap(); assert_eq!( client.protocol_version(), Some(rustls::ProtocolVersion::TLSv1_3) @@ -417,7 +390,7 @@ fn tls13_handshake_and_application_data() { #[test] fn imported_ca_signs_with_openssl() { - init(); + init_openssl(); let key = pki::generate_keypair().unwrap(); let mut params = rcgen::CertificateParams::default(); params.is_ca = rcgen::IsCa::Ca(rcgen::BasicConstraints::Unconstrained); @@ -443,7 +416,7 @@ fn imported_ca_signs_with_openssl() { #[test] fn malformed_hmac_signatures_are_rejected() { - init(); + init_openssl(); for algorithm in [ jsonwebtoken::Algorithm::HS256, jsonwebtoken::Algorithm::HS384, @@ -475,7 +448,7 @@ fn malformed_hmac_signatures_are_rejected() { #[cfg(feature = "aws-lc")] fn backends_preserve_ciphertext_keys_and_jwt_signatures() { use jsonwebtoken::{Algorithm, DecodingKey, EncodingKey}; - init(); + init_openssl(); let aws = CryptoContext::default(); assert_eq!(aws.backend().capabilities().backend, "aws-lc"); let openssl: &dyn ProtocolBackend = &OpenSsl; diff --git a/crates/openshell-crypto/tests/openssl_key_validation.rs b/crates/openshell-crypto/tests/openssl_key_validation.rs index e20337bc83..91badf80fe 100644 --- a/crates/openshell-crypto/tests/openssl_key_validation.rs +++ b/crates/openshell-crypto/tests/openssl_key_validation.rs @@ -124,3 +124,81 @@ fn pki_and_jwt_reject_inconsistent_ec_private_keys() { assert!((provider.signer_factory)(&algorithm, &EncodingKey::from_ec_der(&sec1)).is_ok()); } } + +#[test] +fn pki_enforces_rsa_minimum_size_for_pem_and_pkcs8() { + // Certificate construction uses the facade for its random serial number. + openshell_crypto::install_default_context(openshell_crypto::CryptoContext::new(Box::new( + OpenSsl, + ))) + .unwrap(); + #[cfg(feature = "aws-lc")] + let aws = openshell_crypto::CryptoContext::default(); + let backends: &[&dyn ProtocolBackend] = &[ + &OpenSsl, + #[cfg(feature = "aws-lc")] + aws.backend(), + ]; + for bits in [1024, 2048] { + let rsa = Rsa::generate(bits).unwrap(); + let traditional_pem = String::from_utf8(rsa.private_key_to_pem().unwrap()).unwrap(); + let native = PKey::from_rsa(rsa).unwrap(); + let der = native.private_key_to_pkcs8().unwrap(); + let pem = String::from_utf8(native.private_key_to_pem_pkcs8().unwrap()).unwrap(); + for backend in backends { + for (format, imported) in [ + ("PKCS#8 DER", backend.import_keypair_der(&der)), + ("PKCS#8 PEM", backend.import_keypair_pem(&pem)), + ("PKCS#1 PEM", backend.import_keypair_pem(&traditional_pem)), + ] { + if bits < 2048 { + assert!( + imported.is_err(), + "{} accepted a {bits}-bit RSA key from {format}", + backend.capabilities().backend + ); + } else { + let key = imported.unwrap(); + assert_eq!(key.public_key_der(), native.public_key_to_der().unwrap()); + let certificate = openshell_crypto::pki::self_signed( + rcgen::CertificateParams::new(vec!["localhost".into()]).unwrap(), + &key, + ) + .unwrap(); + let x509 = openssl::x509::X509::from_der(certificate.der()).unwrap(); + assert!(x509.verify(&native).unwrap()); + } + } + } + } +} + +#[test] +fn pki_rejects_oversized_rsa_imports() { + // A parseable oversized modulus isolates the size gate without expensive + // 8193-bit prime generation. This synthetic key is not a consistent RSA pair. + let seed = Rsa::generate(2048).unwrap(); + let mut modulus = seed.n().to_owned().unwrap(); + modulus.set_bit(8192).unwrap(); + let oversized = Rsa::from_private_components( + modulus, + seed.e().to_owned().unwrap(), + seed.d().to_owned().unwrap(), + seed.p().unwrap().to_owned().unwrap(), + seed.q().unwrap().to_owned().unwrap(), + seed.dmp1().unwrap().to_owned().unwrap(), + seed.dmq1().unwrap().to_owned().unwrap(), + seed.iqmp().unwrap().to_owned().unwrap(), + ) + .unwrap(); + let traditional_pem = String::from_utf8(oversized.private_key_to_pem().unwrap()).unwrap(); + let native = PKey::from_rsa(oversized).unwrap(); + assert_eq!(native.bits(), 8193); + let der = native.private_key_to_pkcs8().unwrap(); + let pem = String::from_utf8(native.private_key_to_pem_pkcs8().unwrap()).unwrap(); + assert!(PKey::private_key_from_pkcs8(&der).is_ok()); + assert!(PKey::private_key_from_pem(pem.as_bytes()).is_ok()); + assert!(OpenSsl.import_keypair_der(&der).is_err()); + assert!(OpenSsl.import_keypair_pem(&pem).is_err()); + assert!(OpenSsl.import_keypair_pem(&traditional_pem).is_err()); +} diff --git a/crates/openshell-crypto/tests/openssl_tls.rs b/crates/openshell-crypto/tests/openssl_tls.rs index 57592f4190..2b17bf7c4d 100644 --- a/crates/openshell-crypto/tests/openssl_tls.rs +++ b/crates/openshell-crypto/tests/openssl_tls.rs @@ -3,43 +3,11 @@ #![cfg(feature = "openssl")] -use openshell_crypto::{CryptoContext, OpenSsl, ProtocolBackend, pki, tls}; -use std::sync::{Arc, Once}; +mod common; -fn init() { - static INIT: Once = Once::new(); - INIT.call_once(|| { - #[cfg(feature = "aws-lc")] - rustls::crypto::aws_lc_rs::default_provider() - .install_default() - .unwrap(); - openshell_crypto::install_default_context(CryptoContext::new(Box::new(OpenSsl))).unwrap(); - }); -} - -fn handshake( - client: rustls::ClientConfig, - server: rustls::ServerConfig, - host: &'static str, -) -> Result<(), rustls::Error> { - let mut client = - rustls::ClientConnection::new(Arc::new(client), host.try_into().unwrap()).unwrap(); - let mut server = rustls::ServerConnection::new(Arc::new(server)).unwrap(); - for _ in 0..10 { - let mut wire = Vec::new(); - client.write_tls(&mut wire).unwrap(); - server.read_tls(&mut wire.as_slice()).unwrap(); - server.process_new_packets()?; - wire.clear(); - server.write_tls(&mut wire).unwrap(); - client.read_tls(&mut wire.as_slice()).unwrap(); - client.process_new_packets()?; - if !client.is_handshaking() && !server.is_handshaking() { - return Ok(()); - } - } - Err(rustls::Error::General("handshake did not finish".into())) -} +use common::{handshake, init_openssl}; +use openshell_crypto::{OpenSsl, ProtocolBackend, pki, tls}; +use std::sync::Arc; fn leaf( ca: &pki::Certificate, @@ -56,7 +24,7 @@ fn leaf( #[test] fn tls12_and_tls13_enforce_server_and_client_authentication() { - init(); + init_openssl(); // An explicit empty protocol policy must not be replaced with defaults. assert!(tls::try_client_builder_with_protocol_versions(&[]).is_err()); assert!(tls::try_server_builder_with_protocol_versions(&[]).is_err()); diff --git a/crates/openshell-crypto/tests/openssl_unavailable.rs b/crates/openshell-crypto/tests/openssl_unavailable.rs index f04f692643..d0bbab1bb5 100644 --- a/crates/openshell-crypto/tests/openssl_unavailable.rs +++ b/crates/openshell-crypto/tests/openssl_unavailable.rs @@ -4,12 +4,16 @@ #![cfg(feature = "openssl")] use openshell_crypto::{CryptoBackend, CryptoContext, CryptoError, OpenSsl, ProtocolBackend, tls}; +use std::{ + process::{Command, Stdio}, + time::{Duration, Instant}, +}; #[test] fn unavailable_provider_fails_without_fallback() { // OpenSSL reads configuration once. Exercise failure in a fresh process, // without changing the test runner's environment or process-global state. - let result = std::process::Command::new(std::env::current_exe().unwrap()) + let mut child = Command::new(std::env::current_exe().unwrap()) .args([ "--ignored", "--exact", @@ -24,8 +28,26 @@ fn unavailable_provider_fails_without_fallback() { "/tests/fixtures/openssl-unavailable.cnf" ), ) - .output() + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() .unwrap(); + let deadline = Instant::now() + Duration::from_secs(10); + while child.try_wait().unwrap().is_none() { + if Instant::now() >= deadline { + // The child may have exited since try_wait; still reap it below. + let _ = child.kill(); + let result = child.wait_with_output().unwrap(); + panic!( + "unavailable-provider child exceeded the 10-second deadline\n{}\n{}", + String::from_utf8_lossy(&result.stdout), + String::from_utf8_lossy(&result.stderr) + ); + } + std::thread::sleep(Duration::from_millis(20)); + } + let result = child.wait_with_output().unwrap(); assert!( result.status.success(), "{}\n{}", diff --git a/deny.toml b/deny.toml index 6366c028b9..8a1802276b 100644 --- a/deny.toml +++ b/deny.toml @@ -100,6 +100,10 @@ wrappers = [ [[bans.deny]] name = "aws-lc-fips-sys" # Keep this boundary even when the FIPS build feature is disabled. +# The aws-lc-rs wrapper is intentionally unused in the current resolved graph; +# cargo-deny's unused-wrapper warning for this entry is expected. Revisit this +# acknowledgement if the FIPS dependency is enabled; other unused wrappers +# still require review. reason = "Native crypto bindings belong behind their provider libraries, not application crates." wrappers = [ "aws-lc-rs", @@ -114,6 +118,10 @@ wrappers = [ "rustls-openssl", ] +[[bans.deny]] +name = "openssl-src" +reason = "Use system-provided OpenSSL; vendored OpenSSL is not permitted." + [[bans.deny]] name = "openssl-sys" reason = "Native crypto bindings belong behind their provider libraries, not application crates." diff --git a/tasks/scripts/test-crypto-dependency-policy.py b/tasks/scripts/test-crypto-dependency-policy.py index 6845a32204..6ade9ba798 100644 --- a/tasks/scripts/test-crypto-dependency-policy.py +++ b/tasks/scripts/test-crypto-dependency-policy.py @@ -5,11 +5,14 @@ No crypto code is built or downloaded. Package names and dependency edges are enough to check that an approved wrapper cannot mask an application bypass. +This standalone runner needs no pytest environment in the Rust policy job. +Graph failures are collected so every case runs before the command fails. """ import json import subprocess import tempfile +import tomllib from pathlib import Path ROOT = Path(__file__).resolve().parents[2] @@ -19,7 +22,7 @@ def check_graph( directory: Path, edges: list[tuple[str, str, str, str]], - banned: str | None = None, + banned: tuple[str, ...] = (), ) -> None: """Each edge is (parent, package, manifest section, dependency alias).""" packages = {name for parent, child, _, _ in edges for name in (parent, child)} @@ -74,12 +77,12 @@ def check_graph( check=False, ) output = result.stdout + result.stderr - if banned is None: + if not banned: passed = result.returncode == 0 else: # A metadata/configuration failure must not count as a successful ban. - passed = ( - result.returncode != 0 and f"error[banned]: crate '{banned} =" in output + passed = result.returncode != 0 and all( + f"error[banned]: crate '{package} =" in output for package in banned ) if not passed: raise AssertionError(f"{directory.name}: unexpected policy result\n{output}") @@ -93,6 +96,20 @@ def edge( def main() -> None: + tools = tomllib.loads((ROOT / "mise.toml").read_text(encoding="utf-8"))["tools"] + expected = tools["github:EmbarkStudios/cargo-deny"]["version"] + version = subprocess.run( + ["cargo", "deny", "--version"], + capture_output=True, + text=True, + check=False, + ) + if version.returncode != 0 or version.stdout.strip() != f"cargo-deny {expected}": + actual = (version.stdout + version.stderr).strip() + raise RuntimeError( + f"Crypto policy fixtures require cargo-deny {expected} from mise.toml; " + f"found: {actual}. Run mise install, then mise run rust:deny:policy." + ) facade = [ edge("policy-test-app", "openshell-crypto"), edge("openshell-crypto", "aws-lc-rs"), @@ -106,10 +123,28 @@ def main() -> None: edge("openshell-crypto", "jsonwebtoken-openssl"), edge("jsonwebtoken-openssl", "openssl"), ] + failures: list[AssertionError] = [] + + def run_graph( + directory: Path, + edges: list[tuple[str, str, str, str]], + banned: tuple[str, ...] = (), + ) -> None: + try: + check_graph(directory, edges, banned) + except AssertionError as error: + failures.append(error) + print(f"FAIL {directory.name}") + with tempfile.TemporaryDirectory(prefix="openshell-crypto-policy-") as temp: root = Path(temp) - check_graph(root / "facade-backends", facade) - check_graph( + run_graph(root / "facade-backends", facade) + run_graph( + root / "vendored-openssl", + [*facade, edge("openssl-sys", "openssl-src", "build-dependencies")], + ("openssl-src",), + ) + run_graph( root / "documented-exceptions", [ edge("openshell-server", "rsa", "dev-dependencies"), @@ -122,12 +157,13 @@ def main() -> None: ) # Keep the valid facade path present in every negative case. A shared # implementation must still reject each unapproved direct parent. - for package in ( + direct_packages = ( "aws-lc-rs", "aws-lc-sys", "aws-lc-fips-sys", "openssl", "openssl-sys", + "openssl-src", "rustls-openssl", "jsonwebtoken-openssl", "ring", @@ -139,33 +175,41 @@ def main() -> None: "blake3", "ecdsa", "ml-kem", - ): - check_graph( - root / f"direct-{package}", - [*facade, edge("policy-test-app", package)], - package, - ) + ) + # cargo-deny reports every violation; require each diagnostic so one + # working ban cannot hide another missing or weakened rule. + run_graph( + root / "direct-dependencies", + [ + *facade, + *(edge("policy-test-app", package) for package in direct_packages), + ], + direct_packages, + ) for name, section in ( ("renamed", "dependencies"), ("build", "build-dependencies"), ("dev", "dev-dependencies"), ("windows-only", "target.'cfg(windows)'.dependencies"), ): - check_graph( + run_graph( root / name, [*facade, edge("policy-test-app", "openssl", section, "crypto_alias")], - "openssl", + ("openssl",), ) - check_graph( + run_graph( root / "unapproved-wrapper", [ *facade, edge("policy-test-app", "another-crypto-wrapper"), edge("another-crypto-wrapper", "openssl"), ], - "openssl", + ("openssl",), ) + if failures: + raise ExceptionGroup("Crypto dependency policy fixtures failed", failures) + if __name__ == "__main__": main() From 28a14ed42ccff808d31c2c4e79305a52165f6452 Mon Sep 17 00:00:00 2001 From: Varsha Prasad Narsing Date: Fri, 23 Oct 2026 13:55:54 -0700 Subject: [PATCH 6/7] test(crypto): verify OpenSSL integration and linkage Signed-off-by: Varsha Prasad Narsing --- .github/workflows/branch-checks.yml | 21 +++++++++++ crates/openshell-crypto/README.md | 35 +++++++++++++++++++ .../openshell-crypto/src/openssl_backend.rs | 3 ++ .../tests/jwt_rejection_parity.rs | 23 ++++++++++-- crates/openshell-crypto/tests/openssl_tls.rs | 10 ++++++ 5 files changed, 89 insertions(+), 3 deletions(-) diff --git a/.github/workflows/branch-checks.yml b/.github/workflows/branch-checks.yml index 1f835025d0..3c1771673f 100644 --- a/.github/workflows/branch-checks.yml +++ b/.github/workflows/branch-checks.yml @@ -341,9 +341,30 @@ jobs: fi cargo clippy --locked -p openshell-crypto --no-default-features --features openssl --all-targets -- -D warnings cargo test --locked -p openshell-crypto --no-default-features --features openssl + for feature in tls-tonic tls-kube tls-sqlx; do + cargo check --locked -p openshell-crypto --no-default-features --features "openssl,$feature" + done cargo clippy --locked -p openshell-crypto --features openssl --all-targets -- -D warnings cargo test --locked -p openshell-crypto --features openssl + - name: Verify dynamic OpenSSL linkage on Linux + if: runner.os == 'Linux' + shell: nix develop -c bash -euo pipefail {0} + env: + OPENSSL_NO_VENDOR: "1" + OPENSSL_STATIC: "0" + run: | + cargo test --locked -p openshell-crypto --no-default-features --features openssl --test openssl_tls --no-run --message-format=json > "$RUNNER_TEMP/openssl-artifacts.jsonl" + executable=$(uv run --no-project python -c 'import json, sys; artifacts = [json.loads(line) for line in sys.stdin]; paths = [a["executable"] for a in artifacts if a.get("reason") == "compiler-artifact" and a.get("target", {}).get("name") == "openssl_tls" and a.get("executable")]; assert len(paths) == 1, paths; print(paths[0])' < "$RUNNER_TEMP/openssl-artifacts.jsonl") + ldd "$executable" > "$RUNNER_TEMP/openssl-linkage.txt" + cat "$RUNNER_TEMP/openssl-linkage.txt" + for library in libssl libcrypto; do + if ! grep -Eq "^[[:space:]]*$library\\.so[^[:space:]]* => /" "$RUNNER_TEMP/openssl-linkage.txt"; then + echo "ERROR: $library did not resolve to a dynamic library" >&2 + exit 1 + fi + done + - name: Test shell: nix develop -c bash -euo pipefail {0} env: diff --git a/crates/openshell-crypto/README.md b/crates/openshell-crypto/README.md index 5d849be5d1..745c3d63b4 100644 --- a/crates/openshell-crypto/README.md +++ b/crates/openshell-crypto/README.md @@ -114,6 +114,41 @@ OpenSSL key instead of parsing it again through the upstream adapter. Claim validation stays in jsonwebtoken. AEAD setup errors remain distinct from authentication errors; tentative plaintext is cleared on failure and returned only after authentication. +At AEAD finalization, all OpenSSL errors map to `CryptoError::Authentication`. +The [EVP AEAD interface](https://docs.openssl.org/3.0/man3/EVP_EncryptInit/#aead-interface) +reports unsuccessful authentication through the finalization result; it does not +provide a portable error-stack discriminator between a bad tag and a late provider +failure across providers. A provider failure at this stage therefore shares that +classification. Setup and update failures remain `CryptoError::Operation`. +No failure releases tentative plaintext or triggers a backend fallback. + +The optional `tls-tonic`, `tls-kube`, and `tls-sqlx` features can each compile with +`--no-default-features --features openssl,`; CI checks those combinations +individually. These features expose integration dependencies, not OpenSSL transport +adapters for them. Compilation does not establish that dependency-owned clients +use OpenSSL at runtime or make an OpenSSL-only whole-application build supported. + +Linux CI checks that the OpenSSL-only TLS test executable dynamically resolves +both `libssl` and `libcrypto`. The Nix devshell supplies those libraries; this is +a linkage regression check, not validation against a distro's OpenSSL packages, +provider configuration, or FIPS operating mode. Distro qualification remains +downstream work. +The test explicitly creates a native OpenSSL TLS context to retain `libssl` for +this assertion; the backend itself uses Rustls for TLS protocol handling. + +Both protocol adapters are external community-maintained dependencies: +[`rustls-openssl`](https://github.com/tofay/rustls-openssl) and +[`jsonwebtoken-openssl`](https://github.com/arckoor/jsonwebtoken-providers). +As of 2026-10-23, upstream rustls-openssl has released 0.4.2, while the JWT adapter's +repository remains unarchived with recent activity. This is not a support or +security-maintenance commitment from OpenShell or those projects. The exact +`rustls-openssl =0.4.1` pin preserves the reviewed provider behavior until a +deliberate upgrade is validated. `jsonwebtoken-openssl =1.0.0` matches our +jsonwebtoken 10 provider API; version 2 targets jsonwebtoken 11. Dependency +updates must rerun interoperability, rejection, provider-failure, and linkage +checks. Distro packagers need both Rust adapter crates as well as system OpenSSL; +packaging Rust sources does not permit vendoring the native OpenSSL library. + Protocol limitations remain: Rustls hash/HKDF callbacks and JWT JWK thumbprint hashing cannot return backend errors; adapter helpers can panic on provider failure. The TLS adapter also uses direct OpenSSL SHA operations and low-level diff --git a/crates/openshell-crypto/src/openssl_backend.rs b/crates/openshell-crypto/src/openssl_backend.rs index 88f73b5ea8..bfc2ef03d4 100644 --- a/crates/openshell-crypto/src/openssl_backend.rs +++ b/crates/openshell-crypto/src/openssl_backend.rs @@ -112,6 +112,9 @@ impl CryptoBackend for OpenSsl { // failure path and return it only after finalization verifies the tag. let mut plaintext = Zeroizing::new(vec![0; output_size(data.len())?]); let written = update_data(&mut crypter, data, &mut plaintext)?; + // EVP finalization has no portable provider-independent classification + // for bad tags versus late provider failures. Treat all finalization + // failures as authentication failures; never release tentative plaintext. let final_len = crypter .finalize(&mut plaintext[written..]) .map_err(|_| CryptoError::Authentication)?; diff --git a/crates/openshell-crypto/tests/jwt_rejection_parity.rs b/crates/openshell-crypto/tests/jwt_rejection_parity.rs index 3516960ba6..32ee5123c4 100644 --- a/crates/openshell-crypto/tests/jwt_rejection_parity.rs +++ b/crates/openshell-crypto/tests/jwt_rejection_parity.rs @@ -102,13 +102,30 @@ fn malformed_keys_are_rejected_by_both_backends() { reject_signing("undersized RSA key", algorithm, &encoding); } - for (curve, wrong_algorithm) in [ - (&rcgen::PKCS_ECDSA_P256_SHA256, Algorithm::ES384), - (&rcgen::PKCS_ECDSA_P384_SHA384, Algorithm::ES256), + for (curve, algorithm, wrong_algorithm) in [ + ( + &rcgen::PKCS_ECDSA_P256_SHA256, + Algorithm::ES256, + Algorithm::ES384, + ), + ( + &rcgen::PKCS_ECDSA_P384_SHA384, + Algorithm::ES384, + Algorithm::ES256, + ), ] { let key = OpenSsl.generate_keypair(curve).unwrap(); let encoding = EncodingKey::from_ec_pem(key.serialize_pem().unwrap().as_bytes()).unwrap(); reject_signing("mismatched EC curve", wrong_algorithm, &encoding); + let decoding = DecodingKey::from_ec_pem(key.public_key_pem().as_bytes()).unwrap(); + let signer = (OpenSsl.jwt_provider().signer_factory)(&algorithm, &encoding).unwrap(); + let signature = signer.try_sign(MESSAGE).unwrap(); + reject_verification( + "mismatched EC curve", + wrong_algorithm, + &decoding, + &signature, + ); } } diff --git a/crates/openshell-crypto/tests/openssl_tls.rs b/crates/openshell-crypto/tests/openssl_tls.rs index 2b17bf7c4d..5560e81957 100644 --- a/crates/openshell-crypto/tests/openssl_tls.rs +++ b/crates/openshell-crypto/tests/openssl_tls.rs @@ -9,6 +9,16 @@ use common::{handshake, init_openssl}; use openshell_crypto::{OpenSsl, ProtocolBackend, pki, tls}; use std::sync::Arc; +#[test] +fn native_tls_library_is_available_for_linkage_check() { + // The backend uses Rustls for TLS and libcrypto for primitives. Exercise + // libssl explicitly so Linux's --as-needed linker retains both native + // libraries for CI's dynamic-linkage assertion on this test executable. + let _context = openssl::ssl::SslContext::builder(openssl::ssl::SslMethod::tls()) + .unwrap() + .build(); +} + fn leaf( ca: &pki::Certificate, ca_key: &pki::KeyPair, From c084996aeaa6012b0197aea2e478110e8a6fa0e9 Mon Sep 17 00:00:00 2001 From: Varsha Prasad Narsing Date: Fri, 23 Oct 2026 14:13:46 -0700 Subject: [PATCH 7/7] fix(crypto): reject inconsistent RSA private keys Signed-off-by: Varsha Prasad Narsing --- crates/openshell-crypto/README.md | 2 + .../openshell-crypto/src/openssl_backend.rs | 13 +-- .../src/openssl_backend/jwt.rs | 5 ++ .../tests/openssl_key_validation.rs | 88 +++++++++++++++++++ 4 files changed, 103 insertions(+), 5 deletions(-) diff --git a/crates/openshell-crypto/README.md b/crates/openshell-crypto/README.md index 745c3d63b4..79535b4d47 100644 --- a/crates/openshell-crypto/README.md +++ b/crates/openshell-crypto/README.md @@ -109,6 +109,8 @@ guards for malformed HMAC signatures, EC key consistency and algorithm/curve binding, RSA public-component bounds, JOSE RSA-PSS salt lengths and fixed-width EC JWK coordinates. Certificate key import also checks EC key consistency and requires RSA keys in the 2048–8192-bit range, matching AWS-LC. +RSA certificate imports and JWT signing also validate private-key consistency, +including the private exponent, primes, and CRT parameters, before using the key. RSA signing/verification, EC signing, and EC JWK extraction reuse the validated OpenSSL key instead of parsing it again through the upstream adapter. Claim validation stays in jsonwebtoken. AEAD setup errors remain distinct from authentication errors; diff --git a/crates/openshell-crypto/src/openssl_backend.rs b/crates/openshell-crypto/src/openssl_backend.rs index bfc2ef03d4..5a62634699 100644 --- a/crates/openshell-crypto/src/openssl_backend.rs +++ b/crates/openshell-crypto/src/openssl_backend.rs @@ -188,11 +188,14 @@ impl Key { if !(2048..=8192).contains(&inner.bits()) { return Err(rcgen::Error::CouldNotParseKeyPair); } - inner - .rsa() - .map_err(key_error)? - .public_key_to_der_pkcs1() - .map_err(key_error)? + let rsa = inner.rsa().map_err(key_error)?; + // Decoding and size checks do not establish consistency of the + // private exponent, primes, and CRT parameters. Signing may + // still succeed through CRT even when the private exponent is wrong. + if !matches!(rsa.check_key(), Ok(true)) { + return Err(rcgen::Error::CouldNotParseKeyPair); + } + rsa.public_key_to_der_pkcs1().map_err(key_error)? } _ => return Err(rcgen::Error::UnsupportedSignatureAlgorithm), }; diff --git a/crates/openshell-crypto/src/openssl_backend/jwt.rs b/crates/openshell-crypto/src/openssl_backend/jwt.rs index 4a0774950b..4f972f6bcb 100644 --- a/crates/openshell-crypto/src/openssl_backend/jwt.rs +++ b/crates/openshell-crypto/src/openssl_backend/jwt.rs @@ -75,6 +75,11 @@ fn signer(algorithm: &Algorithm, key: &EncodingKey) -> Result let key = PKey::private_key_from_der(key.inner()).map_err(|_| ErrorKind::InvalidKeyFormat)?; check_rsa_key(&key)?; + // Public-component checks are insufficient for signing keys. Keep this + // private-key validation out of the public verifier path. + if !matches!(key.rsa().and_then(|rsa| rsa.check_key()), Ok(true)) { + return Err(ErrorKind::InvalidKeyFormat.into()); + } return Ok(Box::new(RsaSigner { key, algorithm: *algorithm, diff --git a/crates/openshell-crypto/tests/openssl_key_validation.rs b/crates/openshell-crypto/tests/openssl_key_validation.rs index 91badf80fe..a7f441322d 100644 --- a/crates/openshell-crypto/tests/openssl_key_validation.rs +++ b/crates/openshell-crypto/tests/openssl_key_validation.rs @@ -202,3 +202,91 @@ fn pki_rejects_oversized_rsa_imports() { assert!(OpenSsl.import_keypair_pem(&pem).is_err()); assert!(OpenSsl.import_keypair_pem(&traditional_pem).is_err()); } + +#[test] +fn pki_and_jwt_reject_inconsistent_rsa_private_keys() { + let valid = Rsa::generate(2048).unwrap(); + #[cfg(feature = "aws-lc")] + let aws = openshell_crypto::CryptoContext::default(); + let backends: &[&dyn ProtocolBackend] = &[ + &OpenSsl, + #[cfg(feature = "aws-lc")] + aws.backend(), + ]; + for component in 0..4 { + let mut private = [ + valid.d().to_owned().unwrap(), + valid.dmp1().unwrap().to_owned().unwrap(), + valid.dmq1().unwrap().to_owned().unwrap(), + valid.iqmp().unwrap().to_owned().unwrap(), + ]; + private[component].add_word(2).unwrap(); + let [ + private_exponent, + first_exponent, + second_exponent, + coefficient, + ] = private; + let malformed = Rsa::from_private_components( + valid.n().to_owned().unwrap(), + valid.e().to_owned().unwrap(), + private_exponent, + valid.p().unwrap().to_owned().unwrap(), + valid.q().unwrap().to_owned().unwrap(), + first_exponent, + second_exponent, + coefficient, + ) + .unwrap(); + assert!(!matches!(malformed.check_key(), Ok(true))); + let pkcs1 = malformed.private_key_to_der().unwrap(); + let traditional_pem = String::from_utf8(malformed.private_key_to_pem().unwrap()).unwrap(); + let native = PKey::from_rsa(malformed).unwrap(); + let der = native.private_key_to_pkcs8().unwrap(); + let pem = String::from_utf8(native.private_key_to_pem_pkcs8().unwrap()).unwrap(); + // Parsing succeeds; consistency validation must reject these keys. + assert!(PKey::private_key_from_pkcs8(&der).is_ok()); + assert!(PKey::private_key_from_pem(pem.as_bytes()).is_ok()); + for backend in backends { + for imported in [ + backend.import_keypair_der(&der), + backend.import_keypair_pem(&pem), + backend.import_keypair_pem(&traditional_pem), + ] { + assert!( + imported.is_err(), + "{} accepted corrupted RSA component {component}", + backend.capabilities().backend + ); + } + for encoding in [ + EncodingKey::from_rsa_der(&pkcs1), + EncodingKey::from_rsa_pem(pem.as_bytes()).unwrap(), + ] { + for algorithm in [ + Algorithm::RS256, + Algorithm::RS384, + Algorithm::RS512, + Algorithm::PS256, + Algorithm::PS384, + Algorithm::PS512, + ] { + let signer = (backend.jwt_provider().signer_factory)(&algorithm, &encoding); + // OpenSSL validates at construction; AWS-LC may defer it + // until signing. Neither may successfully use the key. + if backend.capabilities().backend == "openssl" { + assert!( + signer.is_err(), + "OpenSSL accepted corrupted RSA component {component} for {algorithm:?}" + ); + } else { + assert!( + !signer.is_ok_and(|signer| signer.try_sign(b"header.claims").is_ok()), + "AWS-LC signed with corrupted RSA component {component} for {algorithm:?}" + ); + } + } + } + } + } +}