From e9c9c69d28fa861fe9a381543d57ada8b257433b Mon Sep 17 00:00:00 2001 From: matthew-pilot Date: Fri, 29 May 2026 18:28:20 +0000 Subject: [PATCH] fix: add HMAC-SHA256 signing and verification to identity webhook VerifyToken (PILOT-240) VerifyToken sends tokens to the configured identity webhook via HTTP POST but neither signed the request nor verified the response. An attacker who compromises the webhook endpoint, hijacks its DNS, or MITMs its TLS could return {verified:true, external_id: victim} for any token, bypassing the entire IDP-backed registration model. Changes: - Store gains identityWebhookSecret field + Set/Get accessors - VerifyToken uses http.NewRequest + client.Do (was client.Post) - When secret is set: outbound requests carry X-Pilot-Signature-256 (matching the PILOT-239 webhook event pattern) - When secret is set: response X-Pilot-Signature-256 is verified; unsigned or mis-signed responses are rejected - When secret is empty: full backward compatibility (no headers) Tested: 6 new tests covering signed request verification, unsigned response rejection, wrong-signature rejection, no-signature-when-empty, empty-token shortcut with secret, and getter/setter roundtrip. --- identity.go | 11 ++++ identity/identity.go | 64 ++++++++++++++++++-- identity/zz_store_test.go | 122 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 193 insertions(+), 4 deletions(-) diff --git a/identity.go b/identity.go index 884a49f..18a62cb 100644 --- a/identity.go +++ b/identity.go @@ -152,6 +152,17 @@ func (s *Server) GetIdentityWebhookURL() string { return s.identity.GetWebhookURL() } +// SetIdentityWebhookSecret sets the HMAC-SHA256 pre-shared secret for +// identity webhook request/response signing (PILOT-240). +func (s *Server) SetIdentityWebhookSecret(secret string) { + s.identity.SetIdentityWebhookSecret(secret) +} + +// GetIdentityWebhookSecret returns the current identity webhook HMAC secret. +func (s *Server) GetIdentityWebhookSecret() string { + return s.identity.GetIdentityWebhookSecret() +} + func (s *Server) provisionCallbacks() identpkg.ProvisionCallbacks { return identpkg.ProvisionCallbacks{ FindOrCreateNetwork: s.findOrCreateNetwork, diff --git a/identity/identity.go b/identity/identity.go index 74aa8f2..2995ebb 100644 --- a/identity/identity.go +++ b/identity/identity.go @@ -13,6 +13,7 @@ import ( "crypto/rsa" "crypto/sha256" "encoding/base64" + "encoding/hex" "encoding/json" "fmt" "io" @@ -141,9 +142,10 @@ type Store struct { nodes NodeView cb Callbacks - mu sync.RWMutex - identityWebhookURL string - idpConfig *BlueprintIdentityProvider + mu sync.RWMutex + identityWebhookURL string + identityWebhookSecret string + idpConfig *BlueprintIdentityProvider jwksCache *JWKSCache } @@ -179,11 +181,34 @@ func (st *Store) GetWebhookURL() string { return st.identityWebhookURL } +// SetIdentityWebhookSecret sets the HMAC-SHA256 pre-shared secret for +// identity webhook request/response signing (PILOT-240). When non-empty, +// VerifyToken signs outbound requests and verifies response signatures. +func (st *Store) SetIdentityWebhookSecret(secret string) { + st.mu.Lock() + st.identityWebhookSecret = secret + st.mu.Unlock() +} + +// GetIdentityWebhookSecret returns the currently configured identity +// webhook HMAC secret. +func (st *Store) GetIdentityWebhookSecret() string { + st.mu.RLock() + defer st.mu.RUnlock() + return st.identityWebhookSecret +} + // VerifyToken sends the token to the configured identity webhook and returns // the verified external ID. Returns ("", nil) if no webhook is configured. +// +// When a webhook secret is configured, outbound requests carry an +// X-Pilot-Signature-256 HMAC-SHA256 header, and the response MUST include +// a matching X-Pilot-Signature-256 header — unsigned responses are rejected +// (PILOT-240). func (st *Store) VerifyToken(token string) (string, error) { st.mu.RLock() url := st.identityWebhookURL + secret := st.identityWebhookSecret st.mu.RUnlock() if url == "" { @@ -198,7 +223,21 @@ func (st *Store) VerifyToken(token string) (string, error) { return "", fmt.Errorf("marshal identity request: %w", err) } - resp, err := sharedHTTPClient.Post(url, "application/json", bytes.NewReader(body)) + req, err := http.NewRequest(http.MethodPost, url, bytes.NewReader(body)) + if err != nil { + return "", fmt.Errorf("build identity request: %w", err) + } + req.Header.Set("Content-Type", "application/json") + + // HMAC-SHA256 request signing (PILOT-240): when a secret is configured, + // sign the request body so the webhook can verify the caller. + if secret != "" { + reqMac := hmac.New(sha256.New, []byte(secret)) + reqMac.Write(body) + req.Header.Set("X-Pilot-Signature-256", hex.EncodeToString(reqMac.Sum(nil))) + } + + resp, err := sharedHTTPClient.Do(req) if err != nil { slog.Warn("identity webhook request failed", "error", err) return "", fmt.Errorf("identity verification failed: %w", err) @@ -214,6 +253,23 @@ func (st *Store) VerifyToken(token string) (string, error) { return "", fmt.Errorf("read identity response: %w", err) } + // HMAC-SHA256 response verification (PILOT-240): when a secret is + // configured, the response MUST carry a valid X-Pilot-Signature-256 + // header. Unsigned or mis-signed responses are rejected to prevent + // webhook-spoofing attacks. + if secret != "" { + respSig := resp.Header.Get("X-Pilot-Signature-256") + if respSig == "" { + return "", fmt.Errorf("identity webhook response missing X-Pilot-Signature-256 header") + } + respMac := hmac.New(sha256.New, []byte(secret)) + respMac.Write(respBody) + expected := hex.EncodeToString(respMac.Sum(nil)) + if !hmac.Equal([]byte(respSig), []byte(expected)) { + return "", fmt.Errorf("identity webhook response signature mismatch") + } + } + var result identityVerifyResponse if err := json.Unmarshal(respBody, &result); err != nil { return "", fmt.Errorf("parse identity response: %w", err) diff --git a/identity/zz_store_test.go b/identity/zz_store_test.go index 27da633..49eb8e6 100644 --- a/identity/zz_store_test.go +++ b/identity/zz_store_test.go @@ -3,6 +3,9 @@ package identity import ( + "crypto/hmac" + "crypto/sha256" + "encoding/hex" "encoding/json" "net/http" "net/http/httptest" @@ -278,3 +281,122 @@ func TestJsonUint32_Helper(t *testing.T) { t.Errorf("non-float: got %d", got) } } + +// TestStore_SetIdentityWebhookSecret_GetRoundtrip (PILOT-240). +func TestStore_SetIdentityWebhookSecret_GetRoundtrip(t *testing.T) { + t.Parallel() + st := newTestStore() + if got := st.GetIdentityWebhookSecret(); got != "" { + t.Errorf("initial = %q, want empty", got) + } + st.SetIdentityWebhookSecret("my-secret") + if got := st.GetIdentityWebhookSecret(); got != "my-secret" { + t.Errorf("after Set = %q", got) + } + st.SetIdentityWebhookSecret("") + if got := st.GetIdentityWebhookSecret(); got != "" { + t.Errorf("after clear = %q", got) + } +} + +// TestStore_VerifyToken_SignsRequest (PILOT-240). +func TestStore_VerifyToken_SignsRequest(t *testing.T) { + t.Parallel() + const secret = "my-secret" + var gotSig string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotSig = r.Header.Get("X-Pilot-Signature-256") + respBody := []byte(`{"verified":true,"external_id":"user-42"}`) + mac := hmac.New(sha256.New, []byte(secret)) + mac.Write(respBody) + w.Header().Set("X-Pilot-Signature-256", hex.EncodeToString(mac.Sum(nil))) + w.Write(respBody) + })) + defer srv.Close() + + st := newTestStore() + st.SetWebhookURL(srv.URL) + st.SetIdentityWebhookSecret(secret) + + got, err := st.VerifyToken("my-token") + if err != nil { + t.Fatalf("VerifyToken: %v", err) + } + if got != "user-42" { + t.Errorf("got %q, want user-42", got) + } + if gotSig == "" { + t.Fatal("X-Pilot-Signature-256 request header not set") + } + expectedBody, _ := json.Marshal(identityVerifyRequest{Token: "my-token"}) + mac := hmac.New(sha256.New, []byte(secret)) + mac.Write(expectedBody) + want := hex.EncodeToString(mac.Sum(nil)) + if !hmac.Equal([]byte(gotSig), []byte(want)) { + t.Errorf("request HMAC mismatch: got %s, want %s", gotSig, want) + } +} + +// TestStore_VerifyToken_RejectsUnsignedResponse (PILOT-240). +func TestStore_VerifyToken_RejectsUnsignedResponse(t *testing.T) { + t.Parallel() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte(`{"verified":true,"external_id":"user-42"}`)) + })) + defer srv.Close() + st := newTestStore() + st.SetWebhookURL(srv.URL) + st.SetIdentityWebhookSecret("my-secret") + if _, err := st.VerifyToken("my-token"); err == nil { + t.Fatal("expected rejection when response is unsigned") + } +} + +// TestStore_VerifyToken_RejectsWrongSignature (PILOT-240). +func TestStore_VerifyToken_RejectsWrongSignature(t *testing.T) { + t.Parallel() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + respBody := []byte(`{"verified":true,"external_id":"user-42"}`) + w.Header().Set("X-Pilot-Signature-256", "deadbeef") + w.Write(respBody) + })) + defer srv.Close() + st := newTestStore() + st.SetWebhookURL(srv.URL) + st.SetIdentityWebhookSecret("my-secret") + if _, err := st.VerifyToken("my-token"); err == nil { + t.Fatal("expected rejection when response signature mismatches") + } +} + +// TestStore_VerifyToken_NoSignatureWithoutSecret (PILOT-240): backward compat. +func TestStore_VerifyToken_NoSignatureWithoutSecret(t *testing.T) { + t.Parallel() + var gotSig string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotSig = r.Header.Get("X-Pilot-Signature-256") + w.Write([]byte(`{"verified":true,"external_id":"user-42"}`)) + })) + defer srv.Close() + st := newTestStore() + st.SetWebhookURL(srv.URL) + got, err := st.VerifyToken("my-token") + if err != nil { + t.Fatalf("VerifyToken: %v", err) + } + if got != "user-42" || gotSig != "" { + t.Errorf("got %q sig=%q, want user-42 sig empty", got, gotSig) + } +} + +// TestStore_VerifyToken_WithSecret_EmptyTokenShortCircuits (PILOT-240). +func TestStore_VerifyToken_WithSecret_EmptyTokenShortCircuits(t *testing.T) { + t.Parallel() + st := newTestStore() + st.SetWebhookURL("https://idp/verify") + st.SetIdentityWebhookSecret("my-secret") + got, err := st.VerifyToken("") + if err != nil || got != "" { + t.Errorf("empty token with secret: got (%q, %v)", got, err) + } +}