web: reject a client CA that holds no certificates - #438
Open
mrueg wants to merge 1 commit into
Open
Conversation
x509.CertPool.AppendCertsFromPEM reports whether it parsed any certificate at all, and both call sites discarded it. A client_ca_file that exists but holds no usable PEM - a truncated copy, a DER encoded file, a path pointing at the wrong artifact - was accepted, and produced a pool with no subjects. The same applied to an inline client_ca. Under RequireAndVerifyClientCert that fails closed, so it is not an authentication bypass, but the operator gets a server that rejects every client with an opaque handshake error while Validate reports the configuration as good. Reject it at configuration time instead, naming the file so the cause is obvious. The emptiness check runs after the client auth policy has been validated. Two existing fixtures pin the ordering from opposite directions: one configures a readable but empty CA with no policy and expects the policy error, the other configures a missing CA file with no policy and expects the read error. Reporting emptiness last keeps both reporting what they did before, and reports the more fundamental fault first when a configuration carries several. The existing web_config_auth_clientCAs_invalid.bad.yml fixture only covers a missing file, which is why this went unnoticed, so add fixtures for a file that parses to nothing and for the inline equivalent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Manuel Rüger <manuel@rueg.eu>
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.
x509.CertPool.AppendCertsFromPEMreports whether it parsed any certificate at all, and both call sites discard it. Aclient_ca_filethat exists but holds no usable PEM — a truncated copy, a DER-encoded file, a path pointing at the wrong artifact — is accepted and produces a pool with no subjects. The same applies to an inlineclient_ca.Under
RequireAndVerifyClientCertthis fails closed, so it is not an authentication bypass. But the operator gets a server that rejects every client with an opaque handshake error, whileValidatereports the configuration as good.Reproduced against master: a CA file containing the literal text
this is definitely not a PEM certificatewas accepted, and the resulting pool had 0 subjects.Ordering
The emptiness check runs after the client auth policy has been validated. Two existing fixtures pin the ordering from opposite directions — one configures a readable but empty CA with no policy and expects the policy error, the other configures a missing CA file with no policy and expects the read error. Reporting emptiness last keeps both reporting what they did before.
Tests
web_config_auth_clientCAs_invalid.bad.ymlonly covers a missing file, which is why this went unnoticed. Added fixtures for a file that parses to nothing and for the inline equivalent; both fail against master.🤖 Generated with Claude Code