Add ability to specify allowed CNs for RequestHeader proxy client cert #8443
Conversation
|
suggestions welcome for the config field name |
| if a.allowedCommonNames.Has(subject.CommonName) { | ||
| return nil | ||
| } | ||
| return fmt.Errorf("x509: subject with cn=%s is not allowed", subject.CommonName) |
There was a problem hiding this comment.
I'm going to assume that you're intentionally withholding the allowed set on the assumption that while a compromised signer means you're owned, they wouldn't be able to also trick openshift without either access to the master-config or the client cert from the existing proxy?
Seems a little thin to me, but if you want it that way, you should add a comment and maybe indicate where in the master-config they should look when they're rejected?
|
lgtm. Is this for 1.2 or 1.2.x? |
|
added integration tests (and made sure the old integration test still passed against the current code):
|
|
[test] |
|
lgtm @smarterclayton for approval. fwiw, I'd vote yes. |
|
Risk? |
Given the testing around it and the simplicity of the code, I'd say low. |
|
bindata failure, re[test] |
smarterclayton
added
the
approved
|
Approved per importance to setting up request proxies |
|
flake on #7429, re[test] |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/5578/) (Image: devenv-rhel7_3957) |
|
Evaluated for origin merge up to 0ffdfdd |
|
Evaluated for origin test up to 0ffdfdd |
|
continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/2947/) |
Allows requiring a particular CN, so that not all client certs issued by the CA have to be trusted.