git.iko.soy/typeable/oauth2-proxy
on master help sign in
Handle cookie signing errors
commit1c8c5b08d70aaa82e1331858d4196e850b8a2688 authorNick Meves <nick.meves@greenhouse.io> dateMon 20 Jul 2020, 18:18 (6 years ago) parent65c2283
6 files changed+64−39
compare with master
unifiedsplit
pkg/encryption/utils.go+19−18 pkg/encryption/utils_test.go+4−2 pkg/sessions/cookie/session_store.go+16−8 pkg/sessions/persistence/manager.go+5−1 pkg/sessions/persistence/ticket.go+17−8 pkg/sessions/tests/session_store_tests.go+3−2
pkg/encryption/utils.go+19−18
@@ -2,19 +2,16 @@ package encryption
22
33import (
44 "crypto/hmac"
5− "crypto/rand"
5+ // TODO (@NickMeves): Remove SHA1 signed cookie support in V7
66 "crypto/sha1" // #nosec G505
77 "crypto/sha256"
88 "encoding/base64"
99 "fmt"
1010 "hash"
11− "io"
1211 "net/http"
1312 "strconv"
1413 "strings"
1514 "time"
16−
17− "github.com/oauth2-proxy/oauth2-proxy/pkg/logger"
1815)
1916
2017// SecretBytes attempts to base64 decode the secret, if that fails it treats the secret as binary
@@ -69,40 +66,44 @@ func Validate(cookie *http.Cookie, seed string, expiration time.Duration) (value
6966}
7067
7168// SignedValue returns a cookie that is signed and can later be checked with Validate
72−func SignedValue(seed string, key string, value []byte, now time.Time) string {
69+func SignedValue(seed string, key string, value []byte, now time.Time) (string, error) {
7370 encodedValue := base64.URLEncoding.EncodeToString(value)
7471 timeStr := fmt.Sprintf("%d", now.Unix())
75− sig := cookieSignature(sha256.New, seed, key, encodedValue, timeStr)
72+ sig, err := cookieSignature(sha256.New, seed, key, encodedValue, timeStr)
73+ if err != nil {
74+ return "", err
75+ }
7676 cookieVal := fmt.Sprintf("%s|%s|%s", encodedValue, timeStr, sig)
77− return cookieVal
77+ return cookieVal, nil
7878}
7979
80−func cookieSignature(signer func() hash.Hash, args ...string) string {
80+func cookieSignature(signer func() hash.Hash, args ...string) (string, error) {
8181 h := hmac.New(signer, []byte(args[0]))
8282 for _, arg := range args[1:] {
8383 _, err := h.Write([]byte(arg))
84− // If signing fails, fail closed and return something that won't validate
8584 if err != nil {
86− garbage := make([]byte, 16)
87− if _, err := io.ReadFull(rand.Reader, garbage); err != nil {
88− logger.Fatal("HMAC & RNG functions both failing. Shutting down for security")
89− }
90− return base64.URLEncoding.EncodeToString(garbage)
85+ return "", err
9186 }
9287 }
9388 var b []byte
9489 b = h.Sum(b)
95− return base64.URLEncoding.EncodeToString(b)
90+ return base64.URLEncoding.EncodeToString(b), nil
9691}
9792
9893func checkSignature(signature string, args ...string) bool {
99− checkSig := cookieSignature(sha256.New, args...)
94+ checkSig, err := cookieSignature(sha256.New, args...)
95+ if err != nil {
96+ return false
97+ }
10098 if checkHmac(signature, checkSig) {
10199 return true
102100 }
103101
104− // TODO: After appropriate rollout window, remove support for SHA1
105− legacySig := cookieSignature(sha1.New, args...)
102+ // TODO (@NickMeves): Remove SHA1 signed cookie support in V7
103+ legacySig, err := cookieSignature(sha1.New, args...)
104+ if err != nil {
105+ return false
106+ }
106107 return checkHmac(signature, legacySig)
107108}
108109
pkg/encryption/utils_test.go+4−2
@@ -88,8 +88,10 @@ func TestSignAndValidate(t *testing.T) {
8888 value := base64.URLEncoding.EncodeToString([]byte("I am soooo encoded"))
8989 epoch := "123456789"
9090
91− sha256sig := cookieSignature(sha256.New, seed, key, value, epoch)
92− sha1sig := cookieSignature(sha1.New, seed, key, value, epoch)
91+ sha256sig, err := cookieSignature(sha256.New, seed, key, value, epoch)
92+ assert.NoError(t, err)
93+ sha1sig, err := cookieSignature(sha1.New, seed, key, value, epoch)
94+ assert.NoError(t, err)
9395
9496 assert.True(t, checkSignature(sha256sig, seed, key, value, epoch))
9597 // This should be switched to False after fully deprecating SHA1
pkg/sessions/cookie/session_store.go+16−8
@@ -44,8 +44,7 @@ func (s *SessionStore) Save(rw http.ResponseWriter, req *http.Request, ss *sessi
4444 if err != nil {
4545 return err
4646 }
47− s.setSessionCookie(rw, req, value, *ss.CreatedAt)
48− return nil
47+ return s.setSessionCookie(rw, req, value, *ss.CreatedAt)
4948}
5049
5150// Load reads sessions.SessionState information from Cookies within the
@@ -114,24 +113,33 @@ func sessionFromCookie(v []byte, c encryption.Cipher) (s *sessions.SessionState,
114113}
115114
116115// setSessionCookie adds the user's session cookie to the response
117−func (s *SessionStore) setSessionCookie(rw http.ResponseWriter, req *http.Request, val []byte, created time.Time) {
118− for _, c := range s.makeSessionCookie(req, val, created) {
116+func (s *SessionStore) setSessionCookie(rw http.ResponseWriter, req *http.Request, val []byte, created time.Time) error {
117+ cookies, err := s.makeSessionCookie(req, val, created)
118+ if err != nil {
119+ return err
120+ }
121+ for _, c := range cookies {
119122 http.SetCookie(rw, c)
120123 }
124+ return nil
121125}
122126
123127// makeSessionCookie creates an http.Cookie containing the authenticated user's
124128// authentication details
125−func (s *SessionStore) makeSessionCookie(req *http.Request, value []byte, now time.Time) []*http.Cookie {
129+func (s *SessionStore) makeSessionCookie(req *http.Request, value []byte, now time.Time) ([]*http.Cookie, error) {
126130 strValue := string(value)
127131 if strValue != "" {
128− strValue = encryption.SignedValue(s.Cookie.Secret, s.Cookie.Name, value, now)
132+ var err error
133+ strValue, err = encryption.SignedValue(s.Cookie.Secret, s.Cookie.Name, value, now)
134+ if err != nil {
135+ return nil, err
136+ }
129137 }
130138 c := s.makeCookie(req, s.Cookie.Name, strValue, s.Cookie.Expire, now)
131139 if len(c.String()) > maxCookieLength {
132− return splitCookie(c)
140+ return splitCookie(c), nil
133141 }
134− return []*http.Cookie{c}
142+ return []*http.Cookie{c}, nil
135143}
136144
137145func (s *SessionStore) makeCookie(req *http.Request, name string, value string, expiration time.Duration, now time.Time) *http.Cookie {
pkg/sessions/persistence/manager.go+5−1
@@ -48,7 +48,11 @@ func (m *Manager) Save(rw http.ResponseWriter, req *http.Request, s *sessions.Se
4848 if err != nil {
4949 return err
5050 }
51− tckt.setCookie(rw, req, s)
51+
52+ err = tckt.setCookie(rw, req, s)
53+ if err != nil {
54+ return err
55+ }
5256
5357 return nil
5458}
pkg/sessions/persistence/ticket.go+17−8
@@ -148,33 +148,42 @@ func (t *ticket) clearSession(clearer clearFunc) error {
148148}
149149
150150// setCookie sets the encoded ticket as a cookie
151−func (t *ticket) setCookie(rw http.ResponseWriter, req *http.Request, s *sessions.SessionState) {
152− ticketCookie := t.makeCookie(
151+func (t *ticket) setCookie(rw http.ResponseWriter, req *http.Request, s *sessions.SessionState) error {
152+ ticketCookie, err := t.makeCookie(
153153 req,
154154 t.encodeTicket(),
155155 t.options.Expire,
156156 *s.CreatedAt,
157157 )
158+ if err != nil {
159+ return err
160+ }
158161
159162 http.SetCookie(rw, ticketCookie)
163+ return nil
160164}
161165
162166// clearCookie removes any cookies that would be where this ticket
163167// would set them
164168func (t *ticket) clearCookie(rw http.ResponseWriter, req *http.Request) {
165− clearCookie := t.makeCookie(
169+ http.SetCookie(rw, cookies.MakeCookieFromOptions(
166170 req,
171+ t.options.Name,
167172 "",
173+ t.options,
168174 time.Hour*-1,
169175 time.Now(),
170− )
171− http.SetCookie(rw, clearCookie)
176+ ))
172177}
173178
174179// makeCookie makes a cookie, signing the value if present
175−func (t *ticket) makeCookie(req *http.Request, value string, expires time.Duration, now time.Time) *http.Cookie {
180+func (t *ticket) makeCookie(req *http.Request, value string, expires time.Duration, now time.Time) (*http.Cookie, error) {
176181 if value != "" {
177− value = encryption.SignedValue(t.options.Secret, t.options.Name, []byte(value), now)
182+ var err error
183+ value, err = encryption.SignedValue(t.options.Secret, t.options.Name, []byte(value), now)
184+ if err != nil {
185+ return nil, err
186+ }
178187 }
179188 return cookies.MakeCookieFromOptions(
180189 req,
@@ -183,7 +192,7 @@ func (t *ticket) makeCookie(req *http.Request, value string, expires time.Durati
183192 t.options,
184193 expires,
185194 now,
186− )
195+ ), nil
187196}
188197
189198// makeCipher makes a AES-GCM cipher out of the ticket's secret
pkg/sessions/tests/session_store_tests.go+3−2
@@ -311,11 +311,12 @@ func SessionStoreInterfaceTests(in *testInput) {
311311 BeforeEach(func() {
312312 By("Using a valid cookie with a different providers session encoding")
313313 broken := "BrokenSessionFromADifferentSessionImplementation"
314− value := encryption.SignedValue(in.cookieOpts.Secret, in.cookieOpts.Name, []byte(broken), time.Now())
314+ value, err := encryption.SignedValue(in.cookieOpts.Secret, in.cookieOpts.Name, []byte(broken), time.Now())
315+ Expect(err).ToNot(HaveOccurred())
315316 cookie := cookiesapi.MakeCookieFromOptions(in.request, in.cookieOpts.Name, value, in.cookieOpts, in.cookieOpts.Expire, time.Now())
316317 in.request.AddCookie(cookie)
317318
318− err := in.ss().Save(in.response, in.request, in.session)
319+ err = in.ss().Save(in.response, in.request, in.session)
319320 Expect(err).ToNot(HaveOccurred())
320321 })
321322