ff761d2 · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

fix: runtime error: index out of range (0) with length 0 (#2328)

ff761d2Nuno Miguel Micaelo Borges2 years ago

CHANGELOG.md+1

@@ -10,6 +10,7 @@

11- [#2803](https://github.com/oauth2-proxy/oauth2-proxy/pull/2803) fix: self signed certificate handling in v7.7.0 (@tuunit)

12- [#2619](https://github.com/oauth2-proxy/oauth2-proxy/pull/2619) fix: unable to use hyphen in JSON path for oidc-groups-claim option (@rd-danny-fleer)

13+- [#2311](https://github.com/oauth2-proxy/oauth2-proxy/pull/2311) fix: runtime error: index out of range (0) with length 0 (@miguelborges99 / @tuunit)

14# V7.7.0

15# V7.7.0

oauthproxy.go+39−9

@@ -870,9 +870,22 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) {

870 return

871 }

872

873+ nonce, appRedirect, err := decodeState(req.Form.Get("state"), p.encodeState)

874+ if err != nil {

875+ logger.Errorf("Error while parsing OAuth2 state: %v", err)

876+ p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error())

877+ return

878+ }

879+

880+ // calculate the cookie name

881+ cookieName := cookies.GenerateCookieName(p.CookieOptions, nonce)

882+ // Try to find the CSRF cookie and decode it

873− csrf, err := cookies.LoadCSRFCookie(req, p.CookieOptions)

883+ csrf, err := cookies.LoadCSRFCookie(req, cookieName, p.CookieOptions)

874 if err != nil {

884 if err != nil {

885+ // There are a lot of issues opened complaining about missing CSRF cookies.

886+ // Try to log the INs and OUTs of OAuthProxy, to be easier to analyse these issues.

887+ LoggingCSRFCookiesInOAuthCallback(req, cookieName)

875− logger.Println(req, logger.AuthFailure, "Invalid authentication via OAuth2. Error while loading CSRF cookie:", err.Error())

888+ logger.Println(req, logger.AuthFailure, "Invalid authentication via OAuth2: unable to obtain CSRF cookie: %s (state=%s)", err, nonce)

876 p.ErrorPage(rw, req, http.StatusForbidden, err.Error(), "Login Failed: Unable to find a valid CSRF token. Please try again.")

889 p.ErrorPage(rw, req, http.StatusForbidden, err.Error(), "Login Failed: Unable to find a valid CSRF token. Please try again.")

877 return

890 return

878 }

891 }

@@ -893,13 +906,6 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) {

893

906

894 csrf.ClearCookie(rw, req)

907 csrf.ClearCookie(rw, req)

895

908

896− nonce, appRedirect, err := decodeState(req.Form.Get("state"), p.encodeState)

897− if err != nil {

898− logger.Errorf("Error while parsing OAuth2 state: %v", err)

899− p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error())

900− return

901− }

902−

903 if !csrf.CheckOAuthState(nonce) {

909 if !csrf.CheckOAuthState(nonce) {

904 logger.PrintAuthf(session.Email, req, logger.AuthFailure, "Invalid authentication via OAuth2: CSRF token mismatch, potential attack")

910 logger.PrintAuthf(session.Email, req, logger.AuthFailure, "Invalid authentication via OAuth2: CSRF token mismatch, potential attack")

905 p.ErrorPage(rw, req, http.StatusForbidden, "CSRF token mismatch, potential attack", "Login Failed: Unable to find a valid CSRF token. Please try again.")

911 p.ErrorPage(rw, req, http.StatusForbidden, "CSRF token mismatch, potential attack", "Login Failed: Unable to find a valid CSRF token. Please try again.")

@@ -1297,3 +1303,27 @@ func (p *OAuthProxy) errorJSON(rw http.ResponseWriter, code int) {

1297 // application/json

1303 // application/json

1298 rw.Write([]byte("{}"))

1304 rw.Write([]byte("{}"))

1299}

1305}

1306+

1307+// LoggingCSRFCookiesInOAuthCallback Log all CSRF cookies found in HTTP request OAuth callback,

1308+// which were successfully parsed

1309+func LoggingCSRFCookiesInOAuthCallback(req *http.Request, cookieName string) {

1310+ cookies := req.Cookies()

1311+ if len(cookies) == 0 {

1312+ logger.Println(req, logger.AuthFailure, "No cookies were found in OAuth callback.")

1313+ return

1314+ }

1315+

1316+ for _, c := range cookies {

1317+ if cookieName == c.Name {

1318+ logger.Println(req, logger.AuthFailure, "CSRF cookie %s was found in OAuth callback.", c.Name)

1319+ return

1320+ }

1321+

1322+ if strings.HasSuffix(c.Name, "_csrf") {

1323+ logger.Println(req, logger.AuthFailure, "CSRF cookie %s was found in OAuth callback, but it is not the expected one (%s).", c.Name, cookieName)

1324+ return

1325+ }

1326+ }

1327+

1328+ logger.Println(req, logger.AuthFailure, "Cookies were found in OAuth callback, but none was a CSRF cookie.")

1329+}

pkg/cookies/csrf.go+8−15

@@ -72,9 +72,7 @@ func NewCSRF(opts *options.Cookie, codeVerifier string) (CSRF, error) {

72}

74// LoadCSRFCookie loads a CSRF object from a request's CSRF cookie

75−func LoadCSRFCookie(req *http.Request, opts *options.Cookie) (CSRF, error) {

75+func LoadCSRFCookie(req *http.Request, cookieName string, opts *options.Cookie) (CSRF, error) {

76− cookieName := GenerateCookieName(req, opts)

77−

78 cookies := req.Cookies()

76 cookies := req.Cookies()

79 for _, cookie := range cookies {

77 for _, cookie := range cookies {

80 if cookie.Name != cookieName {

78 if cookie.Name != cookieName {

@@ -89,17 +87,17 @@ func LoadCSRFCookie(req *http.Request, opts *options.Cookie) (CSRF, error) {

89 return csrf, nil

87 return csrf, nil

90 }

88 }

92− return nil, errors.New("CSRF cookie not found")

90+ return nil, fmt.Errorf("CSRF cookie with name '%v' was not found", cookieName)

93}

91}

95// GenerateCookieName in case cookie options state that CSRF cookie has fixed name then set fixed name, otherwise

93// GenerateCookieName in case cookie options state that CSRF cookie has fixed name then set fixed name, otherwise

96// build name based on the state

94// build name based on the state

97−func GenerateCookieName(req *http.Request, opts *options.Cookie) string {

95+func GenerateCookieName(opts *options.Cookie, state string) string {

98 stateSubstring := ""

96 stateSubstring := ""

99 if opts.CSRFPerRequest {

97 if opts.CSRFPerRequest {

100 // csrfCookieName will include a substring of the state to enable multiple csrf cookies

98 // csrfCookieName will include a substring of the state to enable multiple csrf cookies

101 // in case of parallel requests

99 // in case of parallel requests

102− stateSubstring = ExtractStateSubstring(req)

100+ stateSubstring = ExtractStateSubstring(state)

103 }

101 }

104 return csrfCookieName(opts, stateSubstring)

102 return csrfCookieName(opts, stateSubstring)

105}

103}

@@ -218,20 +216,15 @@ func csrfCookieName(opts *options.Cookie, stateSubstring string) string {

218 if stateSubstring == "" {

216 if stateSubstring == "" {

219 return fmt.Sprintf("%v_csrf", opts.Name)

217 return fmt.Sprintf("%v_csrf", opts.Name)

220 }

218 }

221− return fmt.Sprintf("%v_csrf_%v", opts.Name, stateSubstring)

219+ return fmt.Sprintf("%v_%v_csrf", opts.Name, stateSubstring)

222}

220}

223

221

224// ExtractStateSubstring extract the initial state characters, to add it to the CSRF cookie name

222// ExtractStateSubstring extract the initial state characters, to add it to the CSRF cookie name

225−func ExtractStateSubstring(req *http.Request) string {

223+func ExtractStateSubstring(state string) string {

226 lastChar := csrfStateLength - 1

224 lastChar := csrfStateLength - 1

227 stateSubstring := ""

225 stateSubstring := ""

228−

229− state := req.URL.Query()["state"]

230− if state[0] != "" {

231− state := state[0]

232− if lastChar <= len(state) {

226+ if lastChar <= len(state) {

233− stateSubstring = state[0:lastChar]

227+ stateSubstring = state[0:lastChar]

234− }

235 }

228 }

236 return stateSubstring

229 return stateSubstring

237}

230}

pkg/cookies/csrf_test.go+6−4

@@ -19,6 +19,7 @@ var _ = Describe("CSRF Cookie Tests", func() {

19 cookieOpts *options.Cookie

20 publicCSRF CSRF

21 privateCSRF *csrf

22+ csrfName string

22 )

23 )

24 BeforeEach(func() {

25 BeforeEach(func() {

@@ -39,6 +40,7 @@ var _ = Describe("CSRF Cookie Tests", func() {

39 Expect(err).ToNot(HaveOccurred())

40 Expect(err).ToNot(HaveOccurred())

41 privateCSRF = publicCSRF.(*csrf)

42 privateCSRF = publicCSRF.(*csrf)

43+ csrfName = GenerateCookieName(cookieOpts, csrfNonce)

42 })

44 })

44 Context("NewCSRF", func() {

46 Context("NewCSRF", func() {

@@ -175,7 +177,7 @@ var _ = Describe("CSRF Cookie Tests", func() {

175 })

177 })

176

178

177 It("should return error when no cookie is set", func() {

179 It("should return error when no cookie is set", func() {

178− csrf, err := LoadCSRFCookie(req, cookieOpts)

180+ csrf, err := LoadCSRFCookie(req, csrfName, cookieOpts)

179 Expect(err).To(HaveOccurred())

181 Expect(err).To(HaveOccurred())

180 Expect(csrf).To(BeNil())

182 Expect(csrf).To(BeNil())

181 })

183 })

@@ -191,7 +193,7 @@ var _ = Describe("CSRF Cookie Tests", func() {

191 Value: encoded,

193 Value: encoded,

192 })

194 })

193

195

194− csrf, err := LoadCSRFCookie(req, cookieOpts)

196+ csrf, err := LoadCSRFCookie(req, csrfName, cookieOpts)

195 Expect(err).ToNot(HaveOccurred())

197 Expect(err).ToNot(HaveOccurred())

196 Expect(csrf).ToNot(BeNil())

198 Expect(csrf).ToNot(BeNil())

197 })

199 })

@@ -202,7 +204,7 @@ var _ = Describe("CSRF Cookie Tests", func() {

202 Value: "invalid",

204 Value: "invalid",

203 })

205 })

204

206

205− csrf, err := LoadCSRFCookie(req, cookieOpts)

207+ csrf, err := LoadCSRFCookie(req, csrfName, cookieOpts)

206 Expect(err).To(HaveOccurred())

208 Expect(err).To(HaveOccurred())

207 Expect(csrf).To(BeNil())

209 Expect(csrf).To(BeNil())

208 })

210 })

@@ -223,7 +225,7 @@ var _ = Describe("CSRF Cookie Tests", func() {

223 Value: encoded,

225 Value: encoded,

224 })

226 })

225

227

226− csrf, err := LoadCSRFCookie(req, cookieOpts)

228+ csrf, err := LoadCSRFCookie(req, csrfName, cookieOpts)

227 Expect(err).ToNot(HaveOccurred())

229 Expect(err).ToNot(HaveOccurred())

228 Expect(csrf).ToNot(BeNil())

230 Expect(csrf).ToNot(BeNil())

229 })

231 })