Bug 1739262: RequestHeaders IdP: fix TLS handshake [4.1] #24124
Conversation
|
@stlaz: No Bugzilla bug is referenced in the title of this pull request. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-ci-robot
added
the
size/XS
stlaz
changed the title
openshift-ci-robot
added
the
bugzilla/valid-bug
|
@stlaz: This pull request references Bugzilla bug 1739262, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
stlaz
force-pushed
the
oauth_client_certs_41
branch
from
457ee82 to
50b0c82
Compare
pkg/oauthserver/oauthserver/auth.go
Outdated
|
|
||
| // we need to add our CA data to secure serving as well to have the OAuth server | ||
| // advertise them for client auth during TLS handshake | ||
| c.GenericConfig.SecureServing.ClientCA = append(c.GenericConfig.SecureServing.ClientCA, provider.ClientCA) |
There was a problem hiding this comment.
/hold
This flag's backend changed in later versions, I need to see that this still works
There was a problem hiding this comment.
/hold cancel
the proper CA names are being announced
openshift-ci-robot
added
the
do-not-merge/hold
stlaz
force-pushed
the
oauth_client_certs_41
branch
2 times, most recently
from
7edaaea to
75afdd3
Compare
|
/retest |
openshift-ci-robot
removed
the
do-not-merge/hold
|
unit tests are failing because https://www.cvedetails.com/cve/CVE-2019-14809/ got fixed in golang 1.11, I wonder whether the rest is the same |
stlaz
changed the title
|
depends on #24111 for integration tests fix |
openshift-ci-robot
added
size/M
stlaz
force-pushed
the
oauth_client_certs_41
branch
from
a1dffef to
385db4e
Compare
|
/retest |
OAuth server uses the APIServer wrapping for its endpoints. This wrapping has its own RequestHeader IdP for the cluster admin login with client cert. As a part of that, before the RequestHeader is set, DelegatingAuthenticationOptions() adds its own ClientCA to the serving info config. The result of the above behavior is that during TLS handshake, when the names for acceptable client certificate CAs are announced, the CAs of the above mentioned apiserver RequestHeader IdP are announced. When users configure their own RequestHeader IdP to be used in OpenShift for the oauth-server to try to retrieve the users from, oauth-server was not adding this RequestHeader's client CA name among the acceptable client certificate CA names mentioned above. This commit adds the client CA data from the user specified RequestHeader IdP to 'GenericConfig.SecureServing.ClientCA', which is later converted to tlsConfig object which defines the data to be used in TLS handshakes.
stlaz
force-pushed
the
oauth_client_certs_41
branch
from
385db4e to
56bb935
Compare
openshift-ci-robot
added
size/XS
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k, stlaz The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
knobunc
added
the
cherry-pick-approved
|
@stlaz: All pull requests linked via external trackers have merged. Bugzilla bug 1739262 has been moved to the MODIFIED state. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
6 participants
OAuth server uses the APIServer wrapping for its endpoints. This
wrapping has its own RequestHeader IdP for the cluster admin login
with client cert. As a part of that, before the RequestHeader is
set, DelegatingAuthenticationOptions() adds its own ClientCA to
the serving info config.
The result of the above behavior is that during TLS handshake, when
the names for acceptable client certificate CAs are announced, the
CAs of the above mentioned apiserver RequestHeader IdP are announced.
When users configure their own RequestHeader IdP to be used in
OpenShift for the oauth-server to try to retrieve the users from,
oauth-server was not adding this RequestHeader's client CA name
among the acceptable client certificate CA names mentioned above.
This commit adds the client CA data from the user specified
RequestHeader IdP to 'GenericConfig.SecureServing.ClientCA',
which is later converted to tlsConfig object which defines the
data to be used in TLS handshakes.
/assign @deads2k