zrhoffman commented on code in PR #7110: URL: https://github.com/apache/trafficcontrol/pull/7110#discussion_r1110453151
########## experimental/certificate_auth/example/client/client.go: ########## @@ -0,0 +1,79 @@ +package main + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import ( + "bytes" + "crypto/tls" + "fmt" + "io/ioutil" + "log" + "net/http" + "time" +) + +func main() { + // LoadX509KeyPair can also load certificate chain with intermediates + cert, _ := tls.LoadX509KeyPair("../certs/client-intermediate-chain.crt.pem", "../certs/client.key.pem") + + transport := &http.Transport{ + TLSClientConfig: &tls.Config{ + Certificates: []tls.Certificate{cert}, + }, + } + + client := http.Client{ + Timeout: time.Second * 60, + Transport: transport, + } + + // Send standard username/password form combo + // reqBody, err := json.Marshal(map[string]string{ + // "u": "userid", + // "p": "exampleuseridpassword", + // }) + // if err != nil { + // log.Fatalln(err) + // } Review Comment: Can these commented lines be removed? ########## experimental/certificate_auth/example/client/client.go: ########## @@ -0,0 +1,79 @@ +package main + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import ( + "bytes" + "crypto/tls" + "fmt" + "io/ioutil" + "log" + "net/http" + "time" +) + +func main() { + // LoadX509KeyPair can also load certificate chain with intermediates + cert, _ := tls.LoadX509KeyPair("../certs/client-intermediate-chain.crt.pem", "../certs/client.key.pem") + + transport := &http.Transport{ + TLSClientConfig: &tls.Config{ + Certificates: []tls.Certificate{cert}, + }, + } + + client := http.Client{ + Timeout: time.Second * 60, + Transport: transport, + } + + // Send standard username/password form combo + // reqBody, err := json.Marshal(map[string]string{ + // "u": "userid", + // "p": "exampleuseridpassword", + // }) + // if err != nil { + // log.Fatalln(err) + // } + + req, err := http.NewRequest( + http.MethodPost, + "https://server.local:8443/api/4.0/user/login";, + bytes.NewBufferString(""), + // bytes.NewBuffer(reqBody), // username/password Review Comment: Commented that can maybe be removed ########## experimental/certificate_auth/certs/generate_certs.go: ########## @@ -0,0 +1,446 @@ +package main + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import ( + "bytes" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "crypto/x509/pkix" + "encoding/asn1" + "encoding/pem" + "flag" + "fmt" + "io/ioutil" + "log" + "math" + "math/big" + "net" + "time" +) + +// CertificateKeyPair contains the parsed representation of a certificate +// and private key. +type CertificateKeyPair struct { + Certificate *x509.Certificate + PrivateKey *rsa.PrivateKey +} + +// CertificatePEMPair contains the PEM encoded certificate and private key. +type CertificatePEMPair struct { + CertificatePEM, PrivateKeyPEM string +} + +var ( + rootCN = "root.local" + interCN = "intermediate.local" + clientCN = "client.local" + serverCN = "server.local" + + uid = "userid" + // useEcdsa = false //TODO: Enable and refactor +) + +func main() { + flag.StringVar(&uid, "uid", uid, "[Optional] The User ID value to be added to the client certificate") + // flag.BoolVar(&useEcdsa, "useEcdsa", useEcdsa, "[Optional] Use ECDSA 256 when generating the keys. Default is RSA 4096") Review Comment: Can this commented line be removed? ########## traffic_ops/traffic_ops_golang/login/login.go: ########## @@ -108,135 +108,184 @@ Subject: {{.InstanceName}} Password Reset Request` + "\r\n\r" + ` </html> `)) +// 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 { return func(w http.ResponseWriter, r *http.Request) { defer r.Body.Close() authenticated := false form := auth.PasswordForm{} - if err := json.NewDecoder(r.Body).Decode(&form); err != nil { - api.HandleErr(w, r, nil, http.StatusBadRequest, err, nil) - return - } - if form.Username == "" || form.Password == "" { - api.HandleErr(w, r, nil, http.StatusBadRequest, errors.New("username and password are required"), nil) - return - } - resp := struct { - tc.Alerts - }{} + var resp tc.Alerts dbCtx, cancelTx := context.WithTimeout(r.Context(), time.Duration(cfg.DBQueryTimeoutSeconds)*time.Second) defer cancelTx() - userAllowed, err, blockingErr := auth.CheckLocalUserIsAllowed(form, db, dbCtx) - if blockingErr != nil { - api.HandleErr(w, r, nil, http.StatusServiceUnavailable, nil, fmt.Errorf("error checking local user password: %s\n", blockingErr.Error())) - return - } - if err != nil { - log.Errorf("checking local user: %s\n", err.Error()) + + // Attempt to perform client certificate authentication. If fails, goto standard form auth. If the + // certificate was verified, has a UID, and the UID matches an existing user we consider this to + // be a successful login. + { + // No certs provided by the client. Skip to form authentication + if r.TLS == nil || len(r.TLS.PeerCertificates) == 0 { + goto FormAuth + } + + // If no configuration is set, skip to form auth + if cfg.ClientCertAuth == nil || len(cfg.ClientCertAuth.RootCertsDir) == 0 { + goto FormAuth + } + + // Perform certificate verification to ensure it is valid against Root CAs + err := auth.VerifyClientCertificate(r, cfg.ClientCertAuth.RootCertsDir) + if err != nil { + log.Warnf("ClientCertAuth: error attempting to verify client provided TLS certificate. err: %s\n", err) + goto FormAuth + } + + // Client provided a verified certificate. Extract UID value. + form.Username = auth.ParseClientCertificateUID(r.TLS.PeerCertificates[0]) + if len(form.Username) == 0 { + log.Infoln("ClientCertAuth: client provided certificate did not contain a UID object identifier or value") + goto FormAuth + } + + // 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 + } + if err != nil { + log.Warnf("ClientCertAuth: 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("ClientCertAuth: checking ldap user: %s\n", err.Error()) + } + } } Review Comment: Can this be restructured so that `goto` is not required, like splitting this part into another function? ########## traffic_ops/traffic_ops_golang/login/login.go: ########## @@ -108,135 +108,184 @@ Subject: {{.InstanceName}} Password Reset Request` + "\r\n\r" + ` </html> `)) +// 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 { return func(w http.ResponseWriter, r *http.Request) { defer r.Body.Close() authenticated := false form := auth.PasswordForm{} - if err := json.NewDecoder(r.Body).Decode(&form); err != nil { - api.HandleErr(w, r, nil, http.StatusBadRequest, err, nil) - return - } - if form.Username == "" || form.Password == "" { - api.HandleErr(w, r, nil, http.StatusBadRequest, errors.New("username and password are required"), nil) - return - } - resp := struct { - tc.Alerts - }{} + var resp tc.Alerts dbCtx, cancelTx := context.WithTimeout(r.Context(), time.Duration(cfg.DBQueryTimeoutSeconds)*time.Second) defer cancelTx() - userAllowed, err, blockingErr := auth.CheckLocalUserIsAllowed(form, db, dbCtx) - if blockingErr != nil { - api.HandleErr(w, r, nil, http.StatusServiceUnavailable, nil, fmt.Errorf("error checking local user password: %s\n", blockingErr.Error())) - return - } - if err != nil { - log.Errorf("checking local user: %s\n", err.Error()) + + // Attempt to perform client certificate authentication. If fails, goto standard form auth. If the + // certificate was verified, has a UID, and the UID matches an existing user we consider this to + // be a successful login. + { + // No certs provided by the client. Skip to form authentication + if r.TLS == nil || len(r.TLS.PeerCertificates) == 0 { + goto FormAuth + } + + // If no configuration is set, skip to form auth + if cfg.ClientCertAuth == nil || len(cfg.ClientCertAuth.RootCertsDir) == 0 { + goto FormAuth + } + + // Perform certificate verification to ensure it is valid against Root CAs + err := auth.VerifyClientCertificate(r, cfg.ClientCertAuth.RootCertsDir) + if err != nil { + log.Warnf("ClientCertAuth: error attempting to verify client provided TLS certificate. err: %s\n", err) + goto FormAuth + } + + // Client provided a verified certificate. Extract UID value. + form.Username = auth.ParseClientCertificateUID(r.TLS.PeerCertificates[0]) + if len(form.Username) == 0 { + log.Infoln("ClientCertAuth: client provided certificate did not contain a UID object identifier or value") + goto FormAuth + } + + // 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 + } + if err != nil { + log.Warnf("ClientCertAuth: 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("ClientCertAuth: checking ldap user: %s\n", err.Error()) + } + } } - if userAllowed { + + FormAuth: + // Failed certificate-based auth, perform standard form auth + if !authenticated { + // Perform form authentication + if err := json.NewDecoder(r.Body).Decode(&form); err != nil { + api.HandleErr(w, r, nil, http.StatusBadRequest, err, nil) + return + } + if form.Username == "" || form.Password == "" { + api.HandleErr(w, r, nil, http.StatusBadRequest, errors.New("username and password are required"), nil) + return + } + + // Check if user exists and has a role + userAllowed, 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 + } + if err != nil { + log.Errorf("checking local user: %s\n", err.Error()) + } + + // User w/ role does not exist, return unauthorized + if !userAllowed { + resp = tc.CreateAlerts(tc.ErrorLevel, "Invalid username or password.") + w.WriteHeader(http.StatusUnauthorized) + api.WriteRespRaw(w, r, resp) + return + } + + // Check local DB or LDAP authenticated, err, blockingErr = auth.CheckLocalUserPassword(form, db, dbCtx) if blockingErr != nil { - api.HandleErr(w, r, nil, http.StatusServiceUnavailable, nil, fmt.Errorf("error checking local user password: %s\n", blockingErr.Error())) + api.HandleErr(w, r, nil, http.StatusServiceUnavailable, nil, fmt.Errorf("error checking local user password: %s", blockingErr.Error())) return } if err != nil { log.Errorf("checking local user password: %s\n", err.Error()) } var ldapErr error - if !authenticated { - if cfg.LDAPEnabled { - authenticated, ldapErr = auth.CheckLDAPUser(form, cfg.ConfigLDAP) - if ldapErr != nil { - log.Errorf("checking ldap user: %s\n", ldapErr.Error()) - } + if !authenticated && cfg.LDAPEnabled { + authenticated, ldapErr = auth.CheckLDAPUser(form, cfg.ConfigLDAP) + if ldapErr != nil { + log.Errorf("checking ldap user: %s\n", ldapErr.Error()) } } - if authenticated { - httpCookie := tocookie.GetCookie(form.Username, defaultCookieDuration, cfg.Secrets[0]) - http.SetCookie(w, httpCookie) - - var jwtToken jwt.Token - var jwtSigned []byte - jwtBuilder := jwt.NewBuilder() - - emptyConf := config.CdniConf{} - if cfg.Cdni != nil && *cfg.Cdni != emptyConf { - ucdn, err := auth.GetUserUcdn(form, db, dbCtx) - if err != nil { - // log but do not error out since this is optional in the JWT for CDNi integration - log.Errorf("getting ucdn for user %s: %v", form.Username, err) - } - jwtBuilder.Claim("iss", ucdn) - jwtBuilder.Claim("aud", cfg.Cdni.DCdnId) - } + } - jwtBuilder.Claim("exp", httpCookie.Expires.Unix()) - jwtBuilder.Claim(api.MojoCookie, httpCookie.Value) - jwtToken, err = jwtBuilder.Build() - if err != nil { - api.HandleErr(w, r, nil, http.StatusInternalServerError, nil, fmt.Errorf("building token: %s", err)) - return - } + // Failed to authenticate in either local DB or LDAP, return unauthorized + if !authenticated { + resp = tc.CreateAlerts(tc.ErrorLevel, "Invalid username or password.") + w.WriteHeader(http.StatusUnauthorized) + api.WriteRespRaw(w, r, resp) + return + } - jwtSigned, err = jwt.Sign(jwtToken, jwa.HS256, []byte(cfg.Secrets[0])) - if err != nil { - api.HandleErr(w, r, nil, http.StatusInternalServerError, nil, err) - return - } + // Successful authentication, write cookie and return + httpCookie := tocookie.GetCookie(form.Username, defaultCookieDuration, cfg.Secrets[0]) + http.SetCookie(w, httpCookie) - http.SetCookie(w, &http.Cookie{ - Name: api.AccessToken, - Value: string(jwtSigned), - Path: "/", - MaxAge: httpCookie.MaxAge, - Expires: httpCookie.Expires, - HttpOnly: true, // prevents the cookie being accessed by Javascript. DO NOT remove, security vulnerability - }) - - // If all's well until here, then update last authenticated time - tx, txErr := db.BeginTx(dbCtx, nil) - if txErr != nil { - api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("beginning transaction: %w", txErr)) - return - } - defer func() { - if err := tx.Commit(); err != nil && err != sql.ErrTxDone { - log.Errorln("committing transaction: " + err.Error()) - } - }() - _, dbErr := tx.Exec(UpdateLoginTimeQuery, form.Username) - if dbErr != nil { - log.Errorf("unable to update authentication time for a given user: %s\n", dbErr.Error()) - resp = struct { - tc.Alerts - }{tc.CreateAlerts(tc.ErrorLevel, "Unable to update authentication time for a given user")} - } else { - resp = struct { - tc.Alerts - }{tc.CreateAlerts(tc.SuccessLevel, "Successfully logged in.")} - } + var jwtToken jwt.Token + var jwtSigned []byte + jwtBuilder := jwt.NewBuilder() - } else { - resp = struct { - tc.Alerts - }{tc.CreateAlerts(tc.ErrorLevel, "Invalid username or password.")} + emptyConf := config.CdniConf{} + if cfg.Cdni != nil && *cfg.Cdni != emptyConf { + ucdn, err := auth.GetUserUcdn(form, db, dbCtx) + if err != nil { + // log but do not error out since this is optional in the JWT for CDNi integration + log.Errorf("getting ucdn for user %s: %v", form.Username, err) } - } else { - resp = struct { - tc.Alerts - }{tc.CreateAlerts(tc.ErrorLevel, "Invalid username or password.")} + jwtBuilder.Claim("iss", ucdn) + jwtBuilder.Claim("aud", cfg.Cdni.DCdnId) Review Comment: `"aud"` can be `jwt.AudienceKey` ########## traffic_ops/traffic_ops_golang/login/login.go: ########## @@ -108,135 +108,184 @@ Subject: {{.InstanceName}} Password Reset Request` + "\r\n\r" + ` </html> `)) +// 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 { return func(w http.ResponseWriter, r *http.Request) { defer r.Body.Close() authenticated := false form := auth.PasswordForm{} - if err := json.NewDecoder(r.Body).Decode(&form); err != nil { - api.HandleErr(w, r, nil, http.StatusBadRequest, err, nil) - return - } - if form.Username == "" || form.Password == "" { - api.HandleErr(w, r, nil, http.StatusBadRequest, errors.New("username and password are required"), nil) - return - } - resp := struct { - tc.Alerts - }{} + var resp tc.Alerts dbCtx, cancelTx := context.WithTimeout(r.Context(), time.Duration(cfg.DBQueryTimeoutSeconds)*time.Second) defer cancelTx() - userAllowed, err, blockingErr := auth.CheckLocalUserIsAllowed(form, db, dbCtx) - if blockingErr != nil { - api.HandleErr(w, r, nil, http.StatusServiceUnavailable, nil, fmt.Errorf("error checking local user password: %s\n", blockingErr.Error())) - return - } - if err != nil { - log.Errorf("checking local user: %s\n", err.Error()) + + // Attempt to perform client certificate authentication. If fails, goto standard form auth. If the + // certificate was verified, has a UID, and the UID matches an existing user we consider this to + // be a successful login. + { + // No certs provided by the client. Skip to form authentication + if r.TLS == nil || len(r.TLS.PeerCertificates) == 0 { + goto FormAuth + } + + // If no configuration is set, skip to form auth + if cfg.ClientCertAuth == nil || len(cfg.ClientCertAuth.RootCertsDir) == 0 { + goto FormAuth + } + + // Perform certificate verification to ensure it is valid against Root CAs + err := auth.VerifyClientCertificate(r, cfg.ClientCertAuth.RootCertsDir) + if err != nil { + log.Warnf("ClientCertAuth: error attempting to verify client provided TLS certificate. err: %s\n", err) + goto FormAuth + } + + // Client provided a verified certificate. Extract UID value. + form.Username = auth.ParseClientCertificateUID(r.TLS.PeerCertificates[0]) + if len(form.Username) == 0 { + log.Infoln("ClientCertAuth: client provided certificate did not contain a UID object identifier or value") + goto FormAuth + } + + // 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 + } + if err != nil { + log.Warnf("ClientCertAuth: 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("ClientCertAuth: checking ldap user: %s\n", err.Error()) + } + } } - if userAllowed { + + FormAuth: + // Failed certificate-based auth, perform standard form auth + if !authenticated { + // Perform form authentication + if err := json.NewDecoder(r.Body).Decode(&form); err != nil { + api.HandleErr(w, r, nil, http.StatusBadRequest, err, nil) + return + } + if form.Username == "" || form.Password == "" { + api.HandleErr(w, r, nil, http.StatusBadRequest, errors.New("username and password are required"), nil) + return + } + + // Check if user exists and has a role + userAllowed, 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 + } + if err != nil { + log.Errorf("checking local user: %s\n", err.Error()) + } + + // User w/ role does not exist, return unauthorized + if !userAllowed { + resp = tc.CreateAlerts(tc.ErrorLevel, "Invalid username or password.") + w.WriteHeader(http.StatusUnauthorized) + api.WriteRespRaw(w, r, resp) + return + } + + // Check local DB or LDAP authenticated, err, blockingErr = auth.CheckLocalUserPassword(form, db, dbCtx) if blockingErr != nil { - api.HandleErr(w, r, nil, http.StatusServiceUnavailable, nil, fmt.Errorf("error checking local user password: %s\n", blockingErr.Error())) + api.HandleErr(w, r, nil, http.StatusServiceUnavailable, nil, fmt.Errorf("error checking local user password: %s", blockingErr.Error())) return } if err != nil { log.Errorf("checking local user password: %s\n", err.Error()) } var ldapErr error - if !authenticated { - if cfg.LDAPEnabled { - authenticated, ldapErr = auth.CheckLDAPUser(form, cfg.ConfigLDAP) - if ldapErr != nil { - log.Errorf("checking ldap user: %s\n", ldapErr.Error()) - } + if !authenticated && cfg.LDAPEnabled { + authenticated, ldapErr = auth.CheckLDAPUser(form, cfg.ConfigLDAP) + if ldapErr != nil { + log.Errorf("checking ldap user: %s\n", ldapErr.Error()) } } - if authenticated { - httpCookie := tocookie.GetCookie(form.Username, defaultCookieDuration, cfg.Secrets[0]) - http.SetCookie(w, httpCookie) - - var jwtToken jwt.Token - var jwtSigned []byte - jwtBuilder := jwt.NewBuilder() - - emptyConf := config.CdniConf{} - if cfg.Cdni != nil && *cfg.Cdni != emptyConf { - ucdn, err := auth.GetUserUcdn(form, db, dbCtx) - if err != nil { - // log but do not error out since this is optional in the JWT for CDNi integration - log.Errorf("getting ucdn for user %s: %v", form.Username, err) - } - jwtBuilder.Claim("iss", ucdn) - jwtBuilder.Claim("aud", cfg.Cdni.DCdnId) - } + } - jwtBuilder.Claim("exp", httpCookie.Expires.Unix()) - jwtBuilder.Claim(api.MojoCookie, httpCookie.Value) - jwtToken, err = jwtBuilder.Build() - if err != nil { - api.HandleErr(w, r, nil, http.StatusInternalServerError, nil, fmt.Errorf("building token: %s", err)) - return - } + // Failed to authenticate in either local DB or LDAP, return unauthorized + if !authenticated { + resp = tc.CreateAlerts(tc.ErrorLevel, "Invalid username or password.") + w.WriteHeader(http.StatusUnauthorized) + api.WriteRespRaw(w, r, resp) + return + } - jwtSigned, err = jwt.Sign(jwtToken, jwa.HS256, []byte(cfg.Secrets[0])) - if err != nil { - api.HandleErr(w, r, nil, http.StatusInternalServerError, nil, err) - return - } + // Successful authentication, write cookie and return + httpCookie := tocookie.GetCookie(form.Username, defaultCookieDuration, cfg.Secrets[0]) + http.SetCookie(w, httpCookie) - http.SetCookie(w, &http.Cookie{ - Name: api.AccessToken, - Value: string(jwtSigned), - Path: "/", - MaxAge: httpCookie.MaxAge, - Expires: httpCookie.Expires, - HttpOnly: true, // prevents the cookie being accessed by Javascript. DO NOT remove, security vulnerability - }) - - // If all's well until here, then update last authenticated time - tx, txErr := db.BeginTx(dbCtx, nil) - if txErr != nil { - api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("beginning transaction: %w", txErr)) - return - } - defer func() { - if err := tx.Commit(); err != nil && err != sql.ErrTxDone { - log.Errorln("committing transaction: " + err.Error()) - } - }() - _, dbErr := tx.Exec(UpdateLoginTimeQuery, form.Username) - if dbErr != nil { - log.Errorf("unable to update authentication time for a given user: %s\n", dbErr.Error()) - resp = struct { - tc.Alerts - }{tc.CreateAlerts(tc.ErrorLevel, "Unable to update authentication time for a given user")} - } else { - resp = struct { - tc.Alerts - }{tc.CreateAlerts(tc.SuccessLevel, "Successfully logged in.")} - } + var jwtToken jwt.Token + var jwtSigned []byte + jwtBuilder := jwt.NewBuilder() - } else { - resp = struct { - tc.Alerts - }{tc.CreateAlerts(tc.ErrorLevel, "Invalid username or password.")} + emptyConf := config.CdniConf{} + if cfg.Cdni != nil && *cfg.Cdni != emptyConf { + ucdn, err := auth.GetUserUcdn(form, db, dbCtx) + if err != nil { + // log but do not error out since this is optional in the JWT for CDNi integration + log.Errorf("getting ucdn for user %s: %v", form.Username, err) } - } else { - resp = struct { - tc.Alerts - }{tc.CreateAlerts(tc.ErrorLevel, "Invalid username or password.")} + jwtBuilder.Claim("iss", ucdn) Review Comment: Because this part is getting touched/moved: `"iss"` can be `jwt.IssuerKey` ########## traffic_ops/traffic_ops_golang/login/login.go: ########## @@ -108,135 +108,184 @@ Subject: {{.InstanceName}} Password Reset Request` + "\r\n\r" + ` </html> `)) +// 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 { return func(w http.ResponseWriter, r *http.Request) { defer r.Body.Close() authenticated := false form := auth.PasswordForm{} - if err := json.NewDecoder(r.Body).Decode(&form); err != nil { - api.HandleErr(w, r, nil, http.StatusBadRequest, err, nil) - return - } - if form.Username == "" || form.Password == "" { - api.HandleErr(w, r, nil, http.StatusBadRequest, errors.New("username and password are required"), nil) - return - } - resp := struct { - tc.Alerts - }{} + var resp tc.Alerts dbCtx, cancelTx := context.WithTimeout(r.Context(), time.Duration(cfg.DBQueryTimeoutSeconds)*time.Second) defer cancelTx() - userAllowed, err, blockingErr := auth.CheckLocalUserIsAllowed(form, db, dbCtx) - if blockingErr != nil { - api.HandleErr(w, r, nil, http.StatusServiceUnavailable, nil, fmt.Errorf("error checking local user password: %s\n", blockingErr.Error())) - return - } - if err != nil { - log.Errorf("checking local user: %s\n", err.Error()) + + // Attempt to perform client certificate authentication. If fails, goto standard form auth. If the + // certificate was verified, has a UID, and the UID matches an existing user we consider this to + // be a successful login. + { + // No certs provided by the client. Skip to form authentication + if r.TLS == nil || len(r.TLS.PeerCertificates) == 0 { + goto FormAuth + } + + // If no configuration is set, skip to form auth + if cfg.ClientCertAuth == nil || len(cfg.ClientCertAuth.RootCertsDir) == 0 { + goto FormAuth + } + + // Perform certificate verification to ensure it is valid against Root CAs + err := auth.VerifyClientCertificate(r, cfg.ClientCertAuth.RootCertsDir) + if err != nil { + log.Warnf("ClientCertAuth: error attempting to verify client provided TLS certificate. err: %s\n", err) + goto FormAuth + } + + // Client provided a verified certificate. Extract UID value. + form.Username = auth.ParseClientCertificateUID(r.TLS.PeerCertificates[0]) + if len(form.Username) == 0 { + log.Infoln("ClientCertAuth: client provided certificate did not contain a UID object identifier or value") + goto FormAuth + } + + // 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 + } + if err != nil { + log.Warnf("ClientCertAuth: checking local user: %s\n", err.Error()) Review Comment: Nit: Why PascalCase instead of a lowercase string? ########## traffic_ops/traffic_ops_golang/login/login.go: ########## @@ -108,135 +108,184 @@ Subject: {{.InstanceName}} Password Reset Request` + "\r\n\r" + ` </html> `)) +// 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 { return func(w http.ResponseWriter, r *http.Request) { defer r.Body.Close() authenticated := false form := auth.PasswordForm{} - if err := json.NewDecoder(r.Body).Decode(&form); err != nil { - api.HandleErr(w, r, nil, http.StatusBadRequest, err, nil) - return - } - if form.Username == "" || form.Password == "" { - api.HandleErr(w, r, nil, http.StatusBadRequest, errors.New("username and password are required"), nil) - return - } - resp := struct { - tc.Alerts - }{} + var resp tc.Alerts dbCtx, cancelTx := context.WithTimeout(r.Context(), time.Duration(cfg.DBQueryTimeoutSeconds)*time.Second) defer cancelTx() - userAllowed, err, blockingErr := auth.CheckLocalUserIsAllowed(form, db, dbCtx) - if blockingErr != nil { - api.HandleErr(w, r, nil, http.StatusServiceUnavailable, nil, fmt.Errorf("error checking local user password: %s\n", blockingErr.Error())) - return - } - if err != nil { - log.Errorf("checking local user: %s\n", err.Error()) + + // Attempt to perform client certificate authentication. If fails, goto standard form auth. If the + // certificate was verified, has a UID, and the UID matches an existing user we consider this to + // be a successful login. + { + // No certs provided by the client. Skip to form authentication + if r.TLS == nil || len(r.TLS.PeerCertificates) == 0 { + goto FormAuth + } + + // If no configuration is set, skip to form auth + if cfg.ClientCertAuth == nil || len(cfg.ClientCertAuth.RootCertsDir) == 0 { + goto FormAuth + } + + // Perform certificate verification to ensure it is valid against Root CAs + err := auth.VerifyClientCertificate(r, cfg.ClientCertAuth.RootCertsDir) + if err != nil { + log.Warnf("ClientCertAuth: error attempting to verify client provided TLS certificate. err: %s\n", err) + goto FormAuth + } + + // Client provided a verified certificate. Extract UID value. + form.Username = auth.ParseClientCertificateUID(r.TLS.PeerCertificates[0]) + if len(form.Username) == 0 { + log.Infoln("ClientCertAuth: client provided certificate did not contain a UID object identifier or value") + goto FormAuth + } + + // 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 + } + if err != nil { + log.Warnf("ClientCertAuth: 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("ClientCertAuth: checking ldap user: %s\n", err.Error()) + } + } } - if userAllowed { + + FormAuth: + // Failed certificate-based auth, perform standard form auth + if !authenticated { + // Perform form authentication + if err := json.NewDecoder(r.Body).Decode(&form); err != nil { + api.HandleErr(w, r, nil, http.StatusBadRequest, err, nil) + return + } + if form.Username == "" || form.Password == "" { + api.HandleErr(w, r, nil, http.StatusBadRequest, errors.New("username and password are required"), nil) + return + } + + // Check if user exists and has a role + userAllowed, 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 + } + if err != nil { + log.Errorf("checking local user: %s\n", err.Error()) + } + + // User w/ role does not exist, return unauthorized + if !userAllowed { + resp = tc.CreateAlerts(tc.ErrorLevel, "Invalid username or password.") + w.WriteHeader(http.StatusUnauthorized) + api.WriteRespRaw(w, r, resp) + return + } + + // Check local DB or LDAP authenticated, err, blockingErr = auth.CheckLocalUserPassword(form, db, dbCtx) if blockingErr != nil { - api.HandleErr(w, r, nil, http.StatusServiceUnavailable, nil, fmt.Errorf("error checking local user password: %s\n", blockingErr.Error())) + api.HandleErr(w, r, nil, http.StatusServiceUnavailable, nil, fmt.Errorf("error checking local user password: %s", blockingErr.Error())) return } if err != nil { log.Errorf("checking local user password: %s\n", err.Error()) } var ldapErr error - if !authenticated { - if cfg.LDAPEnabled { - authenticated, ldapErr = auth.CheckLDAPUser(form, cfg.ConfigLDAP) - if ldapErr != nil { - log.Errorf("checking ldap user: %s\n", ldapErr.Error()) - } + if !authenticated && cfg.LDAPEnabled { + authenticated, ldapErr = auth.CheckLDAPUser(form, cfg.ConfigLDAP) + if ldapErr != nil { + log.Errorf("checking ldap user: %s\n", ldapErr.Error()) } } - if authenticated { - httpCookie := tocookie.GetCookie(form.Username, defaultCookieDuration, cfg.Secrets[0]) - http.SetCookie(w, httpCookie) - - var jwtToken jwt.Token - var jwtSigned []byte - jwtBuilder := jwt.NewBuilder() - - emptyConf := config.CdniConf{} - if cfg.Cdni != nil && *cfg.Cdni != emptyConf { - ucdn, err := auth.GetUserUcdn(form, db, dbCtx) - if err != nil { - // log but do not error out since this is optional in the JWT for CDNi integration - log.Errorf("getting ucdn for user %s: %v", form.Username, err) - } - jwtBuilder.Claim("iss", ucdn) - jwtBuilder.Claim("aud", cfg.Cdni.DCdnId) - } + } - jwtBuilder.Claim("exp", httpCookie.Expires.Unix()) - jwtBuilder.Claim(api.MojoCookie, httpCookie.Value) - jwtToken, err = jwtBuilder.Build() - if err != nil { - api.HandleErr(w, r, nil, http.StatusInternalServerError, nil, fmt.Errorf("building token: %s", err)) - return - } + // Failed to authenticate in either local DB or LDAP, return unauthorized + if !authenticated { + resp = tc.CreateAlerts(tc.ErrorLevel, "Invalid username or password.") + w.WriteHeader(http.StatusUnauthorized) + api.WriteRespRaw(w, r, resp) + return + } - jwtSigned, err = jwt.Sign(jwtToken, jwa.HS256, []byte(cfg.Secrets[0])) - if err != nil { - api.HandleErr(w, r, nil, http.StatusInternalServerError, nil, err) - return - } + // Successful authentication, write cookie and return + httpCookie := tocookie.GetCookie(form.Username, defaultCookieDuration, cfg.Secrets[0]) + http.SetCookie(w, httpCookie) - http.SetCookie(w, &http.Cookie{ - Name: api.AccessToken, - Value: string(jwtSigned), - Path: "/", - MaxAge: httpCookie.MaxAge, - Expires: httpCookie.Expires, - HttpOnly: true, // prevents the cookie being accessed by Javascript. DO NOT remove, security vulnerability - }) - - // If all's well until here, then update last authenticated time - tx, txErr := db.BeginTx(dbCtx, nil) - if txErr != nil { - api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("beginning transaction: %w", txErr)) - return - } - defer func() { - if err := tx.Commit(); err != nil && err != sql.ErrTxDone { - log.Errorln("committing transaction: " + err.Error()) - } - }() - _, dbErr := tx.Exec(UpdateLoginTimeQuery, form.Username) - if dbErr != nil { - log.Errorf("unable to update authentication time for a given user: %s\n", dbErr.Error()) - resp = struct { - tc.Alerts - }{tc.CreateAlerts(tc.ErrorLevel, "Unable to update authentication time for a given user")} - } else { - resp = struct { - tc.Alerts - }{tc.CreateAlerts(tc.SuccessLevel, "Successfully logged in.")} - } + var jwtToken jwt.Token + var jwtSigned []byte + jwtBuilder := jwt.NewBuilder() - } else { - resp = struct { - tc.Alerts - }{tc.CreateAlerts(tc.ErrorLevel, "Invalid username or password.")} + emptyConf := config.CdniConf{} + if cfg.Cdni != nil && *cfg.Cdni != emptyConf { + ucdn, err := auth.GetUserUcdn(form, db, dbCtx) + if err != nil { + // log but do not error out since this is optional in the JWT for CDNi integration + log.Errorf("getting ucdn for user %s: %v", form.Username, err) } - } else { - resp = struct { - tc.Alerts - }{tc.CreateAlerts(tc.ErrorLevel, "Invalid username or password.")} + jwtBuilder.Claim("iss", ucdn) + jwtBuilder.Claim("aud", cfg.Cdni.DCdnId) } - respBts, err := json.Marshal(resp) + + jwtBuilder.Claim("exp", httpCookie.Expires.Unix()) Review Comment: `"exp"` can be `jwt.ExpirationKey` ########## traffic_ops/traffic_ops_golang/login/login.go: ########## @@ -108,135 +108,184 @@ Subject: {{.InstanceName}} Password Reset Request` + "\r\n\r" + ` </html> `)) +// 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 { return func(w http.ResponseWriter, r *http.Request) { defer r.Body.Close() authenticated := false form := auth.PasswordForm{} - if err := json.NewDecoder(r.Body).Decode(&form); err != nil { - api.HandleErr(w, r, nil, http.StatusBadRequest, err, nil) - return - } - if form.Username == "" || form.Password == "" { - api.HandleErr(w, r, nil, http.StatusBadRequest, errors.New("username and password are required"), nil) - return - } - resp := struct { - tc.Alerts - }{} + var resp tc.Alerts dbCtx, cancelTx := context.WithTimeout(r.Context(), time.Duration(cfg.DBQueryTimeoutSeconds)*time.Second) defer cancelTx() - userAllowed, err, blockingErr := auth.CheckLocalUserIsAllowed(form, db, dbCtx) - if blockingErr != nil { - api.HandleErr(w, r, nil, http.StatusServiceUnavailable, nil, fmt.Errorf("error checking local user password: %s\n", blockingErr.Error())) - return - } - if err != nil { - log.Errorf("checking local user: %s\n", err.Error()) + + // Attempt to perform client certificate authentication. If fails, goto standard form auth. If the + // certificate was verified, has a UID, and the UID matches an existing user we consider this to + // be a successful login. + { + // No certs provided by the client. Skip to form authentication + if r.TLS == nil || len(r.TLS.PeerCertificates) == 0 { + goto FormAuth + } + + // If no configuration is set, skip to form auth + if cfg.ClientCertAuth == nil || len(cfg.ClientCertAuth.RootCertsDir) == 0 { + goto FormAuth + } + + // Perform certificate verification to ensure it is valid against Root CAs + err := auth.VerifyClientCertificate(r, cfg.ClientCertAuth.RootCertsDir) + if err != nil { + log.Warnf("ClientCertAuth: error attempting to verify client provided TLS certificate. err: %s\n", err) + goto FormAuth + } + + // Client provided a verified certificate. Extract UID value. + form.Username = auth.ParseClientCertificateUID(r.TLS.PeerCertificates[0]) + if len(form.Username) == 0 { + log.Infoln("ClientCertAuth: client provided certificate did not contain a UID object identifier or value") + goto FormAuth + } + + // 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 + } + if err != nil { + log.Warnf("ClientCertAuth: 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("ClientCertAuth: checking ldap user: %s\n", err.Error()) Review Comment: Nit: Why PascalCase instead of a lowercase string? -- 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]
