From 793bcc4b61ebbb0a67c642c249a968f27ee32cdf Mon Sep 17 00:00:00 2001
From: Bob Callaway <bcallaway@google.com>
Date: Mon, 26 Sep 2022 15:16:18 -0400
Subject: [PATCH] address review comments
Signed-off-by: Bob Callaway <bcallaway@google.com>
---
server/handlers.go | 4 +++-
storage/sql/crud.go | 3 ++-
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/server/handlers.go b/server/handlers.go
index d982f0be..11dcdd07 100755
--- a/server/handlers.go
+++ b/server/handlers.go
@@ -502,6 +502,8 @@ func (s *Server) finalizeLogin(identity connector.Identity, authReq storage.Auth
// 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)
@@ -576,7 +578,7 @@ func (s *Server) handleApproval(w http.ResponseWriter, r *http.Request) {
// build expected hmac with secret key
h := hmac.New(sha256.New, authReq.HMACKey)
- h.Write([]byte(r.FormValue("req")))
+ h.Write([]byte(authReq.ID))
expectedMAC := h.Sum(nil)
// constant time comparison
if !hmac.Equal(mac, expectedMAC) {
diff --git a/storage/sql/crud.go b/storage/sql/crud.go
index 8ac4204f..1583c177 100644
--- a/storage/sql/crud.go
+++ b/storage/sql/crud.go
@@ -144,7 +144,8 @@ func (c *conn) CreateAuthRequest(a storage.AuthRequest) error {
a.Claims.Email, a.Claims.EmailVerified, encoder(a.Claims.Groups),
a.ConnectorID, a.ConnectorData,
a.Expiry,
- a.PKCE.CodeChallenge, a.PKCE.CodeChallengeMethod, a.HMACKey,
+ a.PKCE.CodeChallenge, a.PKCE.CodeChallengeMethod,
+ a.HMACKey,
)
if err != nil {
if c.alreadyExistsCheck(err) {
--
GitLab