zrhoffman commented on code in PR #7392:
URL: https://github.com/apache/trafficcontrol/pull/7392#discussion_r1187821844
##########
traffic_ops/traffic_ops_golang/login/login.go:
##########
@@ -108,135 +108,187 @@ Subject: {{.InstanceName}} Password Reset Request` +
"\r\n\r" + `
</html>
`))
+func clientCertAuthentication(w http.ResponseWriter, r *http.Request, db
*sqlx.DB, cfg config.Config, dbCtx context.Context, cancelTx
context.CancelFunc, form auth.PasswordForm, authenticated bool) bool {
+ // No certs provided by the client. Skip to form authentication
+ if r.TLS == nil || len(r.TLS.PeerCertificates) == 0 {
+ return false
+ }
+
+ // If no configuration is set, skip to form auth
+ if cfg.ClientCertAuth == nil || len(cfg.ClientCertAuth.RootCertsDir) ==
0 {
+ return false
+ }
+
+ // Perform certificate verification to ensure it is valid against Root
CAs
+ err := auth.VerifyClientCertificate(r, cfg.ClientCertAuth.RootCertsDir)
+ if err != nil {
+ log.Warnf("client cert auth: error attempting to verify client
provided TLS certificate. err: %s\n", err)
+ return false
+ }
+
+ // Client provided a verified certificate. Extract UID value.
+ form.Username =
auth.ParseClientCertificateUID(r.TLS.PeerCertificates[0])
+ if len(form.Username) == 0 {
+ log.Infoln("client cert auth: client provided certificate did
not contain a UID object identifier or value")
+ return false
+ }
+
+ // Check if user exists locally (TODB) and has a role.
+ var blockingErr error
+ authenticated, err, blockingErr =
auth.CheckLocalUserIsAllowed(form.Username, db, dbCtx)
+ if blockingErr != nil {
+ api.HandleErr(w, r, nil, http.StatusServiceUnavailable, nil,
fmt.Errorf("error checking local user has role: %s", blockingErr.Error()))
+ return false
+ }
+ if err != nil {
+ log.Warnf("client cert auth: checking local user: %s\n",
err.Error())
+ }
+
+ // Check LDAP if enabled
+ if !authenticated && cfg.LDAPEnabled {
+ _, authenticated, err = auth.LookupUserDN(form.Username,
cfg.ConfigLDAP)
+ if err != nil {
+ log.Warnf("Client Cert Auth: checking ldap user: %s\n",
err.Error())
+ }
+ }
+
+ return authenticated
+}
+
+// LoginHandler first attempts to verify and parse user information from an
optionally
+// provided client TLS certificate. If it fails at any point, it will fall
back and
+// continue with the standard submitted form authentication.
func LoginHandler(db *sqlx.DB, cfg config.Config) http.HandlerFunc {
Review Comment:
> This handles certificates as a replacement for uid/pass credentials, but
not as a replacement for the mojo cookie. Given that the certificate can be
seamlessly provided on every request, asking clients to use the login method to
obtain a token that is then used instead of the certificate seems to create
complexity where it isn't required. Additionally, certificates can ideally
enable flows that do not use tokens, which prevents tokens from being stolen.
The private key of a certificate isn't transmitted, so even if the certificate
were leaked, it couldn't be used.
You're right that this is a good idea, I opened #7498 for it. I'll make sure
we implement it, but it should not block #7392 being accepted, IMO.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
