Merge pull request #237 from cyli/mutual-auth

Support mutual authentication between the server and signer

cmd/notary-server/config.json

@@ -8,8 +8,10 @@
 	  "type": "remote",
 	  "hostname": "notarysigner",
 	  "port": "7899",
-	  "tls_ca_file": "./fixtures/root-ca.crt",			
-	  "key_algorithm": "ecdsa"
+	  "tls_ca_file": "./fixtures/root-ca.crt",
+	  "key_algorithm": "ecdsa",
+	  "tls_client_cert": "./fixtures/notary-server.crt",
+	  "tls_client_key": "./fixtures/notary-server.key"
 	},
 	"logging": {
 		"level": "debug"

cmd/notary-server/main.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"crypto/tls"
 	_ "expvar"
 	"flag"
 	"fmt"
@@ -24,6 +25,7 @@ import (
 	"github.com/docker/notary/server"
 	"github.com/docker/notary/server/storage"
 	"github.com/docker/notary/signer"
+	"github.com/docker/notary/utils"
 	"github.com/docker/notary/version"
 	"github.com/spf13/viper"
 )
@@ -46,6 +48,52 @@ func init() {
 	flag.BoolVar(&debug, "debug", false, "Enable the debugging server on localhost:8080")
 }
 
+// optionally sets up TLS for the server - if no TLS configuration is
+// specified, TLS is not enabled.
+func serverTLS(configuration *viper.Viper) (*tls.Config, error) {
+	tlsCertFile := configuration.GetString("server.tls_cert_file")
+	tlsKeyFile := configuration.GetString("server.tls_key_file")
+
+	if tlsCertFile == "" && tlsKeyFile == "" {
+		return nil, nil
+	} else if tlsCertFile == "" || tlsKeyFile == "" {
+		return nil, fmt.Errorf("Partial TLS configuration found. Either include both a cert and key file in the configuration, or include neither to disable TLS.")
+	}
+
+	tlsConfig, err := utils.ConfigureServerTLS(&utils.ServerTLSOpts{
+		ServerCertFile: tlsCertFile,
+		ServerKeyFile:  tlsKeyFile,
+	})
+	if err != nil {
+		return nil, fmt.Errorf("Unable to set up TLS: %s", err.Error())
+	}
+	return tlsConfig, nil
+}
+
+// sets up TLS for the GRPC connection to notary-signer
+func grpcTLS(configuration *viper.Viper) (*tls.Config, error) {
+	rootCA := configuration.GetString("trust_service.tls_ca_file")
+	serverName := configuration.GetString("trust_service.hostname")
+	clientCert := configuration.GetString("trust_service.tls_client_cert")
+	clientKey := configuration.GetString("trust_service.tls_client_key")
+
+	if (clientCert == "" && clientKey != "") || (clientCert != "" && clientKey == "") {
+		return nil, fmt.Errorf("Partial TLS configuration found. Either include both a client cert and client key file in the configuration, or include neither.")
+	}
+
+	tlsConfig, err := utils.ConfigureClientTLS(&utils.ClientTLSOpts{
+		RootCAFile:     rootCA,
+		ServerName:     serverName,
+		ClientCertFile: clientCert,
+		ClientKeyFile:  clientKey,
+	})
+	if err != nil {
+		return nil, fmt.Errorf(
+			"Unable to configure TLS to the trust service: %s", err.Error())
+	}
+	return tlsConfig, nil
+}
+
 func main() {
 	flag.Usage = usage
 	flag.Parse()
@@ -111,10 +159,14 @@ func main() {
 	var trust signed.CryptoService
 	if mainViper.GetString("trust_service.type") == "remote" {
 		logrus.Info("Using remote signing service")
+		clientTLS, err := grpcTLS(mainViper)
+		if err != nil {
+			logrus.Fatal(err.Error())
+		}
 		notarySigner := signer.NewNotarySigner(
 			mainViper.GetString("trust_service.hostname"),
 			mainViper.GetString("trust_service.port"),
-			mainViper.GetString("trust_service.tls_ca_file"),
+			clientTLS,
 		)
 		trust = notarySigner
 		minute := 1 * time.Minute
@@ -151,12 +203,17 @@ func main() {
 		logrus.Debug("Using memory backend")
 		ctx = context.WithValue(ctx, "metaStore", storage.NewMemStorage())
 	}
+
+	tlsConfig, err := serverTLS(mainViper)
+	if err != nil {
+		logrus.Fatal(err.Error())
+	}
+
 	logrus.Info("Starting Server")
 	err = server.Run(
 		ctx,
 		mainViper.GetString("server.addr"),
-		mainViper.GetString("server.tls_cert_file"),
-		mainViper.GetString("server.tls_key_file"),
+		tlsConfig,
 		trust,
 		mainViper.GetString("auth.type"),
 		mainViper.Get("auth.options"),

cmd/notary-server/main_test.go

@@ -0,0 +1,144 @@
+package main
+
+import (
+	"bytes"
+	"crypto/tls"
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/spf13/viper"
+	"github.com/stretchr/testify/assert"
+)
+
+const (
+	Cert = "../../fixtures/notary-server.crt"
+	Key  = "../../fixtures/notary-server.key"
+	Root = "../../fixtures/root-ca.crt"
+)
+
+// initializes a viper object with test configuration
+func configure(jsonConfig []byte) *viper.Viper {
+	config := viper.New()
+	config.SetConfigType("json")
+	config.ReadConfig(bytes.NewBuffer(jsonConfig))
+	return config
+}
+
+// If neither the cert nor the key are provided, a nil tls config is returned.
+func TestServerTLSMissingCertAndKey(t *testing.T) {
+	tlsConfig, err := serverTLS(configure([]byte(`{"server": {}}`)))
+	assert.NoError(t, err)
+	assert.Nil(t, tlsConfig)
+}
+
+// Cert and Key either both have to be empty or both have to be provided.
+func TestServerTLSMissingCertAndOrKey(t *testing.T) {
+	configs := []string{
+		fmt.Sprintf(`{"tls_cert_file": "%s"}`, Cert),
+		fmt.Sprintf(`{"tls_key_file": "%s"}`, Key),
+	}
+	for _, serverConfig := range configs {
+		config := configure(
+			[]byte(fmt.Sprintf(`{"server": %s}`, serverConfig)))
+		tlsConfig, err := serverTLS(config)
+		assert.Error(t, err)
+		assert.Nil(t, tlsConfig)
+		assert.True(t,
+			strings.Contains(err.Error(), "Partial TLS configuration found."))
+	}
+}
+
+// The rest of the functionality of serverTLS depends upon
+// utils.ConfigureServerTLS, so this test just asserts that if successful,
+// the correct tls.Config is returned based on all the configuration parameters
+func TestServerTLSSuccess(t *testing.T) {
+	keypair, err := tls.LoadX509KeyPair(Cert, Key)
+	assert.NoError(t, err, "Unable to load cert and key for testing")
+
+	config := fmt.Sprintf(
+		`{"server": {"tls_cert_file": "%s", "tls_key_file": "%s"}}`,
+		Cert, Key)
+	tlsConfig, err := serverTLS(configure([]byte(config)))
+	assert.NoError(t, err)
+	assert.Equal(t, []tls.Certificate{keypair}, tlsConfig.Certificates)
+}
+
+// The rest of the functionality of serverTLS depends upon
+// utils.ConfigureServerTLS, so this test just asserts that if it fails,
+// the error is propogated.
+func TestServerTLSFailure(t *testing.T) {
+	config := fmt.Sprintf(
+		`{"server": {"tls_cert_file": "non-exist", "tls_key_file": "%s"}}`,
+		Key)
+	tlsConfig, err := serverTLS(configure([]byte(config)))
+	assert.Error(t, err)
+	assert.Nil(t, tlsConfig)
+	assert.True(t, strings.Contains(err.Error(), "Unable to set up TLS"))
+}
+
+// Client cert and Key either both have to be empty or both have to be
+// provided.
+func TestGrpcTLSMissingCertOrKey(t *testing.T) {
+	configs := []string{
+		fmt.Sprintf(`"tls_client_cert": "%s"`, Cert),
+		fmt.Sprintf(`"tls_client_key": "%s"`, Key),
+	}
+	for _, trustConfig := range configs {
+		jsonConfig := fmt.Sprintf(
+			`{"trust_service": {"hostname": "notary-signer", %s}}`,
+			trustConfig)
+		config := configure([]byte(jsonConfig))
+		tlsConfig, err := grpcTLS(config)
+		assert.Error(t, err)
+		assert.Nil(t, tlsConfig)
+		assert.True(t,
+			strings.Contains(err.Error(), "Partial TLS configuration found."))
+	}
+}
+
+// If no TLS configuration is provided for the host server, a tls config with
+// the provided serverName is still returned.
+func TestGrpcTLSNoConfig(t *testing.T) {
+	tlsConfig, err := grpcTLS(
+		configure([]byte(`{"trust_service": {"hostname": "notary-signer"}}`)))
+	assert.NoError(t, err)
+	assert.Equal(t, "notary-signer", tlsConfig.ServerName)
+	assert.Nil(t, tlsConfig.RootCAs)
+	assert.Nil(t, tlsConfig.Certificates)
+}
+
+// The rest of the functionality of grpcTLS depends upon
+// utils.ConfigureClientTLS, so this test just asserts that if successful,
+// the correct tls.Config is returned based on all the configuration parameters
+func TestGrpcTLSSuccess(t *testing.T) {
+	keypair, err := tls.LoadX509KeyPair(Cert, Key)
+	assert.NoError(t, err, "Unable to load cert and key for testing")
+
+	config := fmt.Sprintf(
+		`{"trust_service": {
+            "hostname": "notary-server",
+            "tls_client_cert": "%s",
+            "tls_client_key": "%s"}}`,
+		Cert, Key)
+	tlsConfig, err := grpcTLS(configure([]byte(config)))
+	assert.NoError(t, err)
+	assert.Equal(t, []tls.Certificate{keypair}, tlsConfig.Certificates)
+}
+
+// The rest of the functionality of grpcTLS depends upon
+// utils.ConfigureServerTLS, so this test just asserts that if it fails,
+// the error is propogated.
+func TestGrpcTLSFailure(t *testing.T) {
+	config := fmt.Sprintf(
+		`{"trust_service": {
+            "hostname": "notary-server",
+            "tls_client_cert": "no-exist",
+            "tls_client_key": "%s"}}`,
+		Key)
+	tlsConfig, err := grpcTLS(configure([]byte(config)))
+	assert.Error(t, err)
+	assert.Nil(t, tlsConfig)
+	assert.True(t, strings.Contains(err.Error(),
+		"Unable to configure TLS to the trust service"))
+}

cmd/notary-signer/config.json

@@ -3,7 +3,8 @@
 		"http_addr": ":4444",
 		"grpc_addr": ":7899",
 		"cert_file": "./fixtures/notary-signer.crt",
-		"key_file": "./fixtures/notary-signer.key"
+		"key_file": "./fixtures/notary-signer.key",
+		"client_ca_file": "./fixtures/notary-server.crt"
 	},
 	"crypto": {
 		"pkcslib": "/usr/local/lib/softhsm/libsofthsm2.so"

cmd/notary-signer/main.go

@@ -1,10 +1,12 @@
 package main
 
 import (
+	"crypto/tls"
 	"database/sql"
 	"errors"
 	_ "expvar"
 	"flag"
+	"fmt"
 	"log"
 	"net"
 	"net/http"
@@ -68,6 +70,30 @@ func passphraseRetriever(keyName, alias string, createNew bool, attempts int) (p
 	return passphrase, false, nil
 }
 
+// parses and sets up the TLS for the signer http + grpc server
+func signerTLS(configuration *viper.Viper, printUsage bool) (*tls.Config, error) {
+	certFile := configuration.GetString("server.cert_file")
+	keyFile := configuration.GetString("server.key_file")
+	if certFile == "" || keyFile == "" {
+		if printUsage {
+			usage()
+		}
+		return nil, fmt.Errorf("Certificate and key are mandatory")
+	}
+
+	clientCAFile := configuration.GetString("server.client_ca_file")
+	tlsConfig, err := utils.ConfigureServerTLS(&utils.ServerTLSOpts{
+		ServerCertFile:    certFile,
+		ServerKeyFile:     keyFile,
+		RequireClientAuth: clientCAFile != "",
+		ClientCAFile:      clientCAFile,
+	})
+	if err != nil {
+		return nil, fmt.Errorf("Unable to set up TLS: %s", err.Error())
+	}
+	return tlsConfig, nil
+}
+
 func main() {
 	flag.Usage = usage
 	flag.Parse()
@@ -95,19 +121,9 @@ func main() {
 
 	logrus.SetLevel(logrus.Level(mainViper.GetInt("logging.level")))
 
-	certFile := mainViper.GetString("server.cert_file")
-	keyFile := mainViper.GetString("server.key_file")
-	if certFile == "" || keyFile == "" {
-		usage()
-		log.Fatalf("Certificate and key are mandatory")
-	}
-
-	tlsConfig, err := utils.ConfigureServerTLS(&utils.ServerTLSOpts{
-		ServerCertFile: certFile,
-		ServerKeyFile:  keyFile,
-	})
+	tlsConfig, err := signerTLS(mainViper, true)
 	if err != nil {
-		logrus.Fatalf("Unable to set up TLS: %s", err.Error())
+		logrus.Fatalf(err.Error())
 	}
 
 	cryptoServices := make(signer.CryptoServiceIndex)
@@ -163,10 +179,7 @@ func main() {
 	if err != nil {
 		log.Fatalf("failed to listen %v", err)
 	}
-	creds, err := credentials.NewServerTLSFromFile(certFile, keyFile)
-	if err != nil {
-		log.Fatalf("failed to generate credentials %v", err)
-	}
+	creds := credentials.NewTLS(tlsConfig)
 	opts := []grpc.ServerOption{grpc.Creds(creds)}
 	grpcServer := grpc.NewServer(opts...)
 
@@ -191,9 +204,9 @@ func main() {
 		log.Println("HTTP server listening on", httpAddr)
 	}
 
-	err = server.ListenAndServeTLS(certFile, keyFile)
+	err = server.ListenAndServeTLS("", "")
 	if err != nil {
-		log.Fatal("HTTP server failed to start:", err)
+		log.Fatal("HTTPS server failed to start:", err)
 	}
 }
 

cmd/notary-signer/main_test.go

@@ -1 +1,71 @@
 package main
+
+import (
+	"bytes"
+	"crypto/tls"
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/spf13/viper"
+	"github.com/stretchr/testify/assert"
+)
+
+const (
+	Cert = "../../fixtures/notary-signer.crt"
+	Key  = "../../fixtures/notary-signer.key"
+	Root = "../../fixtures/root-ca.crt"
+)
+
+// initializes a viper object with test configuration
+func configure(jsonConfig []byte) *viper.Viper {
+	config := viper.New()
+	config.SetConfigType("json")
+	config.ReadConfig(bytes.NewBuffer(jsonConfig))
+	return config
+}
+
+func TestSignerTLSMissingCertAndOrKey(t *testing.T) {
+	configs := []string{
+		"{}",
+		fmt.Sprintf(`{"cert_file": "%s"}`, Cert),
+		fmt.Sprintf(`{"key_file": "%s"}`, Key),
+	}
+	for _, serverConfig := range configs {
+		config := configure(
+			[]byte(fmt.Sprintf(`{"server": %s}`, serverConfig)))
+		tlsConfig, err := signerTLS(config, false)
+		assert.Error(t, err)
+		assert.Nil(t, tlsConfig)
+		assert.Equal(t, "Certificate and key are mandatory", err.Error())
+	}
+}
+
+// The rest of the functionality of signerTLS depends upon
+// utils.ConfigureServerTLS, so this test just asserts that if successful,
+// the correct tls.Config is returned based on all the configuration parameters
+func TestSignerTLSSuccess(t *testing.T) {
+	keypair, err := tls.LoadX509KeyPair(Cert, Key)
+	assert.NoError(t, err, "Unable to load cert and key for testing")
+
+	config := fmt.Sprintf(
+		`{"server": {"cert_file": "%s", "key_file": "%s", "client_ca_file": "%s"}}`,
+		Cert, Key, Cert)
+	tlsConfig, err := signerTLS(configure([]byte(config)), false)
+	assert.NoError(t, err)
+	assert.Equal(t, []tls.Certificate{keypair}, tlsConfig.Certificates)
+	assert.NotNil(t, tlsConfig.ClientCAs)
+}
+
+// The rest of the functionality of signerTLS depends upon
+// utils.ConfigureServerTLS, so this test just asserts that if it fails,
+// the error is propogated.
+func TestSignerTLSFailure(t *testing.T) {
+	config := fmt.Sprintf(
+		`{"server": {"cert_file": "%s", "key_file": "%s", "client_ca_file": "%s"}}`,
+		Cert, Key, "non-existant")
+	tlsConfig, err := signerTLS(configure([]byte(config)), false)
+	assert.Error(t, err)
+	assert.Nil(t, tlsConfig)
+	assert.True(t, strings.Contains(err.Error(), "Unable to set up TLS"))
+}

server/server.go

@@ -29,7 +29,7 @@ func init() {
 // Run sets up and starts a TLS server that can be cancelled using the
 // given configuration. The context it is passed is the context it should
 // use directly for the TLS server, and generate children off for requests
-func Run(ctx context.Context, addr, tlsCertFile, tlsKeyFile string, trust signed.CryptoService, authMethod string, authOpts interface{}) error {
+func Run(ctx context.Context, addr string, tlsConfig *tls.Config, trust signed.CryptoService, authMethod string, authOpts interface{}) error {
 
 	tcpAddr, err := net.ResolveTCPAddr("tcp", addr)
 	if err != nil {
@@ -41,18 +41,9 @@ func Run(ctx context.Context, addr, tlsCertFile, tlsKeyFile string, trust signed
 		return err
 	}
 
-	if tlsCertFile != "" && tlsKeyFile != "" {
-		tlsConfig, err := utils.ConfigureServerTLS(&utils.ServerTLSOpts{
-			ServerCertFile: tlsCertFile,
-			ServerKeyFile:  tlsKeyFile,
-		})
-		if err != nil {
-			return err
-		}
+	if tlsConfig != nil {
 		logrus.Info("Enabling TLS")
 		lsnr = tls.NewListener(lsnr, tlsConfig)
-	} else if tlsCertFile != "" || tlsKeyFile != "" {
-		return fmt.Errorf("Partial TLS configuration found. Either include both a cert and key file in the configuration, or include neither to disable TLS.")
 	}
 
 	var ac auth.AccessController

server/server_test.go

@@ -14,8 +14,7 @@ func TestRunBadAddr(t *testing.T) {
 	err := Run(
 		context.Background(),
 		"testAddr",
-		"../fixtures/notary-server.crt",
-		"../fixtures/notary-server.crt",
+		nil,
 		signed.NewEd25519(),
 		"",
 		nil,
@@ -31,8 +30,7 @@ func TestRunReservedPort(t *testing.T) {
 	err := Run(
 		ctx,
 		"localhost:80",
-		"../fixtures/notary-server.crt",
-		"../fixtures/notary-server.crt",
+		nil,
 		signed.NewEd25519(),
 		"",
 		nil,

signer/signer_trust.go

@@ -1,6 +1,7 @@
 package signer
 
 import (
+	"crypto/tls"
 	"fmt"
 	"net"
 	"time"
@@ -8,7 +9,6 @@ import (
 	"github.com/Sirupsen/logrus"
 	pb "github.com/docker/notary/proto"
 	"github.com/docker/notary/tuf/data"
-	"github.com/docker/notary/utils"
 	"golang.org/x/net/context"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
@@ -28,16 +28,9 @@ type NotarySigner struct {
 }
 
 // NewNotarySigner is a convinience method that returns NotarySigner
-func NewNotarySigner(hostname string, port string, tlscafile string) *NotarySigner {
+func NewNotarySigner(hostname string, port string, tlsConfig *tls.Config) *NotarySigner {
 	var opts []grpc.DialOption
 	netAddr := net.JoinHostPort(hostname, port)
-	tlsConfig, err := utils.ConfigureClientTLS(&utils.ClientTLSOpts{
-		RootCAFile: tlscafile,
-		ServerName: hostname,
-	})
-	if err != nil {
-		logrus.Fatal("Unable to set up TLS: ", err)
-	}
 	creds := credentials.NewTLS(tlsConfig)
 	opts = append(opts, grpc.WithTransportCredentials(creds))
 	conn, err := grpc.Dial(netAddr, opts...)

trustmanager/keyfilestore_test.go

@@ -9,7 +9,7 @@ import (
 	"testing"
 
 	"github.com/docker/notary/pkg/passphrase"
-	"github.com/endophage/gotuf/data"
+	"github.com/docker/notary/tuf/data"
 	"github.com/stretchr/testify/assert"
 )