b0375e8 · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

Fix #635: Support specifying alternative provider TLS trust source(s) (#645)

* Fix #635: Support specifying alternative provider TLS trust source(s) * Update pkg/apis/options/options.go Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk> * Update pkg/validation/options.go Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk> * Address review comments * upd CHANGELOG.md * refactor test to assert textual subjects + add openssl gen cmd Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>

commitb0375e85fa8bd5a74632f74c545b6d35d56aa34f authork-wall <kwall@apache.org> dateFri 3 Jul 2020, 16:09 (6 years ago) parent390d479

7 files changed+163−18

compare with master

unifiedsplit

CHANGELOG.md+1 docs/configuration/configuration.md+1 pkg/apis/options/options.go+18−16 pkg/util/util.go+24 pkg/util/util_test.go+91 pkg/validation/options.go+15−2 pkg/validation/options_test.go+13

CHANGELOG.md+1

@@ -10,6 +10,7 @@

1010

1111- [#542](https://github.com/oauth2-proxy/oauth2-proxy/pull/542) Move SessionStore tests to independent package (@JoelSpeed)

1212- [#577](https://github.com/oauth2-proxy/oauth2-proxy/pull/577) Move Cipher and Session Store initialisation out of Validation (@JoelSpeed)

13+- [#635](https://github.com/oauth2-proxy/oauth2-proxy/pull/635) Support specifying alternative provider TLS trust source(s) (@k-wall)

1314

1415# v6.0.0

1516

docs/configuration/configuration.md+1

@@ -86,6 +86,7 @@ An example [oauth2-proxy.cfg]({{ site.gitweb }}/contrib/oauth2-proxy.cfg.example

pkg/apis/options/options.go+18−16

@@ -85,22 +85,23 @@ type Options struct {

8585

8686 // These options allow for other providers besides Google, with

8787 // potential overrides.

88− ProviderType string `flag:"provider" cfg:"provider"`

89− ProviderName string `flag:"provider-display-name" cfg:"provider_display_name"`

90− OIDCIssuerURL string `flag:"oidc-issuer-url" cfg:"oidc_issuer_url"`

91− InsecureOIDCAllowUnverifiedEmail bool `flag:"insecure-oidc-allow-unverified-email" cfg:"insecure_oidc_allow_unverified_email"`

92− InsecureOIDCSkipIssuerVerification bool `flag:"insecure-oidc-skip-issuer-verification" cfg:"insecure_oidc_skip_issuer_verification"`

93− SkipOIDCDiscovery bool `flag:"skip-oidc-discovery" cfg:"skip_oidc_discovery"`

94− OIDCJwksURL string `flag:"oidc-jwks-url" cfg:"oidc_jwks_url"`

95− LoginURL string `flag:"login-url" cfg:"login_url"`

96− RedeemURL string `flag:"redeem-url" cfg:"redeem_url"`

97− ProfileURL string `flag:"profile-url" cfg:"profile_url"`

98− ProtectedResource string `flag:"resource" cfg:"resource"`

99− ValidateURL string `flag:"validate-url" cfg:"validate_url"`

100− Scope string `flag:"scope" cfg:"scope"`

101− Prompt string `flag:"prompt" cfg:"prompt"`

102− ApprovalPrompt string `flag:"approval-prompt" cfg:"approval_prompt"` // Deprecated by OIDC 1.0

103− UserIDClaim string `flag:"user-id-claim" cfg:"user_id_claim"`

88+ ProviderType string `flag:"provider" cfg:"provider"`

89+ ProviderName string `flag:"provider-display-name" cfg:"provider_display_name"`

90+ ProviderCAFiles []string `flag:"provider-ca-file" cfg:"provider_ca_files"`

91+ OIDCIssuerURL string `flag:"oidc-issuer-url" cfg:"oidc_issuer_url"`

92+ InsecureOIDCAllowUnverifiedEmail bool `flag:"insecure-oidc-allow-unverified-email" cfg:"insecure_oidc_allow_unverified_email"`

93+ InsecureOIDCSkipIssuerVerification bool `flag:"insecure-oidc-skip-issuer-verification" cfg:"insecure_oidc_skip_issuer_verification"`

94+ SkipOIDCDiscovery bool `flag:"skip-oidc-discovery" cfg:"skip_oidc_discovery"`

95+ OIDCJwksURL string `flag:"oidc-jwks-url" cfg:"oidc_jwks_url"`

96+ LoginURL string `flag:"login-url" cfg:"login_url"`

97+ RedeemURL string `flag:"redeem-url" cfg:"redeem_url"`

98+ ProfileURL string `flag:"profile-url" cfg:"profile_url"`

99+ ProtectedResource string `flag:"resource" cfg:"resource"`

100+ ValidateURL string `flag:"validate-url" cfg:"validate_url"`

101+ Scope string `flag:"scope" cfg:"scope"`

102+ Prompt string `flag:"prompt" cfg:"prompt"`

103+ ApprovalPrompt string `flag:"approval-prompt" cfg:"approval_prompt"` // Deprecated by OIDC 1.0

104+ UserIDClaim string `flag:"user-id-claim" cfg:"user_id_claim"`

104105

105106 SignatureKey string `flag:"signature-key" cfg:"signature_key"`

106107 AcrValues string `flag:"acr-values" cfg:"acr_values"`

@@ -267,6 +268,7 @@ func NewFlagSet() *pflag.FlagSet {

267268

268269 flagSet.String("provider", "google", "OAuth provider")

269270 flagSet.String("provider-display-name", "", "Provider display name")

271+ 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.")

270272 flagSet.String("oidc-issuer-url", "", "OpenID Connect issuer URL (ie: https://accounts.google.com)")

271273 flagSet.Bool("insecure-oidc-allow-unverified-email", false, "Don't fail if an email address in an id_token is not verified")

272274 flagSet.Bool("insecure-oidc-skip-issuer-verification", false, "Do not verify if issuer matches OIDC discovery URL")

pkg/util/util.gonew file+24

@@ -0,0 +1,24 @@

1+package util

3+import (

4+ "crypto/x509"

5+ "fmt"

6+ "io/ioutil"

7+)

9+func GetCertPool(paths []string) (*x509.CertPool, error) {

10+ if len(paths) == 0 {

11+ return nil, fmt.Errorf("invalid empty list of Root CAs file paths")

12+ }

13+ pool := x509.NewCertPool()

14+ for _, path := range paths {

15+ data, err := ioutil.ReadFile(path)

16+ if err != nil {

17+ return nil, fmt.Errorf("certificate authority file (%s) could not be read - %s", path, err)

18+ }

19+ if !pool.AppendCertsFromPEM(data) {

20+ return nil, fmt.Errorf("loading certificate authority (%s) failed", path)

21+ }

22+ }

23+ return pool, nil

24+}

pkg/util/util_test.gonew file+91

@@ -0,0 +1,91 @@

1+package util

3+import (

4+ "crypto/x509/pkix"

5+ "encoding/asn1"

6+ "io/ioutil"

7+ "os"

8+ "testing"

10+ "github.com/stretchr/testify/assert"

11+)

12+

13+// Test certificate created with an OpenSSL command in the following form:

14+// openssl req -x509 -newkey rsa:4096 -keyout key-unused.pem -out cert.pem -nodes -subj "/CN=oauth-proxy test ca"

15+

16+var (

17+ testCA1Subj = "CN=oauth-proxy test ca"

18+ testCA1 = `-----BEGIN CERTIFICATE-----

19+MIICuTCCAaGgAwIBAgIFAKuKEWowDQYJKoZIhvcNAQELBQAwHjEcMBoGA1UEAxMT

20+b2F1dGgtcHJveHkgdGVzdCBjYTAeFw0xNzEwMjQyMDExMzJaFw0xOTEwMjQyMDEx

21+MzJaMB4xHDAaBgNVBAMTE29hdXRoLXByb3h5IHRlc3QgY2EwggEiMA0GCSqGSIb3

22+DQEBAQUAA4IBDwAwggEKAoIBAQC5/kmgKNiECuxlj27yTWBWOMVvIB0AaRhQrMA7

23+3iSCk/SHhaTabUuXUGRwmCAewT/y9oX3rTdfnSPCn7praU/27lRFBgOGFrTzAZH6

24+voisF54I3ZxWZgHDJ/ig/KFwd0Y8OATj9/k9uAJSCe6aT7BouJPZVWNGF2dF5BOJ

25+EwFsJiN2s8HpF14DhxFOMMtlckdMHGxi3wj3E/hBCfGvGGU4Wezz48vEWWC1ajWM

26+qVq2vVWi1bcNft8FjWa5wTGpdlDQJM7yvKYJPwRkEjgIXtF1ra3JM3WTTFZO9Yhd

27+QXwO7IWRTdTaypKTNbTDKuWQZsm7xQM9sNcFkukGb3o+uBpLAgMBAAEwDQYJKoZI

28+hvcNAQELBQADggEBAHJNrUfHhN7VOUF60pG8sOEkx0ztjbtbYMj2N9Kb0oSya+re

29+Kmb2Z4JgyV7XHCZ03Jch6L7UBI3Y6/Lp1zdwU03LFayVUchLkvFonoXpRRP5UFYN

30++36xP3ZL1qBYFphARsCk6/tl36czH4oF5gTlhWCRy3upNzn+INk467hnCKt5xuse

31+zhm+xQv/VN1poI0S/oCg9HLA9iKpoqGJByN32yoFr3QViLPqkmJ1v8EiH0Ns+1m3

32+pP5YlVqdRCVrxgT80PIMsvQhfcuIrbbeiRDEUdEX7FqebuGCEa2757MTdW7UYQiB

33+7kgECMnwAOlJME8aDKnmTBajaMy6xCSC87V7wps=

34+-----END CERTIFICATE-----

35+`

36+ testCA2Subj = "CN=oauth-proxy second test ca"

37+ testCA2 = `-----BEGIN CERTIFICATE-----

38+MIICxzCCAa+gAwIBAgIFAKuMKewwDQYJKoZIhvcNAQELBQAwJTEjMCEGA1UEAxMa

39+b2F1dGgtcHJveHkgc2Vjb25kIHRlc3QgY2EwHhcNMTcxMDI1MTYxMTQxWhcNMTkx

40+MDI1MTYxMTQxWjAlMSMwIQYDVQQDExpvYXV0aC1wcm94eSBzZWNvbmQgdGVzdCBj

41+YTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAKdTkEOJ+QpOHy0PqGDR

42+fu8NFyo7BJwAnI+P1G32UXMeecCwBgGJEyv6eHEFV6jH/U2K2H0hynaCFxRuIdTA

43+EeS4s4BAbKqFhQ62I9lF3HVuqRPOe5FYdUl80eQynME22fWQ6/sZdQds0sFqaJBz

44+R4KQQxVULT19Br/6zwQZZhC1NtzSwCqi4CoO2OM7ctUKRvtC87LNGWapz5I4eh0A

45+/q4XJaSObsBCAJD7OVMa1LM3sSINUnvvGoSBKTuJ8MRk/BQRAO/PwXxsa+2h+k+w

46+D6sLExrBgWzAAPQKRKF+nLYVhz9AKn4JBpZt9j4PvTKz1SDcJ5wVEzOfVmii7Ui3

47+EFcCAwEAATANBgkqhkiG9w0BAQsFAAOCAQEAiy58XvhOka3drXv2bwl95FwNtodj

48+L2MmIdF0pp01O0ryREcC1kogamdOS/UHQs4okuCjwinR/UgU+cFGCDYHfeENtUTw

49+Ox2OikYD7bXUpNzbQ4QyF0+cKwAgxD4ai5xSV/NUvMkL1aE8tLyxGm6VkhhyvxU1

50+U9kvLha6KBWOCNd2fBJxgg8RAxFV3vR+xLdEtXnBAeTURrHM19gwMtd16y6gUZTZ

51+Xbl3Ix0t2+sqi0hpEF/iVFdCp5TXiicSnZCtePzCfHePAEfbh5hS0bq8Lbb9DZ6d

52++2jX3AVuYhQPuutxla+vNp2XRcMTbzwXyi/Ig4nHKmPLFXsEbv+4tSwxyQ==

53+-----END CERTIFICATE-----

54+`

55+)

56+

57+func makeTestCertFile(t *testing.T, pem, dir string) *os.File {

58+ file, err := ioutil.TempFile(dir, "test-certfile")

59+ assert.NoError(t, err)

60+ _, err = file.Write([]byte(pem))

61+ assert.NoError(t, err)

62+ return file

63+}

64+

65+func TestGetCertPool_NoRoots(t *testing.T) {

66+ _, err := GetCertPool([]string(nil))

67+ assert.Error(t, err, "invalid empty list of Root CAs file paths")

68+}

69+

70+func TestGetCertPool(t *testing.T) {

71+ tempDir, err := ioutil.TempDir("", "certtest")

72+ assert.NoError(t, err)

73+ defer os.RemoveAll(tempDir)

74+ certFile1 := makeTestCertFile(t, testCA1, tempDir)

75+ certFile2 := makeTestCertFile(t, testCA2, tempDir)

76+

77+ certPool, err := GetCertPool([]string{certFile1.Name(), certFile2.Name()})

78+ assert.NoError(t, err)

79+

80+ subj := certPool.Subjects()

81+ got := make([]string, 0)

82+ for i := range subj {

83+ var subject pkix.RDNSequence

84+ _, err := asn1.Unmarshal(subj[i], &subject)

85+ assert.NoError(t, err)

86+ got = append(got, subject.String())

87+ }

88+

89+ expectedSubjects := []string{testCA1Subj, testCA2Subj}

90+ assert.Equal(t, expectedSubjects, got)

91+}

pkg/validation/options.go+15−2

@@ -21,21 +21,34 @@ import (

2121 "github.com/oauth2-proxy/oauth2-proxy/pkg/ip"

2222 "github.com/oauth2-proxy/oauth2-proxy/pkg/logger"

2323 "github.com/oauth2-proxy/oauth2-proxy/pkg/requests"

24+ "github.com/oauth2-proxy/oauth2-proxy/pkg/util"

2425 "github.com/oauth2-proxy/oauth2-proxy/providers"

2526)

2627

2728// Validate checks that required options are set and validates those that they

2829// are of the correct format

2930func Validate(o *options.Options) error {

31+ msgs := make([]string, 0)

3032 if o.SSLInsecureSkipVerify {

31− // TODO: Accept a certificate bundle.

3233 insecureTransport := &http.Transport{

3334 TLSClientConfig: &tls.Config{InsecureSkipVerify: true},

3435 }

3536 http.DefaultClient = &http.Client{Transport: insecureTransport}

37+ } else if len(o.ProviderCAFiles) > 0 {

38+ pool, err := util.GetCertPool(o.ProviderCAFiles)

39+ if err == nil {

40+ transport := &http.Transport{

41+ TLSClientConfig: &tls.Config{

42+ RootCAs: pool,

43+ },

44+ }

45+

46+ http.DefaultClient = &http.Client{Transport: transport}

47+ } else {

48+ msgs = append(msgs, fmt.Sprintf("unable to load provider CA file(s): %v", err))

49+ }

3650 }

3751

38− msgs := make([]string, 0)

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

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

4154 } else {

pkg/validation/options_test.go+13

@@ -366,3 +366,16 @@ func TestRealClientIPHeader(t *testing.T) {

366366 assert.Equal(t, expected, err.Error())

367367 assert.Nil(t, o.GetRealClientIPParser())

368368}

369+

370+func TestProviderCAFilesError(t *testing.T) {

371+ file, err := ioutil.TempFile("", "absent.*.crt")

372+ assert.NoError(t, err)

373+ assert.NoError(t, file.Close())

374+ assert.NoError(t, os.Remove(file.Name()))

375+

376+ o := testOptions()

377+ o.ProviderCAFiles = append(o.ProviderCAFiles, file.Name())

378+ err = Validate(o)

379+ assert.Error(t, err)

380+ assert.Contains(t, err.Error(), "unable to load provider CA file(s)")

381+}