From 49fb00c8952e31195bdfb030b51fdabf4e02ded3 Mon Sep 17 00:00:00 2001
From: Caleb Doxsey <cdoxsey@pomerium.com>
Date: Mon, 10 Jan 2022 10:51:56 -0700
Subject: [PATCH] envoy: check certificates for must-staple flag and drop them
 if they are missing the response (#2909)

* envoy: check certificates for must-staple flag and drop them if they are missing the response

* Update config/envoyconfig/tls_test.go

Co-authored-by: Denis Mishin <dmishin@pomerium.com>

Co-authored-by: Denis Mishin <dmishin@pomerium.com>
---
 config/envoyconfig/listeners.go |  6 ++++++
 config/envoyconfig/tls.go       | 35 +++++++++++++++++++++++++++++++++
 config/envoyconfig/tls_test.go  | 16 +++++++++++++++
 pkg/cryptutil/certificates.go   |  9 ++++++---
 4 files changed, 63 insertions(+), 3 deletions(-)

diff --git a/config/envoyconfig/listeners.go b/config/envoyconfig/listeners.go
index eb37e9076..2f08cfdd3 100644
--- a/config/envoyconfig/listeners.go
+++ b/config/envoyconfig/listeners.go
@@ -648,6 +648,12 @@ func (b *Builder) buildDownstreamTLSContext(ctx context.Context,
 		return nil
 	}
 
+	err = validateCertificate(cert)
+	if err != nil {
+		log.Warn(ctx).Str("domain", domain).Err(err).Msg("invalid certificate for domain")
+		return nil
+	}
+
 	var alpnProtocols []string
 	switch cfg.Options.GetCodecType() {
 	case config.CodecTypeHTTP1:
diff --git a/config/envoyconfig/tls.go b/config/envoyconfig/tls.go
index 0ea50bfcb..f16830f2a 100644
--- a/config/envoyconfig/tls.go
+++ b/config/envoyconfig/tls.go
@@ -1,6 +1,10 @@
 package envoyconfig
 
 import (
+	"crypto/tls"
+	"crypto/x509"
+	"encoding/asn1"
+	"fmt"
 	"net/url"
 	"regexp"
 	"strings"
@@ -8,6 +12,8 @@ import (
 	envoy_type_matcher_v3 "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3"
 )
 
+var oidMustStaple = asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 1, 24}
+
 func (b *Builder) buildSubjectAlternativeNameMatcher(
 	dst *url.URL,
 	overrideName string,
@@ -50,3 +56,32 @@ func (b *Builder) buildSubjectNameIndication(
 	sni = strings.Replace(sni, "*", "example", -1)
 	return sni
 }
+
+// validateCertificate validates that a certificate can be used with Envoy's TLS stack.
+func validateCertificate(cert *tls.Certificate) error {
+	if len(cert.Certificate) == 0 {
+		return nil
+	}
+
+	// parse the x509 certificate because leaf isn't always filled in
+	x509cert, err := x509.ParseCertificate(cert.Certificate[0])
+	if err != nil {
+		return err
+	}
+
+	// check to make sure that if we require an OCSP staple that its available.
+	if len(cert.OCSPStaple) == 0 && hasMustStaple(x509cert) {
+		return fmt.Errorf("certificate requires OCSP stapling but has no OCSP staple response")
+	}
+
+	return nil
+}
+
+func hasMustStaple(cert *x509.Certificate) bool {
+	for _, ext := range cert.Extensions {
+		if ext.Id.Equal(oidMustStaple) {
+			return true
+		}
+	}
+	return false
+}
diff --git a/config/envoyconfig/tls_test.go b/config/envoyconfig/tls_test.go
index d6149b0da..848d0ffb7 100644
--- a/config/envoyconfig/tls_test.go
+++ b/config/envoyconfig/tls_test.go
@@ -1,12 +1,16 @@
 package envoyconfig
 
 import (
+	"crypto/x509"
+	"crypto/x509/pkix"
 	"net/url"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/pomerium/pomerium/internal/testutil"
+	"github.com/pomerium/pomerium/pkg/cryptutil"
 )
 
 func TestBuildSubjectAlternativeNameMatcher(t *testing.T) {
@@ -31,3 +35,15 @@ func TestBuildSubjectNameIndication(t *testing.T) {
 	assert.Equal(t, "example.org", b.buildSubjectNameIndication(&url.URL{Host: "example.com:1234"}, "example.org"))
 	assert.Equal(t, "example.example.org", b.buildSubjectNameIndication(&url.URL{Host: "example.com:1234"}, "*.example.org"))
 }
+
+func TestValidateCertificate(t *testing.T) {
+	cert, err := cryptutil.GenerateSelfSignedCertificate("example.com", func(tpl *x509.Certificate) {
+		// set the must staple flag on the cert
+		tpl.ExtraExtensions = append(tpl.ExtraExtensions, pkix.Extension{
+			Id: oidMustStaple,
+		})
+	})
+	require.NoError(t, err)
+
+	assert.Error(t, validateCertificate(cert), "should return an error for a must-staple TLS certificate that has no stapled OCSP response")
+}
diff --git a/pkg/cryptutil/certificates.go b/pkg/cryptutil/certificates.go
index f21f04665..841de58b2 100644
--- a/pkg/cryptutil/certificates.go
+++ b/pkg/cryptutil/certificates.go
@@ -163,7 +163,7 @@ func EncodePrivateKey(key *ecdsa.PrivateKey) ([]byte, error) {
 // GenerateSelfSignedCertificate generates a self-signed TLS certificate.
 //
 // mostly copied from https://golang.org/src/crypto/tls/generate_cert.go
-func GenerateSelfSignedCertificate(domain string) (*tls.Certificate, error) {
+func GenerateSelfSignedCertificate(domain string, configure ...func(*x509.Certificate)) (*tls.Certificate, error) {
 	privateKey, err := rsa.GenerateKey(rand.Reader, 2048)
 	if err != nil {
 		return nil, fmt.Errorf("failed to geneate private key: %w", err)
@@ -175,7 +175,7 @@ func GenerateSelfSignedCertificate(domain string) (*tls.Certificate, error) {
 		return nil, fmt.Errorf("failed to generate serial number: %w", err)
 	}
 
-	template := x509.Certificate{
+	template := &x509.Certificate{
 		SerialNumber: serialNumber,
 		Subject: pkix.Name{
 			Organization: []string{"Pomerium"},
@@ -191,9 +191,12 @@ func GenerateSelfSignedCertificate(domain string) (*tls.Certificate, error) {
 	} else {
 		template.DNSNames = append(template.DNSNames, domain)
 	}
+	for _, f := range configure {
+		f(template)
+	}
 
 	publicKeyBytes, err := x509.CreateCertificate(rand.Reader,
-		&template, &template,
+		template, template,
 		privateKey.Public(), privateKey,
 	)
 	if err != nil {