fix(mtls): fail-open accepted any certificate, and path RBAC denied everyone - #20
Merged
Merged
Conversation
docs/MTLS.md still called enforcement per-route and listed per-path selectivity as a limitation, which read as though per-route were the working baseline. It never was: a TLS connection policy is chosen on SNI, before any path exists. The document now describes the per-SNI merge, what happens when two routes on one hostname disagree, the force_https derivation and migration 00145, and the three write paths that refuse client-cert enforcement with SSL off. README repeated the same per-route claim in two places and never mentioned the preshared keys or post-quantum-only TLS at all. MULTI_NODE gains the HA peer-group rotation note - the whole group rotates now, so the customer has to re-download their .conf - and names supports_psk in the join flow. Also: concurrency groups on both workflows. Three commits in five minutes queued three full multi-arch builds and cancelled none of them. A tag run is exempt, since that is the one that publishes the release images.
…veryone
Three defects, all older than 1.5.0, found while correcting the docs.
Fail-open emitted Caddy's "request" mode, which asks for a client certificate
and never validates it, while both the documentation and the settings help
text said "verify_if_given". Since the subject of that certificate is
forwarded on as X-Mtls-Subject, fail-open did not mean "clients without a
certificate are allowed" - it meant any client could claim any identity. It
now uses verify_if_given: no certificate still passes, a presented one must
chain to the CA.
X-Mtls-Subject was only ever set, never stripped, and only on routes with path
rules - so on every other route a client-supplied header reached the upstream
indistinguishable from a real one. Every route now deletes it first.
Path RBAC compared Caddy's rendered DN ("CN=device-42") against the bare
common name issuance stores, so it matched nothing and 403'd every covered
path, legitimate holders included. The check now accepts either form. The old
test passed only because its fixture seeded a subject the issue path cannot
produce; the new test seeds what mtls.Issue actually writes and fails on the
pre-fix code.
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.
Security remediation. Details withheld until operators have had time to upgrade; see the CHANGELOG for impact and upgrade notes.