276d1c6 · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

Always encrypt sessions regardless of configuration

commit276d1c6f1952cefed0806309eb4b526ce93f838c authorTimo Beckers <timo@incline.eu> dateMon 24 Feb 2020, 12:23 (7 years ago) parent0c9795a

4 files changed+49−40

compare with master

unifiedsplit

CHANGELOG.md+6 oauthproxy_test.go+13−6 pkg/validation/options.go+29−33 pkg/validation/options_test.go+1−1

CHANGELOG.md+6

@@ -43,6 +43,11 @@

4343 - Multiple cookie domains may now be configured. The longest domain that matches will be used.

4444 - The config options `cookie_domain` is now `cookie_domains`

4545 - The environment variable `OAUTH2_PROXY_COOKIE_DOMAIN` is now `OAUTH2_PROXY_COOKIE_DOMAINS`

46+- [#414](https://github.com/oauth2-proxy/oauth2-proxy/pull/414) Always encrypt sessions regardless of config

47+ - Previously, sessions were encrypted only when certain options were configured.

48+ This lead to confusion and misconfiguration as it was not obvious when a session should be encrypted.

49+ - Cookie Secrets must now be 16, 24 or 32 bytes.

50+ - If you need to change your secret, this will force users to reauthenticate.

4651

4752## Changes since v5.1.1

4853

@@ -86,6 +91,7 @@

8691- [#488](https://github.com/oauth2-proxy/oauth2-proxy/pull/488) Set-Basic-Auth should default to false (@JoelSpeed)

8792- [#494](https://github.com/oauth2-proxy/oauth2-proxy/pull/494) Upstream websockets TLS certificate validation now depends on ssl-upstream-insecure-skip-verify (@yaroslavros)

8893- [#497](https://github.com/oauth2-proxy/oauth2-proxy/pull/497) Restrict access using Github collaborators (@jsclayton)

94+- [#414](https://github.com/oauth2-proxy/oauth2-proxy/pull/414) Always encrypt sessions regardless of config (@ti-mo)

8995

9096# v5.1.1

9197

oauthproxy_test.go+13−6

@@ -29,6 +29,13 @@ import (

2929 "golang.org/x/net/websocket"

3030)

3131

32+const (

33+ // The rawCookieSecret is 32 bytes and the base64CookieSecret is the base64

34+ // encoded version of this.

35+ rawCookieSecret = "secretthirtytwobytes+abcdefghijk"

36+ base64CookieSecret = "c2VjcmV0dGhpcnR5dHdvYnl0ZXMrYWJjZGVmZ2hpamsK"

37+)

38+

3239func init() {

3340 logger.SetFlags(logger.Lshortfile)

3441

@@ -166,7 +173,7 @@ func TestRobotsTxt(t *testing.T) {

166173 opts := options.NewOptions()

167174 opts.ClientID = "asdlkjx"

168175 opts.ClientSecret = "alkgks"

169− opts.Cookie.Secret = "asdkugkj"

176+ opts.Cookie.Secret = rawCookieSecret

170177 validation.Validate(opts)

171178

172179 proxy := NewOAuthProxy(opts, func(string) bool { return true })

@@ -181,7 +188,7 @@ func TestIsValidRedirect(t *testing.T) {

181188 opts := options.NewOptions()

182189 opts.ClientID = "skdlfj"

183190 opts.ClientSecret = "fgkdsgj"

184− opts.Cookie.Secret = "ljgiogbj"

191+ opts.Cookie.Secret = base64CookieSecret

185192 // Should match domains that are exactly foo.bar and any subdomain of bar.foo

186193 opts.WhitelistDomains = []string{

187194 "foo.bar",

@@ -794,7 +801,7 @@ func NewSignInPageTest(skipProvider bool) *SignInPageTest {

794801 var sipTest SignInPageTest

795802

796803 sipTest.opts = options.NewOptions()

797− sipTest.opts.Cookie.Secret = "adklsj2"

804+ sipTest.opts.Cookie.Secret = rawCookieSecret

798805 sipTest.opts.ClientID = "lkdgj"

799806 sipTest.opts.ClientSecret = "sgiufgoi"

800807 sipTest.opts.SkipProviderButton = skipProvider

@@ -1208,7 +1215,7 @@ func TestAuthSkippedForPreflightRequests(t *testing.T) {

12081215 opts.Upstreams = append(opts.Upstreams, upstream.URL)

12091216 opts.ClientID = "aljsal"

12101217 opts.ClientSecret = "jglkfsdgj"

1211− opts.Cookie.Secret = "dkfjgdls"

1218+ opts.Cookie.Secret = base64CookieSecret

12121219 opts.SkipAuthPreflight = true

12131220 validation.Validate(opts)

12141221

@@ -1255,7 +1262,7 @@ type SignatureTest struct {

12551262

12561263func NewSignatureTest() *SignatureTest {

12571264 opts := options.NewOptions()

1258− opts.Cookie.Secret = "cookie secret"

1265+ opts.Cookie.Secret = rawCookieSecret

12591266 opts.ClientID = "client ID"

12601267 opts.ClientSecret = "client secret"

12611268 opts.EmailDomains = []string{"acm.org"}

@@ -1402,7 +1409,7 @@ type ajaxRequestTest struct {

14021409func newAjaxRequestTest() *ajaxRequestTest {

14031410 test := &ajaxRequestTest{}

14041411 test.opts = options.NewOptions()

1405− test.opts.Cookie.Secret = "sdflsw"

1412+ test.opts.Cookie.Secret = base64CookieSecret

14061413 test.opts.ClientID = "gkljfdl"

14071414 test.opts.ClientSecret = "sdflkjs"

14081415 validation.Validate(test.opts)

pkg/validation/options.go+29−33

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

1212 "regexp"

1313 "sort"

1414 "strings"

15− "time"

1615

1716 "github.com/coreos/go-oidc"

1817 "github.com/dgrijalva/jwt-go"

@@ -39,9 +38,38 @@ func Validate(o *options.Options) error {

3938 }

4039

4140 msgs := make([]string, 0)

41+

42+ var cipher *encryption.Cipher

4243 if o.Cookie.Secret == "" {

4344 msgs = append(msgs, "missing setting: cookie-secret")

45+ } else {

46+ validCookieSecretSize := false

47+ for _, i := range []int{16, 24, 32} {

48+ if len(encryption.SecretBytes(o.Cookie.Secret)) == i {

49+ validCookieSecretSize = true

50+ }

51+ }

52+ var decoded bool

53+ if string(encryption.SecretBytes(o.Cookie.Secret)) != o.Cookie.Secret {

54+ decoded = true

55+ }

56+ if !validCookieSecretSize {

57+ var suffix string

58+ if decoded {

59+ suffix = " note: cookie secret was base64 decoded"

60+ }

61+ msgs = append(msgs,

62+ fmt.Sprintf("Cookie secret must be 16, 24, or 32 bytes to create an AES cipher. Got %d bytes.%s",

63+ len(encryption.SecretBytes(o.Cookie.Secret)), suffix))

64+ } else {

65+ var err error

66+ cipher, err = encryption.NewCipher(encryption.SecretBytes(o.Cookie.Secret))

67+ if err != nil {

68+ msgs = append(msgs, fmt.Sprintf("cookie-secret error: %v", err))

69+ }

70+ }

4471 }

72+

4573 if o.ClientID == "" {

4674 msgs = append(msgs, "missing setting: client-id")

4775 }

@@ -195,38 +223,6 @@ func Validate(o *options.Options) error {

195223 }

196224 msgs = parseProviderInfo(o, msgs)

197225

198− var cipher *encryption.Cipher

199− if o.PassAccessToken || o.SetAuthorization || o.PassAuthorization || (o.Cookie.Refresh != time.Duration(0)) {

200− validCookieSecretSize := false

201− for _, i := range []int{16, 24, 32} {

202− if len(encryption.SecretBytes(o.Cookie.Secret)) == i {

203− validCookieSecretSize = true

204− }

205− }

206− var decoded bool

207− if string(encryption.SecretBytes(o.Cookie.Secret)) != o.Cookie.Secret {

208− decoded = true

209− }

210− if !validCookieSecretSize {

211− var suffix string

212− if decoded {

213− suffix = fmt.Sprintf(" note: cookie secret was base64 decoded from %q", o.Cookie.Secret)

214− }

215− msgs = append(msgs, fmt.Sprintf(

216− "cookie_secret must be 16, 24, or 32 bytes "+

217− "to create an AES cipher when "+

218− "pass_access_token == true or "+

219− "cookie_refresh != 0, but is %d bytes.%s",

220− len(encryption.SecretBytes(o.Cookie.Secret)), suffix))

221− } else {

222− var err error

223− cipher, err = encryption.NewCipher(encryption.SecretBytes(o.Cookie.Secret))

224− if err != nil {

225− msgs = append(msgs, fmt.Sprintf("cookie-secret error: %v", err))

226− }

227− }

228− }

229−

230226 o.Session.Cipher = cipher

231227 sessionStore, err := sessions.NewSessionStore(&o.Session, &o.Cookie)

232228 if err != nil {

pkg/validation/options_test.go+1−1

@@ -15,7 +15,7 @@ import (

1515)

1616

1717const (

18− cookieSecret = "foobar"

18+ cookieSecret = "secretthirtytwobytes+abcdefghijk"

1919 clientID = "bazquux"

2020 clientSecret = "xyzzyplugh"

2121)