From f1bd28caf4efcd475fb57d4e638b1ac9458a835e Mon Sep 17 00:00:00 2001 From: Ying Li Date: Fri, 20 Nov 2015 01:17:45 -0800 Subject: [PATCH] The TLS certificates are now relative to the config file used. Previously, if a relative path was provided, it was relative to the current working directory to wherever the binaries were run. Now it is relative to whatever config file was used. Signed-off-by: Ying Li --- Dockerfile.server | 2 +- Dockerfile.signer | 2 +- cmd/notary-server/main.go | 6 +++--- cmd/notary-server/main_test.go | 18 +++++++++------- .../server-config.json | 10 ++++----- .../signer-config.json | 6 +++--- utils/configuration.go | 21 ++++++++++++++++--- utils/configuration_test.go | 21 +++++++++++++++++++ 8 files changed, 63 insertions(+), 23 deletions(-) rename cmd/notary-server/config.json => fixtures/server-config.json (53%) rename cmd/notary-signer/config.json => fixtures/signer-config.json (55%) diff --git a/Dockerfile.server b/Dockerfile.server index be430f778b..eac83a4067 100644 --- a/Dockerfile.server +++ b/Dockerfile.server @@ -20,4 +20,4 @@ RUN go install \ ${NOTARYPKG}/cmd/notary-server ENTRYPOINT [ "notary-server" ] -CMD [ "-config", "cmd/notary-server/config.json" ] +CMD [ "-config", "fixtures/server-config.json" ] diff --git a/Dockerfile.signer b/Dockerfile.signer index e4400bdbb0..de85aa9fdb 100644 --- a/Dockerfile.signer +++ b/Dockerfile.signer @@ -38,4 +38,4 @@ RUN go install \ ENTRYPOINT [ "notary-signer" ] -CMD [ "-config=cmd/notary-signer/config.json" ] +CMD [ "-config=fixtures/signer-config.json" ] diff --git a/cmd/notary-server/main.go b/cmd/notary-server/main.go index e9f738ecc7..37d4c09d7a 100644 --- a/cmd/notary-server/main.go +++ b/cmd/notary-server/main.go @@ -73,10 +73,10 @@ func getAddrAndTLSConfig(configuration *viper.Viper) (string, *tls.Config, error // 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") + rootCA := utils.GetPathRelativeToConfig(configuration, "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") + clientCert := utils.GetPathRelativeToConfig(configuration, "trust_service.tls_client_cert") + clientKey := utils.GetPathRelativeToConfig(configuration, "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.") diff --git a/cmd/notary-server/main_test.go b/cmd/notary-server/main_test.go index f11cafe38c..ed36fd3809 100644 --- a/cmd/notary-server/main_test.go +++ b/cmd/notary-server/main_test.go @@ -121,18 +121,22 @@ func TestGrpcTLSNoConfig(t *testing.T) { // 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 +// the correct tls.Config is returned based on all the configuration parameters, +// and that it gets the path relative to the config file 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": { + configJSON := `{ + "trust_service": { "hostname": "notary-server", - "tls_client_cert": "%s", - "tls_client_key": "%s"}}`, - Cert, Key) - tlsConfig, err := grpcTLS(configure(config)) + "tls_client_cert": "notary-server.crt", + "tls_client_key": "notary-server.key" + } + }` + config := configure(configJSON) + config.SetConfigFile("../../fixtures/config.json") + tlsConfig, err := grpcTLS(config) assert.NoError(t, err) assert.Equal(t, []tls.Certificate{keypair}, tlsConfig.Certificates) } diff --git a/cmd/notary-server/config.json b/fixtures/server-config.json similarity index 53% rename from cmd/notary-server/config.json rename to fixtures/server-config.json index 05f94b97fb..5a1b56cdcd 100644 --- a/cmd/notary-server/config.json +++ b/fixtures/server-config.json @@ -1,17 +1,17 @@ { "server": { "http_addr": ":4443", - "tls_key_file": "./fixtures/notary-server.key", - "tls_cert_file": "./fixtures/notary-server.crt" + "tls_key_file": "./notary-server.key", + "tls_cert_file": "./notary-server.crt" }, "trust_service": { "type": "remote", "hostname": "notarysigner", "port": "7899", - "tls_ca_file": "./fixtures/root-ca.crt", + "tls_ca_file": "./root-ca.crt", "key_algorithm": "ecdsa", - "tls_client_cert": "./fixtures/notary-server.crt", - "tls_client_key": "./fixtures/notary-server.key" + "tls_client_cert": "./notary-server.crt", + "tls_client_key": "./notary-server.key" }, "logging": { "level": "debug" diff --git a/cmd/notary-signer/config.json b/fixtures/signer-config.json similarity index 55% rename from cmd/notary-signer/config.json rename to fixtures/signer-config.json index ee29719846..780b7c5a27 100644 --- a/cmd/notary-signer/config.json +++ b/fixtures/signer-config.json @@ -2,9 +2,9 @@ "server": { "http_addr": ":4444", "grpc_addr": ":7899", - "tls_cert_file": "./fixtures/notary-signer.crt", - "tls_key_file": "./fixtures/notary-signer.key", - "client_ca_file": "./fixtures/notary-server.crt" + "tls_cert_file": "./notary-signer.crt", + "tls_key_file": "./notary-signer.key", + "client_ca_file": "./notary-server.crt" }, "logging": { "level": "debug" diff --git a/utils/configuration.go b/utils/configuration.go index 85cb0ebabc..37216400da 100644 --- a/utils/configuration.go +++ b/utils/configuration.go @@ -4,6 +4,7 @@ package utils import ( "fmt" + "path/filepath" "strings" "github.com/Sirupsen/logrus" @@ -18,16 +19,30 @@ type Storage struct { Source string } +// GetPathRelativeToConfig gets a configuration key which is a path, and if +// it is not empty or an absolute path, returns the absolute path relative +// to the configuration file +func GetPathRelativeToConfig(configuration *viper.Viper, key string) string { + configFile := configuration.ConfigFileUsed() + p := configuration.GetString(key) + if p == "" || filepath.IsAbs(p) { + return p + } + return filepath.Join(filepath.Dir(configFile), p) +} + // ParseServerTLS tries to parse out a valid ServerTLSOpts from a Viper: // - If TLS is required, both the cert and key must be provided // - If TLS is not requried, either both the cert and key must be provided or // neither must be provided +// The files are relative to the config file used to populate the instance +// of viper. func ParseServerTLS(configuration *viper.Viper, tlsRequired bool) (*ServerTLSOpts, error) { // unmarshalling into objects does not seem to pick up env vars tlsOpts := ServerTLSOpts{ - ServerCertFile: configuration.GetString("server.tls_cert_file"), - ServerKeyFile: configuration.GetString("server.tls_key_file"), - ClientCAFile: configuration.GetString("server.client_ca_file"), + ServerCertFile: GetPathRelativeToConfig(configuration, "server.tls_cert_file"), + ServerKeyFile: GetPathRelativeToConfig(configuration, "server.tls_key_file"), + ClientCAFile: GetPathRelativeToConfig(configuration, "server.client_ca_file"), } cert, key := tlsOpts.ServerCertFile, tlsOpts.ServerKeyFile diff --git a/utils/configuration_test.go b/utils/configuration_test.go index b587c25632..00a3ab5ba3 100644 --- a/utils/configuration_test.go +++ b/utils/configuration_test.go @@ -274,6 +274,27 @@ func TestParseTLSWithTLS(t *testing.T) { assert.Equal(t, expected, *tlsOpts) } +func TestParseTLSWithTLSRelativeToConfigFile(t *testing.T) { + config := configure(`{ + "server": { + "tls_cert_file": "path/to/cert", + "tls_key_file": "/abspath/to/key", + "client_ca_file": "" + } + }`) + config.SetConfigFile("/opt/me.json") + + expected := ServerTLSOpts{ + ServerCertFile: "/opt/path/to/cert", + ServerKeyFile: "/abspath/to/key", + ClientCAFile: "", + } + + tlsOpts, err := ParseServerTLS(config, false) + assert.NoError(t, err) + assert.Equal(t, expected, *tlsOpts) +} + func TestParseTLSWithEnvironmentVariables(t *testing.T) { config := configure(`{ "server": {