From dc482bf9058faf7a192486eb52caa1d42646f6b3 Mon Sep 17 00:00:00 2001 From: Daniel Bennett Date: Mon, 10 Mar 2025 12:12:02 -0400 Subject: [PATCH] auth: redact auth method client secret (#25328) OIDC client secrets that users provide in auth method configuration are, well, secret, so we should hide them from API calls and event streams. --- .changelog/25328.txt | 3 +++ nomad/acl_endpoint.go | 2 +- nomad/state/events.go | 2 ++ nomad/structs/acl.go | 15 +++++++++++++++ nomad/structs/acl_test.go | 18 ++++++++++++++++++ 5 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 .changelog/25328.txt diff --git a/.changelog/25328.txt b/.changelog/25328.txt new file mode 100644 index 000000000..30af6a1ef --- /dev/null +++ b/.changelog/25328.txt @@ -0,0 +1,3 @@ +```release-note:security +auth: Redact OIDC client secret from API responses and event stream ([CVE-2025-1296](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2025-1296)) +``` diff --git a/nomad/acl_endpoint.go b/nomad/acl_endpoint.go index 61cafae09..a80cb5837 100644 --- a/nomad/acl_endpoint.go +++ b/nomad/acl_endpoint.go @@ -2084,7 +2084,7 @@ func (a *ACL) GetAuthMethod( // We didn't encounter an error looking up the index; set the auth // method on the reply and exit successfully. - reply.AuthMethod = out + reply.AuthMethod = out.Sanitize() return nil }, }) diff --git a/nomad/state/events.go b/nomad/state/events.go index 29abd6d9f..67e3fd93a 100644 --- a/nomad/state/events.go +++ b/nomad/state/events.go @@ -110,6 +110,7 @@ func eventFromChange(change memdb.Change) (structs.Event, bool) { if !ok { return structs.Event{}, false } + before = before.Sanitize() return structs.Event{ Topic: structs.TopicACLAuthMethod, Key: before.Name, @@ -283,6 +284,7 @@ func eventFromChange(change memdb.Change) (structs.Event, bool) { if !ok { return structs.Event{}, false } + after = after.Sanitize() return structs.Event{ Topic: structs.TopicACLAuthMethod, Key: after.Name, diff --git a/nomad/structs/acl.go b/nomad/structs/acl.go index afdea9b60..41aea10e7 100644 --- a/nomad/structs/acl.go +++ b/nomad/structs/acl.go @@ -978,6 +978,21 @@ func (a *ACLAuthMethod) Validate(minTTL, maxTTL time.Duration) error { return mErr.ErrorOrNil() } +// Sanitize returns a copy of the ACLAuthMethod with any secrets redacted +func (a *ACLAuthMethod) Sanitize() *ACLAuthMethod { + if a == nil || a.Config == nil { + return a + } + // copy to ensure we do not mutate a pointer pulled directly out of state. + clean := a.Copy() + // clean nested structs here, so it's obvious what all is being cleaned + // in one spot, rather than follow a stack of sanitization calls. + if clean.Config.OIDCClientSecret != "" { + clean.Config.OIDCClientSecret = "redacted" + } + return clean +} + // TokenLocalityIsGlobal returns whether the auth method creates global ACL // tokens or not. func (a *ACLAuthMethod) TokenLocalityIsGlobal() bool { diff --git a/nomad/structs/acl_test.go b/nomad/structs/acl_test.go index 8c1abbd40..90bc6b804 100644 --- a/nomad/structs/acl_test.go +++ b/nomad/structs/acl_test.go @@ -1263,6 +1263,24 @@ func TestACLAuthMethod_Validate(t *testing.T) { } } +// Sanitize method should redact sensitive values +func TestACLAuthMethod_Sanitize(t *testing.T) { + // these just shouldn't nil panic + am := &ACLAuthMethod{} + am.Sanitize() + am.Config = &ACLAuthMethodConfig{} + am.Sanitize() + + t.Run("client secret", func(t *testing.T) { + am := am.Copy() + am.Config.OIDCClientSecret = "very private secret" + dirty := am.Config.OIDCClientSecret + clean := am.Sanitize().Config.OIDCClientSecret + must.Eq(t, "very private secret", dirty) + must.Eq(t, "redacted", clean) + }) +} + func TestACLAuthMethod_Merge(t *testing.T) { ci.Parallel(t)