-
Notifications
You must be signed in to change notification settings - Fork 345
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update TO to handle Client-Cert-Subject HTTP header for client cert authentication. #8013
Update TO to handle Client-Cert-Subject HTTP header for client cert authentication. #8013
Conversation
1bf6e3d
to
d138a40
Compare
|
||
// skipped certificate-based auth, log and continue | ||
if !triedAuthentication { | ||
log.Infof("skipped certificate-based auth because either no certs provided by the client or no configuration is set") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should remove this otherwise it will fill up our logs every time someone tries to authenticate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
|
||
// Failed certificate-based auth, perform standard form auth | ||
if !authenticated { | ||
log.Infof("user %s could not be successfully authenticated using client certificates", form.Username) | ||
if triedAuthentication && !authenticated { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the case that cert authentication isn't tried (triedAuthentication = false) the code will not enter this conditional, which means form auth wont work unless a user tries to authenticate with a certificate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Missed it. Fixed.
Not Planned For Now |
Overview,
Traffic Ops needs to accept Impersonation Certificates as described in the blueprint: https://github.com/apache/trafficcontrol/blob/master/blueprints/client-certificate-auth.md#traffic-ops-impact
Currently, TO only accepts a UID presented directly in the Subject of a client certificate. TO MUST accept a UID presented in the Client-Cert-Subject field as well. If a client certificate with an empty UID in the Subject field is presented in a request that also contains a Client-Cert-Subject HTTP header, TO MUST use the contents of the Client-Cert-Subject header to determine the UID of the client.
Which Traffic Control components are affected by this PR?
What is the best way to verify this PR?
Manual Testing,
trafficcontrol/experimental/certificate_auth/certs/generate_certs.go
Ensure that the UID field is empty at line,
trafficcontrol/experimental/certificate_auth/certs/generate_certs.go
Line 58 in a892235
uid = ""
Running
go run generate_certs.go
will produce private keys and certificates for Root, Intermediate, and Client. Place the Root certificate in the directory location specified in thecdn.conf
file,Launch a Traffic Ops instance.
In the
trafficcontrol/experimental/certificate_auth/example/client.go
file, uncomment the following line to set UID viaClient-Cert-Subject
header and change theUID
value to match the user you want to authenticate,req.Header.Set("Client-Cert-Subject", "CN=client,OU=client,O=client,L=client,ST=client,C=US,UID=userID")
client.go
file to send a request to the user/login Traffic Ops with the Client and Intermediate certs along with the http header Client-Cert-Subject ,go run client.go
If this is a bugfix, which Traffic Control versions contained the bug?
PR submission checklist