From 51e80f24ef25252d65476cf33e31242e43919af9 Mon Sep 17 00:00:00 2001 From: stagswtf <142280349+stagswtf@users.noreply.github.com> Date: Tue, 28 Oct 2025 00:37:25 -0700 Subject: [PATCH] fix: use GetSecret() in ticket.go makeCookie to respect cookie-secret-file (#3228) * fix: use GetSecret() in ticket.go makeCookie The makeCookie method in ticket.go was using t.options.Secret directly, which meant cookie-secret-file was not being respected. Updated to use GetSecret() which handles both cookie-secret and cookie-secret-file properly. Also added test coverage for cookie-secret-file functionality. Fixes #3224 Signed-off-by: stagswtf <142280349+stagswtf@users.noreply.github.com> * docs: update CHANGELOG.md for cookie-secret-file fix Signed-off-by: stagswtf <142280349+stagswtf@users.noreply.github.com> * correct PR link and undo file formatting Signed-off-by: stagswtf <142280349+stagswtf@users.noreply.github.com> * fix: error wrapping Signed-off-by: Jan Larwig --------- Signed-off-by: stagswtf <142280349+stagswtf@users.noreply.github.com> Signed-off-by: Jan Larwig Co-authored-by: Jan Larwig --- CHANGELOG.md | 4 ++- pkg/sessions/persistence/ticket.go | 11 +++++-- pkg/sessions/tests/session_store_tests.go | 37 +++++++++++++++++++++++ 3 files changed, 48 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f2c9b441..5aefaafa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ ## Changes since v7.12.0 +- [#3228](https://github.com/oauth2-proxy/oauth2-proxy/pull/3228) fix: use GetSecret() in ticket.go makeCookie to respect cookie-secret-file (@stagswtf) + # V7.12.0 ## Release Highlights @@ -119,7 +121,7 @@ For detailed information, migration guidance, and security implications, see the - 🕵️‍♀️ Vulnerabilities have been addressed - [CVE-2025-22871](https://github.com/advisories/GHSA-g9pc-8g42-g6vq) - 🐛 Squashed some bugs - + ## Important Notes ## Breaking Changes diff --git a/pkg/sessions/persistence/ticket.go b/pkg/sessions/persistence/ticket.go index 7855db45..56d6bd9b 100644 --- a/pkg/sessions/persistence/ticket.go +++ b/pkg/sessions/persistence/ticket.go @@ -233,12 +233,17 @@ func (t *ticket) clearCookie(rw http.ResponseWriter, req *http.Request) { // makeCookie makes a cookie, signing the value if present func (t *ticket) makeCookie(req *http.Request, value string, expires time.Duration, now time.Time) (*http.Cookie, error) { if value != "" { - var err error - value, err = encryption.SignedValue(t.options.Secret, t.options.Name, []byte(value), now) + secret, err := t.options.GetSecret() if err != nil { - return nil, err + return nil, fmt.Errorf("retrieving secret failed: %w", err) + } + + value, err = encryption.SignedValue(secret, t.options.Name, []byte(value), now) + if err != nil { + return nil, fmt.Errorf("signing cookie value failed: %w", err) } } + return cookies.MakeCookieFromOptions( req, t.options.Name, diff --git a/pkg/sessions/tests/session_store_tests.go b/pkg/sessions/tests/session_store_tests.go index a4818ef2..05b67d8d 100644 --- a/pkg/sessions/tests/session_store_tests.go +++ b/pkg/sessions/tests/session_store_tests.go @@ -4,6 +4,7 @@ import ( "crypto/rand" "net/http" "net/http/httptest" + "os" "strconv" "strings" "time" @@ -133,6 +134,42 @@ func RunSessionStoreTests(newSS NewSessionStoreFunc, persistentFastForward Persi PersistentSessionStoreInterfaceTests(&input) } }) + + Context("with cookie secret file", func() { + var tmpfile *os.File + var err error + BeforeEach(func() { + tmpfile, err = os.CreateTemp("", "cookie-secret-test") + secretBytes := make([]byte, 32) + tmpfile.Write(secretBytes) + tmpfile.Close() + + input.cookieOpts = &options.Cookie{ + Name: "_oauth2_proxy_file", + Path: "/", + Expire: time.Duration(168) * time.Hour, + Refresh: time.Duration(1) * time.Hour, + Secure: true, + HTTPOnly: true, + SameSite: "", + Secret: "", + SecretFile: tmpfile.Name(), + } + ss, err = newSS(opts, input.cookieOpts) + Expect(err).ToNot(HaveOccurred()) + }) + + AfterEach(func() { + if tmpfile != nil { + os.Remove(tmpfile.Name()) + } + }) + + SessionStoreInterfaceTests(&input) + if persistentFastForward != nil { + PersistentSessionStoreInterfaceTests(&input) + } + }) }) }