diff --git a/pkg/agent/manager/manager_test.go b/pkg/agent/manager/manager_test.go index fad4bd22e0..246a1e28ab 100644 --- a/pkg/agent/manager/manager_test.go +++ b/pkg/agent/manager/manager_test.go @@ -1106,25 +1106,23 @@ func (h *mockNodeAPIHandler) getGRPCServerConfig(hello *tls.ClientHelloInfo) (*t } func (h *mockNodeAPIHandler) getCertFromCtx(ctx context.Context) (certificate *x509.Certificate, err error) { - ctxPeer, ok := peer.FromContext(ctx) if !ok { - return nil, errors.New("It was not posible to extract peer from request") + return nil, errors.New("no peer information") } tlsInfo, ok := ctxPeer.AuthInfo.(credentials.TLSInfo) if !ok { - return nil, errors.New("It was not posible to extract AuthInfo from request") + return nil, errors.New("no TLS auth info for peer") } if len(tlsInfo.State.VerifiedChains) == 0 { - return nil, errors.New("VerifiedChains was empty") + return nil, errors.New("no verified client certificate presented by peer") } - chain := tlsInfo.State.VerifiedChains[0] if len(chain) == 0 { // this shouldn't be possible with the tls package, but we should be // defensive. - return nil, errors.New("VerifiedChain was empty") + return nil, errors.New("verified client chain is missing certificates") } return chain[0], nil diff --git a/pkg/server/endpoints/endpoints_test.go b/pkg/server/endpoints/endpoints_test.go index 1982b4e0fa..f04c491c9a 100644 --- a/pkg/server/endpoints/endpoints_test.go +++ b/pkg/server/endpoints/endpoints_test.go @@ -8,6 +8,8 @@ import ( "errors" "net" "net/url" + "strings" + "sync" "testing" "time" @@ -15,6 +17,7 @@ import ( "github.com/sirupsen/logrus/hooks/test" "github.com/spiffe/spire/pkg/common/bundleutil" "github.com/spiffe/spire/pkg/server/svid" + "github.com/spiffe/spire/proto/common" "github.com/spiffe/spire/proto/server/datastore" "github.com/spiffe/spire/test/fakes/fakedatastore" "github.com/spiffe/spire/test/fakes/fakeservercatalog" @@ -221,3 +224,107 @@ func (s *EndpointsTestSuite) configureBundle() ([]tls.Certificate, *x509.CertPoo }, }, caPool } + +func (s *EndpointsTestSuite) TestClientCertificateVerification() { + caTmpl, err := util.NewCATemplate("example.org") + s.Require().NoError(err) + caCert, caKey, err := util.SelfSign(caTmpl) + s.Require().NoError(err) + + serverTmpl, err := util.NewSVIDTemplate("spiffe://example.org/server") + s.Require().NoError(err) + serverTmpl.DNSNames = []string{"just-for-validation"} + serverCert, serverKey, err := util.Sign(serverTmpl, caCert, caKey) + s.Require().NoError(err) + + clientTmpl, err := util.NewSVIDTemplate("spiffe://example.org/agent") + s.Require().NoError(err) + clientCert, clientKey, err := util.Sign(clientTmpl, caCert, caKey) + s.Require().NoError(err) + + otherCaTmpl, err := util.NewCATemplate("example.org") + s.Require().NoError(err) + otherCaCert, otherCaKey, err := util.SelfSign(otherCaTmpl) + s.Require().NoError(err) + + otherClientTmpl, err := util.NewSVIDTemplate("spiffe://example.org/agent") + s.Require().NoError(err) + otherClientCert, otherClientKey, err := util.Sign(otherClientTmpl, otherCaCert, otherCaKey) + s.Require().NoError(err) + + rootCAs := x509.NewCertPool() + rootCAs.AddCert(caCert) + + // set the trust bundle and plumb a CA certificate + _, err = s.ds.CreateBundle(context.Background(), &datastore.CreateBundleRequest{ + Bundle: &common.Bundle{ + TrustDomainId: "spiffe://example.org", + RootCas: []*common.Certificate{ + {DerBytes: caCert.Raw}, + }, + }, + }) + s.Require().NoError(err) + s.svidState.Update(svid.State{ + SVID: []*x509.Certificate{serverCert}, + Key: serverKey, + }) + + var wg sync.WaitGroup + defer wg.Wait() + + ctx, cancel := context.WithCancel(ctx) + defer cancel() + + wg.Add(1) + go func() { + defer wg.Done() + s.e.ListenAndServe(ctx) + }() + + // This helper function attempts a TLS connection to the gRPC server. It + // uses the supplied client certificate, if any. It gives up the 2 seconds + // for the server to start listening, which is generous. Any non-dial + // related errors (i.e. TLS handshake failures) are returned. + try := func(cert *tls.Certificate) error { + tlsConfig := &tls.Config{ + RootCAs: rootCAs, + // this override is just so we don't have to set up spiffe peer + // validation of the server by the client, which is outside the + // scope of this test. + ServerName: "just-for-validation", + } + if cert != nil { + tlsConfig.Certificates = append(tlsConfig.Certificates, *cert) + } + for i := 0; i < 20; i++ { + conn, err := tls.Dial("tcp", "127.0.0.1:8000", tlsConfig) + if err != nil { + if strings.HasPrefix(err.Error(), "dial") { + time.Sleep(time.Millisecond * 100) + continue + } + return err + } + conn.Close() + return nil + } + s.FailNow("unable to connect to server within 2 seconds") + return errors.New("unreachable") + } + + err = try(nil) + s.Require().NoError(err, "client should be allowed if no cert presented") + + err = try(&tls.Certificate{ + Certificate: [][]byte{clientCert.Raw}, + PrivateKey: clientKey, + }) + s.Require().NoError(err, "client should be allowed if proper cert presented") + + err = try(&tls.Certificate{ + Certificate: [][]byte{otherClientCert.Raw}, + PrivateKey: otherClientKey, + }) + s.Require().Error(err, "client should NOT be allowed if cert presented is not trusted") +} diff --git a/pkg/server/endpoints/node/handler.go b/pkg/server/endpoints/node/handler.go index ec342109b9..a6ab788958 100644 --- a/pkg/server/endpoints/node/handler.go +++ b/pkg/server/endpoints/node/handler.go @@ -560,24 +560,23 @@ func (h *Handler) getAttestResponse(ctx context.Context, } func (h *Handler) getCertFromCtx(ctx context.Context) (certificate *x509.Certificate, err error) { - ctxPeer, ok := peer.FromContext(ctx) if !ok { - return nil, errors.New("It was not posible to extract peer from request") + return nil, errors.New("no peer information") } tlsInfo, ok := ctxPeer.AuthInfo.(credentials.TLSInfo) if !ok { - return nil, errors.New("It was not posible to extract AuthInfo from request") + return nil, errors.New("no TLS auth info for peer") } if len(tlsInfo.State.VerifiedChains) == 0 { - return nil, errors.New("VerifiedChains was empty") + return nil, errors.New("no verified client certificate presented by peer") } chain := tlsInfo.State.VerifiedChains[0] if len(chain) == 0 { // this shouldn't be possible with the tls package, but we should be // defensive. - return nil, errors.New("VerifiedChain was empty") + return nil, errors.New("verified client chain is missing certificates") } return chain[0], nil diff --git a/test/util/cert_generation.go b/test/util/cert_generation.go index fccd9dcc55..6e9f38e1e6 100644 --- a/test/util/cert_generation.go +++ b/test/util/cert_generation.go @@ -9,9 +9,8 @@ import ( "crypto/x509/pkix" "math/big" mrand "math/rand" + "net/url" "time" - - "github.com/spiffe/go-spiffe/uri" ) // NewSVIDTemplate returns a default SVID template with the specified SPIFFE ID. Must @@ -136,18 +135,11 @@ func defaultCATemplate() *x509.Certificate { // Create an x509 extension with the URI SAN of the given SPIFFE ID, and set it onto // the referenced certificate func addSpiffeExtension(spiffeID string, cert *x509.Certificate) error { - uriSANs, err := uri.MarshalUriSANs([]string{spiffeID}) + u, err := url.Parse(spiffeID) if err != nil { return err } - - ext := []pkix.Extension{{ - Id: uri.OidExtensionSubjectAltName, - Value: uriSANs, - Critical: true, - }} - - cert.ExtraExtensions = ext + cert.URIs = append(cert.URIs, u) return nil }