55085d9 · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

csrf protection; always set state

commit55085d9697962668fd4e43e8e4644144fe83cd93 authorColin Arnott <arnottcr@gmail.com> dateMon 27 Mar 2017, 21:14 (10 years ago) parent6c690b6

4 files changed+112−44

compare with master

unifiedsplit

cookie/nonce.go+16 oauthproxy.go+68−28 oauthproxy_test.go+26−11 providers/provider_default.go+2−5

cookie/nonce.gonew file+16

@@ -0,0 +1,16 @@

1+package cookie

3+import (

4+ "crypto/rand"

5+ "fmt"

6+)

8+func Nonce() (nonce string, err error) {

9+ b := make([]byte, 16)

10+ _, err = rand.Read(b)

11+ if err != nil {

12+ return

13+ }

14+ nonce = fmt.Sprintf("%x", b)

15+ return

16+}

oauthproxy.go+68−28

@@ -37,6 +37,7 @@ var SignatureHeaders []string = []string{

3737type OAuthProxy struct {

3838 CookieSeed string

3939 CookieName string

40+ CSRFCookieName string

4041 CookieDomain string

4142 CookieSecure bool

4243 CookieHttpOnly bool

@@ -174,6 +175,7 @@ func NewOAuthProxy(opts *Options, validator func(string) bool) *OAuthProxy {

174175

175176 return &OAuthProxy{

176177 CookieName: opts.CookieName,

178+ CSRFCookieName: fmt.Sprintf("%v_%v", opts.CookieName, "csrf"),

177179 CookieSeed: opts.CookieSecret,

178180 CookieDomain: opts.CookieDomain,

179181 CookieSecure: opts.CookieSecure,

@@ -245,7 +247,22 @@ func (p *OAuthProxy) redeemCode(host, code string) (s *providers.SessionState, e

245247 return

246248}

247249

248−func (p *OAuthProxy) MakeCookie(req *http.Request, value string, expiration time.Duration, now time.Time) *http.Cookie {

250+func (p *OAuthProxy) MakeSessionCookie(req *http.Request, value string, expiration time.Duration, now time.Time) *http.Cookie {

251+ if value != "" {

252+ value = cookie.SignedValue(p.CookieSeed, p.CookieName, value, now)

253+ if len(value) > 4096 {

254+ // Cookies cannot be larger than 4kb

255+ log.Printf("WARNING - Cookie Size: %d bytes", len(value))

256+ }

257+ }

258+ return p.makeCookie(req, p.CookieName, value, expiration, now)

259+}

260+

261+func (p *OAuthProxy) MakeCSRFCookie(req *http.Request, value string, expiration time.Duration, now time.Time) *http.Cookie {

262+ return p.makeCookie(req, p.CSRFCookieName, value, expiration, now)

263+}

264+

265+func (p *OAuthProxy) makeCookie(req *http.Request, name string, value string, expiration time.Duration, now time.Time) *http.Cookie {

249266 domain := req.Host

250267 if h, _, err := net.SplitHostPort(domain); err == nil {

251268 domain = h

@@ -257,15 +274,8 @@ func (p *OAuthProxy) MakeCookie(req *http.Request, value string, expiration time

257274 domain = p.CookieDomain

258275 }

259276

260− if value != "" {

261− value = cookie.SignedValue(p.CookieSeed, p.CookieName, value, now)

262− if len(value) > 4096 {

263− // Cookies cannot be larger than 4kb

264− log.Printf("WARNING - Cookie Size: %d bytes", len(value))

265− }

266− }

267277 return &http.Cookie{

268− Name: p.CookieName,

278+ Name: name,

269279 Value: value,

270280 Path: "/",

271281 Domain: domain,

@@ -275,12 +285,20 @@ func (p *OAuthProxy) MakeCookie(req *http.Request, value string, expiration time

275285 }

276286}

277287

278−func (p *OAuthProxy) ClearCookie(rw http.ResponseWriter, req *http.Request) {

279− http.SetCookie(rw, p.MakeCookie(req, "", time.Hour*-1, time.Now()))

288+func (p *OAuthProxy) ClearCSRFCookie(rw http.ResponseWriter, req *http.Request) {

289+ http.SetCookie(rw, p.MakeCSRFCookie(req, "", time.Hour*-1, time.Now()))

290+}

291+

292+func (p *OAuthProxy) SetCSRFCookie(rw http.ResponseWriter, req *http.Request, val string) {

293+ http.SetCookie(rw, p.MakeCSRFCookie(req, val, p.CookieExpire, time.Now()))

280294}

281295

282−func (p *OAuthProxy) SetCookie(rw http.ResponseWriter, req *http.Request, val string) {

283− http.SetCookie(rw, p.MakeCookie(req, val, p.CookieExpire, time.Now()))

296+func (p *OAuthProxy) ClearSessionCookie(rw http.ResponseWriter, req *http.Request) {

297+ http.SetCookie(rw, p.MakeSessionCookie(req, "", time.Hour*-1, time.Now()))

298+}

299+

300+func (p *OAuthProxy) SetSessionCookie(rw http.ResponseWriter, req *http.Request, val string) {

301+ http.SetCookie(rw, p.MakeSessionCookie(req, val, p.CookieExpire, time.Now()))

284302}

285303

286304func (p *OAuthProxy) LoadCookiedSession(req *http.Request) (*providers.SessionState, time.Duration, error) {

@@ -309,7 +327,7 @@ func (p *OAuthProxy) SaveSession(rw http.ResponseWriter, req *http.Request, s *p

309327 if err != nil {

310328 return err

311329 }

312− p.SetCookie(rw, req, value)

330+ p.SetSessionCookie(rw, req, value)

313331 return nil

314332}

315333

@@ -339,7 +357,7 @@ func (p *OAuthProxy) ErrorPage(rw http.ResponseWriter, code int, title string, m

339357}

340358

341359func (p *OAuthProxy) SignInPage(rw http.ResponseWriter, req *http.Request, code int) {

342− p.ClearCookie(rw, req)

360+ p.ClearSessionCookie(rw, req)

343361 rw.WriteHeader(code)

344362

345363 redirect_url := req.URL.RequestURI()

@@ -384,20 +402,18 @@ func (p *OAuthProxy) ManualSignIn(rw http.ResponseWriter, req *http.Request) (st

384402 return "", false

385403}

386404

387−func (p *OAuthProxy) GetRedirect(req *http.Request) (string, error) {

388− err := req.ParseForm()

389−

405+func (p *OAuthProxy) GetRedirect(req *http.Request) (redirect string, err error) {

406+ err = req.ParseForm()

390407 if err != nil {

391− return "", err

408+ return

392409 }

393410

394− redirect := req.FormValue("rd")

395−

396− if redirect == "" {

411+ redirect = req.Form.Get("rd")

412+ if redirect == "" || !strings.HasPrefix(redirect, "/") || strings.HasPrefix(redirect, "//") {

397413 redirect = "/"

398414 }

399415

400− return redirect, err

416+ return

401417}

402418

403419func (p *OAuthProxy) IsWhitelistedPath(path string) (ok bool) {

@@ -459,18 +475,24 @@ func (p *OAuthProxy) SignIn(rw http.ResponseWriter, req *http.Request) {

459475}

460476

461477func (p *OAuthProxy) SignOut(rw http.ResponseWriter, req *http.Request) {

462− p.ClearCookie(rw, req)

478+ p.ClearSessionCookie(rw, req)

463479 http.Redirect(rw, req, "/", 302)

464480}

465481

466482func (p *OAuthProxy) OAuthStart(rw http.ResponseWriter, req *http.Request) {

483+ nonce, err := cookie.Nonce()

484+ if err != nil {

485+ p.ErrorPage(rw, 500, "Internal Error", err.Error())

486+ return

487+ }

488+ p.SetCSRFCookie(rw, req, nonce)

467489 redirect, err := p.GetRedirect(req)

468490 if err != nil {

469491 p.ErrorPage(rw, 500, "Internal Error", err.Error())

470492 return

471493 }

472494 redirectURI := p.GetRedirectURI(req.Host)

473− http.Redirect(rw, req, p.provider.GetLoginURL(redirectURI, redirect), 302)

495+ http.Redirect(rw, req, p.provider.GetLoginURL(redirectURI, fmt.Sprintf("%v:%v", nonce, redirect)), 302)

474496}

475497

476498func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) {

@@ -495,8 +517,26 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) {

495517 return

496518 }

497519

498− redirect := req.Form.Get("state")

499− if !strings.HasPrefix(redirect, "/") || strings.HasPrefix(redirect, "//") {

520+ s := strings.SplitN(req.Form.Get("state"), ":", 2)

521+ if len(s) != 2 {

522+ p.ErrorPage(rw, 500, "Internal Error", "Invalid State")

523+ return

524+ }

525+ nonce := s[0]

526+ redirect := s[1]

527+ c, err := req.Cookie(p.CSRFCookieName)

528+ if err != nil {

529+ p.ErrorPage(rw, 403, "Permission Denied", err.Error())

530+ return

531+ }

532+ p.ClearCSRFCookie(rw, req)

533+ if c.Value != nonce {

534+ log.Printf("%s csrf token mismatch, potential attack", remoteAddr)

535+ p.ErrorPage(rw, 403, "Permission Denied", "csrf failed")

536+ return

537+ }

538+

539+ if !strings.HasPrefix(redirect, "/") || strings.HasPrefix(redirect, "//") {

500540 redirect = "/"

501541 }

502542

@@ -595,7 +635,7 @@ func (p *OAuthProxy) Authenticate(rw http.ResponseWriter, req *http.Request) int

595635 }

596636

597637 if clearSession {

598− p.ClearCookie(rw, req)

638+ p.ClearSessionCookie(rw, req)

599639 }

600640

601641 if session == nil {

oauthproxy_test.go+26−11

@@ -170,10 +170,14 @@ func TestBasicAuthPassword(t *testing.T) {

170170 })

171171

172172 rw := httptest.NewRecorder()

173− req, _ := http.NewRequest("GET", "/oauth2/callback?code=callback_code",

173+ req, _ := http.NewRequest("GET", "/oauth2/callback?code=callback_code&state=nonce:",

174174 strings.NewReader(""))

175+ req.AddCookie(proxy.MakeCSRFCookie(req, "nonce", proxy.CookieExpire, time.Now()))

175176 proxy.ServeHTTP(rw, req)

176− cookie := rw.HeaderMap["Set-Cookie"][0]

177+ if rw.Code >= 400 {

178+ t.Fatalf("expected 3xx got %d", rw.Code)

179+ }

180+ cookie := rw.HeaderMap["Set-Cookie"][1]

177181

178182 cookieName := proxy.CookieName

179183 var value string

@@ -196,9 +200,11 @@ func TestBasicAuthPassword(t *testing.T) {

196200 Expires: time.Now().Add(time.Duration(24)),

197201 HttpOnly: true,

198202 })

203+ req.AddCookie(proxy.MakeCSRFCookie(req, "nonce", proxy.CookieExpire, time.Now()))

199204

200205 rw = httptest.NewRecorder()

201206 proxy.ServeHTTP(rw, req)

207+

202208 expectedHeader := "Basic " + base64.StdEncoding.EncodeToString([]byte(user_name+":"+opts.BasicAuthPassword))

203209 assert.Equal(t, expectedHeader, rw.Body.String())

204210 provider_server.Close()

@@ -263,13 +269,14 @@ func (pat_test *PassAccessTokenTest) Close() {

263269func (pat_test *PassAccessTokenTest) getCallbackEndpoint() (http_code int,

264270 cookie string) {

265271 rw := httptest.NewRecorder()

266− req, err := http.NewRequest("GET", "/oauth2/callback?code=callback_code",

272+ req, err := http.NewRequest("GET", "/oauth2/callback?code=callback_code&state=nonce:",

267273 strings.NewReader(""))

268274 if err != nil {

269275 return 0, ""

270276 }

277+ req.AddCookie(pat_test.proxy.MakeCSRFCookie(req, "nonce", time.Hour, time.Now()))

271278 pat_test.proxy.ServeHTTP(rw, req)

272− return rw.Code, rw.HeaderMap["Set-Cookie"][0]

279+ return rw.Code, rw.HeaderMap["Set-Cookie"][1]

273280}

274281

275282func (pat_test *PassAccessTokenTest) getRootEndpoint(cookie string) (http_code int, access_token string) {

@@ -314,14 +321,18 @@ func TestForwardAccessTokenUpstream(t *testing.T) {

314321

315322 // A successful validation will redirect and set the auth cookie.

316323 code, cookie := pat_test.getCallbackEndpoint()

317− assert.Equal(t, 302, code)

324+ if code != 302 {

325+ t.Fatalf("expected 302; got %d", code)

326+ }

318327 assert.NotEqual(t, nil, cookie)

319328

320329 // Now we make a regular request; the access_token from the cookie is

321330 // forwarded as the "X-Forwarded-Access-Token" header. The token is

322331 // read by the test provider server and written in the response body.

323332 code, payload := pat_test.getRootEndpoint(cookie)

324− assert.Equal(t, 200, code)

333+ if code != 200 {

334+ t.Fatalf("expected 200; got %d", code)

335+ }

325336 assert.Equal(t, "my_auth_token", payload)

326337}

327338

@@ -333,13 +344,17 @@ func TestDoNotForwardAccessTokenUpstream(t *testing.T) {

333344

334345 // A successful validation will redirect and set the auth cookie.

335346 code, cookie := pat_test.getCallbackEndpoint()

336− assert.Equal(t, 302, code)

347+ if code != 302 {

348+ t.Fatalf("expected 302; got %d", code)

349+ }

337350 assert.NotEqual(t, nil, cookie)

338351

339352 // Now we make a regular request, but the access token header should

340353 // not be present.

341354 code, payload := pat_test.getRootEndpoint(cookie)

342− assert.Equal(t, 200, code)

355+ if code != 200 {

356+ t.Fatalf("expected 200; got %d", code)

357+ }

343358 assert.Equal(t, "No access token found.", payload)

344359}

345360

@@ -457,7 +472,7 @@ func NewProcessCookieTestWithDefaults() *ProcessCookieTest {

457472}

458473

459474func (p *ProcessCookieTest) MakeCookie(value string, ref time.Time) *http.Cookie {

460− return p.proxy.MakeCookie(p.req, value, p.opts.CookieExpire, ref)

475+ return p.proxy.MakeSessionCookie(p.req, value, p.opts.CookieExpire, ref)

461476}

462477

463478func (p *ProcessCookieTest) SaveSession(s *providers.SessionState, ref time.Time) error {

@@ -465,7 +480,7 @@ func (p *ProcessCookieTest) SaveSession(s *providers.SessionState, ref time.Time

465480 if err != nil {

466481 return err

467482 }

468− p.req.AddCookie(p.proxy.MakeCookie(p.req, value, p.proxy.CookieExpire, ref))

483+ p.req.AddCookie(p.proxy.MakeSessionCookie(p.req, value, p.proxy.CookieExpire, ref))

469484 return nil

470485}

471486

@@ -697,7 +712,7 @@ func (st *SignatureTest) MakeRequestWithExpectedKey(method, body, key string) {

697712 if err != nil {

698713 panic(err)

699714 }

700− cookie := proxy.MakeCookie(req, value, proxy.CookieExpire, time.Now())

715+ cookie := proxy.MakeSessionCookie(req, value, proxy.CookieExpire, time.Now())

701716 req.AddCookie(cookie)

702717 // This is used by the upstream to validate the signature.

703718 st.authenticator.auth = hmacauth.NewHmacAuth(

providers/provider_default.go+2−5

@@ -8,7 +8,6 @@ import (

88 "io/ioutil"

99 "net/http"

1010 "net/url"

11− "strings"

1211

1312 "github.com/bitly/oauth2_proxy/cookie"

1413)

@@ -79,7 +78,7 @@ func (p *ProviderData) Redeem(redirectURL, code string) (s *SessionState, err er

7978}

8079

8180// GetLoginURL with typical oauth parameters

82−func (p *ProviderData) GetLoginURL(redirectURI, finalRedirect string) string {

81+func (p *ProviderData) GetLoginURL(redirectURI, state string) string {

8382 var a url.URL

8483 a = *p.LoginURL

8584 params, _ := url.ParseQuery(a.RawQuery)

@@ -88,9 +87,7 @@ func (p *ProviderData) GetLoginURL(redirectURI, finalRedirect string) string {

8887 params.Add("scope", p.Scope)

8988 params.Set("client_id", p.ClientID)

9089 params.Set("response_type", "code")

91− if strings.HasPrefix(finalRedirect, "/") && !strings.HasPrefix(finalRedirect,"//") {

92− params.Add("state", finalRedirect)

93− }

90+ params.Add("state", state)

9491 a.RawQuery = params.Encode()

9592 return a.String()

9693}