diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c087c2..bf6e7c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ ## Changes since v7.5.1 +- [#2237](https://github.com/oauth2-proxy/oauth2-proxy/pull/2237) adds an option to append CA certificates (@emsixteeen) - [#2128](https://github.com/oauth2-proxy/oauth2-proxy/pull/2128) Update dependencies (@vllvll) - [#2274](https://github.com/oauth2-proxy/oauth2-proxy/pull/2274) Upgrade golang.org/x/net to v0.17.0 (@pierluigilenoci) - [#2282](https://github.com/oauth2-proxy/oauth2-proxy/pull/2282) Fixed checking Google Groups membership using Google Application Credentials (@kvanzuijlen) diff --git a/docs/docs/configuration/alpha_config.md b/docs/docs/configuration/alpha_config.md index a72dc63..8141f47 100644 --- a/docs/docs/configuration/alpha_config.md +++ b/docs/docs/configuration/alpha_config.md @@ -415,6 +415,7 @@ Provider holds all configuration for a single provider | `provider` | _[ProviderType](#providertype)_ | Type is the OAuth provider
must be set from the supported providers group,
otherwise 'Google' is set as default | | `name` | _string_ | Name is the providers display name
if set, it will be shown to the users in the login page. | | `caFiles` | _[]string_ | CAFiles is a list of paths to CA certificates that should be used when connecting to the provider.
If not specified, the default Go trust sources are used instead | +| `useSystemTrustStore` | _bool_ | UseSystemTrustStore determines if your custom CA files and the system trust store are used
If set to true, your custom CA files and the system trust store are used otherwise only your custom CA files. | | `loginURL` | _string_ | LoginURL is the authentication endpoint | | `loginURLParameters` | _[[]LoginURLParameter](#loginurlparameter)_ | LoginURLParameters defines the parameters that can be passed from the start URL to the IdP login URL | | `redeemURL` | _string_ | RedeemURL is the token redemption endpoint | diff --git a/docs/docs/configuration/overview.md b/docs/docs/configuration/overview.md index a362949..36848ca 100644 --- a/docs/docs/configuration/overview.md +++ b/docs/docs/configuration/overview.md @@ -154,6 +154,7 @@ An example [oauth2-proxy.cfg](https://github.com/oauth2-proxy/oauth2-proxy/blob/ | `--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. | +| `--use-system-trust-store` | bool | Determines if `provider-ca-file` files and the system trust store are used. If set to true, your custom CA files and the system trust store are used otherwise only your custom CA files. | false | | `--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/legacy_options.go b/pkg/apis/options/legacy_options.go index 99be34b..1bb0610 100644 --- a/pkg/apis/options/legacy_options.go +++ b/pkg/apis/options/legacy_options.go @@ -507,6 +507,7 @@ type LegacyProvider struct { 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"` + UseSystemTrustStore bool `flag:"use-system-trust-store" cfg:"use_system_trust_store"` 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"` @@ -561,6 +562,7 @@ func legacyProviderFlagSet() *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.Bool("use-system-trust-store", false, "Determines if 'provider-ca-file' files and the system trust store are used. If set to true, your custom CA files and the system trust store are used otherwise only your custom CA files.") 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") @@ -659,6 +661,7 @@ func (l *LegacyProvider) convert() (Providers, error) { ClientSecretFile: l.ClientSecretFile, Type: ProviderType(l.ProviderType), CAFiles: l.ProviderCAFiles, + UseSystemTrustStore: l.UseSystemTrustStore, LoginURL: l.LoginURL, RedeemURL: l.RedeemURL, ProfileURL: l.ProfileURL, diff --git a/pkg/apis/options/providers.go b/pkg/apis/options/providers.go index 9599f23..8820f34 100644 --- a/pkg/apis/options/providers.go +++ b/pkg/apis/options/providers.go @@ -59,7 +59,9 @@ type Provider struct { // CAFiles is a list of paths to CA certificates that should be used when connecting to the provider. // If not specified, the default Go trust sources are used instead CAFiles []string `json:"caFiles,omitempty"` - + // UseSystemTrustStore determines if your custom CA files and the system trust store are used + // If set to true, your custom CA files and the system trust store are used otherwise only your custom CA files. + UseSystemTrustStore bool `json:"useSystemTrustStore,omitempty"` // LoginURL is the authentication endpoint LoginURL string `json:"loginURL,omitempty"` // LoginURLParameters defines the parameters that can be passed from the start URL to the IdP login URL diff --git a/pkg/util/util.go b/pkg/util/util.go index a4425d9..0f3d70a 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -14,11 +14,40 @@ import ( "time" ) -func GetCertPool(paths []string) (*x509.CertPool, error) { +func GetCertPool(paths []string, useSystemPool bool) (*x509.CertPool, error) { if len(paths) == 0 { return nil, fmt.Errorf("invalid empty list of Root CAs file paths") } - pool := x509.NewCertPool() + + var pool *x509.CertPool + if useSystemPool { + rootPool, err := getSystemCertPool() + if err != nil { + return nil, fmt.Errorf("unable to get SystemCertPool when append is true - #{err}") + } + pool = rootPool + } else { + pool = x509.NewCertPool() + } + + return loadCertsFromPaths(paths, pool) + +} + +func getSystemCertPool() (*x509.CertPool, error) { + rootPool, err := x509.SystemCertPool() + if err != nil { + return nil, err + } + + if rootPool == nil { + return nil, fmt.Errorf("SystemCertPool is empty") + } + + return rootPool, nil +} + +func loadCertsFromPaths(paths []string, pool *x509.CertPool) (*x509.CertPool, error) { for _, path := range paths { // Cert paths are a configurable option data, err := os.ReadFile(path) // #nosec G304 diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go index b8eff50..167c3e5 100644 --- a/pkg/util/util_test.go +++ b/pkg/util/util_test.go @@ -190,7 +190,7 @@ func makeTestCertFile(t *testing.T, pem, dir string) *os.File { } func TestGetCertPool_NoRoots(t *testing.T) { - _, err := GetCertPool([]string(nil)) + _, err := GetCertPool([]string(nil), false) assert.Error(t, err, "invalid empty list of Root CAs file paths") } @@ -204,34 +204,52 @@ func TestGetCertPool(t *testing.T) { } }(tempDir) + rootPool, _ := x509.SystemCertPool() + cleanPool := x509.NewCertPool() + + tests := []struct { + appendCerts bool + pool *x509.CertPool + }{ + {false, cleanPool}, + {true, rootPool}, + } + certFile1 := makeTestCertFile(t, root1Cert, tempDir) certFile2 := makeTestCertFile(t, root2Cert, tempDir) - certPool, err := GetCertPool([]string{certFile1.Name(), certFile2.Name()}) - assert.NoError(t, err) + for _, tc := range tests { + // Append certs to "known" pool so we can compare them + assert.True(t, tc.pool.AppendCertsFromPEM([]byte(root1Cert))) + assert.True(t, tc.pool.AppendCertsFromPEM([]byte(root2Cert))) - cert1Block, _ := pem.Decode([]byte(cert1Cert)) - cert1, _ := x509.ParseCertificate(cert1Block.Bytes) - assert.Equal(t, cert1.Subject.String(), cert1CertSubj) + certPool, err := GetCertPool([]string{certFile1.Name(), certFile2.Name()}, tc.appendCerts) + assert.NoError(t, err) + assert.True(t, tc.pool.Equal(certPool)) - cert2Block, _ := pem.Decode([]byte(cert2Cert)) - cert2, _ := x509.ParseCertificate(cert2Block.Bytes) - assert.Equal(t, cert2.Subject.String(), cert2CertSubj) + cert1Block, _ := pem.Decode([]byte(cert1Cert)) + cert1, _ := x509.ParseCertificate(cert1Block.Bytes) + assert.Equal(t, cert1.Subject.String(), cert1CertSubj) - cert3Block, _ := pem.Decode([]byte(cert3Cert)) - cert3, _ := x509.ParseCertificate(cert3Block.Bytes) - assert.Equal(t, cert3.Subject.String(), cert3CertSubj) + cert2Block, _ := pem.Decode([]byte(cert2Cert)) + cert2, _ := x509.ParseCertificate(cert2Block.Bytes) + assert.Equal(t, cert2.Subject.String(), cert2CertSubj) - opts := x509.VerifyOptions{ - Roots: certPool, + cert3Block, _ := pem.Decode([]byte(cert3Cert)) + cert3, _ := x509.ParseCertificate(cert3Block.Bytes) + assert.Equal(t, cert3.Subject.String(), cert3CertSubj) + + opts := x509.VerifyOptions{ + Roots: certPool, + } + + // "cert1" and "cert2" should be valid because "root1" and "root2" are in the certPool + // "cert3" should not be valid because "root3" is not in the certPool + _, err1 := cert1.Verify(opts) + assert.NoError(t, err1) + _, err2 := cert2.Verify(opts) + assert.NoError(t, err2) + _, err3 := cert3.Verify(opts) + assert.Error(t, err3) } - - // "cert1" and "cert2" should be valid because "root1" and "root2" are in the certPool - // "cert3" should not be valid because "root3" is not in the certPool - _, err1 := cert1.Verify(opts) - assert.NoError(t, err1) - _, err2 := cert2.Verify(opts) - assert.NoError(t, err2) - _, err3 := cert3.Verify(opts) - assert.Error(t, err3) } diff --git a/pkg/validation/options.go b/pkg/validation/options.go index a3ce051..8c80482 100644 --- a/pkg/validation/options.go +++ b/pkg/validation/options.go @@ -37,7 +37,7 @@ func Validate(o *options.Options) error { } http.DefaultClient = &http.Client{Transport: insecureTransport} } else if len(o.Providers[0].CAFiles) > 0 { - pool, err := util.GetCertPool(o.Providers[0].CAFiles) + pool, err := util.GetCertPool(o.Providers[0].CAFiles, o.Providers[0].UseSystemTrustStore) if err == nil { transport := http.DefaultTransport.(*http.Transport).Clone() transport.TLSClientConfig = &tls.Config{