Skip to content

Commit

Permalink
Issue: 2236 - adds an option to append CA certificates
Browse files Browse the repository at this point in the history
  • Loading branch information
l-lafin committed Jul 12, 2024
1 parent 7ca3ec6 commit 6aa359a
Show file tree
Hide file tree
Showing 8 changed files with 81 additions and 30 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

# 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)
Expand Down
1 change: 1 addition & 0 deletions docs/docs/configuration/alpha_config.md
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,7 @@ Provider holds all configuration for a single provider
| `provider` | _[ProviderType](#providertype)_ | Type is the OAuth provider<br/>must be set from the supported providers group,<br/>otherwise 'Google' is set as default |
| `name` | _string_ | Name is the providers display name<br/>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.<br/>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<br/>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 |
Expand Down
1 change: 1 addition & 0 deletions docs/docs/configuration/overview.md
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,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) |
Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/options/legacy_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,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"`
Expand Down Expand Up @@ -596,6 +597,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")
Expand Down Expand Up @@ -698,6 +700,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,
Expand Down
4 changes: 3 additions & 1 deletion pkg/apis/options/providers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
32 changes: 30 additions & 2 deletions pkg/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,39 @@ 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
Expand Down
67 changes: 41 additions & 26 deletions pkg/util/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ mmu20Q5sWqggb0WNk1GoHIlpiObgY38KfnNSZ5Ra/QuXLF3QM2NdK5bT1iNFfPG3
fQklEouz/2lqZkRhsKSeXFKQ+h8GtNevKwFg6y3wWvH62WF5mTe9eyUE9VWl64y6
js5ESoVXA+e+QfsMsJrI5XfLV1O8ZxXKAVrYxBnC+WQbrNOjI7VBkjcn/QDmDjBw
sC1lo4YZwxEQ/bE0kEWI7PT/Skml4bTLw0jsgXNV9Nd8
-----END CERTIFICATE-----
-----END CERTIFICATE-----
`
cert1CertSubj = "CN=oauth2-proxy test cert1 ca"
cert1Cert = `-----BEGIN CERTIFICATE-----
Expand Down Expand Up @@ -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")
}

Expand All @@ -204,36 +204,51 @@ 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)
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)))

certPool, err := GetCertPool([]string{certFile1.Name(), certFile2.Name()})
certFile1.Close()
certFile2.Close()
assert.NoError(t, err)
certPool, err := GetCertPool([]string{certFile1.Name(), certFile2.Name()}, tc.appendCerts)
assert.NoError(t, err)
assert.True(t, tc.pool.Equal(certPool))

cert1Block, _ := pem.Decode([]byte(cert1Cert))
cert1, _ := x509.ParseCertificate(cert1Block.Bytes)
assert.Equal(t, cert1.Subject.String(), cert1CertSubj)
cert1Block, _ := pem.Decode([]byte(cert1Cert))
cert1, _ := x509.ParseCertificate(cert1Block.Bytes)
assert.Equal(t, cert1.Subject.String(), cert1CertSubj)

cert2Block, _ := pem.Decode([]byte(cert2Cert))
cert2, _ := x509.ParseCertificate(cert2Block.Bytes)
assert.Equal(t, cert2.Subject.String(), cert2CertSubj)
cert2Block, _ := pem.Decode([]byte(cert2Cert))
cert2, _ := x509.ParseCertificate(cert2Block.Bytes)
assert.Equal(t, cert2.Subject.String(), cert2CertSubj)

cert3Block, _ := pem.Decode([]byte(cert3Cert))
cert3, _ := x509.ParseCertificate(cert3Block.Bytes)
assert.Equal(t, cert3.Subject.String(), cert3CertSubj)
cert3Block, _ := pem.Decode([]byte(cert3Cert))
cert3, _ := x509.ParseCertificate(cert3Block.Bytes)
assert.Equal(t, cert3.Subject.String(), cert3CertSubj)

opts := x509.VerifyOptions{
Roots: certPool,
}
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)
}
}
2 changes: 1 addition & 1 deletion pkg/validation/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down

0 comments on commit 6aa359a

Please sign in to comment.