Repository navigation
fix(tap): only match allowed names against the verified client cert - #15743
Open
musthaf-bughunter wants to merge 1 commit into
Open
musthaf-bughunter wants to merge 1 commit into
musthaf-bughunter wants to merge 1 commit into
Conversation
The tap APIServer checks `requestheader-allowed-names` against every certificate in `req.TLS.PeerCertificates`. Only the first one, the leaf, is verified against `requestheader-client-ca-file`; the rest are whatever the client chose to send after it. A client holding any certificate signed by that CA can append a self-signed certificate with CN `front-proxy-client` (or any other allowed name) and pass `validate()`. From there it controls `X-Remote-User` and `X-Remote-Group`, so it can tap any workload as any user, for example `system:masters`. This matters where the requestheader CA also issues other client certificates, which is the case `requestheader-allowed-names` exists to guard against. Match the allowed names against `PeerCertificates[0]` only. With `RequireAndVerifyClientCert` that is the certificate Go verified against the client CA, and it is the same certificate the Kubernetes requestheader authenticator checks. Validation: added `TestValidate_IgnoresUnverifiedPeerCertificates`, which does a real TLS handshake with a CA-signed leaf (CN=name-untrusted) followed by a self-signed CN=name-trusted certificate. It is accepted on main and rejected with this change. The existing `TestValidate*` cases are unchanged and pass. `go test ./viz/tap/...`, `go vet`, goimports and golangci-lint are clean. Signed-off-by: mushtaf saheb <musthaf@medios.digiscrypt.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.
The tap APIServer checks
requestheader-allowed-namesagainst everycertificate in
req.TLS.PeerCertificates. Only the first one, the leaf,is verified against
requestheader-client-ca-file; the rest arewhatever the client chose to send after it. A client holding any
certificate signed by that CA can append a self-signed certificate with
CN
front-proxy-client(or any other allowed name) and passvalidate(). From there it controlsX-Remote-UserandX-Remote-Group, so it can tap any workload as any user, for examplesystem:masters.This matters where the requestheader CA also issues other client
certificates, which is the case
requestheader-allowed-namesexists toguard against.
Match the allowed names against
PeerCertificates[0]only. WithRequireAndVerifyClientCertthat is the certificate Go verified againstthe client CA, and it is the same certificate the Kubernetes
requestheader authenticator checks.
Validation: added
TestValidate_IgnoresUnverifiedPeerCertificates,which does a real TLS handshake with a CA-signed leaf
(CN=name-untrusted) followed by a self-signed CN=name-trusted
certificate. It is accepted on main and rejected with this change. The
existing
TestValidate*cases are unchanged and pass.go test ./viz/tap/...,go vet, goimports and golangci-lint are clean.