From 4e61013428d6efbfd197c056bd7c17594858a4d2 Mon Sep 17 00:00:00 2001 From: Denis Mishin Date: Tue, 9 May 2023 18:54:50 -0400 Subject: [PATCH] fix WillHaveCertificateForServerName check to be strict match for derived cert name (#4167) --- config/config.go | 2 +- config/config_test.go | 28 ++++++++++++++++++++++++++++ config/envoyconfig/listeners_test.go | 17 +++++++++++------ config/options.go | 2 +- 4 files changed, 41 insertions(+), 8 deletions(-) diff --git a/config/config.go b/config/config.go index 9e42d66fa91..f7fe5173263 100644 --- a/config/config.go +++ b/config/config.go @@ -213,7 +213,7 @@ func (cfg *Config) WillHaveCertificateForServerName(serverName string) (bool, er } } - return cfg.Options.DeriveInternalDomainCert != nil, nil + return cfg.Options.GetDeriveInternalDomain() == serverName, nil } // GetCertificatePool gets the certificate pool for the config. diff --git a/config/config_test.go b/config/config_test.go index 7073a13dc0d..bffaf26420d 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -2,9 +2,12 @@ package config import ( "crypto/tls" + "crypto/x509" "testing" + "github.com/golang/protobuf/proto" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/pomerium/pomerium/pkg/cryptutil" ) @@ -63,4 +66,29 @@ func TestConfig_GetCertificateForServerName(t *testing.T) { } assert.NotNil(t, found) }) + t.Run("generate for specific name", func(t *testing.T) { + cfg := &Config{Options: NewDefaultOptions()} + cfg.Options.DeriveInternalDomainCert = proto.String("databroker.int.example.com") + + ok, err := cfg.WillHaveCertificateForServerName("databroker.int.example.com") + require.NoError(t, err) + assert.True(t, ok) + + found, err := cfg.GetCertificateForServerName("databroker.int.example.com") + require.NoError(t, err) + assert.True(t, cryptutil.MatchesServerName(found, "databroker.int.example.com")) + + certPool, err := cfg.GetCertificatePool() + require.NoError(t, err) + + xc, err := x509.ParseCertificate(found.Certificate[0]) + require.NoError(t, err) + + _, err = xc.Verify(x509.VerifyOptions{ + DNSName: "databroker.int.example.com", + KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageAny}, + Roots: certPool, + }) + require.NoError(t, err) + }) } diff --git a/config/envoyconfig/listeners_test.go b/config/envoyconfig/listeners_test.go index 282f7e0717a..d856ebaa999 100644 --- a/config/envoyconfig/listeners_test.go +++ b/config/envoyconfig/listeners_test.go @@ -208,12 +208,13 @@ func Test_getAllDomains(t *testing.T) { require.NoError(t, err) options := &config.Options{ - Addr: "127.0.0.1:9000", - GRPCAddr: "127.0.0.1:9001", - Services: "all", - AuthenticateURLString: "https://authenticate.example.com", - AuthorizeURLString: "https://authorize.example.com:9001", - DataBrokerURLString: "https://cache.example.com:9001", + Addr: "127.0.0.1:9000", + GRPCAddr: "127.0.0.1:9001", + Services: "all", + AuthenticateURLString: "https://authenticate.example.com", + AuthenticateInternalURLString: "https://authenticate.int.example.com", + AuthorizeURLString: "https://authorize.example.com:9001", + DataBrokerURLString: "https://cache.example.com:9001", Policies: []config.Policy{ {From: "http://a.example.com"}, {From: "https://b.example.com"}, @@ -232,6 +233,8 @@ func Test_getAllDomains(t *testing.T) { "a.example.com:80", "authenticate.example.com", "authenticate.example.com:443", + "authenticate.int.example.com", + "authenticate.int.example.com:443", "b.example.com", "b.example.com:443", "c.example.com", @@ -260,6 +263,8 @@ func Test_getAllDomains(t *testing.T) { "a.example.com:80", "authenticate.example.com", "authenticate.example.com:443", + "authenticate.int.example.com", + "authenticate.int.example.com:443", "authorize.example.com:9001", "b.example.com", "b.example.com:443", diff --git a/config/options.go b/config/options.go index 136e6d85484..3fe2fe9729b 100644 --- a/config/options.go +++ b/config/options.go @@ -740,7 +740,7 @@ func (o *Options) GetDeriveInternalDomain() string { if o.DeriveInternalDomainCert == nil { return "" } - return *o.DeriveInternalDomainCert + return strings.ToLower(*o.DeriveInternalDomainCert) } // GetAuthenticateURL returns the AuthenticateURL in the options or 127.0.0.1.