From 489d49380ba54800c49847fef1cdbe1cfdb9f315 Mon Sep 17 00:00:00 2001 From: Inverse Integral Date: Mon, 16 Sep 2024 20:21:39 +0200 Subject: [PATCH] Support verifying server certificate via CA (#5058) --- .../hashicorpvault/hashicorp_vault.go | 6 +- .../testdata/invalid-client-cert.pem | 1 + .../testdata/invalid-client-key.pem | 1 + .../testdata/invalid-root-cert.pem | 1 + .../keymanager/hashicorpvault/vault_client.go | 2 - .../hashicorpvault/vault_client_test.go | 177 +++++++++++++++++- 6 files changed, 179 insertions(+), 9 deletions(-) create mode 100644 pkg/server/plugin/keymanager/hashicorpvault/testdata/invalid-client-cert.pem create mode 100644 pkg/server/plugin/keymanager/hashicorpvault/testdata/invalid-client-key.pem create mode 100644 pkg/server/plugin/keymanager/hashicorpvault/testdata/invalid-root-cert.pem diff --git a/pkg/server/plugin/keymanager/hashicorpvault/hashicorp_vault.go b/pkg/server/plugin/keymanager/hashicorpvault/hashicorp_vault.go index ddf9c76b8ea..61c22b4cdac 100644 --- a/pkg/server/plugin/keymanager/hashicorpvault/hashicorp_vault.go +++ b/pkg/server/plugin/keymanager/hashicorpvault/hashicorp_vault.go @@ -59,8 +59,9 @@ type Config struct { // If true, vault client accepts any server certificates. // It should be used only test environment so on. InsecureSkipVerify bool `hcl:"insecure_skip_verify" json:"insecure_skip_verify"` - - // TODO: Support CA certificate path here instead of insecure skip verify + // Path to a CA certificate file that the client verifies the server certificate. + // Only PEM format is supported. + CACertPath string `hcl:"ca_cert_path" json:"ca_cert_path"` // Configuration for the Token authentication method TokenAuth *TokenAuthConfig `hcl:"token_auth" json:"token_auth,omitempty"` @@ -228,6 +229,7 @@ func (p *Plugin) genClientParams(method AuthMethod, config *Config) (*ClientPara VaultAddr: p.getEnvOrDefault(envVaultAddr, config.VaultAddr), Namespace: p.getEnvOrDefault(envVaultNamespace, config.Namespace), TransitEnginePath: p.getEnvOrDefault(envVaultTransitEnginePath, config.TransitEnginePath), + CACertPath: p.getEnvOrDefault(envVaultCACert, config.CACertPath), TLSSKipVerify: config.InsecureSkipVerify, } diff --git a/pkg/server/plugin/keymanager/hashicorpvault/testdata/invalid-client-cert.pem b/pkg/server/plugin/keymanager/hashicorpvault/testdata/invalid-client-cert.pem new file mode 100644 index 00000000000..7ec3efa7578 --- /dev/null +++ b/pkg/server/plugin/keymanager/hashicorpvault/testdata/invalid-client-cert.pem @@ -0,0 +1 @@ +"invalid-client-cert-file" diff --git a/pkg/server/plugin/keymanager/hashicorpvault/testdata/invalid-client-key.pem b/pkg/server/plugin/keymanager/hashicorpvault/testdata/invalid-client-key.pem new file mode 100644 index 00000000000..2ce22f3da63 --- /dev/null +++ b/pkg/server/plugin/keymanager/hashicorpvault/testdata/invalid-client-key.pem @@ -0,0 +1 @@ +"invalid-client-key-file" diff --git a/pkg/server/plugin/keymanager/hashicorpvault/testdata/invalid-root-cert.pem b/pkg/server/plugin/keymanager/hashicorpvault/testdata/invalid-root-cert.pem new file mode 100644 index 00000000000..c224998f837 --- /dev/null +++ b/pkg/server/plugin/keymanager/hashicorpvault/testdata/invalid-root-cert.pem @@ -0,0 +1 @@ +"invalid-root-cert-file" diff --git a/pkg/server/plugin/keymanager/hashicorpvault/vault_client.go b/pkg/server/plugin/keymanager/hashicorpvault/vault_client.go index a1ecfde3c4e..bb60233a101 100644 --- a/pkg/server/plugin/keymanager/hashicorpvault/vault_client.go +++ b/pkg/server/plugin/keymanager/hashicorpvault/vault_client.go @@ -18,8 +18,6 @@ import ( "github.com/spiffe/spire/pkg/common/pemutil" ) -// TODO: Delete everything that is unused in here - const ( envVaultAddr = "VAULT_ADDR" envVaultToken = "VAULT_TOKEN" diff --git a/pkg/server/plugin/keymanager/hashicorpvault/vault_client_test.go b/pkg/server/plugin/keymanager/hashicorpvault/vault_client_test.go index d626167cf67..1b243617566 100644 --- a/pkg/server/plugin/keymanager/hashicorpvault/vault_client_test.go +++ b/pkg/server/plugin/keymanager/hashicorpvault/vault_client_test.go @@ -1,7 +1,12 @@ package hashicorpvault import ( + "crypto/tls" + "crypto/x509" "fmt" + vapi "github.com/hashicorp/vault/api" + "net/http" + "os" "testing" "time" @@ -13,11 +18,14 @@ import ( ) const ( - testRootCert = "testdata/root-cert.pem" - testServerCert = "testdata/server-cert.pem" - testServerKey = "testdata/server-key.pem" - testClientCert = "testdata/client-cert.pem" - testClientKey = "testdata/client-key.pem" + testRootCert = "testdata/root-cert.pem" + testInvalidRootCert = "testdata/invalid-root-cert.pem" + testServerCert = "testdata/server-cert.pem" + testServerKey = "testdata/server-key.pem" + testClientCert = "testdata/client-cert.pem" + testInvalidClientCert = "testdata/invalid-client-cert.pem" + testClientKey = "testdata/client-key.pem" + testInvalidClientKey = "testdata/invalid-client-key.pem" ) func TestNewClientConfigWithDefaultValues(t *testing.T) { @@ -470,9 +478,168 @@ func TestRenewTokenFailed(t *testing.T) { } } +func TestConfigureTLSWithCertAuth(t *testing.T) { + cp := &ClientParams{ + VaultAddr: "http://example.org:8200", + ClientCertPath: testClientCert, + ClientKeyPath: testClientKey, + CACertPath: testRootCert, + } + cc, err := NewClientConfig(cp, hclog.Default()) + require.NoError(t, err) + + vc := vapi.DefaultConfig() + err = cc.configureTLS(vc) + require.NoError(t, err) + + tcc := vc.HttpClient.Transport.(*http.Transport).TLSClientConfig + cert, err := tcc.GetClientCertificate(&tls.CertificateRequestInfo{}) + require.NoError(t, err) + + testCert, err := testClientCertificatePair() + require.NoError(t, err) + require.Equal(t, testCert.Certificate, cert.Certificate) + + testPool, err := testRootCAs() + require.NoError(t, err) + require.True(t, testPool.Equal(tcc.RootCAs)) +} + +func TestConfigureTLSWithTokenAuth(t *testing.T) { + cp := &ClientParams{ + VaultAddr: "http://example.org:8200", + CACertPath: testRootCert, + Token: "test-token", + } + cc, err := NewClientConfig(cp, hclog.Default()) + require.NoError(t, err) + + vc := vapi.DefaultConfig() + err = cc.configureTLS(vc) + require.NoError(t, err) + + tcc := vc.HttpClient.Transport.(*http.Transport).TLSClientConfig + require.Nil(t, tcc.GetClientCertificate) + + testPool, err := testRootCAs() + require.NoError(t, err) + require.Equal(t, testPool.Subjects(), tcc.RootCAs.Subjects()) //nolint:staticcheck // these pools are not system pools so the use of Subjects() is ok for now +} + +func TestConfigureTLSWithAppRoleAuth(t *testing.T) { + cp := &ClientParams{ + VaultAddr: "http://example.org:8200", + CACertPath: testRootCert, + AppRoleID: "test-approle-id", + AppRoleSecretID: "test-approle-secret", + } + cc, err := NewClientConfig(cp, hclog.Default()) + require.NoError(t, err) + + vc := vapi.DefaultConfig() + err = cc.configureTLS(vc) + require.NoError(t, err) + + tcc := vc.HttpClient.Transport.(*http.Transport).TLSClientConfig + require.Nil(t, tcc.GetClientCertificate) + + testPool, err := testRootCAs() + require.NoError(t, err) + require.Equal(t, testPool.Subjects(), tcc.RootCAs.Subjects()) //nolint:staticcheck // these pools are not system pools so the use of Subjects() is ok for now +} + +func TestConfigureTLSInvalidCACert(t *testing.T) { + cp := &ClientParams{ + VaultAddr: "http://example.org:8200", + ClientCertPath: testClientCert, + ClientKeyPath: testClientKey, + CACertPath: testInvalidRootCert, + } + cc, err := NewClientConfig(cp, hclog.Default()) + require.NoError(t, err) + + vc := vapi.DefaultConfig() + err = cc.configureTLS(vc) + spiretest.RequireGRPCStatusHasPrefix(t, err, codes.InvalidArgument, "failed to load CA certificate: no PEM blocks") +} + +func TestConfigureTLSInvalidClientKey(t *testing.T) { + cp := &ClientParams{ + VaultAddr: "http://example.org:8200", + ClientCertPath: testClientCert, + ClientKeyPath: testInvalidClientKey, + CACertPath: testRootCert, + } + cc, err := NewClientConfig(cp, hclog.Default()) + require.NoError(t, err) + + vc := vapi.DefaultConfig() + err = cc.configureTLS(vc) + spiretest.RequireGRPCStatusHasPrefix(t, err, codes.InvalidArgument, "failed to parse client cert and private-key: tls: failed to find any PEM data in key input") +} + +func TestConfigureTLSInvalidClientCert(t *testing.T) { + cp := &ClientParams{ + VaultAddr: "http://example.org:8200", + ClientCertPath: testInvalidClientCert, + ClientKeyPath: testClientKey, + CACertPath: testRootCert, + } + cc, err := NewClientConfig(cp, hclog.Default()) + require.NoError(t, err) + + vc := vapi.DefaultConfig() + err = cc.configureTLS(vc) + spiretest.RequireGRPCStatusHasPrefix(t, err, codes.InvalidArgument, "failed to parse client cert and private-key: tls: failed to find any PEM data in certificate input") +} + +func TestConfigureTLSRequireClientCertAndKey(t *testing.T) { + cp := &ClientParams{ + VaultAddr: "http://example.org:8200", + ClientCertPath: testClientCert, + CACertPath: testRootCert, + } + cc, err := NewClientConfig(cp, hclog.Default()) + require.NoError(t, err) + + vc := vapi.DefaultConfig() + err = cc.configureTLS(vc) + spiretest.RequireGRPCStatus(t, err, codes.InvalidArgument, "both client cert and client key are required") +} + +// TODO: Test CreateKey +// TODO: Test GetKey +// TODO: Test SignData + func newFakeVaultServer() *FakeVaultServerConfig { fakeVaultServer := NewFakeVaultServerConfig() fakeVaultServer.RenewResponseCode = 200 fakeVaultServer.RenewResponse = []byte(testRenewResponse) return fakeVaultServer } + +func testClientCertificatePair() (tls.Certificate, error) { + cert, err := os.ReadFile(testClientCert) + if err != nil { + return tls.Certificate{}, err + } + key, err := os.ReadFile(testClientKey) + if err != nil { + return tls.Certificate{}, err + } + + return tls.X509KeyPair(cert, key) +} + +func testRootCAs() (*x509.CertPool, error) { + pool := x509.NewCertPool() + pem, err := os.ReadFile(testRootCert) + if err != nil { + return nil, err + } + ok := pool.AppendCertsFromPEM(pem) + if !ok { + return nil, err + } + return pool, nil +}