Use the shared configuration parsing code in notary-signer's parsing.

Also add bugsnag support to notary-signer.

This also changes the 'server.cert_file' and 'server.key_file'
parameters to 'server.tls_cert_file' and 'server.tls_key_file',
respectively, to match notary-server.

Previously, the default alias, which was under the environment
variable NOTARY_SERVER_DEFAULT_ALIAS is now also available in
the config file in storage.default_alias.  The password has
not changed.

Finally, this removes some of the HSM references in notary-signer.

Signed-off-by: Ying Li <ying.li@docker.com>

cmd/notary-signer/config.json

@@ -2,15 +2,12 @@
 	"server": {
 		"http_addr": ":4444",
 		"grpc_addr": ":7899",
-		"cert_file": "./fixtures/notary-signer.crt",
-		"key_file": "./fixtures/notary-signer.key",
+		"tls_cert_file": "./fixtures/notary-signer.crt",
+		"tls_key_file": "./fixtures/notary-signer.key",
 		"client_ca_file": "./fixtures/notary-server.crt"
 	},
-	"crypto": {
-		"pkcslib": "/usr/local/lib/softhsm/libsofthsm2.so"
-	},
 	"logging": {
-		"level": 5
+		"level": "debug"
 	},
 	"storage": {
 		"backend": "mysql",

cmd/notary-signer/main.go

@@ -29,7 +29,7 @@ import (
 	"github.com/docker/notary/utils"
 	"github.com/docker/notary/version"
 	_ "github.com/go-sql-driver/mysql"
-	"github.com/miekg/pkcs11"
+	_ "github.com/mattn/go-sqlite3"
 	"github.com/spf13/viper"
 
 	"github.com/Sirupsen/logrus"
@@ -38,10 +38,8 @@ import (
 
 const (
 	debugAddr       = "localhost:8080"
-	dbType          = "mysql"
 	envPrefix       = "NOTARY_SIGNER"
 	defaultAliasEnv = "DEFAULT_ALIAS"
-	pinCode         = "PIN"
 )
 
 var (
@@ -51,13 +49,7 @@ var (
 )
 
 func init() {
-	// set default log level to Error
-	mainViper.SetDefault("logging", map[string]interface{}{"level": 2})
-
-	mainViper.SetEnvPrefix(envPrefix)
-	mainViper.SetEnvKeyReplacer(strings.NewReplacer(".", "_"))
-	mainViper.AutomaticEnv()
-
+	utils.SetupViper(mainViper, envPrefix)
 	// Setup flags
 	flag.StringVar(&configFile, "config", "", "Path to configuration file")
 	flag.BoolVar(&debug, "debug", false, "show the version and exit")
@@ -73,21 +65,113 @@ func passphraseRetriever(keyName, alias string, createNew bool, attempts int) (p
 	return passphrase, false, nil
 }
 
-// validates TLS configuration options and sets up the TLS for the signer
-// http + grpc server
-func signerTLS(tlsConfig *utils.ServerTLSOpts, printUsage bool) (*tls.Config, error) {
-	if tlsConfig.ServerCertFile == "" || tlsConfig.ServerKeyFile == "" {
-		if printUsage {
-			usage()
-		}
-		return nil, fmt.Errorf("Certificate and key are mandatory")
+// Reads the configuration file for storage setup, and sets up the cryptoservice
+// mapping
+func setUpCryptoservices(configuration *viper.Viper, allowedBackends []string) (
+	signer.CryptoServiceIndex, error) {
+
+	storeConfig, err := utils.ParseStorage(configuration, allowedBackends)
+	if err != nil {
+		return nil, err
 	}
 
-	tlsConfig, err := utils.ConfigureServerTLS(config)
-	if err != nil {
-		return nil, fmt.Errorf("Unable to set up TLS: %s", err.Error())
+	if storeConfig == nil {
+		return nil, fmt.Errorf("DB storage configuration is mandatory")
 	}
-	return tlsConfig, nil
+
+	dbSQL, err := sql.Open(storeConfig.Backend, storeConfig.Source)
+	if err != nil {
+		return nil, fmt.Errorf("failed to open the %s database: %s, %v",
+			storeConfig.Backend, storeConfig.Source, err)
+	}
+	logrus.Debugf("Using %s DB: %s", storeConfig.Backend, storeConfig.Source)
+
+	defaultAlias := configuration.GetString("storage.default_alias")
+	if defaultAlias == "" {
+		// backwards compatibility - support this environment variable
+		defaultAlias = configuration.GetString(defaultAliasEnv)
+	}
+
+	if defaultAlias == "" {
+		return nil, fmt.Errorf("must provide a default alias for the key DB")
+	}
+	logrus.Debug("Default Alias: ", defaultAlias)
+
+	keyStore, err := keydbstore.NewKeyDBStore(
+		passphraseRetriever, defaultAlias, storeConfig.Backend, dbSQL)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create a new keydbstore: %v", err)
+	}
+
+	health.RegisterPeriodicFunc(
+		"DB operational", keyStore.HealthCheck, time.Second*60)
+
+	cryptoService := cryptoservice.NewCryptoService("", keyStore)
+
+	cryptoServices := make(signer.CryptoServiceIndex)
+	cryptoServices[data.ED25519Key] = cryptoService
+	cryptoServices[data.ECDSAKey] = cryptoService
+
+	return cryptoServices, nil
+}
+
+// set up the GRPC server
+func setupGRPCServer(grpcAddr string, tlsConfig *tls.Config,
+	cryptoServices signer.CryptoServiceIndex) (*grpc.Server, net.Listener, error) {
+
+	//RPC server setup
+	kms := &api.KeyManagementServer{CryptoServices: cryptoServices,
+		HealthChecker: health.CheckStatus}
+	ss := &api.SignerServer{CryptoServices: cryptoServices,
+		HealthChecker: health.CheckStatus}
+
+	lis, err := net.Listen("tcp", grpcAddr)
+	if err != nil {
+		return nil, nil, fmt.Errorf("grpc server failed to listen on %s: %v",
+			grpcAddr, err)
+	}
+
+	creds := credentials.NewTLS(tlsConfig)
+	opts := []grpc.ServerOption{grpc.Creds(creds)}
+	grpcServer := grpc.NewServer(opts...)
+
+	pb.RegisterKeyManagementServer(grpcServer, kms)
+	pb.RegisterSignerServer(grpcServer, ss)
+
+	return grpcServer, lis, nil
+}
+
+func setupHTTPServer(httpAddr string, tlsConfig *tls.Config,
+	cryptoServices signer.CryptoServiceIndex) http.Server {
+
+	return http.Server{
+		Addr:      httpAddr,
+		Handler:   api.Handlers(cryptoServices),
+		TLSConfig: tlsConfig,
+	}
+}
+
+func getAddrAndTLSConfig(configuration *viper.Viper) (string, string, *tls.Config, error) {
+	tlsOpts, err := utils.ParseServerTLS(configuration, true)
+	if err != nil {
+		return "", "", nil, fmt.Errorf("unable to set up TLS: %s", err.Error())
+	}
+	tlsConfig, err := utils.ConfigureServerTLS(tlsOpts)
+	if err != nil {
+		return "", "", nil, fmt.Errorf("unable to set up TLS: %s", err.Error())
+	}
+
+	grpcAddr := configuration.GetString("server.grpc_addr")
+	if grpcAddr == "" {
+		return "", "", nil, fmt.Errorf("grpc listen address required for server")
+	}
+
+	httpAddr := configuration.GetString("server.http_addr")
+	if httpAddr == "" {
+		return "", "", nil, fmt.Errorf("http listen address required for server")
+	}
+
+	return httpAddr, grpcAddr, tlsConfig, nil
 }
 
 func main() {
@@ -108,8 +192,6 @@ func main() {
 	mainViper.SetConfigType(strings.TrimPrefix(ext, "."))
 	mainViper.SetConfigName(strings.TrimSuffix(filename, ext))
 	mainViper.AddConfigPath(configPath)
-	// set default log level to Error
-	mainViper.SetDefault("logging.level", "error")
 	err := mainViper.ReadInConfig()
 	if err != nil {
 		logrus.Error("Viper Error: ", err.Error())
@@ -117,87 +199,46 @@ func main() {
 		os.Exit(1)
 	}
 
-	var config util.ServerConfiguration
-	err = mainViper.Unmarshal(&config)
+	// default is error level
+	lvl, err := utils.ParseLogLevel(mainViper, logrus.ErrorLevel)
 	if err != nil {
-		logrus.Fatalf(err.Error())
+		logrus.Fatal(err.Error())
 	}
+	logrus.SetLevel(lvl)
 
-	if config.Logging.Level != nil {
-		fmt.Println("LOGGING level", config.Logging.Level)
-		logrus.SetLevel(config.Logging.Level.ToLogrus())
-	}
-
-	tlsConfig, err := signerTLS(mainViper, true)
+	// parse bugsnag config
+	bugsnagConf, err := utils.ParseBugsnag(mainViper)
 	if err != nil {
-		logrus.Fatalf(err.Error())
+		logrus.Fatal(err.Error())
 	}
+	utils.SetUpBugsnag(bugsnagConf)
 
-	cryptoServices := make(signer.CryptoServiceIndex)
-
-	emptyStorage := utils.Storage{}
-	if config.Storage == emptyStorage {
-		usage()
-		log.Fatalf("Must specify a MySQL backend.")
-	}
-
-	dbSQL, err := sql.Open(config.Storage.Backend, config.Storage.URL)
+	// parse server config
+	httpAddr, grpcAddr, tlsConfig, err := getAddrAndTLSConfig(mainViper)
 	if err != nil {
-		log.Fatalf("failed to open the database: %s, %v", config.Storage.URL, err)
+		logrus.Fatal(err.Error())
 	}
 
-	defaultAlias := mainViper.GetString(defaultAliasEnv)
-	logrus.Debug("Default Alias: ", defaultAlias)
-	keyStore, err := keydbstore.NewKeyDBStore(passphraseRetriever, defaultAlias, configDBType, dbSQL)
+	// setup the cryptoservices
+	cryptoServices, err := setUpCryptoservices(mainViper, []string{"mysql"})
 	if err != nil {
-		log.Fatalf("failed to create a new keydbstore: %v", err)
+		logrus.Fatal(err.Error())
 	}
 
-	health.RegisterPeriodicFunc(
-		"DB operational", keyStore.HealthCheck, time.Second*60)
-
-	cryptoService := cryptoservice.NewCryptoService("", keyStore)
-
-	cryptoServices[data.ED25519Key] = cryptoService
-	cryptoServices[data.ECDSAKey] = cryptoService
-
-	//RPC server setup
-	kms := &api.KeyManagementServer{CryptoServices: cryptoServices,
-		HealthChecker: health.CheckStatus}
-	ss := &api.SignerServer{CryptoServices: cryptoServices,
-		HealthChecker: health.CheckStatus}
-
-	rpcAddr := mainViper.GetString("server.grpc_addr")
-	lis, err := net.Listen("tcp", rpcAddr)
+	grpcServer, lis, err := setupGRPCServer(grpcAddr, tlsConfig, cryptoServices)
 	if err != nil {
-		log.Fatalf("failed to listen %v", err)
+		logrus.Fatal(err.Error())
 	}
-	creds := credentials.NewTLS(tlsConfig)
-	opts := []grpc.ServerOption{grpc.Creds(creds)}
-	grpcServer := grpc.NewServer(opts...)
 
-	pb.RegisterKeyManagementServer(grpcServer, kms)
-	pb.RegisterSignerServer(grpcServer, ss)
-
-	go grpcServer.Serve(lis)
-
-	httpAddr := mainViper.GetString("server.http_addr")
-	if httpAddr == "" {
-		log.Fatalf("Server address is required")
-	}
-	//HTTP server setup
-	server := http.Server{
-		Addr:      httpAddr,
-		Handler:   api.Handlers(cryptoServices),
-		TLSConfig: tlsConfig,
-	}
+	httpServer := setupHTTPServer(httpAddr, tlsConfig, cryptoServices)
 
 	if debug {
-		log.Println("RPC server listening on", rpcAddr)
+		log.Println("RPC server listening on", grpcAddr)
 		log.Println("HTTP server listening on", httpAddr)
 	}
 
-	err = server.ListenAndServeTLS("", "")
+	go grpcServer.Serve(lis)
+	err = httpServer.ListenAndServeTLS("", "")
 	if err != nil {
 		log.Fatal("HTTPS server failed to start:", err)
 	}
@@ -217,45 +258,3 @@ func debugServer(addr string) {
 		log.Fatalf("error listening on debug interface: %v", err)
 	}
 }
-
-// SetupHSMEnv is a method that depends on the existences
-func SetupHSMEnv(libraryPath, pin string) (*pkcs11.Ctx, pkcs11.SessionHandle) {
-	p := pkcs11.New(libraryPath)
-
-	if p == nil {
-		log.Fatalf("Failed to init library")
-	}
-
-	if err := p.Initialize(); err != nil {
-		log.Fatalf("Initialize error %s\n", err.Error())
-	}
-
-	slots, err := p.GetSlotList(true)
-	if err != nil {
-		log.Fatalf("Failed to list HSM slots %s", err)
-	}
-	// Check to see if we got any slots from the HSM.
-	if len(slots) < 1 {
-		log.Fatalln("No HSM Slots found")
-	}
-
-	// CKF_SERIAL_SESSION: TRUE if cryptographic functions are performed in serial with the application; FALSE if the functions may be performed in parallel with the application.
-	// CKF_RW_SESSION: TRUE if the session is read/write; FALSE if the session is read-only
-	session, err := p.OpenSession(slots[0], pkcs11.CKF_SERIAL_SESSION|pkcs11.CKF_RW_SESSION)
-	if err != nil {
-		log.Fatalf("Failed to Start Session with HSM %s", err)
-	}
-
-	if err = p.Login(session, pkcs11.CKU_USER, pin); err != nil {
-		log.Fatalf("User PIN %s\n", err.Error())
-	}
-
-	return p, session
-}
-
-func cleanup(ctx *pkcs11.Ctx, session pkcs11.SessionHandle) {
-	ctx.Destroy()
-	ctx.Finalize()
-	ctx.CloseSession(session)
-	ctx.Logout(session)
-}

cmd/notary-signer/main_test.go

@@ -6,9 +6,12 @@ import (
 	"bytes"
 	"crypto/tls"
 	"fmt"
-	"strings"
+	"io/ioutil"
+	"os"
 	"testing"
 
+	"github.com/docker/notary/signer"
+	"github.com/docker/notary/tuf/data"
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
 )
@@ -20,54 +23,127 @@ const (
 )
 
 // initializes a viper object with test configuration
-func configure(jsonConfig []byte) *viper.Viper {
+func configure(jsonConfig string) *viper.Viper {
 	config := viper.New()
 	config.SetConfigType("json")
-	config.ReadConfig(bytes.NewBuffer(jsonConfig))
+	config.ReadConfig(bytes.NewBuffer([]byte(jsonConfig)))
 	return config
 }
 
-func TestSignerTLSMissingCertAndOrKey(t *testing.T) {
-	configs := []string{
-		"{}",
-		fmt.Sprintf(`{"cert_file": "%s"}`, Cert),
-		fmt.Sprintf(`{"key_file": "%s"}`, Key),
+func TestGetAddrAndTLSConfigInvalidTLS(t *testing.T) {
+	invalids := []string{
+		`{"server": {"http_addr": ":1234", "grpc_addr": ":2345"}}`,
+		`{"server": {
+				"http_addr": ":1234",
+				"grpc_addr": ":2345",
+				"tls_cert_file": "nope",
+				"tls_key_file": "nope"
+		}}`,
 	}
-	for _, serverConfig := range configs {
-		config := configure(
-			[]byte(fmt.Sprintf(`{"server": %s}`, serverConfig)))
-		tlsConfig, err := signerTLS(config, false)
+	for _, configJSON := range invalids {
+		_, _, _, err := getAddrAndTLSConfig(configure(configJSON))
 		assert.Error(t, err)
-		assert.Nil(t, tlsConfig)
-		assert.Equal(t, "Certificate and key are mandatory", err.Error())
+		assert.Contains(t, err.Error(), "unable to set up TLS")
 	}
 }
 
-// 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)
+func TestGetAddrAndTLSConfigNoGRPCAddr(t *testing.T) {
+	_, _, _, err := getAddrAndTLSConfig(configure(fmt.Sprintf(`{
+		"server": {
+			"http_addr": ":1234",
+			"tls_cert_file": "%s",
+			"tls_key_file": "%s"
+		}
+	}`, Cert, Key)))
 	assert.Error(t, err)
-	assert.Nil(t, tlsConfig)
-	assert.True(t, strings.Contains(err.Error(), "Unable to set up TLS"))
+	assert.Contains(t, err.Error(), "grpc listen address required for server")
+}
+
+func TestGetAddrAndTLSConfigNoHTTPAddr(t *testing.T) {
+	_, _, _, err := getAddrAndTLSConfig(configure(fmt.Sprintf(`{
+		"server": {
+			"grpc_addr": ":1234",
+			"tls_cert_file": "%s",
+			"tls_key_file": "%s"
+		}
+	}`, Cert, Key)))
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "http listen address required for server")
+}
+
+func TestGetAddrAndTLSConfigSuccess(t *testing.T) {
+	httpAddr, grpcAddr, tlsConf, err := getAddrAndTLSConfig(configure(fmt.Sprintf(`{
+		"server": {
+			"http_addr": ":2345",
+			"grpc_addr": ":1234",
+			"tls_cert_file": "%s",
+			"tls_key_file": "%s"
+		}
+	}`, Cert, Key)))
+	assert.NoError(t, err)
+	assert.Equal(t, ":2345", httpAddr)
+	assert.Equal(t, ":1234", grpcAddr)
+	assert.NotNil(t, tlsConf)
+}
+
+func TestSetupCryptoServicesNoDefaultAlias(t *testing.T) {
+	tmpFile, err := ioutil.TempFile("/tmp", "sqlite3")
+	assert.NoError(t, err)
+	tmpFile.Close()
+	defer os.Remove(tmpFile.Name())
+
+	_, err = setUpCryptoservices(
+		configure(fmt.Sprintf(
+			`{"storage": {"backend": "sqlite3", "db_url": "%s"}}`,
+			tmpFile.Name())),
+		[]string{"sqlite3"})
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "must provide a default alias for the key DB")
+}
+
+func TestSetupCryptoServicesSuccess(t *testing.T) {
+	tmpFile, err := ioutil.TempFile("/tmp", "sqlite3")
+	assert.NoError(t, err)
+	tmpFile.Close()
+	defer os.Remove(tmpFile.Name())
+
+	cryptoServices, err := setUpCryptoservices(
+		configure(fmt.Sprintf(
+			`{"storage": {"backend": "sqlite3", "db_url": "%s"},
+			"default_alias": "timestamp"}`,
+			tmpFile.Name())),
+		[]string{"sqlite3"})
+	assert.NoError(t, err)
+	assert.Len(t, cryptoServices, 2)
+
+	edService, ok := cryptoServices[data.ED25519Key]
+	assert.True(t, ok)
+
+	ecService, ok := cryptoServices[data.ECDSAKey]
+	assert.True(t, ok)
+
+	assert.Equal(t, edService, ecService)
+}
+
+func TestSetupHTTPServer(t *testing.T) {
+	httpServer := setupHTTPServer(":4443", nil, make(signer.CryptoServiceIndex))
+	assert.Equal(t, ":4443", httpServer.Addr)
+	assert.Nil(t, httpServer.TLSConfig)
+}
+
+func TestSetupGRPCServerInvalidAddress(t *testing.T) {
+	_, _, err := setupGRPCServer("nope", nil, make(signer.CryptoServiceIndex))
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "grpc server failed to listen on nope")
+}
+
+func TestSetupGRPCServerSuccess(t *testing.T) {
+	tlsConf := tls.Config{InsecureSkipVerify: true}
+	grpcServer, lis, err := setupGRPCServer(":7899", &tlsConf,
+		make(signer.CryptoServiceIndex))
+	defer lis.Close()
+	assert.NoError(t, err)
+	assert.Equal(t, "[::]:7899", lis.Addr().String())
+	assert.Equal(t, "tcp", lis.Addr().Network())
+	assert.NotNil(t, grpcServer)
 }