From 900061b88a59dcd79b462efb3ba57b6ca21b1663 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Sun, 24 May 2020 11:16:45 +0100 Subject: [PATCH] Move CookieOptions validation to it's own file --- pkg/validation/cookie.go | 69 +++++++++++++++++++++++++++++++++++++++ pkg/validation/options.go | 58 ++------------------------------ 2 files changed, 71 insertions(+), 56 deletions(-) create mode 100644 pkg/validation/cookie.go diff --git a/pkg/validation/cookie.go b/pkg/validation/cookie.go new file mode 100644 index 00000000..c3cc44aa --- /dev/null +++ b/pkg/validation/cookie.go @@ -0,0 +1,69 @@ +package validation + +import ( + "fmt" + "net/http" + "sort" + + "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/options" + "github.com/oauth2-proxy/oauth2-proxy/pkg/encryption" +) + +func validateCookieOptions(o options.CookieOptions) []string { + msgs := validateCookieSecret(o.Secret) + + if o.Refresh >= o.Expire { + msgs = append(msgs, fmt.Sprintf( + "cookie_refresh (%q) must be less than cookie_expire (%q)", + o.Refresh.String(), + o.Expire.String())) + } + + switch o.SameSite { + case "", "none", "lax", "strict": + default: + msgs = append(msgs, fmt.Sprintf("cookie_samesite (%q) must be one of ['', 'lax', 'strict', 'none']", o.SameSite)) + } + + // Sort cookie domains by length, so that we try longer (and more specific) domains first + sort.Slice(o.Domains, func(i, j int) bool { + return len(o.Domains[i]) > len(o.Domains[j]) + }) + + msgs = append(msgs, validateCookieName(o.Name)...) + return msgs +} + +func validateCookieName(name string) []string { + cookie := &http.Cookie{Name: name} + if cookie.String() == "" { + return []string{fmt.Sprintf("invalid cookie name: %q", name)} + } + return []string{} +} + +func validateCookieSecret(secret string) []string { + if secret == "" { + return []string{"missing setting: cookie-secret"} + } + + secretBytes := encryption.SecretBytes(secret) + // Check if the secret is a valid length + switch len(secretBytes) { + case 16, 24, 32: + // Valid secret size found + return []string{} + } + // Invalid secret size found, return a message + + // If the secretBytes is different to the raw secret, it was decoded from Base64 + // Add a note to the error message + var decodedSuffix string + if string(secretBytes) != secret { + decodedSuffix = " note: cookie secret was base64 decoded" + } + + return []string{fmt.Sprintf( + "cookie_secret must be 16, 24, or 32 bytes to create an AES cipher, but is %d bytes.%s", + len(secretBytes), decodedSuffix)} +} diff --git a/pkg/validation/options.go b/pkg/validation/options.go index e7ea18ba..d7d4270b 100644 --- a/pkg/validation/options.go +++ b/pkg/validation/options.go @@ -10,14 +10,12 @@ import ( "net/url" "os" "regexp" - "sort" "strings" "github.com/coreos/go-oidc" "github.com/dgrijalva/jwt-go" "github.com/mbland/hmacauth" "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/options" - "github.com/oauth2-proxy/oauth2-proxy/pkg/encryption" "github.com/oauth2-proxy/oauth2-proxy/pkg/ip" "github.com/oauth2-proxy/oauth2-proxy/pkg/logger" "github.com/oauth2-proxy/oauth2-proxy/pkg/requests" @@ -28,7 +26,8 @@ import ( // Validate checks that required options are set and validates those that they // are of the correct format func Validate(o *options.Options) error { - msgs := make([]string, 0) + msgs := validateCookieOptions(o.Cookie) + if o.SSLInsecureSkipVerify { insecureTransport := &http.Transport{ TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, @@ -49,30 +48,6 @@ func Validate(o *options.Options) error { } } - if o.Cookie.Secret == "" { - msgs = append(msgs, "missing setting: cookie-secret") - } else { - validCookieSecretSize := false - for _, i := range []int{16, 24, 32} { - if len(encryption.SecretBytes(o.Cookie.Secret)) == i { - validCookieSecretSize = true - } - } - var decoded bool - if string(encryption.SecretBytes(o.Cookie.Secret)) != o.Cookie.Secret { - decoded = true - } - if !validCookieSecretSize { - var suffix string - if decoded { - suffix = " note: cookie secret was base64 decoded" - } - msgs = append(msgs, - fmt.Sprintf("Cookie secret must be 16, 24, or 32 bytes to create an AES cipher. Got %d bytes.%s", - len(encryption.SecretBytes(o.Cookie.Secret)), suffix)) - } - } - if o.ClientID == "" { msgs = append(msgs, "missing setting: client-id") } @@ -222,14 +197,6 @@ func Validate(o *options.Options) error { } msgs = parseProviderInfo(o, msgs) - if o.Cookie.Refresh >= o.Cookie.Expire { - msgs = append(msgs, fmt.Sprintf( - "cookie_refresh (%s) must be less than "+ - "cookie_expire (%s)", - o.Cookie.Refresh.String(), - o.Cookie.Expire.String())) - } - if len(o.GoogleGroups) > 0 || o.GoogleAdminEmail != "" || o.GoogleServiceAccountJSON != "" { if len(o.GoogleGroups) < 1 { msgs = append(msgs, "missing setting: google-group") @@ -242,20 +209,7 @@ func Validate(o *options.Options) error { } } - switch o.Cookie.SameSite { - case "", "none", "lax", "strict": - default: - msgs = append(msgs, fmt.Sprintf("cookie_samesite (%s) must be one of ['', 'lax', 'strict', 'none']", o.Cookie.SameSite)) - } - - // Sort cookie domains by length, so that we try longer (and more specific) - // domains first - sort.Slice(o.Cookie.Domains, func(i, j int) bool { - return len(o.Cookie.Domains[i]) > len(o.Cookie.Domains[j]) - }) - msgs = parseSignatureKey(o, msgs) - msgs = validateCookieName(o, msgs) msgs = configureLogger(o.Logging, msgs) if o.ReverseProxy { @@ -442,14 +396,6 @@ func newVerifierFromJwtIssuer(jwtIssuer jwtIssuer) (*oidc.IDTokenVerifier, error return verifier, nil } -func validateCookieName(o *options.Options, msgs []string) []string { - cookie := &http.Cookie{Name: o.Cookie.Name} - if cookie.String() == "" { - return append(msgs, fmt.Sprintf("invalid cookie name: %q", o.Cookie.Name)) - } - return msgs -} - // jwtIssuer hold parsed JWT issuer info that's used to construct a verifier. type jwtIssuer struct { issuerURI string