From 572082f1782859a8541a250f8a64d9c598e77b1c Mon Sep 17 00:00:00 2001 From: Jagan Parthiban Date: Tue, 21 May 2024 19:16:33 +0530 Subject: [PATCH 1/7] Update TO to handle impersonation certificate --- traffic_ops/traffic_ops_golang/login/login.go | 27 ++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/traffic_ops/traffic_ops_golang/login/login.go b/traffic_ops/traffic_ops_golang/login/login.go index c083da260c..861bbf9ebd 100644 --- a/traffic_ops/traffic_ops_golang/login/login.go +++ b/traffic_ops/traffic_ops_golang/login/login.go @@ -32,6 +32,7 @@ import ( "net/http" "net/url" "path/filepath" + "strings" "time" "github.com/apache/trafficcontrol/v8/lib/go-log" @@ -126,10 +127,16 @@ func clientCertAuthentication(w http.ResponseWriter, r *http.Request, db *sqlx.D return false } - // Client provided a verified certificate. Extract UID value. + // Client provided a verified certificate. Extract UID value. Try Certificate first and then HTTP Header + clientCertSubject := r.Header.Get("Client-Cert-Subject") if username, err := auth.ParseClientCertificateUID(r.TLS.PeerCertificates[0]); err != nil { - log.Errorf("parsing client certificate: %s\n", err) - return false + log.Infof("parsing client certificate: %s\n", err) + if username, err = extractUID(clientCertSubject); err != nil { + log.Errorf("extracting UID from http header client-cert-subject: %s\n", err) + return false + } else { + form.Username = username + } } else { form.Username = username } @@ -729,3 +736,17 @@ func ResetPassword(db *sqlx.DB, cfg config.Config) http.HandlerFunc { api.WriteAndLogErr(w, r, append(respBts, '\n')) } } + +// Extract UID from the Client-Cert-Subject header +func extractUID(subject string) (string, error) { + err := error(nil) + subjects := strings.Split(subject, ",") + for _, s := range subjects { + if strings.Contains(s, "UID=") { + return strings.TrimSpace(strings.TrimPrefix(s, "UID=")), err + } + } + + err = fmt.Errorf("UID not found in Client-Cert-Subject header") + return "", err +} From d138a407da0641214f3dffbbc601b40fc9ca01aa Mon Sep 17 00:00:00 2001 From: Jagan Parthiban Date: Wed, 22 May 2024 11:56:43 +0530 Subject: [PATCH 2/7] Update example client.go to handle Client-Cert-Subject header to determine the UID of the client --- experimental/certificate_auth/example/client/client.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/experimental/certificate_auth/example/client/client.go b/experimental/certificate_auth/example/client/client.go index b6cbd629d9..c1a6b6252a 100644 --- a/experimental/certificate_auth/example/client/client.go +++ b/experimental/certificate_auth/example/client/client.go @@ -53,6 +53,9 @@ func main() { log.Fatalln(err) } + // Uncomment the following line to set UID via Client-Cert-Subject header and change the UID 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") + resp, err := client.Do(req) if err != nil { log.Fatalln(err) From 9a3ac8f02b3e99ebd460f093add95ed847330336 Mon Sep 17 00:00:00 2001 From: Jagan Parthiban Date: Thu, 23 May 2024 19:20:06 +0530 Subject: [PATCH 3/7] update ParseClientCertificateUID to handle empty subject name in OID --- traffic_ops/traffic_ops_golang/auth/certificate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/traffic_ops/traffic_ops_golang/auth/certificate.go b/traffic_ops/traffic_ops_golang/auth/certificate.go index e5dbf36eac..cb5adf7ac7 100644 --- a/traffic_ops/traffic_ops_golang/auth/certificate.go +++ b/traffic_ops/traffic_ops_golang/auth/certificate.go @@ -180,7 +180,7 @@ func ParseClientCertificateUID(cert *x509.Certificate) (string, error) { foundUID = true } } - if !foundUID { + if !foundUID || uid == "" { err = fmt.Errorf("no UID found") } return uid, err From 597d13280710f2533e4ec6dfdff7d95bec7613b4 Mon Sep 17 00:00:00 2001 From: Jagan Parthiban Date: Thu, 23 May 2024 19:22:24 +0530 Subject: [PATCH 4/7] update login error message to differentiate triedAuthentication vs Authenticated --- traffic_ops/traffic_ops_golang/login/login.go | 40 +++++++++++++------ 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/traffic_ops/traffic_ops_golang/login/login.go b/traffic_ops/traffic_ops_golang/login/login.go index 861bbf9ebd..dc3df1313e 100644 --- a/traffic_ops/traffic_ops_golang/login/login.go +++ b/traffic_ops/traffic_ops_golang/login/login.go @@ -29,6 +29,7 @@ import ( "errors" "fmt" "html/template" + "net" "net/http" "net/url" "path/filepath" @@ -109,36 +110,39 @@ Subject: {{.InstanceName}} Password Reset Request` + "\r\n\r" + ` `)) -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 { +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, bool) { // No certs provided by the client. Skip to form authentication if r.TLS == nil || len(r.TLS.PeerCertificates) == 0 { - return false + return false, false } // If no configuration is set, skip to form auth if cfg.ClientCertAuth == nil || len(cfg.ClientCertAuth.RootCertsDir) == 0 { - return false + return false, false } // Perform certificate verification to ensure it is valid against Root CAs err := auth.VerifyClientCertificate(r, cfg.ClientCertAuth.RootCertsDir, cfg.Insecure) if err != nil { log.Warnf("client cert auth: error attempting to verify client provided TLS certificate. err: %s\n", err) - return false + return true, false } // Client provided a verified certificate. Extract UID value. Try Certificate first and then HTTP Header clientCertSubject := r.Header.Get("Client-Cert-Subject") + remoteIp, _, _ := net.SplitHostPort(r.RemoteAddr) if username, err := auth.ParseClientCertificateUID(r.TLS.PeerCertificates[0]); err != nil { - log.Infof("parsing client certificate: %s\n", err) + log.Warnf("parsing client certificate: %s\n", err) if username, err = extractUID(clientCertSubject); err != nil { - log.Errorf("extracting UID from http header client-cert-subject: %s\n", err) - return false + log.Warnf("extracting UID from http header client-cert-subject: %s\n", err) + return true, false } else { form.Username = username + log.Infof("extracted UID from http-header client-cert-subject: %s , remoteAddress: %s", form.Username, remoteIp) } } else { form.Username = username + log.Infof("extracted UID from client certificate: %s , remoteAddress: %s", form.Username, remoteIp) } // Check if user exists locally (TODB) and has a role. @@ -146,7 +150,7 @@ func clientCertAuthentication(w http.ResponseWriter, r *http.Request, db *sqlx.D 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 + return true, false } if err != nil { log.Warnf("client cert auth: checking local user: %s\n", err) @@ -160,7 +164,7 @@ func clientCertAuthentication(w http.ResponseWriter, r *http.Request, db *sqlx.D } } - return authenticated + return true, authenticated } // LoginHandler first attempts to verify and parse user information from an optionally @@ -169,6 +173,7 @@ func clientCertAuthentication(w http.ResponseWriter, r *http.Request, db *sqlx.D func LoginHandler(db *sqlx.DB, cfg config.Config) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { defer r.Body.Close() + triedAuthentication := false authenticated := false form := auth.PasswordForm{} var resp tc.Alerts @@ -178,18 +183,27 @@ func LoginHandler(db *sqlx.DB, cfg config.Config) http.HandlerFunc { // 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. - authenticated = clientCertAuthentication(w, r, db, cfg, dbCtx, cancelTx, &form, authenticated) + triedAuthentication, authenticated = clientCertAuthentication(w, r, db, cfg, dbCtx, cancelTx, &form, authenticated) + + // 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") + } // 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 { + if form.Username == "" { + log.Infof("could not extract UID from client certificate or HTTP header & could not successfully authenticate using client certificates") + } else { + log.Infof("user %s could not be successfully authenticated using client certificates", form.Username) + } // 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) + api.HandleErr(w, r, nil, http.StatusBadRequest, errors.New("certificate-based auth failed. hence username and password are required"), nil) return } From a2047c8217673438aa36a34a1104ef14b43c7f8e Mon Sep 17 00:00:00 2001 From: Jagan Parthiban Date: Thu, 23 May 2024 19:51:59 +0530 Subject: [PATCH 5/7] added CHANGELOG.md entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index be7d1090ea..29b93e8797 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). ## [unreleased] ### Added +- -[#8013](https://github.com/apache/trafficcontrol/pull/8013) *Traffic Ops* Update TO to handle Client-Cert-Subject HTTP header for client cert authentication. - [#8014](https://github.com/apache/trafficcontrol/pull/8014) *Traffic Ops* Added logs to indicate which mechanism a client used to login to TO. - [#7812](https://github.com/apache/trafficcontrol/pull/7812) *Traffic Portal*: Expose the `configUpdateFailed` and `revalUpdateFailed` fields on the server table. - [#7870](https://github.com/apache/trafficcontrol/pull/7870) *Traffic Portal*: Adds a hyperlink to the DSR page to the DS itself for ease of navigation. From 405c254079d4faa8c10d8afd30ebb2ea66e1ce2f Mon Sep 17 00:00:00 2001 From: "Parthiban, Jagan" Date: Thu, 30 May 2024 15:02:16 +0530 Subject: [PATCH 6/7] Update else statement for successful authentication using certs --- traffic_ops/traffic_ops_golang/login/login.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/traffic_ops/traffic_ops_golang/login/login.go b/traffic_ops/traffic_ops_golang/login/login.go index dc3df1313e..b239c55d4e 100644 --- a/traffic_ops/traffic_ops_golang/login/login.go +++ b/traffic_ops/traffic_ops_golang/login/login.go @@ -246,7 +246,7 @@ func LoginHandler(db *sqlx.DB, cfg config.Config) http.HandlerFunc { log.Infof("user %s successfully authenticated using LDAP", form.Username) } } - } else { + } else if triedAuthentication && authenticated { log.Infof("user %s successfully authenticated using client certificates", form.Username) } From 35753e01c8818d7a22614aef7e9c3aa5adc6108b Mon Sep 17 00:00:00 2001 From: "Parthiban, Jagan" Date: Fri, 31 May 2024 14:14:33 +0530 Subject: [PATCH 7/7] Fix PR Review Comments --- traffic_ops/traffic_ops_golang/login/login.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/traffic_ops/traffic_ops_golang/login/login.go b/traffic_ops/traffic_ops_golang/login/login.go index b239c55d4e..b3dd97cf44 100644 --- a/traffic_ops/traffic_ops_golang/login/login.go +++ b/traffic_ops/traffic_ops_golang/login/login.go @@ -185,13 +185,8 @@ func LoginHandler(db *sqlx.DB, cfg config.Config) http.HandlerFunc { // be a successful login. triedAuthentication, authenticated = clientCertAuthentication(w, r, db, cfg, dbCtx, cancelTx, &form, authenticated) - // 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") - } - // Failed certificate-based auth, perform standard form auth - if triedAuthentication && !authenticated { + if !authenticated { if form.Username == "" { log.Infof("could not extract UID from client certificate or HTTP header & could not successfully authenticate using client certificates") } else {