fix WillHaveCertificateForServerName check to be strict match for der…

…ived cert name (#4167)
pomerium · May 9, 2023 · 4e61013 · 4e61013
1 parent 66dadf7
commit 4e61013
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 8 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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.