diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index eeea76f..7fc2072 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -33,9 +33,10 @@ jobs: fail-fast: false matrix: backend: - # The jsonwebtoken crypto backends are mutually exclusive (enabling - # both panics at runtime), so each is exercised on its own leg instead - # of via `--all-features`. + # One leg per crypto backend, plus the build that links both: Cargo + # features are additive, so feature unification (and every tool that + # reaches for `--all-features`, docs.rs and cargo-semver-checks + # included) can produce that combination. - name: rust_crypto # Pure-Rust default backend (RustCrypto / rsa). flags: "--features redis" @@ -49,6 +50,10 @@ jobs: # `builtin_jwt` gate reaches for jsonwebtoken — this leg is what # keeps that honest. flags: "--no-default-features --features redis" + - name: all_features + # Both backends at once, the build cargo-semver-checks and docs.rs + # take. `aws_lc_rs` wins the tie; this leg keeps that wiring honest. + flags: "--all-features" steps: - uses: actions/checkout@v7 with: @@ -59,6 +64,12 @@ jobs: components: clippy, rustfmt # Third-party action: pinned to a commit SHA (dependabot keeps it fresh). - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2 + # Runs each test in its own process, so a hang names the test that hung + # instead of timing the whole job out. Third-party action: pinned to a + # commit SHA (dependabot keeps it fresh). + - uses: taiki-e/install-action@94c31af3204a9f15ab40b35ad084410b905bbc73 # v2 + with: + tool: cargo-nextest - name: Format if: matrix.backend.name == 'rust_crypto' @@ -74,7 +85,11 @@ jobs: env: # Exercises the rate-limit reconciliation against the Redis service. SHIELD_REDIS_TEST_URL: redis://127.0.0.1:6379/ - run: cargo test ${{ matrix.backend.flags }} + run: cargo nextest run ${{ matrix.backend.flags }} + + # nextest does not run doctests; cargo does. + - name: Doc tests + run: cargo test --doc ${{ matrix.backend.flags }} - name: Publish dry-run if: matrix.backend.name == 'rust_crypto' diff --git a/Cargo.toml b/Cargo.toml index 466fce1..70500b0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -114,11 +114,11 @@ default = ["rust_crypto"] # `ProxyServer::with_token_verifier`. builtin_jwt = ["dep:jsonwebtoken"] -# Crypto backend for the BUILT-IN verifier. Mutually exclusive: enable at most -# one. jsonwebtoken picks its provider from these features and panics at runtime -# if both are on, so do NOT build with `--all-features`. They no longer decide -# for the whole dependency graph: a consumer that cannot accept this crate's -# choice supplies its own verifier instead (see `builtin_jwt` above). +# Crypto backend for the BUILT-IN verifier. Additive, like every Cargo feature: +# with both on, jsonwebtoken cannot infer a provider, so the built-in verifier +# installs `aws_lc_rs` (constant-time, advisory-free) for the process. They no +# longer decide for the whole dependency graph: a consumer that cannot accept +# this crate's choice supplies its own verifier instead (see `builtin_jwt`). # # `rust_crypto` (default): pure-Rust RustCrypto backend. Pulls in `rsa`, which # carries RUSTSEC-2023-0071 (Marvin Attack). Not exploitable on our verify-only diff --git a/README.md b/README.md index feb1408..1d51b8e 100644 --- a/README.md +++ b/README.md @@ -339,8 +339,26 @@ from `auth.jwt` (an Ed25519 PEM file or a JWKS endpoint), verified with | `rust_crypto` (default) | RustCrypto | Pure Rust. Pulls in `rsa`, which carries [RUSTSEC-2023-0071](https://rustsec.org/advisories/RUSTSEC-2023-0071); the Marvin attack targets private-key timing, and this path only verifies with public keys (see `deny.toml`). | | `aws_lc_rs` | aws-lc | Constant-time / FIPS-capable, advisory-free, links aws-lc through C FFI. | -They are mutually exclusive, and enabling both is a compile error rather than a -runtime panic. Do **not** build with `--all-features`. +Both can be linked at once, as with any pair of Cargo features. `jsonwebtoken` +cannot infer a provider then, so `ProxyServer::from_config` installs `aws_lc_rs` +for the process: it is constant-time and carries no advisory. A build that wants +RustCrypto regardless drops the `aws_lc_rs` feature. + +If another crate in your process uses `jsonwebtoken` too, it may reach it before +any proxy server is built, and would hit the same ambiguity. It can also turn on +the other backend for `jsonwebtoken` directly, which leaves this crate looking +single-backend while `jsonwebtoken` sees two. Settle it once at the top of +`main`: + +```rust +# fn main() { +structured_proxy::install_default_crypto_provider(); +# } +``` + +The call is idempotent, and installs the backend this crate was built with. It +exists wherever the built-in verifier does, so a `default-features = false` +build with an injected verifier neither has it nor needs it. **An injected verifier** is what you supply when neither of those is the right answer for your binary: a validated / FIPS crypto module, an HSM, or a verifier @@ -373,9 +391,9 @@ ProxyServer::from_config(config) Injection also resolves a problem the features cannot: Cargo unifies features across the whole dependency graph, so `rust_crypto` / `aws_lc_rs` is a property of the *resolution*, not of a binary. Two crates in one workspace that link this -one and want different backends cannot both get their way — and if both features -end up enabled, `jsonwebtoken` refuses the combination. A consumer that injects -its own verifier is not in that argument at all: it takes +one and want different backends cannot both get their way: the resolution enables +both features, and the tie-break above picks `aws_lc_rs` for everyone. A consumer +that injects its own verifier is not in that argument at all: it takes ```toml [dependencies] diff --git a/release-plz.toml b/release-plz.toml index 2162bf7..de870b0 100644 --- a/release-plz.toml +++ b/release-plz.toml @@ -1,5 +1,10 @@ [[package]] name = "structured-proxy" +# cargo-semver-checks builds the baseline from the registry, and every version +# published so far rejects the feature set it uses (both JWT backends at once). +# The published artefacts cannot be changed, so the check stays off until a +# release carrying that fix becomes the baseline; see issue #84. +semver_check = false # Preserve the existing tag scheme (v1.0.0, v1.0.1, ...) instead of the # release-plz default of "{package}-v{version}", so tag history stays continuous. git_tag_name = "v{{ version }}" diff --git a/src/auth/crypto.rs b/src/auth/crypto.rs new file mode 100644 index 0000000..be3728a --- /dev/null +++ b/src/auth/crypto.rs @@ -0,0 +1,58 @@ +//! Process-wide `jsonwebtoken` crypto provider selection. +//! +//! `jsonwebtoken` infers its provider from its own two backend features, and +//! with both on it cannot: it falls back to a provider that panics on first +//! use. Cargo features being additive, that combination arrives on its own. +//! Either this crate's `rust_crypto` and `aws_lc_rs` are both enabled (two +//! dependents asking for different backends, or `--all-features`, which is what +//! docs.rs and `cargo-semver-checks` use), or one of ours is enabled while +//! another crate in the graph turns on the other `jsonwebtoken` feature +//! directly. The second case looks single-backend from here, so the provider is +//! installed explicitly whichever backend this crate compiled with. + +#[cfg(test)] +mod tests; + +/// The provider this crate installs. +/// +/// `aws_lc_rs` wins whenever it is compiled in: it is constant-time and +/// advisory-free, while `rust_crypto` pulls in `rsa` (RUSTSEC-2023-0071). +#[cfg(feature = "aws_lc_rs")] +pub(crate) fn preferred_provider() -> &'static jsonwebtoken::crypto::CryptoProvider { + &jsonwebtoken::crypto::aws_lc::DEFAULT_PROVIDER +} + +/// The provider this crate installs: RustCrypto, the only backend this build +/// compiled with. +#[cfg(all(feature = "rust_crypto", not(feature = "aws_lc_rs")))] +pub(crate) fn preferred_provider() -> &'static jsonwebtoken::crypto::CryptoProvider { + &jsonwebtoken::crypto::rust_crypto::DEFAULT_PROVIDER +} + +/// Select the `jsonwebtoken` crypto provider for this process. +/// +/// Call it once at startup, before anything in the process signs or verifies a +/// JWT. It is idempotent, and installs the backend this crate was built with, +/// so `jsonwebtoken` never has to infer one. +/// +/// A plain [`ProxyServer`](crate::ProxyServer) deployment needs no call: the +/// server does this while it is being built. It is public for the case the +/// server cannot cover, which is also how the ambiguity arises in the first +/// place: another crate in the graph uses `jsonwebtoken` too and may reach it +/// first. Call this at the top of `main` and every consumer is covered, +/// whichever runs first. +/// +/// With both backends linked the choice is `aws_lc_rs`: constant-time, and free +/// of the `rsa` advisory `rust_crypto` carries. A process that wants a different +/// one installs it through +/// [`CryptoProvider::install_default`](jsonwebtoken::crypto::CryptoProvider::install_default) +/// before calling this, and the earlier choice stands. +pub fn install_default_crypto_provider() { + if preferred_provider().install_default().is_err() { + // Something installed a provider before us: an embedder that made its + // own choice, or an earlier call here. Either way it stands: the + // process gets one provider, and the first explicit choice is the one + // the caller meant. + tracing::debug!("jsonwebtoken crypto provider already installed; keeping it"); + } +} diff --git a/src/auth/crypto/tests.rs b/src/auth/crypto/tests.rs new file mode 100644 index 0000000..56c4f01 --- /dev/null +++ b/src/auth/crypto/tests.rs @@ -0,0 +1,53 @@ +/// The selection itself, read before anything installs a provider. Verifying a +/// token proves only that *some* provider is installed (both backends do +/// EdDSA), so the choice is asserted here at its own seam. +/// +/// With both backends linked it must be `aws_lc_rs`, the constant-time and +/// advisory-free one. +#[cfg(all(feature = "rust_crypto", feature = "aws_lc_rs"))] +#[test] +fn tie_break_selects_aws_lc_rs() { + let selected = super::preferred_provider(); + assert!( + std::ptr::eq(selected, &jsonwebtoken::crypto::aws_lc::DEFAULT_PROVIDER), + "the tie-break must select the aws_lc_rs provider" + ); + assert!( + !std::ptr::eq( + selected, + &jsonwebtoken::crypto::rust_crypto::DEFAULT_PROVIDER + ), + "the tie-break must not fall back to RustCrypto" + ); +} + +/// A single-backend build installs that backend rather than leaving +/// `jsonwebtoken` to infer it: another crate in the graph may have turned on the +/// other `jsonwebtoken` feature directly, which is invisible from here and makes +/// inference ambiguous. +#[cfg(all(feature = "aws_lc_rs", not(feature = "rust_crypto")))] +#[test] +fn aws_lc_rs_only_build_installs_aws_lc_rs() { + assert!(std::ptr::eq( + super::preferred_provider(), + &jsonwebtoken::crypto::aws_lc::DEFAULT_PROVIDER + )); +} + +/// The same for the default build: RustCrypto is named explicitly, not inferred. +#[cfg(all(feature = "rust_crypto", not(feature = "aws_lc_rs")))] +#[test] +fn rust_crypto_only_build_installs_rust_crypto() { + assert!(std::ptr::eq( + super::preferred_provider(), + &jsonwebtoken::crypto::rust_crypto::DEFAULT_PROVIDER + )); +} + +/// Installing twice is not an error the caller has to think about: the first +/// choice stands and the second call is a no-op. +#[test] +fn installing_twice_is_harmless() { + super::install_default_crypto_provider(); + super::install_default_crypto_provider(); +} diff --git a/src/auth/mod.rs b/src/auth/mod.rs index fe680ab..9c0bad7 100644 --- a/src/auth/mod.rs +++ b/src/auth/mod.rs @@ -12,6 +12,8 @@ //! which one verified the token. pub mod authz; +#[cfg(feature = "builtin_jwt")] +pub mod crypto; pub mod forward; #[cfg(feature = "builtin_jwt")] pub mod jwks; diff --git a/src/auth/tests.rs b/src/auth/tests.rs index fd9a0c7..c6189ba 100644 --- a/src/auth/tests.rs +++ b/src/auth/tests.rs @@ -471,4 +471,28 @@ mod builtin { }; assert!(err.contains("jwks_uri"), "unexpected error: {err}"); } + + /// A build with both crypto backends linked (feature unification, or + /// `--all-features`) must verify tokens like any other. `jsonwebtoken` can + /// then not pick a provider from its own features and installs one that + /// panics on first use, so the verifier has to select one itself. + #[cfg(all(feature = "rust_crypto", feature = "aws_lc_rs"))] + #[tokio::test] + async fn verifies_with_both_crypto_backends_linked() { + let app = app(auth_with_policy(&["admin"])); + let token = sign(serde_json::json!({ + "iss": "test-iss", "aud": "test-aud", "exp": future_exp(), + "sub": "user-42", "roles": ["admin"] + })); + let resp = app + .oneshot( + HttpRequest::get("/secure") + .header("authorization", format!("Bearer {token}")) + .body(axum::body::Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(resp.status(), 200); + } } diff --git a/src/auth/verifier.rs b/src/auth/verifier.rs index d4f98b0..2685acf 100644 --- a/src/auth/verifier.rs +++ b/src/auth/verifier.rs @@ -36,6 +36,11 @@ impl ConfigVerifier { /// Returns an error string when no key source is configured, the PEM file /// cannot be read, or it is not a valid Ed25519 public key. pub(crate) fn build(jwt: &JwtConfig) -> Result { + // Normally settled in `ProxyServer::from_config` already; repeated here + // because a verifier can also be built without going through the + // server, and the call costs one atomic once the provider is in place. + super::crypto::install_default_crypto_provider(); + let keys = if let Some(uri) = &jwt.jwks_uri { KeySource::Jwks(JwksCache::new(uri.clone())) } else if let Some(pem_path) = &jwt.public_key_pem_file { diff --git a/src/lib.rs b/src/lib.rs index e04307d..2c54afc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -17,8 +17,14 @@ //! //! - the **built-in** verifier (keys from `auth.jwt`), whose crypto backend is //! picked by a feature: `rust_crypto` (default, pure Rust) or `aws_lc_rs` -//! (opt-in, constant-time / FIPS-capable, links aws-lc via C FFI). They are -//! mutually exclusive; enabling both is rejected at compile time below. +//! (opt-in, constant-time / FIPS-capable, links aws-lc via C FFI). Both may be +//! compiled in at once — Cargo features are additive, so a dependency graph +//! with two dependents asking for different backends unifies into exactly that +//! build. `aws_lc_rs` then wins: it is constant-time and free of the `rsa` +//! advisory `rust_crypto` carries. [`ProxyServer::from_config`] settles that +//! choice for the process; a process where another crate may reach +//! `jsonwebtoken` before any server exists calls +//! [`install_default_crypto_provider`] from `main` instead. //! - an **injected** one, supplied by the embedder through //! [`ProxyServer::with_token_verifier`]. Since Cargo unifies features across a //! whole dependency graph, a backend feature cannot be chosen per binary — @@ -26,11 +32,6 @@ //! deciding for everyone else who links this crate. Such a build takes //! `default-features = false` and links no JWT crypto at all. -// jsonwebtoken selects its provider from these features and would otherwise -// panic at runtime on an invalid combination; turn that into a build error. -#[cfg(all(feature = "rust_crypto", feature = "aws_lc_rs"))] -compile_error!("features `rust_crypto` and `aws_lc_rs` are mutually exclusive; enable at most one"); - // `builtin_jwt` is implied by each backend and never meant to stand alone: on // its own it would link jsonwebtoken with no provider, which panics at runtime. #[cfg(all( @@ -52,6 +53,11 @@ pub mod shield; mod tls; pub mod transcode; +/// Settle the process-wide JWT crypto provider. See +/// [`install_default_crypto_provider`] for when a call is needed. +#[cfg(feature = "builtin_jwt")] +pub use auth::crypto::install_default_crypto_provider; + use axum::extract::State; use axum::http::{Request, StatusCode}; use axum::middleware::Next; @@ -113,6 +119,13 @@ pub struct ProxyServer { impl ProxyServer { /// Create from YAML config file. pub fn from_config(config: ProxyConfig) -> Self { + // Earliest point this crate owns: settle the JWT crypto provider here, + // long before the first token arrives. A process whose other crates + // reach jsonwebtoken before any server exists calls + // `install_default_crypto_provider` from `main` instead. + #[cfg(feature = "builtin_jwt")] + auth::crypto::install_default_crypto_provider(); + Self { config, descriptor_pool: None,