From 5333b0a9798aba9d2913bf85ba1ff5f9adc560ff Mon Sep 17 00:00:00 2001 From: Timofey Mischenko Date: Fri, 13 Dec 2024 15:37:00 +0300 Subject: [PATCH] dirty fix+test CRL --- cmd/integration_test.go | 228 ++++++++++++++++++++++++++++++++- cmd/smokescreen.go | 14 +- pkg/smokescreen/config.go | 2 +- pkg/smokescreen/smokescreen.go | 64 +++++++++ 4 files changed, 299 insertions(+), 9 deletions(-) diff --git a/cmd/integration_test.go b/cmd/integration_test.go index cfa53cb1..c48d4c78 100644 --- a/cmd/integration_test.go +++ b/cmd/integration_test.go @@ -18,6 +18,7 @@ import ( "net/http/httptest" "net/url" "os" + "os/exec" "testing" "time" @@ -648,7 +649,7 @@ func startSmokescreen(t *testing.T, useTLS bool, logHook logrus.Hook, httpProxyA ) } - if httpProxyAddr != ""{ + if httpProxyAddr != "" { args = append(args, fmt.Sprintf("--upstream-http-proxy-addr=%s", httpProxyAddr)) args = append(args, fmt.Sprintf("--upstream-https-proxy-addr=%s", httpProxyAddr)) } @@ -683,3 +684,228 @@ func startSmokescreen(t *testing.T, useTLS bool, logHook logrus.Hook, httpProxyA return conf, server, nil } + +func TestCRLValidation(t *testing.T) { + cleanup := generateTestPKI(t) + defer cleanup() + + cleanupACL := generatePermissiveACL(t) + defer cleanupACL() + + // Start smokescreen with TLS and CRL configuration + args := []string{ + "smokescreen", + "--listen-ip=127.0.0.1", + "--egress-acl-file=testdata/pki-crl/acl.yaml", + "--tls-crl-file=testdata/pki-crl/crl.pem", + "--tls-server-bundle-file=testdata/pki-crl/server-bundle.pem", + "--tls-client-ca-file=testdata/pki-crl/ca.pem", + } + conf, err := NewConfiguration(args, nil) + require.NoError(t, err) + + // Add role from request function + conf.RoleFromRequest = testRFRCert + conf.MetricsClient = metrics.NewNoOpMetricsClient() + + handler := smokescreen.BuildProxy(conf) + server := httptest.NewUnstartedServer(handler) + server.TLS = conf.TlsConfig + server.StartTLS() + defer server.Close() + + // Test cases for valid and revoked certificates + testCases := []struct { + name string + certFile string + keyFile string + expectSuccess bool + }{ + { + name: "Valid certificate", + certFile: "testdata/pki-crl/valid-client.pem", + keyFile: "testdata/pki-crl/valid-client-key.pem", + expectSuccess: true, + }, + { + name: "Revoked certificate", + certFile: "testdata/pki-crl/revoked-client.pem", + keyFile: "testdata/pki-crl/revoked-client-key.pem", + expectSuccess: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + cert, err := tls.LoadX509KeyPair(tc.certFile, tc.keyFile) + require.NoError(t, err) + + caCert, err := os.ReadFile("testdata/pki-crl/ca.pem") + require.NoError(t, err) + caCertPool := x509.NewCertPool() + caCertPool.AppendCertsFromPEM(caCert) + + proxyURL, err := url.Parse(server.URL) + require.NoError(t, err) + + client := &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + Certificates: []tls.Certificate{cert}, + RootCAs: caCertPool, + InsecureSkipVerify: true, // For testing only - allows connecting to proxy with self-signed cert + }, + Proxy: http.ProxyURL(proxyURL), + }, + } + + // Make request through proxy + resp, err := client.Get("https://example.com") + + if tc.expectSuccess { + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode) + } else { + require.Error(t, err) + var netErr *net.OpError + require.True(t, errors.As(err, &netErr)) + } + }) + } +} + +func generateTestPKI(t *testing.T) func() { + // Create test directory + err := os.MkdirAll("testdata/pki-crl", 0755) + require.NoError(t, err) + + // Helper function to run openssl commands + runOpenSSL := func(args ...string) { + cmd := exec.Command("openssl", args...) + output, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("openssl %v failed: %v\nOutput: %s", args, err, output) + } + } + + // Generate CA key and certificate + runOpenSSL("genrsa", "-out", "testdata/pki-crl/ca-key.pem", "2048") + runOpenSSL("req", "-new", "-x509", "-days", "365", "-key", "testdata/pki-crl/ca-key.pem", + "-out", "testdata/pki-crl/ca.pem", "-subj", "/CN=Test CA") + + // Generate server key and certificate + runOpenSSL("genrsa", "-out", "testdata/pki-crl/server-key.pem", "2048") + runOpenSSL("req", "-new", "-key", "testdata/pki-crl/server-key.pem", + "-out", "testdata/pki-crl/server.csr", "-subj", "/CN=localhost") + runOpenSSL("x509", "-req", "-days", "365", "-in", "testdata/pki-crl/server.csr", + "-CA", "testdata/pki-crl/ca.pem", "-CAkey", "testdata/pki-crl/ca-key.pem", + "-CAcreateserial", "-out", "testdata/pki-crl/server.pem") + + // Create server bundle by concatenating key and cert + serverKey, err := os.ReadFile("testdata/pki-crl/server-key.pem") + require.NoError(t, err) + serverCert, err := os.ReadFile("testdata/pki-crl/server.pem") + require.NoError(t, err) + err = os.WriteFile("testdata/pki-crl/server-bundle.pem", + append(serverKey, serverCert...), 0644) + require.NoError(t, err) + + // Generate valid client key and certificate + runOpenSSL("genrsa", "-out", "testdata/pki-crl/valid-client-key.pem", "2048") + runOpenSSL("req", "-new", "-key", "testdata/pki-crl/valid-client-key.pem", + "-out", "testdata/pki-crl/valid-client.csr", "-subj", "/CN=valid-client") + runOpenSSL("x509", "-req", "-days", "365", "-in", "testdata/pki-crl/valid-client.csr", + "-CA", "testdata/pki-crl/ca.pem", "-CAkey", "testdata/pki-crl/ca-key.pem", + "-CAcreateserial", "-out", "testdata/pki-crl/valid-client.pem") + + // Generate to-be-revoked client key and certificate + runOpenSSL("genrsa", "-out", "testdata/pki-crl/revoked-client-key.pem", "2048") + runOpenSSL("req", "-new", "-key", "testdata/pki-crl/revoked-client-key.pem", + "-out", "testdata/pki-crl/revoked-client.csr", "-subj", "/CN=revoked-client") + runOpenSSL("x509", "-req", "-days", "365", "-in", "testdata/pki-crl/revoked-client.csr", + "-CA", "testdata/pki-crl/ca.pem", "-CAkey", "testdata/pki-crl/ca-key.pem", + "-CAcreateserial", "-out", "testdata/pki-crl/revoked-client.pem") + + // Create index.txt and crlnumber for CRL generation + err = os.WriteFile("testdata/pki-crl/index.txt", []byte{}, 0644) + require.NoError(t, err) + err = os.WriteFile("testdata/pki-crl/crlnumber", []byte("01\n"), 0644) + require.NoError(t, err) + + // Create openssl CA config with extensions + caConfig := ` +[ ca ] +default_ca = CA_default + +[ CA_default ] +database = testdata/pki-crl/index.txt +crlnumber = testdata/pki-crl/crlnumber +default_days = 365 +default_crl_days = 30 +default_md = sha256 +copy_extensions = copy +crl_extensions = crl_ext + +[ crl_ext ] +authorityKeyIdentifier=keyid:always,issuer:always +` + err = os.WriteFile("testdata/pki-crl/ca-config.conf", []byte(caConfig), 0644) + require.NoError(t, err) + + // Generate initial CRL + runOpenSSL("ca", "-gencrl", + "-keyfile", "testdata/pki-crl/ca-key.pem", + "-cert", "testdata/pki-crl/ca.pem", + "-out", "testdata/pki-crl/crl.pem", + "-config", "testdata/pki-crl/ca-config.conf") + + // Revoke the second certificate + runOpenSSL("ca", "-revoke", "testdata/pki-crl/revoked-client.pem", + "-keyfile", "testdata/pki-crl/ca-key.pem", + "-cert", "testdata/pki-crl/ca.pem", + "-config", "testdata/pki-crl/ca-config.conf") + + // Generate updated CRL with the revoked certificate + runOpenSSL("ca", "-gencrl", + "-keyfile", "testdata/pki-crl/ca-key.pem", + "-cert", "testdata/pki-crl/ca.pem", + "-out", "testdata/pki-crl/crl.pem", + "-config", "testdata/pki-crl/ca-config.conf") + + // Return cleanup function + return func() { + err := os.RemoveAll("testdata/pki-crl") + if err != nil { + t.Logf("Failed to cleanup test PKI: %v", err) + } + } +} + +func generatePermissiveACL(t *testing.T) func() { + aclConfig := `--- +version: v1 +services: + - name: "*" + project: test + action: open + allowed_domains: + - "*.example.com" + +default: + name: unknown-role + project: security + action: open +` + err := os.MkdirAll("testdata/pki-crl", 0755) + require.NoError(t, err) + + err = os.WriteFile("testdata/pki-crl/acl.yaml", []byte(aclConfig), 0644) + require.NoError(t, err) + + return func() { + err := os.Remove("testdata/pki-crl/acl.yaml") + if err != nil { + t.Logf("Failed to cleanup ACL config: %v", err) + } + } +} diff --git a/cmd/smokescreen.go b/cmd/smokescreen.go index be3a75f8..26c4af30 100644 --- a/cmd/smokescreen.go +++ b/cmd/smokescreen.go @@ -95,7 +95,7 @@ func NewConfiguration(args []string, logger *log.Logger) (*smokescreen.Config, e Usage: "Expose prometheus metrics on `ENDPOINT`. Requires --expose-prometheus-metrics to be set. Defaults to \"/metrics\"", }, cli.StringFlag{ - Name: "prometheus-listen-ip", + Name: "prometheus-listen-ip", Value: "0.0.0.0", Usage: "Listen for prometheus metrics on interface with address IP. Requires --expose-prometheus-metrics to be set. Defaults to \"0.0.0.0\"", }, @@ -270,12 +270,6 @@ func NewConfiguration(args []string, logger *log.Logger) (*smokescreen.Config, e } } - if c.IsSet("tls-crl-file") { - if err := conf.SetupCrls(c.StringSlice("tls-crl-file")); err != nil { - return err - } - } - if c.IsSet("unsafe-allow-private-ranges") { conf.UnsafeAllowPrivateRanges = c.Bool("unsafe-allow-private-ranges") } @@ -297,6 +291,12 @@ func NewConfiguration(args []string, logger *log.Logger) (*smokescreen.Config, e } } + if c.IsSet("tls-crl-file") { + if err := conf.SetupCrls(c.StringSlice("tls-crl-file")); err != nil { + return err + } + } + if c.IsSet("upstream-http-proxy-addr") { conf.UpstreamHttpProxyAddr = c.String("upstream-http-proxy-addr") } diff --git a/pkg/smokescreen/config.go b/pkg/smokescreen/config.go index 274d670f..6d916d06 100644 --- a/pkg/smokescreen/config.go +++ b/pkg/smokescreen/config.go @@ -266,7 +266,7 @@ func NewConfig() *Config { func (config *Config) SetupCrls(crlFiles []string) error { for _, crlFile := range crlFiles { - crlBytes, err := ioutil.ReadFile(crlFile) + crlBytes, err := os.ReadFile(crlFile) if err != nil { return err } diff --git a/pkg/smokescreen/smokescreen.go b/pkg/smokescreen/smokescreen.go index 6c49af9b..ae73c47f 100644 --- a/pkg/smokescreen/smokescreen.go +++ b/pkg/smokescreen/smokescreen.go @@ -1,8 +1,11 @@ package smokescreen import ( + "bytes" "context" "crypto/tls" + "crypto/x509" + "encoding/hex" "errors" "fmt" "io" @@ -448,6 +451,67 @@ func newContext(cfg *Config, proxyType string, req *http.Request) *SmokescreenCo func BuildProxy(config *Config) *goproxy.ProxyHttpServer { proxy := goproxy.NewProxyHttpServer(goproxy.WithHttpProxyAddr(config.UpstreamHttpProxyAddr), goproxy.WithHttpsProxyAddr(config.UpstreamHttpsProxyAddr)) proxy.Verbose = false + + // Add CRL verification to TLS config if CRL file is provided + if config.TlsConfig != nil && config.CrlByAuthorityKeyId != nil { + originalVerify := config.TlsConfig.VerifyPeerCertificate + config.TlsConfig.VerifyPeerCertificate = func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error { + config.Log.WithFields(logrus.Fields{ + "rawCerts": rawCerts, + "verifiedChains": verifiedChains, + }).Debug("VerifyPeerCertificate called") + // First run the original verification if it exists + if originalVerify != nil { + if err := originalVerify(rawCerts, verifiedChains); err != nil { + config.Log.WithFields(logrus.Fields{ + "error": err, + }).Error("original verification failed") + return err + } + } + + // Then check CRL for each certificate in the chain + for _, chain := range verifiedChains { + for _, cert := range chain { + // Skip root CA as it's typically not in CRLs + if cert.IsCA && bytes.Equal(cert.RawSubject, cert.RawIssuer) { + config.Log.WithFields(logrus.Fields{ + "cert": cert.Subject.CommonName, + }).Debug("skipping root CA") + continue + } + + // Get CRL for this certificate's issuer + crl := config.CrlByAuthorityKeyId[string(cert.AuthorityKeyId)] + if crl == nil { + // No CRL for this issuer - could be a warning but let's allow for now + config.Log.WithFields(logrus.Fields{ + "cert": cert.Subject.CommonName, + "authorityKeyId": hex.EncodeToString(cert.AuthorityKeyId), + }).Warn("no CRL for certificate") + continue + } + + // Check if certificate is revoked + for _, revoked := range crl.TBSCertList.RevokedCertificates { + if cert.SerialNumber.Cmp(revoked.SerialNumber) == 0 { + config.Log.WithFields(logrus.Fields{ + "cert": cert.Subject.CommonName, + "serial": cert.SerialNumber, + }).Warn("certificate is revoked") + return fmt.Errorf("certificate with serial number %v is revoked", cert.SerialNumber) + } + config.Log.WithFields(logrus.Fields{ + "cert": cert.Subject.CommonName, + "serial": cert.SerialNumber, + }).Debug("certificate is not revoked") + } + } + } + return nil + } + } + configureTransport(proxy.Tr, config) // dialContext will be invoked for both CONNECT and traditional proxy requests