diff --git a/CHANGELOG.md b/CHANGELOG.md index ad4b2f88..cd358258 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - [#542](https://github.com/oauth2-proxy/oauth2-proxy/pull/542) Move SessionStore tests to independent package (@JoelSpeed) - [#577](https://github.com/oauth2-proxy/oauth2-proxy/pull/577) Move Cipher and Session Store initialisation out of Validation (@JoelSpeed) +- [#635](https://github.com/oauth2-proxy/oauth2-proxy/pull/635) Support specifying alternative provider TLS trust source(s) (@k-wall) # v6.0.0 diff --git a/docs/configuration/configuration.md b/docs/configuration/configuration.md index 96974f6f..eaee3610 100644 --- a/docs/configuration/configuration.md +++ b/docs/configuration/configuration.md @@ -86,6 +86,7 @@ An example [oauth2-proxy.cfg]({{ site.gitweb }}/contrib/oauth2-proxy.cfg.example | `--profile-url` | string | Profile access endpoint | | | `--prompt` | string | [OIDC prompt](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest); if present, `approval-prompt` is ignored | `""` | | `--provider` | string | OAuth provider | google | +| `--provider-ca-file` | string \| list | Paths to CA certificates that should be used when connecting to the provider. If not specified, the default Go trust sources are used instead. | | `--provider-display-name` | string | Override the provider's name with the given string; used for the sign-in page | (depends on provider) | | `--ping-path` | string | the ping endpoint that can be used for basic health checks | `"/ping"` | | `--ping-user-agent` | string | a User-Agent that can be used for basic health checks | `""` (don't check user agent) | diff --git a/pkg/apis/options/options.go b/pkg/apis/options/options.go index 65bdaacc..1f113757 100644 --- a/pkg/apis/options/options.go +++ b/pkg/apis/options/options.go @@ -85,22 +85,23 @@ type Options struct { // These options allow for other providers besides Google, with // potential overrides. - ProviderType string `flag:"provider" cfg:"provider"` - ProviderName string `flag:"provider-display-name" cfg:"provider_display_name"` - OIDCIssuerURL string `flag:"oidc-issuer-url" cfg:"oidc_issuer_url"` - InsecureOIDCAllowUnverifiedEmail bool `flag:"insecure-oidc-allow-unverified-email" cfg:"insecure_oidc_allow_unverified_email"` - InsecureOIDCSkipIssuerVerification bool `flag:"insecure-oidc-skip-issuer-verification" cfg:"insecure_oidc_skip_issuer_verification"` - SkipOIDCDiscovery bool `flag:"skip-oidc-discovery" cfg:"skip_oidc_discovery"` - OIDCJwksURL string `flag:"oidc-jwks-url" cfg:"oidc_jwks_url"` - LoginURL string `flag:"login-url" cfg:"login_url"` - RedeemURL string `flag:"redeem-url" cfg:"redeem_url"` - ProfileURL string `flag:"profile-url" cfg:"profile_url"` - ProtectedResource string `flag:"resource" cfg:"resource"` - ValidateURL string `flag:"validate-url" cfg:"validate_url"` - Scope string `flag:"scope" cfg:"scope"` - Prompt string `flag:"prompt" cfg:"prompt"` - ApprovalPrompt string `flag:"approval-prompt" cfg:"approval_prompt"` // Deprecated by OIDC 1.0 - UserIDClaim string `flag:"user-id-claim" cfg:"user_id_claim"` + ProviderType string `flag:"provider" cfg:"provider"` + ProviderName string `flag:"provider-display-name" cfg:"provider_display_name"` + ProviderCAFiles []string `flag:"provider-ca-file" cfg:"provider_ca_files"` + OIDCIssuerURL string `flag:"oidc-issuer-url" cfg:"oidc_issuer_url"` + InsecureOIDCAllowUnverifiedEmail bool `flag:"insecure-oidc-allow-unverified-email" cfg:"insecure_oidc_allow_unverified_email"` + InsecureOIDCSkipIssuerVerification bool `flag:"insecure-oidc-skip-issuer-verification" cfg:"insecure_oidc_skip_issuer_verification"` + SkipOIDCDiscovery bool `flag:"skip-oidc-discovery" cfg:"skip_oidc_discovery"` + OIDCJwksURL string `flag:"oidc-jwks-url" cfg:"oidc_jwks_url"` + LoginURL string `flag:"login-url" cfg:"login_url"` + RedeemURL string `flag:"redeem-url" cfg:"redeem_url"` + ProfileURL string `flag:"profile-url" cfg:"profile_url"` + ProtectedResource string `flag:"resource" cfg:"resource"` + ValidateURL string `flag:"validate-url" cfg:"validate_url"` + Scope string `flag:"scope" cfg:"scope"` + Prompt string `flag:"prompt" cfg:"prompt"` + ApprovalPrompt string `flag:"approval-prompt" cfg:"approval_prompt"` // Deprecated by OIDC 1.0 + UserIDClaim string `flag:"user-id-claim" cfg:"user_id_claim"` SignatureKey string `flag:"signature-key" cfg:"signature_key"` AcrValues string `flag:"acr-values" cfg:"acr_values"` @@ -267,6 +268,7 @@ func NewFlagSet() *pflag.FlagSet { flagSet.String("provider", "google", "OAuth provider") flagSet.String("provider-display-name", "", "Provider display name") + flagSet.StringSlice("provider-ca-file", []string{}, "One or more paths to CA certificates that should be used when connecting to the provider. If not specified, the default Go trust sources are used instead.") flagSet.String("oidc-issuer-url", "", "OpenID Connect issuer URL (ie: https://accounts.google.com)") flagSet.Bool("insecure-oidc-allow-unverified-email", false, "Don't fail if an email address in an id_token is not verified") flagSet.Bool("insecure-oidc-skip-issuer-verification", false, "Do not verify if issuer matches OIDC discovery URL") diff --git a/pkg/util/util.go b/pkg/util/util.go new file mode 100644 index 00000000..e0a3fd3b --- /dev/null +++ b/pkg/util/util.go @@ -0,0 +1,24 @@ +package util + +import ( + "crypto/x509" + "fmt" + "io/ioutil" +) + +func GetCertPool(paths []string) (*x509.CertPool, error) { + if len(paths) == 0 { + return nil, fmt.Errorf("invalid empty list of Root CAs file paths") + } + pool := x509.NewCertPool() + for _, path := range paths { + data, err := ioutil.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("certificate authority file (%s) could not be read - %s", path, err) + } + if !pool.AppendCertsFromPEM(data) { + return nil, fmt.Errorf("loading certificate authority (%s) failed", path) + } + } + return pool, nil +} diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go new file mode 100644 index 00000000..63816209 --- /dev/null +++ b/pkg/util/util_test.go @@ -0,0 +1,91 @@ +package util + +import ( + "crypto/x509/pkix" + "encoding/asn1" + "io/ioutil" + "os" + "testing" + + "github.com/stretchr/testify/assert" +) + +// Test certificate created with an OpenSSL command in the following form: +// openssl req -x509 -newkey rsa:4096 -keyout key-unused.pem -out cert.pem -nodes -subj "/CN=oauth-proxy test ca" + +var ( + testCA1Subj = "CN=oauth-proxy test ca" + testCA1 = `-----BEGIN CERTIFICATE----- +MIICuTCCAaGgAwIBAgIFAKuKEWowDQYJKoZIhvcNAQELBQAwHjEcMBoGA1UEAxMT +b2F1dGgtcHJveHkgdGVzdCBjYTAeFw0xNzEwMjQyMDExMzJaFw0xOTEwMjQyMDEx +MzJaMB4xHDAaBgNVBAMTE29hdXRoLXByb3h5IHRlc3QgY2EwggEiMA0GCSqGSIb3 +DQEBAQUAA4IBDwAwggEKAoIBAQC5/kmgKNiECuxlj27yTWBWOMVvIB0AaRhQrMA7 +3iSCk/SHhaTabUuXUGRwmCAewT/y9oX3rTdfnSPCn7praU/27lRFBgOGFrTzAZH6 +voisF54I3ZxWZgHDJ/ig/KFwd0Y8OATj9/k9uAJSCe6aT7BouJPZVWNGF2dF5BOJ +EwFsJiN2s8HpF14DhxFOMMtlckdMHGxi3wj3E/hBCfGvGGU4Wezz48vEWWC1ajWM +qVq2vVWi1bcNft8FjWa5wTGpdlDQJM7yvKYJPwRkEjgIXtF1ra3JM3WTTFZO9Yhd +QXwO7IWRTdTaypKTNbTDKuWQZsm7xQM9sNcFkukGb3o+uBpLAgMBAAEwDQYJKoZI +hvcNAQELBQADggEBAHJNrUfHhN7VOUF60pG8sOEkx0ztjbtbYMj2N9Kb0oSya+re +Kmb2Z4JgyV7XHCZ03Jch6L7UBI3Y6/Lp1zdwU03LFayVUchLkvFonoXpRRP5UFYN ++36xP3ZL1qBYFphARsCk6/tl36czH4oF5gTlhWCRy3upNzn+INk467hnCKt5xuse +zhm+xQv/VN1poI0S/oCg9HLA9iKpoqGJByN32yoFr3QViLPqkmJ1v8EiH0Ns+1m3 +pP5YlVqdRCVrxgT80PIMsvQhfcuIrbbeiRDEUdEX7FqebuGCEa2757MTdW7UYQiB +7kgECMnwAOlJME8aDKnmTBajaMy6xCSC87V7wps= +-----END CERTIFICATE----- +` + testCA2Subj = "CN=oauth-proxy second test ca" + testCA2 = `-----BEGIN CERTIFICATE----- +MIICxzCCAa+gAwIBAgIFAKuMKewwDQYJKoZIhvcNAQELBQAwJTEjMCEGA1UEAxMa +b2F1dGgtcHJveHkgc2Vjb25kIHRlc3QgY2EwHhcNMTcxMDI1MTYxMTQxWhcNMTkx +MDI1MTYxMTQxWjAlMSMwIQYDVQQDExpvYXV0aC1wcm94eSBzZWNvbmQgdGVzdCBj +YTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAKdTkEOJ+QpOHy0PqGDR +fu8NFyo7BJwAnI+P1G32UXMeecCwBgGJEyv6eHEFV6jH/U2K2H0hynaCFxRuIdTA +EeS4s4BAbKqFhQ62I9lF3HVuqRPOe5FYdUl80eQynME22fWQ6/sZdQds0sFqaJBz +R4KQQxVULT19Br/6zwQZZhC1NtzSwCqi4CoO2OM7ctUKRvtC87LNGWapz5I4eh0A +/q4XJaSObsBCAJD7OVMa1LM3sSINUnvvGoSBKTuJ8MRk/BQRAO/PwXxsa+2h+k+w +D6sLExrBgWzAAPQKRKF+nLYVhz9AKn4JBpZt9j4PvTKz1SDcJ5wVEzOfVmii7Ui3 +EFcCAwEAATANBgkqhkiG9w0BAQsFAAOCAQEAiy58XvhOka3drXv2bwl95FwNtodj +L2MmIdF0pp01O0ryREcC1kogamdOS/UHQs4okuCjwinR/UgU+cFGCDYHfeENtUTw +Ox2OikYD7bXUpNzbQ4QyF0+cKwAgxD4ai5xSV/NUvMkL1aE8tLyxGm6VkhhyvxU1 +U9kvLha6KBWOCNd2fBJxgg8RAxFV3vR+xLdEtXnBAeTURrHM19gwMtd16y6gUZTZ +Xbl3Ix0t2+sqi0hpEF/iVFdCp5TXiicSnZCtePzCfHePAEfbh5hS0bq8Lbb9DZ6d ++2jX3AVuYhQPuutxla+vNp2XRcMTbzwXyi/Ig4nHKmPLFXsEbv+4tSwxyQ== +-----END CERTIFICATE----- +` +) + +func makeTestCertFile(t *testing.T, pem, dir string) *os.File { + file, err := ioutil.TempFile(dir, "test-certfile") + assert.NoError(t, err) + _, err = file.Write([]byte(pem)) + assert.NoError(t, err) + return file +} + +func TestGetCertPool_NoRoots(t *testing.T) { + _, err := GetCertPool([]string(nil)) + assert.Error(t, err, "invalid empty list of Root CAs file paths") +} + +func TestGetCertPool(t *testing.T) { + tempDir, err := ioutil.TempDir("", "certtest") + assert.NoError(t, err) + defer os.RemoveAll(tempDir) + certFile1 := makeTestCertFile(t, testCA1, tempDir) + certFile2 := makeTestCertFile(t, testCA2, tempDir) + + certPool, err := GetCertPool([]string{certFile1.Name(), certFile2.Name()}) + assert.NoError(t, err) + + subj := certPool.Subjects() + got := make([]string, 0) + for i := range subj { + var subject pkix.RDNSequence + _, err := asn1.Unmarshal(subj[i], &subject) + assert.NoError(t, err) + got = append(got, subject.String()) + } + + expectedSubjects := []string{testCA1Subj, testCA2Subj} + assert.Equal(t, expectedSubjects, got) +} diff --git a/pkg/validation/options.go b/pkg/validation/options.go index 611ea92b..e7ea18ba 100644 --- a/pkg/validation/options.go +++ b/pkg/validation/options.go @@ -21,21 +21,34 @@ import ( "github.com/oauth2-proxy/oauth2-proxy/pkg/ip" "github.com/oauth2-proxy/oauth2-proxy/pkg/logger" "github.com/oauth2-proxy/oauth2-proxy/pkg/requests" + "github.com/oauth2-proxy/oauth2-proxy/pkg/util" "github.com/oauth2-proxy/oauth2-proxy/providers" ) // 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) if o.SSLInsecureSkipVerify { - // TODO: Accept a certificate bundle. insecureTransport := &http.Transport{ TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, } http.DefaultClient = &http.Client{Transport: insecureTransport} + } else if len(o.ProviderCAFiles) > 0 { + pool, err := util.GetCertPool(o.ProviderCAFiles) + if err == nil { + transport := &http.Transport{ + TLSClientConfig: &tls.Config{ + RootCAs: pool, + }, + } + + http.DefaultClient = &http.Client{Transport: transport} + } else { + msgs = append(msgs, fmt.Sprintf("unable to load provider CA file(s): %v", err)) + } } - msgs := make([]string, 0) if o.Cookie.Secret == "" { msgs = append(msgs, "missing setting: cookie-secret") } else { diff --git a/pkg/validation/options_test.go b/pkg/validation/options_test.go index 6abd13db..3951c1ae 100644 --- a/pkg/validation/options_test.go +++ b/pkg/validation/options_test.go @@ -366,3 +366,16 @@ func TestRealClientIPHeader(t *testing.T) { assert.Equal(t, expected, err.Error()) assert.Nil(t, o.GetRealClientIPParser()) } + +func TestProviderCAFilesError(t *testing.T) { + file, err := ioutil.TempFile("", "absent.*.crt") + assert.NoError(t, err) + assert.NoError(t, file.Close()) + assert.NoError(t, os.Remove(file.Name())) + + o := testOptions() + o.ProviderCAFiles = append(o.ProviderCAFiles, file.Name()) + err = Validate(o) + assert.Error(t, err) + assert.Contains(t, err.Error(), "unable to load provider CA file(s)") +}