Add a QUIC transport (feature-gated)#110
Draft
plaidfinch wants to merge 18 commits into
Draft
Conversation
The server parsed the client's version message with version_bytes[..4], which panics on a body shorter than 4 bytes — a remotely triggerable panic for any TLS-authenticated peer, and process death under panic = "abort". Parse with a checked conversion and reject anything but exactly 4 bytes as Error::ProtocolVersion. This is stricter than the old code in one way: a version message longer than 4 bytes was previously accepted with the trailing bytes ignored. No client has ever sent one (both protocol versions send exactly CURRENT_PROTOCOL_VERSION.to_le_bytes()), so no interoperability change.
recv_msg allocated the full message buffer from the peer-controlled length prefix before reading any of the body, so a TLS-authenticated (but not yet attested) peer could demand a 4 GiB zeroed allocation with a 4-byte prefix, and hold it by leaving the connection open — repeatable across connections for memory exhaustion. Authenticated-but-unattested is exactly the adversary class the attestation layer exists to screen out, so the framing layer must not trust it with allocation sizes. Bound messages at a named MAX_MSG_SIZE (1 MiB), with compile-time asserts that the bound admits the largest legitimate messages (the hubpacked measurement log and attestation). Oversized lengths are rejected as a new Error::MessageTooLarge before any allocation. No interoperability change: every legitimate protocol message is far below the bound.
cargo clippy --all-targets -- -D warnings fails on illumos (where the cfg(target_os) build-script lines and the unittest build path compile) on lints the ubuntu CI run never sees: uninlined format args in build.rs and a needless Ok(..?) wrapper, plus roots.len() < 1 in both examples. Inline the format args, drop the wrapper, and use is_empty(). No behavior change.
Factor the four rustls config builders down to two transport-free
functions and dedupe the peer-PlatformId extraction, in preparation for
adding a second transport (QUIC) that will consume the same
configuration and identity plumbing.
- Add tls/src/config.rs with pub(crate) load_roots,
new_tls_client_config, and new_tls_server_config. The old
new_tls_{local,ipcc}_{client,server}_config builders differed only in
the ResolveSetting passed to CertResolver::new, so they collapse to
one client and one server builder parameterized by ResolveSetting.
- Add platform_id_from_tls_certs in lib.rs, replacing the two
hand-rolled peer-certificate-chain walks in client.rs and server.rs.
It takes rustls' Option<&[CertificateDer]>, so it serves both TCP's
peer_certificates() and (later) QUIC's peer_identity().
- Client::connect and Server::new no longer load the root PEMs twice.
A missing or malformed root now surfaces as Error::FailedRead{path}
via load_root_cert, rather than a bare Error::Io.
- Correct the stale "use ring; aws_lc doesn't compile on illumos"
comment on crypto_provider(): the crate uses aws-lc-rs, and it does
build and run on illumos (verified natively).
No change to TLS wire behavior, the crypto provider, the verifiers, or
the attestation protocol. TCP tests pass on macOS and natively on
illumos.
Factor the post-handshake attestation exchange out of client.rs and server.rs into a new attest module, in preparation for running the same protocol over a QUIC bidirectional stream as well as TCP. - Add tls/src/attest.rs holding the framing (send_msg/recv_msg with its MAX_MSG_SIZE bound), the DER cert-chain helpers (certs_to_der/certs_from_der), the protocol version constants and result aliases, and corims_from_paths, all moved from lib.rs and server.rs. - Extract client_exchange and server_exchange, each generic over T: AsyncRead + AsyncWrite + Unpin. They take the peer PlatformId already derived from the TLS handshake certificates and return the peer's attested PlatformId plus whether its measurements appraised. The bodies are the former inline exchanges verbatim, including the checked version-message parse and preserving the wire-visible asymmetries: the server sends its own attestation last (and never, under Enforced, after a failed appraisal), the client loads its corims inline at the end of the exchange, and the server loads its corpus before accepting the TLS connection so a malformed corpus fails early. - Client::connect_with_config and SprocketsAcceptor::handshake shrink to TLS setup, peer-PlatformId extraction, and a call into attest. No change to the attestation protocol bytes, the TLS configuration, or the crypto provider. Verified on illumos: clippy --all-targets clean and the TCP test suite passes.
Gate the forthcoming QUIC transport behind a quic feature that pulls in quinn as an optional dependency. Nothing uses quinn yet; this is the additive packaging step so the module can land next without touching the no-feature build. - Add quinn to the workspace dependencies with the tokio runtime and the aws-lc-rs rustls backend, matching the crypto provider sprockets already uses. - Add quinn as an optional dependency of sprockets-tls behind quic = ["dep:quinn"]. The Cargo.lock update is additions-only: no existing crate's resolved version changes, and `cargo tree -p sprockets-tls` without the feature still resolves zero quinn crates. Verified on illumos that both the no-feature build and the --features quic build compile natively with the aws-lc-rs backend (quinn, quinn-proto, and quinn-udp's solarish path).
Add a feature-gated quic module carrying the same mutually-authenticated, RoT-attested channel as the TCP API over QUIC. The transport reuses the crate's rustls config construction and the transport-generic attestation exchange unchanged; only the framing underneath differs. - quic.rs: the Endpoint primitive (one UDP socket that both connects and accepts), the Acceptor, the close_code table, and the quinn config assembly. The TLS config is the same one the TCP path builds, plus ALPN b"sprockets", disabled resumption/0-RTT, and a QuicClientConfig / QuicServerConfig wrapping it with the RFC 9001 AES-128-GCM Initial suite (the handshake still offers only ChaCha20-Poly1305). Transport policy (idle timeout, keep-alive) and server hardening (migration off, bounded pre-auth incoming, Retry-based address validation) are named constants. - quic/stream.rs: BiStream, which rejoins quinn's split send/recv halves into one AsyncRead + AsyncWrite duplex, and AttestedConnection, the QUIC analog of Stream<T> whose attested identity covers every stream opened on the connection. - Four #[cfg(feature = "quic")] Error variants for the QUIC failure modes. The handshake awaits the full connection future, never quinn's into_0rtt path, so a connection is surfaced only after the client's certificate is verified. Verified on illumos: builds, clippy --all-targets, and rustdoc (public and private) are clean with the feature, and the TCP suite passes with and without it.
Thin facades over the quic Endpoint that mirror the TCP Client and Server APIs, for callers that only dial or only listen. - quic::Client::connect binds an ephemeral endpoint on [::]:0, runs the attested handshake, and returns the connection. The endpoint handle is then dropped; quinn keeps its I/O driver alive while the returned connection lives (the EndpointDriver only terminates once no endpoint handles and no connections remain), so the connection stays usable. Callers that dial repeatedly or also listen should share one Endpoint. - quic::Server wraps a listening Endpoint with new / listen_addr / accept, plus an endpoint() accessor for callers that also need to dial or close. Verified on illumos: clippy --all-targets, rustdoc (public and private), and the TCP suite are clean with the feature.
Twelve tests for the quic module, reusing the crate's test PKI and the shared helpers in crate::tests (local_config is made pub for this). Transport and exchange wiring: - basic, no_corpus: enforcing and permissive handshakes with a message echo - unattested_client: valid TLS but no attestation exchange is rejected - spawn_accept: several concurrent clients on one endpoint - multi_stream: a second stream inherits the connection's attested identity - appraisal_failure_close_code: the client observes the APPRAISAL close code - client_auth_required: a client with no certificate is rejected Security pins: - platform_id_mismatch: a peer whose TLS and attestation chains disagree on PlatformId is rejected, and the client observes PLATFORM_ID_MISMATCH - session_cipher_pinned_to_chacha20: a client offering AES-GCM session suites is rejected (the RFC 9001 AES-128-GCM Initial suite does not satisfy the ChaCha20-only session pin) - wrong_alpn_rejected: a client offering the wrong ALPN token is rejected Robustness: - large_payload: 1 MiB round-trips intact both directions, exercising QUIC stream flow control and the BiStream duplex - initial_suite_is_aes_128_gcm: unit test pinning the Initial suite Two behaviors surfaced while writing these: dropping a connection handle before the peer has read closes it with code 0 and truncates undelivered data (so the tests keep connections alive until reads complete, matching the module's delivery-assurance note), and the two test configs provision distinct platforms. Verified on illumos: clippy --all-targets and the full suite pass with and without the feature.
The QUIC analogs of the client and server examples: same CLI and stdin/echo behavior, over a sprockets QUIC connection instead of TCP. AttestedConnection's AsyncRead/AsyncWrite impls let tokio::io::split and copy work as with Stream<TcpStream>; quic::Server::new is synchronous. The shutdown flow deliberately differs from the TCP examples, demonstrating the drop semantics from the quic module docs: the client drains the echo to stream EOF after stdin ends (exiting on stdin close would drop the connection and could truncate the in-flight echo), and the server finishes its stream then holds the connection until the client closes, treating a client that departs by connection close as the end of that connection rather than a fatal error. Both examples are gated behind required-features = ["quic"], so a no-feature --all-targets build skips them and Cargo.lock is unchanged. Verified on illumos: clippy --all-targets clean with the feature, and the example pair run against each other over loopback: two sequential clients, full echoes with no timing dependence, server alive throughout.
Add a features matrix to the clippy and build-and-test jobs so both the default (quic off) and --features quic configurations are linted, built, and tested on every run. check-style is left single because rustfmt is feature-independent, and clippy keeps its existing target scope: --all-targets would enable the unittest feature whose build.rs needs attest-mock, which only the build-and-test job installs (and which already builds all targets and runs the tests in both configurations).
Previously only attestation-exchange failures closed the connection with a close code from close_code; failures between connection establishment and the exchange (peer-identity extraction, opening or accepting the primary stream) returned early and let the dropped connection close with code 0, indistinguishable from a graceful close at the peer. Split the post-establishment work into client_handshake/server_handshake helpers so Endpoint::connect and Acceptor::handshake close with the mapped code on any failure past establishment.
The close reason previously carried the rendered error message, which for local failures can include detail the peer has no need for (file paths, configuration specifics). Close with a short static reason naming only the failure class. Hubpack decode failures, DER parse failures, and oversized length prefixes during the exchange arise from peer-sent bytes (the encode directions cannot fail on our own, already-validated data), so map them to PROTOCOL / ATTESTATION / PROTOCOL respectively instead of letting them fall through to LOCAL_ERROR.
Client::connect went through Endpoint::new, which always installs a server configuration, so the facade's ephemeral socket would field and buffer inbound QUIC connection attempts it could never accept. Bind via quinn::Endpoint::client instead: no server configuration, no listening surface. Endpoint::new is unchanged for callers that want the dual-role socket.
A second shutdown().await on a BiStream errors (quinn's finish rejects an already-finished stream), unlike the TCP-backed Stream; state this on the type since BiStream is pitched as a drop-in duplex. Also note why the retry() result in Endpoint::accept is safe to discard.
QUIC-side pins for the two attest-protocol hardening fixes, mirroring the TCP tests: a raw quinn client with valid credentials sends a version message shorter than 4 bytes (must be rejected as ProtocolVersion, not a panic) and a length prefix claiming a 4 GiB message (must be rejected as MessageTooLarge on the prefix alone, with the client observing the PROTOCOL close code).
A reader landing on the crate root had no way to discover the feature-gated quic module. Plain code span rather than an intra-doc link: the module is conditionally compiled, so a link would break the no-feature doc build.
The examples are part of the crate's contract, and running them caught a real bug (the server panicking when a client departed), so pin them with an end-to-end integration test: spawn server_quic on [::1]:0, read the announced address, run client_quic twice, and assert each client's stdin comes back complete on stdout and that the server outlives its clients. server_quic now announces its bound address on stdout: with port 0 that is the only way any caller, interactive or test, can learn the OS-assigned port. Cargo provides no CARGO_BIN_EXE_<name> for examples, so the test locates the binaries relative to the test executable; every wait is bounded so a wedged process fails the suite rather than hanging it.
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
1 participant
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.
CAVEAT LECTOR: This PR was largely put together by Claude Fable 5, and is an experiment that deserves more discussion before merging.
Summary
Adds a QUIC transport to sprockets-tls as a purely additive, default-off cargo feature (
quic). QUIC connections carry the exact same mutually-authenticated, RoT-attested channel as the TCP API: same rustls configs, same crypto-provider pins (X25519, ChaCha20-Poly1305, TLS 1.3 only), sameRotCertVerifier/CertResolver, same attestation protocol bytes over a QUIC bidirectional stream, same PlatformId binding. No existing signature changes; the no-feature build resolves an unchanged dependency graph (cargo tree | grep -c quinn= 0).The core primitive is
quic::Endpoint— one UDP socket that both dials and listens (QUIC-natural for the trust-quorum full mesh) — plus thinquic::Client/quic::Serverfacades mirroring the TCP API. See thequicmodule docs for the full security model, close-code table, and liveness/drop semantics.Stacked on
The first three commits are stand-ins for #107, #108, #109 (patch-identical). Once those squash-merge, a plain
git rebase maindrops them automatically by patch-id. Recommendation: review primarily the commits from "Extract pure TLS-config construction…" onward.Design decisions
Endpointas the core +Client/Serverfacades.pub use quinnre-export so consumers get version-matched types (semver-lockstep obligation documented in module docs).Security posture (including mitigations for differences from TCP)
QuicClientConfig::with_initial. Pinned by two tests.b"sprockets"(frozen wire constant, not versioned), with strict matching.migration(false)to bind the connection to the initial address, as TLS is inherently bound.MAX_INCOMING= 256, 10 MiB pre-auth buffer (vs quinn's 65,536 / 100 MB defaults).Connectingfuture (regression-tested: a certificate-less client never reachesaccept_bi).Test coverage
12 QUIC integration tests (all invariants doc-commented): basic attested echo, permissive/no-corpus, unattested client rejected, malformed/oversized protocol messages rejected (close-code observed), concurrent accepts, multi-stream identity inheritance, appraisal-failure close code, mandatory client auth, PlatformId mismatch rejected, wrong ALPN rejected, AES-session-suite offer rejected, 1 MiB payload round-trip, Initial-suite unit pin.
Plus an end-to-end test of the example pair itself (
tls/tests/quic_examples.rs): spawns theserver_quic/client_quicbinaries as separate processes over loopback, asserts complete echoes for two sequential clients and that the server outlives them (regression for a close-semantics panic the first manual run of the examples caught).Verification
--all-targetsclean with both feature configs; 8/8 TCP tests, 22/22 withquic.--document-private-itemspasses.cargo check --all-targets, bisectable).Open questions for discussion
MAX_IDLE_TIMEOUT30 s /KEEP_ALIVE_INTERVAL10 s: this is sized around trust-quorum's 1 s pings and 10 s inactivity timeout; needs sign-off from trust-quorum owners.MAX_INCOMING256,INCOMING_BUFFER_SIZE_TOTAL10 MiB: is this permissive enough for the worst-cases on a 32-sled rack?