diff --git a/core/comm/server.go b/core/comm/server.go index 441de327354..62508db1374 100644 --- a/core/comm/server.go +++ b/core/comm/server.go @@ -19,8 +19,11 @@ package comm import ( "crypto/tls" "crypto/x509" + "encoding/pem" "errors" + "fmt" "net" + "sync" "google.golang.org/grpc" "google.golang.org/grpc/credentials" @@ -62,6 +65,12 @@ type GRPCServer interface { //TLSEnabled is a flag indicating whether or not TLS is enabled for this //GRPCServer instance TLSEnabled() bool + //AppendClientRootCAs appends PEM-encoded X509 certificate authorities to + //the list of authorities used to verify client certificates + AppendClientRootCAs(clientRoots [][]byte) error + //RemoveClientRootCAs removes PEM-encoded X509 certificate authorities from + //the list of authorities used to verify client certificates + RemoveClientRootCAs(clientRoots [][]byte) error } type grpcServerImpl struct { @@ -78,9 +87,13 @@ type grpcServerImpl struct { //List of certificate authorities to optionally pass to the client during //the TLS handshake serverRootCAs []tls.Certificate - //List of certificate authorities to be used to authenticate clients if - //client authentication is required - clientRootCAs *x509.CertPool + //lock to protect concurrent access to append / remove + lock *sync.Mutex + //Set of PEM-encoded X509 certificate authorities used to populate + //the tlsConfig.ClientCAs indexed by subject + clientRootCAs map[string]*x509.Certificate + //TLS configuration used by the grpc server + tlsConfig *tls.Config //Is TLS enabled? tlsEnabled bool } @@ -110,6 +123,7 @@ func NewGRPCServerFromListener(listener net.Listener, secureConfig SecureServerC grpcServer := &grpcServerImpl{ address: listener.Addr().String(), listener: listener, + lock: &sync.Mutex{}, } //set up our server options @@ -130,27 +144,29 @@ func NewGRPCServerFromListener(listener net.Listener, secureConfig SecureServerC //base server certificate certificates := []tls.Certificate{grpcServer.serverCertificate} - tlsConfig := &tls.Config{ - Certificates: certificates, + grpcServer.tlsConfig = &tls.Config{ + Certificates: certificates, + SessionTicketsDisabled: true, } //checkif client authentication is required if secureConfig.RequireClientCert { //require TLS client auth - tlsConfig.ClientAuth = tls.RequireAndVerifyClientCert + grpcServer.tlsConfig.ClientAuth = tls.RequireAndVerifyClientCert //if we have client root CAs, create a certPool if len(secureConfig.ClientRootCAs) > 0 { - grpcServer.clientRootCAs = x509.NewCertPool() + grpcServer.clientRootCAs = make(map[string]*x509.Certificate) + grpcServer.tlsConfig.ClientCAs = x509.NewCertPool() for _, clientRootCA := range secureConfig.ClientRootCAs { - if !grpcServer.clientRootCAs.AppendCertsFromPEM(clientRootCA) { - return nil, errors.New("Failed to load client root certificates") + err = grpcServer.appendClientRootCA(clientRootCA) + if err != nil { + return nil, err } } - tlsConfig.ClientCAs = grpcServer.clientRootCAs } } //create credentials - creds := credentials.NewTLS(tlsConfig) + creds := credentials.NewTLS(grpcServer.tlsConfig) //add to server options serverOpts = append(serverOpts, grpc.Creds(creds)) @@ -200,3 +216,116 @@ func (gServer *grpcServerImpl) Start() error { func (gServer *grpcServerImpl) Stop() { gServer.server.Stop() } + +//AppendClientRootCAs appends PEM-encoded X509 certificate authorities to +//the list of authorities used to verify client certificates +func (gServer *grpcServerImpl) AppendClientRootCAs(clientRoots [][]byte) error { + gServer.lock.Lock() + defer gServer.lock.Unlock() + for _, clientRoot := range clientRoots { + err := gServer.appendClientRootCA(clientRoot) + if err != nil { + return err + } + } + return nil +} + +//internal function to add a PEM-encoded clientRootCA +func (gServer *grpcServerImpl) appendClientRootCA(clientRoot []byte) error { + + errMsg := "Failed to append client root certificate(s): %s" + //convert to x509 + certs, subjects, err := pemToX509Certs(clientRoot) + if err != nil { + return fmt.Errorf(errMsg, err.Error()) + } + + if len(certs) < 1 { + return fmt.Errorf(errMsg, "No client root certificates found") + } + + for i, cert := range certs { + //first add to the ClientCAs + gServer.tlsConfig.ClientCAs.AddCert(cert) + //add it to our clientRootCAs map using subject as key + gServer.clientRootCAs[subjects[i]] = cert + } + return nil +} + +//RemoveClientRootCAs removes PEM-encoded X509 certificate authorities from +//the list of authorities used to verify client certificates +func (gServer *grpcServerImpl) RemoveClientRootCAs(clientRoots [][]byte) error { + gServer.lock.Lock() + defer gServer.lock.Unlock() + //remove from internal map + for _, clientRoot := range clientRoots { + err := gServer.removeClientRootCA(clientRoot) + if err != nil { + return err + } + } + + //create a new CertPool and populate with current clientRootCAs + certPool := x509.NewCertPool() + for _, clientRoot := range gServer.clientRootCAs { + certPool.AddCert(clientRoot) + } + + //replace the current ClientCAs pool + gServer.tlsConfig.ClientCAs = certPool + return nil +} + +//internal function to remove a PEM-encoded clientRootCA +func (gServer *grpcServerImpl) removeClientRootCA(clientRoot []byte) error { + + errMsg := "Failed to remove client root certificate(s): %s" + //convert to x509 + certs, subjects, err := pemToX509Certs(clientRoot) + if err != nil { + return fmt.Errorf(errMsg, err.Error()) + } + + if len(certs) < 1 { + return fmt.Errorf(errMsg, "No client root certificates found") + } + + for i, subject := range subjects { + //remove it from our clientRootCAs map using subject as key + //check to see if we have match + if certs[i].Equal(gServer.clientRootCAs[subject]) { + delete(gServer.clientRootCAs, subject) + } + } + return nil +} + +//utility function to parse PEM-encoded certs +func pemToX509Certs(pemCerts []byte) ([]*x509.Certificate, []string, error) { + + //it's possible that multiple certs are encoded + certs := []*x509.Certificate{} + subjects := []string{} + for len(pemCerts) > 0 { + var block *pem.Block + block, pemCerts = pem.Decode(pemCerts) + if block == nil { + break + } + if block.Type != "CERTIFICATE" || len(block.Headers) != 0 { + continue + } + + cert, err := x509.ParseCertificate(block.Bytes) + if err != nil { + return nil, subjects, err + } else { + certs = append(certs, cert) + //extract and append the subject + subjects = append(subjects, string(cert.RawSubject)) + } + } + return certs, subjects, nil +} diff --git a/core/comm/server_test.go b/core/comm/server_test.go index 66fe15ddaed..fe101c909f0 100644 --- a/core/comm/server_test.go +++ b/core/comm/server_test.go @@ -22,8 +22,10 @@ import ( "errors" "fmt" "io/ioutil" + "log" "net" "path/filepath" + "sync" "testing" "time" @@ -65,7 +67,48 @@ zA85vv7JhfMkvZYGPELC7I2K8V7ZAiEA9KcthV3HtDXKNDsA6ULT+qUkyoHRzCzr A4QaL2VU6i4= -----END CERTIFICATE----- ` + +var badPEM = `-----BEGIN CERTIFICATE----- +MIICRDCCAemgAwIBAgIJALwW//dz2ZBvMAoGCCqGSM49BAMCMH4xCzAJBgNVBAYT +AlVTMRMwEQYDVQQIDApDYWxpZm9ybmlhMRYwFAYDVQQHDA1TYW4gRnJhbmNpc2Nv +MRgwFgYDVQQKDA9MaW51eEZvdW5kYXRpb24xFDASBgNVBAsMC0h5cGVybGVkZ2Vy +MRIwEAYDVQQDDAlsb2NhbGhvc3QwHhcNMTYxMjA0MjIzMDE4WhcNMjYxMjAyMjIz +MDE4WjB+MQswCQYDVQQGEwJVUzETMBEGA1UECAwKQ2FsaWZvcm5pYTEWMBQGA1UE +BwwNU2FuIEZyYW5jaXNjbzEYMBYGA1UECgwPTGludXhGb3VuZGF0aW9uMRQwEgYD +VQQLDAtIeXBlcmxlZGdlcjESMBAGA1UEAwwJbG9jYWxob3N0MFkwEwYHKoZIzj0C +-----END CERTIFICATE----- +` + +var pemNoCertificateHeader = `-----BEGIN NOCERT----- +MIICRDCCAemgAwIBAgIJALwW//dz2ZBvMAoGCCqGSM49BAMCMH4xCzAJBgNVBAYT +AlVTMRMwEQYDVQQIDApDYWxpZm9ybmlhMRYwFAYDVQQHDA1TYW4gRnJhbmNpc2Nv +MRgwFgYDVQQKDA9MaW51eEZvdW5kYXRpb24xFDASBgNVBAsMC0h5cGVybGVkZ2Vy +MRIwEAYDVQQDDAlsb2NhbGhvc3QwHhcNMTYxMjA0MjIzMDE4WhcNMjYxMjAyMjIz +MDE4WjB+MQswCQYDVQQGEwJVUzETMBEGA1UECAwKQ2FsaWZvcm5pYTEWMBQGA1UE +BwwNU2FuIEZyYW5jaXNjbzEYMBYGA1UECgwPTGludXhGb3VuZGF0aW9uMRQwEgYD +VQQLDAtIeXBlcmxlZGdlcjESMBAGA1UEAwwJbG9jYWxob3N0MFkwEwYHKoZIzj0C +AQYIKoZIzj0DAQcDQgAEu2FEZVSr30Afey6dwcypeg5P+BuYx5JSYdG0/KJIBjWK +nzYo7FEmgMir7GbNh4pqA8KFrJZkPuxMgnEJBZTv+6NQME4wHQYDVR0OBBYEFAWO +4bfTEr2R6VYzQYrGk/2VWmtYMB8GA1UdIwQYMBaAFAWO4bfTEr2R6VYzQYrGk/2V +WmtYMAwGA1UdEwQFMAMBAf8wCgYIKoZIzj0EAwIDSQAwRgIhAIelqGdxPMHmQqRF +zA85vv7JhfMkvZYGPELC7I2K8V7ZAiEA9KcthV3HtDXKNDsA6ULT+qUkyoHRzCzr +A4QaL2VU6i4= +-----END NOCERT----- +` + var timeout = time.Second * 1 +var testOrgs = []testOrg{} + +func init() { + //load up crypto material for test orgs + for i := 1; i <= numOrgs; i++ { + testOrg, err := loadOrg(i) + if err != nil { + log.Fatalf("Failed to load test organizations due to error: %s", err.Error()) + } + testOrgs = append(testOrgs, testOrg) + } +} //test server to be registered with the GRPCServer type testServiceServer struct{} @@ -402,6 +445,22 @@ func TestNewGRPCServerInvalidParameters(t *testing.T) { if err != nil { t.Log(err.Error()) } + + //bad clientRootCAs + _, err = comm.NewGRPCServer(":9045", + comm.SecureServerConfig{ + UseTLS: true, + ServerCertificate: []byte(selfSignedCertPEM), + ServerKey: []byte(selfSignedKeyPEM), + RequireClientCert: true, + ClientRootCAs: [][]byte{[]byte(pemNoCertificateHeader)}}) + //check for error + msg = "Failed to append client root certificate(s): " + + "No client root certificates found" + assert.EqualError(t, err, msg) + if err != nil { + t.Log(err.Error()) + } } func TestNewGRPCServer(t *testing.T) { @@ -838,9 +897,8 @@ func runMutualAuth(t *testing.T, servers []testServer, trustedClients, unTrusted //loop through all the untrusted clients for k := 0; k < len(unTrustedClients); k++ { //invoke the EmptyCall service - _, err = invokeEmptyCall(servers[i].address, - []grpc.DialOption{grpc.WithTransportCredentials(credentials.NewTLS(unTrustedClients[k]))}) - //we expect failure from trusted clients + _, err = invokeEmptyCall(servers[i].address, []grpc.DialOption{grpc.WithTransportCredentials(credentials.NewTLS(unTrustedClients[k]))}) + //we expect failure from untrusted clients if err != nil { t.Logf("Untrusted client%d was correctly rejected by %s", k, servers[i].address) } else { @@ -855,16 +913,7 @@ func runMutualAuth(t *testing.T, servers []testServer, trustedClients, unTrusted func TestMutualAuth(t *testing.T) { - //load up crypto material for test orgs - var testOrgs = []testOrg{} - for i := 1; i <= numOrgs; i++ { - testOrg, err := loadOrg(i) - if err != nil { - t.Fatalf("Failed to load test organizations due to error: %s", err.Error()) - } - testOrgs = append(testOrgs, testOrg) - } - + t.Parallel() var tests = []struct { name string servers []testServer @@ -919,3 +968,242 @@ func TestMutualAuth(t *testing.T) { } } + +func TestAppendRemoveWithInvalidBytes(t *testing.T) { + + t.Parallel() + + noPEMData := [][]byte{[]byte("badcert1"), []byte("badCert2")} + + //get the config for one of our Org1 test servers + serverConfig := testOrgs[0].testServers(9200, [][]byte{})[0].config + address := testOrgs[0].testServers(9200, [][]byte{})[0].address + + //create a GRPCServer + srv, err := comm.NewGRPCServer(address, serverConfig) + if err != nil { + t.Fatalf("Failed to create GRPCServer due to: %s", err.Error()) + } + + //append/remove nonPEMData + noCertsFound := "No client root certificates found" + err = srv.AppendClientRootCAs(noPEMData) + if err == nil { + t.Fatalf("Expected error: %s", noCertsFound) + } + err = srv.RemoveClientRootCAs(noPEMData) + if err == nil { + t.Fatalf("Expected error: %s", noCertsFound) + } + + //apend/remove PEM without CERTIFICATE header + err = srv.AppendClientRootCAs([][]byte{[]byte(pemNoCertificateHeader)}) + if err == nil { + t.Fatalf("Expected error: %s", noCertsFound) + } + + err = srv.RemoveClientRootCAs([][]byte{[]byte(pemNoCertificateHeader)}) + if err == nil { + t.Fatalf("Expected error: %s", noCertsFound) + } + + //append/remove bad PEM data + err = srv.AppendClientRootCAs([][]byte{[]byte(badPEM)}) + if err == nil { + t.Fatalf("Expected error parsing bad PEM data") + } + + err = srv.RemoveClientRootCAs([][]byte{[]byte(badPEM)}) + if err == nil { + t.Fatalf("Expected error parsing bad PEM data") + } + +} + +func TestAppendClientRootCAs(t *testing.T) { + + t.Parallel() + //get the config for one of our Org1 test servers + serverConfig := testOrgs[0].testServers(9300, [][]byte{})[0].config + address := testOrgs[0].testServers(9300, [][]byte{})[0].address + + //create a GRPCServer + srv, err := comm.NewGRPCServer(address, serverConfig) + if err != nil { + t.Fatalf("Failed to create GRPCServer due to: %s", err.Error()) + } + + //register the GRPC test server and start the GRPCServer + testpb.RegisterTestServiceServer(srv.Server(), &testServiceServer{}) + go srv.Start() + defer srv.Stop() + //should not be needed but just in case + time.Sleep(10 * time.Millisecond) + + //try to connect with untrusted clients from Org2 children + clientConfig1 := testOrgs[1].childOrgs[0].trustedClients([][]byte{testOrgs[0].rootCA})[0] + clientConfig2 := testOrgs[1].childOrgs[1].trustedClients([][]byte{testOrgs[0].rootCA})[0] + clientConfigs := []*tls.Config{clientConfig1, clientConfig2} + + for i, clientConfig := range clientConfigs { + //invoke the EmptyCall service + _, err = invokeEmptyCall(address, []grpc.DialOption{ + grpc.WithTransportCredentials(credentials.NewTLS(clientConfig))}) + //we expect failure as these are currently not trusted clients + if err != nil { + t.Logf("Untrusted client%d was correctly rejected by %s", i, address) + } else { + t.Fatalf("Untrusted client %d should not have been able to connect to %s", i, + address) + } + } + + //now append the root CAs for the untrusted clients + err = srv.AppendClientRootCAs([][]byte{testOrgs[1].childOrgs[0].rootCA, + testOrgs[1].childOrgs[1].rootCA}) + if err != nil { + t.Fatal("Failed to append client root CAs") + } + + //now try to connect again + for j, clientConfig := range clientConfigs { + //invoke the EmptyCall service + _, err = invokeEmptyCall(address, []grpc.DialOption{ + grpc.WithTransportCredentials(credentials.NewTLS(clientConfig))}) + //we expect success as these are now trusted clients + if err != nil { + t.Fatalf("Now trusted client%d failed to connect to %s with error: %s", + j, address, err.Error()) + } else { + t.Logf("Now trusted client%d successfully connected to %s", j, address) + } + } + +} + +func TestRemoveClientRootCAs(t *testing.T) { + + t.Parallel() + //get the config for one of our Org1 test servers and include client CAs from + //Org2 child orgs + serverConfig := testOrgs[0].testServers(9301, + [][]byte{testOrgs[1].childOrgs[0].rootCA, + testOrgs[1].childOrgs[1].rootCA})[0].config + address := testOrgs[0].testServers(9301, [][]byte{})[0].address + + //create a GRPCServer + srv, err := comm.NewGRPCServer(address, serverConfig) + if err != nil { + t.Fatalf("Failed to create GRPCServer due to: %s", err.Error()) + } + + //register the GRPC test server and start the GRPCServer + testpb.RegisterTestServiceServer(srv.Server(), &testServiceServer{}) + go srv.Start() + defer srv.Stop() + //should not be needed but just in case + time.Sleep(10 * time.Millisecond) + + //try to connect with trusted clients from Org2 children + clientConfig1 := testOrgs[1].childOrgs[0].trustedClients([][]byte{testOrgs[0].rootCA})[0] + clientConfig2 := testOrgs[1].childOrgs[1].trustedClients([][]byte{testOrgs[0].rootCA})[0] + clientConfigs := []*tls.Config{clientConfig1, clientConfig2} + + for i, clientConfig := range clientConfigs { + //invoke the EmptyCall service + _, err = invokeEmptyCall(address, []grpc.DialOption{ + grpc.WithTransportCredentials(credentials.NewTLS(clientConfig))}) + + //we expect success as these are trusted clients + if err != nil { + t.Fatalf("Trusted client%d failed to connect to %s with error: %s", + i, address, err.Error()) + } else { + t.Logf("Trusted client%d successfully connected to %s", i, address) + } + } + + //now remove the root CAs for the untrusted clients + err = srv.RemoveClientRootCAs([][]byte{testOrgs[1].childOrgs[0].rootCA, + testOrgs[1].childOrgs[1].rootCA}) + if err != nil { + t.Fatal("Failed to remove client root CAs") + } + + //now try to connect again + for j, clientConfig := range clientConfigs { + //invoke the EmptyCall service + _, err = invokeEmptyCall(address, []grpc.DialOption{ + grpc.WithTransportCredentials(credentials.NewTLS(clientConfig))}) + //we expect failure as these are now untrusted clients + if err != nil { + t.Logf("Now untrusted client%d was correctly rejected by %s", j, address) + } else { + t.Fatalf("Now untrusted client %d should not have been able to connect to %s", j, + address) + } + } + +} + +//test for race conditions - test locally using "go test -race -run TestConcurrentAppendRemove" +func TestConcurrentAppendRemove(t *testing.T) { + + t.Parallel() + //get the config for one of our Org1 test servers and include client CAs from + //Org2 child orgs + serverConfig := testOrgs[0].testServers(9302, + [][]byte{testOrgs[1].childOrgs[0].rootCA, + testOrgs[1].childOrgs[1].rootCA})[0].config + address := testOrgs[0].testServers(9302, [][]byte{})[0].address + + //create a GRPCServer + srv, err := comm.NewGRPCServer(address, serverConfig) + if err != nil { + t.Fatalf("Failed to create GRPCServer due to: %s", err.Error()) + } + + //register the GRPC test server and start the GRPCServer + testpb.RegisterTestServiceServer(srv.Server(), &testServiceServer{}) + go srv.Start() + defer srv.Stop() + + //need to wait for the following go routines to finish + var wg sync.WaitGroup + + wg.Add(1) + go func() { + defer wg.Done() + //now remove the root CAs for the untrusted clients + err := srv.RemoveClientRootCAs([][]byte{testOrgs[1].childOrgs[0].rootCA, + testOrgs[1].childOrgs[1].rootCA}) + if err != nil { + t.Fatal("Failed to remove client root CAs") + } + + }() + + //TODO: enable this after creating a custom type for grpc.TransportCredentials + /* + clientConfig := testOrgs[1].childOrgs[0].trustedClients([][]byte{testOrgs[0].rootCA})[0] + wg.Add(1) + go func() { + defer wg.Done() + _, _ = invokeEmptyCall(address, []grpc.DialOption{ + grpc.WithTransportCredentials(credentials.NewTLS(clientConfig))}) + }() + */ + wg.Add(1) + go func() { + defer wg.Done() + //now append the root CAs for the untrusted clients + err := srv.AppendClientRootCAs([][]byte{testOrgs[1].childOrgs[0].rootCA, + testOrgs[1].childOrgs[1].rootCA}) + if err != nil { + t.Fatal("Failed to append client root CAs") + } + }() + + wg.Wait() + +}