From 417376341a0cba0e422262b641d82dae6f11bffc Mon Sep 17 00:00:00 2001
From: Victor Rodriguez <vrizo@hashicorp.com>
Date: Fri, 24 May 2024 11:12:23 -0400
Subject: [PATCH] Use hash_algorithm parameter on Transit's verify HMAC
 requests. (#27211)

Use hash_algorithm parameter on Transit's verify HMAC requests.

Parameter 'algorithm' has been deprecated in favour of 'hash_algorithm', so
update the pathHMACVerify() handler to use it when it is present.
---
 builtin/logical/transit/path_hmac.go      | 14 +++++++-
 builtin/logical/transit/path_hmac_test.go | 41 ++++++++++++++++++-----
 changelog/27211.txt                       |  3 ++
 3 files changed, 48 insertions(+), 10 deletions(-)
 create mode 100644 changelog/27211.txt

diff --git a/builtin/logical/transit/path_hmac.go b/builtin/logical/transit/path_hmac.go
index 0465b8dfa2be..f71c9516ea5f 100644
--- a/builtin/logical/transit/path_hmac.go
+++ b/builtin/logical/transit/path_hmac.go
@@ -257,7 +257,19 @@ func (b *backend) pathHMACVerify(ctx context.Context, req *logical.Request, d *f
 	name := d.Get("name").(string)
 	algorithm := d.Get("urlalgorithm").(string)
 	if algorithm == "" {
-		algorithm = d.Get("algorithm").(string)
+		hashAlgorithmRaw, hasHashAlgorithm := d.GetOk("hash_algorithm")
+		algorithmRaw, hasAlgorithm := d.GetOk("algorithm")
+
+		// As `algorithm` is deprecated, make sure we only read it if
+		// `hash_algorithm` is not present.
+		switch {
+		case hasHashAlgorithm:
+			algorithm = hashAlgorithmRaw.(string)
+		case hasAlgorithm:
+			algorithm = algorithmRaw.(string)
+		default:
+			algorithm = d.Get("hash_algorithm").(string)
+		}
 	}
 
 	// Get the policy
diff --git a/builtin/logical/transit/path_hmac_test.go b/builtin/logical/transit/path_hmac_test.go
index 4fa0fbce318c..3f21106c4cc9 100644
--- a/builtin/logical/transit/path_hmac_test.go
+++ b/builtin/logical/transit/path_hmac_test.go
@@ -94,17 +94,40 @@ func TestTransit_HMAC(t *testing.T) {
 			}
 
 			// Now verify
+			verify := func() {
+				t.Helper()
+
+				resp, err = b.HandleRequest(context.Background(), req)
+				if err != nil {
+					t.Fatalf("%v: %v", err, resp)
+				}
+				if resp == nil {
+					t.Fatal("expected non-nil response")
+				}
+				if errStr, ok := resp.Data["error"]; ok {
+					t.Fatalf("error validating hmac: %s", errStr)
+				}
+				if resp.Data["valid"].(bool) == false {
+					t.Fatalf(fmt.Sprintf("error validating hmac;\nreq:\n%#v\nresp:\n%#v", *req, *resp))
+				}
+			}
 			req.Path = strings.ReplaceAll(req.Path, "hmac", "verify")
 			req.Data["hmac"] = value.(string)
-			resp, err = b.HandleRequest(context.Background(), req)
-			if err != nil {
-				t.Fatalf("%v: %v", err, resp)
-			}
-			if resp == nil {
-				t.Fatal("expected non-nil response")
-			}
-			if resp.Data["valid"].(bool) == false {
-				panic(fmt.Sprintf("error validating hmac;\nreq:\n%#v\nresp:\n%#v", *req, *resp))
+			verify()
+
+			// If `algorithm` parameter is used, try with `hash_algorithm` as well
+			if algorithm, ok := req.Data["algorithm"]; ok {
+				// Note that `hash_algorithm` takes precedence over `algorithm`, since the
+				// latter is deprecated.
+				req.Data["hash_algorithm"] = algorithm
+				req.Data["algorithm"] = "xxx"
+				defer func() {
+					// Restore the req fields, since it is re-used by the tests below
+					delete(req.Data, "hash_algorithm")
+					req.Data["algorithm"] = algorithm
+				}()
+
+				verify()
 			}
 		}
 
diff --git a/changelog/27211.txt b/changelog/27211.txt
new file mode 100644
index 000000000000..26bf725ebff3
--- /dev/null
+++ b/changelog/27211.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+secrets/transit: Use 'hash_algorithm' parameter if present in HMAC verify requests. Otherwise fall back to deprecated 'algorithm' parameter.
+```