Static musl release image, and fix JWT verification broken by #28 - #29
Merged
Conversation
The release image compiled against glibc from `rust:1` but ran on distroless/cc-debian12. `rust:1` now tracks Debian 13, so that pairing was one base-image bump away from a GLIBC_* load failure. Switch to a fully static musl binary on distroless/static-debian13, removing the glibc version-matching constraint entirely (~25MB -> 12.3MB). Linking statically forced OpenSSL out of the tree, which was worth doing anyway: reqwest pulled in both native-tls/OpenSSL (via default features) and rustls, so two TLS stacks were compiled in and only one was used. Now rustls-only. `http2` and `charset` are re-enabled explicitly because `default-features = false` silently drops them. Fix a live bug found along the way: jsonwebtoken 10 ships no crypto provider by default (`default = ["use_pem"]`), so decode() in Authenticator::decode panicked instead of verifying -- the core path of the service. Enable `rust_crypto`, which is also the pure-Rust backend that keeps the static build free of new C dependencies. CI never caught that because the sub-crates were not workspace members, so `cargo test` at the root only ran the root package's zero tests. Add a [workspace] table and pass --workspace in CI; the three existing sub-crate tests now actually run. That also collapses three independently resolved lockfiles into one and lets mockito -- a test HTTP server that was being compiled into the production binary -- move to [dev-dependencies]. Exclude .devbox from the Docker build context: it is 1.3GB and self-ignores via a nested .gitignore that .dockerignore does not honour, which hard-failed local image builds. Verified on a live system: aarch64 static binary (no interpreter, no GLIBC_ symbols, zero OpenSSL strings), runs on distroless/static, fetches the Access JWKS and the Cloudflare API over rustls using baked-in webpki roots, and rejects malformed and unsigned JWTs with 403 without panicking. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PR #28 bumped jsonwebtoken 9.3.0 -> 10 without selecting a crypto backend. v10 made that choice mandatory, so every decode() call panicked instead of verifying. 3fbb7b9 enabled `rust_crypto`; this commit addresses why the regression reached main unnoticed. Both authenticator tests asserted tautologies: assert_eq!(test.is_ok(), !test.is_err()); assert_eq!(test.is_err(), !test.is_ok()); Neither can fail for any Result. The "success" test could not have passed on its own terms regardless: its token expired 2025-01-29, and every audience in its expected list was the real value with the leading character truncated, so validation returned Err while the assertion still held. Replace them with six tests that mint a throwaway RSA keypair per run and sign tokens at test time, so no token ages into a false failure and no private key is committed. They cover the accept path plus expiry, audience mismatch, unknown kid and missing kid. Also stop taking the algorithm from the token header. Validation::new( header.alg) lets a caller nominate the algorithm their own signature is checked against. jsonwebtoken blocks cross-family swaps (RSA vs HMAC), so this was not exploitable, but a token signed RS384 against an RS256 JWKS was accepted. Use the algorithm the JWKS advertises instead. The regression test for this was confirmed to fail on the old line and pass on the new one. Drop the .unwrap() on DecodingKey::from_rsa_components, which could panic inside a request handler given a malformed JWKS. Point the placeholder devbox `test` script at the real suite and add a matching `build` script. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Clippy reported 10 warnings across the three crates; all were needless borrows, redundant returns or a redundant import. Now zero. Two of them removed real work rather than just noise: fetch_apps cloned `config.api` into a format! argument and cloned `config.token` for every bearer_auth call, so each outbound request allocated two throwaway Strings. Both are borrows now. Also un-ignore devbox.lock. It is 1.9K of nix pins with no secrets, and committing it makes `devbox run` resolve the same toolchain for everyone, same rationale as Cargo.lock. Without it the environment is only pinned as far as "stable". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Two problems, found while answering a question about whether the Dockerfile should target
cc-debian12orcc-debian13.Auth is broken on
main. #28 bumpedjsonwebtoken9.3.0 → 10. The only breaking change in v10 is that selecting a crypto backend became mandatory (rust_cryptooraws_lc_rs); neither was chosen, sodefault = ["use_pem"]left no provider and everydecode()call panicked instead of verifying. It fails closed (panic → 5xx → Traefik denies), so this is not a security hole, but authentication cannot succeed.The base images were mismatched. The binary compiled against glibc from
rust:1but ran ondistroless/cc-debian12.rust:1now tracks Debian 13, so that pairing was one base-image bump away from aGLIBC_*load failure.What changed
Release image
Switched to a fully static musl binary on
distroless/static-debian13, which removes the glibc version-matching constraint entirely rather than answering it. ~25 MB → 12.3 MB.Linking statically forced OpenSSL out of the tree, which was worth doing anyway:
reqwestwas pulling in bothnative-tls/OpenSSL (via default features) andrustls, so two TLS stacks were compiled in and only one was used. Now rustls-only.http2andcharsetare re-enabled explicitly becausedefault-features = falsesilently drops them.The upgrade
Enabled
rust_crypto— also the pure-Rust backend, which keeps the static build free of new C dependencies (aws_lc_rswould have added C + cmake).Why CI missed it
The sub-crates were never workspace members, so
cargo testat the root only ran the root package's zero tests. Verified: plaincargo testruns 1 binary,--workspaceruns 5. Added a[workspace]table and--workspacein CI — note a root that is both a package and a workspace still defaults to just the current package, so both changes are needed.That also collapses three independently-resolved lockfiles into one, and let
mockito— a test HTTP mock server that was being compiled into the production auth binary — move to[dev-dependencies].The tests were vacuous
Both authenticator tests asserted tautologies that cannot fail for any
Result:The "success" test could not have passed on its own terms regardless: its token expired 2025-01-29, and every audience in its expected list was the real value with the leading character truncated (
a4bef…→4bef…). So it returnedErrwhile the assertion still held.Replaced with six tests that mint a throwaway RSA keypair per run and sign tokens at test time — nothing ages into a false failure, and no private key is committed. Coverage: accept path, expiry, audience mismatch, unknown
kid, missingkid, and algorithm pinning. This also removed the old hardcoded JWT, which embedded a real email and a signature from a real account.One hardening fix
Validation::new(header.alg)took the algorithm from the attacker-controlled token header. Not exploitable — jsonwebtoken rejects cross-family swaps, so anHS256downgrade against an RSA key was already blocked. But a token signedRS384against anRS256JWKS was accepted, so a caller could still nominate the algorithm their own signature was checked against. Now useskey.algfrom the JWKS.The regression test for this was confirmed to fail on the old line and pass on the new one — a test that passes both ways is exactly the trap the old suite fell into.
Also dropped the
.unwrap()onDecodingKey::from_rsa_components, which could panic inside a request handler given a malformed JWKS.Verification
Built the image under podman and inspected the artifact:
statically linkedaarch64 ELF, no interpreterGLIBC_symbolsThen ran it against live Cloudflare credentials: it fetched the Access JWKS and 11 apps from
api.cloudflare.comover rustls, confirming musl DNS resolution and TLS with compiled-in trust anchors both work./authreturned 403 to a malformed JWT, a garbage JWT, and a structurally-valid JWT carrying a realkidwith a bogus signature — noCryptoProviderpanic.Also confirmed dropping an initial
CC=musl-gccline produced a byte-identical binary (same BuildID), so it was removed as redundant.Notes for the reviewer
.dockerignorewas letting 1.3 GB into the build context..devboxself-ignores via a nested.gitignore, which.dockerignoredoes not honour. This hard-failed local image builds; CI was unaffected since.devboxdoesn't exist on runners.buildjob pullsrust:1unauthenticated and was exposed to Docker Hub rate limits. That was wrong.buildcompiles natively on the runner and pulls no image at all, and the release-onlydockerjob runsdocker/login-actionbeforebuild-push-action, so itsFROM rust:1pull is authenticated. Thetoomanyrequestsfailure I hit was purely local (no Docker Hub credentials on my machine) and does not apply to CI.mimallocwas therefore not added — no measured justification.cloudflare-dynamic-configandsrc/main.rs). Two were not stylistic:fetch_appsclonedconfig.apiandconfig.tokenon every outbound request.exp— standard clock-skew tolerance, left as-is but now documented in the test rather than being a surprise.🤖 Generated with Claude Code