Merge pull request from GHSA-vh7g-p26c-j2cw

Add HMAC protection on /approval endpoint

server/handlers.go

@@ -1,6 +1,7 @@
 package server
 
 import (
+	"crypto/hmac"
 	"crypto/sha256"
 	"crypto/subtle"
 	"encoding/base64"
@@ -499,7 +500,15 @@ func (s *Server) finalizeLogin(identity connector.Identity, authReq storage.Auth
 	s.logger.Infof("login successful: connector %q, username=%q, preferred_username=%q, email=%q, groups=%q",
 		authReq.ConnectorID, claims.Username, claims.PreferredUsername, email, claims.Groups)
 
-	returnURL := path.Join(s.issuerURL.Path, "/approval") + "?req=" + authReq.ID
+	// TODO: if s.skipApproval or !authReq.ForceApprovalPrompt, we can skip the redirect to /approval and go ahead and send code
+
+	// an HMAC is used here to ensure that the request ID is unpredictable, ensuring that an attacker who intercepted the original
+	// flow would be unable to poll for the result at the /approval endpoint
+	h := hmac.New(sha256.New, authReq.HMACKey)
+	h.Write([]byte(authReq.ID))
+	mac := h.Sum(nil)
+
+	returnURL := path.Join(s.issuerURL.Path, "/approval") + "?req=" + authReq.ID + "&hmac=" + base64.RawURLEncoding.EncodeToString(mac)
 	_, ok := conn.(connector.RefreshConnector)
 	if !ok {
 		return returnURL, nil
@@ -544,6 +553,17 @@ func (s *Server) finalizeLogin(identity connector.Identity, authReq storage.Auth
 }
 
 func (s *Server) handleApproval(w http.ResponseWriter, r *http.Request) {
+	macEncoded := r.FormValue("hmac")
+	if macEncoded == "" {
+		s.renderError(r, w, http.StatusUnauthorized, "Unauthorized request")
+		return
+	}
+	mac, err := base64.RawURLEncoding.DecodeString(macEncoded)
+	if err != nil {
+		s.renderError(r, w, http.StatusUnauthorized, "Unauthorized request")
+		return
+	}
+
 	authReq, err := s.storage.GetAuthRequest(r.FormValue("req"))
 	if err != nil {
 		s.logger.Errorf("Failed to get auth request: %v", err)
@@ -556,6 +576,16 @@ func (s *Server) handleApproval(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// build expected hmac with secret key
+	h := hmac.New(sha256.New, authReq.HMACKey)
+	h.Write([]byte(authReq.ID))
+	expectedMAC := h.Sum(nil)
+	// constant time comparison
+	if !hmac.Equal(mac, expectedMAC) {
+		s.renderError(r, w, http.StatusUnauthorized, "Unauthorized request")
+		return
+	}
+
 	switch r.Method {
 	case http.MethodGet:
 		if s.skipApproval {

server/oauth2.go

@@ -2,6 +2,7 @@ package server
 
 import (
 	"context"
+	"crypto"
 	"crypto/ecdsa"
 	"crypto/elliptic"
 	"crypto/rsa"
@@ -576,6 +577,7 @@ func (s *Server) parseAuthorizationRequest(r *http.Request) (*storage.AuthReques
 			CodeChallenge:       codeChallenge,
 			CodeChallengeMethod: codeChallengeMethod,
 		},
+		HMACKey: storage.NewHMACKey(crypto.SHA256),
 	}, nil
 }
 

storage/conformance/conformance.go

@@ -104,7 +104,8 @@ func testAuthRequestCRUD(t *testing.T, s storage.Storage) {
 			EmailVerified: true,
 			Groups:        []string{"a", "b"},
 		},
-		PKCE: codeChallenge,
+		PKCE:    codeChallenge,
+		HMACKey: []byte("hmac_key"),
 	}
 
 	identity := storage.Claims{Email: "foobar"}
@@ -137,6 +138,7 @@ func testAuthRequestCRUD(t *testing.T, s storage.Storage) {
 			EmailVerified: true,
 			Groups:        []string{"a"},
 		},
+		HMACKey: []byte("hmac_key"),
 	}
 
 	if err := s.CreateAuthRequest(a2); err != nil {
@@ -817,6 +819,7 @@ func testGC(t *testing.T, s storage.Storage) {
 			EmailVerified: true,
 			Groups:        []string{"a", "b"},
 		},
+		HMACKey: []byte("hmac_key"),
 	}
 
 	if err := s.CreateAuthRequest(a); err != nil {

storage/ent/client/authrequest.go

@@ -31,6 +31,7 @@ func (d *Database) CreateAuthRequest(authRequest storage.AuthRequest) error {
 		SetExpiry(authRequest.Expiry.UTC()).
 		SetConnectorID(authRequest.ConnectorID).
 		SetConnectorData(authRequest.ConnectorData).
+		SetHmacKey(authRequest.HMACKey).
 		Save(context.TODO())
 	if err != nil {
 		return convertDBError("create auth request: %w", err)
@@ -94,6 +95,7 @@ func (d *Database) UpdateAuthRequest(id string, updater func(old storage.AuthReq
 		SetExpiry(newAuthRequest.Expiry.UTC()).
 		SetConnectorID(newAuthRequest.ConnectorID).
 		SetConnectorData(newAuthRequest.ConnectorData).
+		SetHmacKey(newAuthRequest.HMACKey).
 		Save(context.TODO())
 	if err != nil {
 		return rollback(tx, "update auth request uploading: %w", err)

storage/ent/client/types.go

@@ -45,6 +45,7 @@ func toStorageAuthRequest(a *db.AuthRequest) storage.AuthRequest {
 			CodeChallenge:       a.CodeChallenge,
 			CodeChallengeMethod: a.CodeChallengeMethod,
 		},
+		HMACKey: a.HmacKey,
 	}
 }