From e03927bb5cfdfbdc94dd2cadedc0463a853c06ac Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Mon, 24 Oct 2016 13:46:22 -0700 Subject: [PATCH] Changed the way TLS config is parsed --- client/config/config.go | 5 +- command/agent/agent.go | 23 ++-- command/agent/config-test-fixtures/basic.hcl | 8 ++ command/agent/config.go | 111 ++++++++++++------- command/agent/config_parse.go | 50 ++++++++- command/agent/config_parse_test.go | 8 ++ command/agent/http.go | 12 +- nomad/config.go | 2 + nomad/server.go | 1 - tlsutil/config.go | 5 +- tlsutil/config_test.go | 1 + 11 files changed, 154 insertions(+), 72 deletions(-) diff --git a/client/config/config.go b/client/config/config.go index 420b653c6..c52aecd24 100644 --- a/client/config/config.go +++ b/client/config/config.go @@ -134,10 +134,10 @@ type Config struct { // allocation metrics to remote Telemetry sinks PublishAllocationMetrics bool - // Don't verify callers identity + // HttpTLS enables TLS for the HTTP endpoints on the clients. HttpTLS bool `mapstructure:"http_tls"` - // Verify inbound and outbound + // RpcTLS enables TLS for the outgoing TLS connections to the Nomad servers. RpcTLS bool `mapstructure:"rpc_tls"` // VerifyServerHostname is used to enable hostname verification of servers. This @@ -254,6 +254,7 @@ func (c *Config) ReadStringListToMapDefault(key, defaultValue string) map[string return list } +// TLSConfig returns a TLSUtil Config based on the client configuration func (c *Config) TLSConfig() *tlsutil.Config { tlsConf := &tlsutil.Config{ VerifyIncoming: true, diff --git a/command/agent/agent.go b/command/agent/agent.go index b737ac4fd..62f38d2ef 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -245,11 +245,12 @@ func (a *Agent) serverConfig() (*nomad.Config, error) { conf.VaultConfig = a.config.Vault // Set the TLS related configs - conf.RpcTLS = a.config.RpcTLS - conf.VerifyServerHostname = a.config.VerifyServerHostname - conf.CAFile = a.config.CAFile - conf.CertFile = a.config.CertFile - conf.KeyFile = a.config.KeyFile + conf.RpcTLS = a.config.TLSConfig.EnableRPC + conf.RequireTLS = conf.RpcTLS + conf.VerifyServerHostname = a.config.TLSConfig.VerifyServerHostname + conf.CAFile = a.config.TLSConfig.CAFile + conf.CertFile = a.config.TLSConfig.CertFile + conf.KeyFile = a.config.TLSConfig.KeyFile return conf, nil } @@ -366,12 +367,12 @@ func (a *Agent) clientConfig() (*clientconfig.Config, error) { conf.PublishAllocationMetrics = a.config.Telemetry.PublishAllocationMetrics // Set the TLS related configs - conf.HttpTLS = a.config.HttpTLS - conf.RpcTLS = a.config.RpcTLS - conf.VerifyServerHostname = a.config.VerifyServerHostname - conf.CAFile = a.config.CAFile - conf.CertFile = a.config.CertFile - conf.KeyFile = a.config.KeyFile + conf.HttpTLS = a.config.TLSConfig.EnableHTTP + conf.RpcTLS = a.config.TLSConfig.EnableRPC + conf.VerifyServerHostname = a.config.TLSConfig.VerifyServerHostname + conf.CAFile = a.config.TLSConfig.CAFile + conf.CertFile = a.config.TLSConfig.CertFile + conf.KeyFile = a.config.TLSConfig.KeyFile return conf, nil } diff --git a/command/agent/config-test-fixtures/basic.hcl b/command/agent/config-test-fixtures/basic.hcl index 96f9e6eb1..602a0011c 100644 --- a/command/agent/config-test-fixtures/basic.hcl +++ b/command/agent/config-test-fixtures/basic.hcl @@ -121,3 +121,11 @@ vault { tls_server_name = "foobar" tls_skip_verify = true } +tls { + http_tls = true + rpc_tls = true + verify_server_hostname = true + ca_file = "foo" + cert_file = "bar" + key_file = "pipe" +} diff --git a/command/agent/config.go b/command/agent/config.go index 383d1232f..7cfbcf053 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -112,31 +112,9 @@ type Config struct { // List of config files that have been loaded (in order) Files []string `mapstructure:"-"` - // Don't verify callers identity - HttpTLS bool `mapstructure:"http_tls"` - - // Verify inbound and outbound - RpcTLS bool `mapstructure:"rpc_tls"` - - // VerifyServerHostname is used to enable hostname verification of servers. This - // ensures that the certificate presented is valid for server... - // This prevents a compromised client from being restarted as a server, and then - // intercepting request traffic as well as being added as a raft peer. This should be - // enabled by default with VerifyOutgoing, but for legacy reasons we cannot break - // existing clients. - VerifyServerHostname bool `mapstructure:"verify_server_hostname"` - - // CAFile is a path to a certificate authority file. This is used with VerifyIncoming - // or VerifyOutgoing to verify the TLS connection. - CAFile string `mapstructure:"ca_file"` - - // CertFile is used to provide a TLS certificate that is used for serving TLS connections. - // Must be provided to serve TLS connections. - CertFile string `mapstructure:"cert_file"` - - // KeyFile is used to provide a TLS key that is used for serving TLS connections. - // Must be provided to serve TLS connections. - KeyFile string `mapstructure:"key_file"` + // TLSConfig provides TLS related configuration for the Nomad server and + // client + TLSConfig *TLSConfig `mapstructure:"tls"` // HTTPAPIResponseHeaders allows users to configure the Nomad http agent to // set arbritrary headers on API responses @@ -161,6 +139,36 @@ type AtlasConfig struct { Endpoint string `mapstructure:"endpoint"` } +// TLSConfig provides TLS related configuration +type TLSConfig struct { + + // EnableHTTP enabled TLS for http traffic to the Nomad server and clients + EnableHTTP bool `mapstructure:"http_tls"` + + // EnableRPC enables TLS for RPC and Raft traffic to the Nomad servers + EnableRPC bool `mapstructure:"rpc_tls"` + + // VerifyServerHostname is used to enable hostname verification of servers. This + // ensures that the certificate presented is valid for server... + // This prevents a compromised client from being restarted as a server, and then + // intercepting request traffic as well as being added as a raft peer. This should be + // enabled by default with VerifyOutgoing, but for legacy reasons we cannot break + // existing clients. + VerifyServerHostname bool `mapstructure:"verify_server_hostname"` + + // CAFile is a path to a certificate authority file. This is used with VerifyIncoming + // or VerifyOutgoing to verify the TLS connection. + CAFile string `mapstructure:"ca_file"` + + // CertFile is used to provide a TLS certificate that is used for serving TLS connections. + // Must be provided to serve TLS connections. + CertFile string `mapstructure:"cert_file"` + + // KeyFile is used to provide a TLS key that is used for serving TLS connections. + // Must be provided to serve TLS connections. + KeyFile string `mapstructure:"key_file"` +} + // ClientConfig is configuration specific to the client mode type ClientConfig struct { // Enabled controls if we are a client @@ -512,6 +520,7 @@ func DefaultConfig() *Config { CollectionInterval: "1s", collectionInterval: 1 * time.Second, }, + TLSConfig: &TLSConfig{}, } } @@ -583,24 +592,6 @@ func (c *Config) Merge(b *Config) *Config { if b.DisableAnonymousSignature { result.DisableAnonymousSignature = true } - if b.HttpTLS { - result.HttpTLS = true - } - if b.RpcTLS { - result.RpcTLS = true - } - if b.VerifyServerHostname { - result.VerifyServerHostname = true - } - if b.CAFile != "" { - result.CAFile = b.CAFile - } - if b.CertFile != "" { - result.CertFile = b.CertFile - } - if b.KeyFile != "" { - result.KeyFile = b.KeyFile - } // Apply the telemetry config if result.Telemetry == nil && b.Telemetry != nil { @@ -610,6 +601,14 @@ func (c *Config) Merge(b *Config) *Config { result.Telemetry = result.Telemetry.Merge(b.Telemetry) } + // Apply the TLS Config + if result.TLSConfig == nil && b.TLSConfig != nil { + tlsConfig := *b.TLSConfig + result.TLSConfig = &tlsConfig + } else if b.TLSConfig != nil { + result.TLSConfig = result.TLSConfig.Merge(b.TLSConfig) + } + // Apply the client config if result.Client == nil && b.Client != nil { client := *b.Client @@ -808,6 +807,32 @@ func (a *ClientConfig) Merge(b *ClientConfig) *ClientConfig { return &result } +// Merge is used to merge two TLS configs together +func (t *TLSConfig) Merge(b *TLSConfig) *TLSConfig { + result := *t + + if b.EnableHTTP { + result.EnableHTTP = true + } + if b.EnableRPC { + result.EnableRPC = true + } + if b.VerifyServerHostname { + result.VerifyServerHostname = true + } + if b.CAFile != "" { + result.CAFile = b.CAFile + } + if b.CertFile != "" { + result.CertFile = b.CertFile + } + if b.KeyFile != "" { + result.KeyFile = b.KeyFile + } + + return &result +} + // Merge is used to merge two telemetry configs together func (a *Telemetry) Merge(b *Telemetry) *Telemetry { result := *a diff --git a/command/agent/config_parse.go b/command/agent/config_parse.go index e88e263c7..3850831b7 100644 --- a/command/agent/config_parse.go +++ b/command/agent/config_parse.go @@ -94,12 +94,7 @@ func parseConfig(result *Config, list *ast.ObjectList) error { "atlas", "consul", "vault", - "http_tls", - "rpc_tls", - "verify_server_hostname", - "ca_file", - "cert_file", - "key_file", + "tls", "http_api_response_headers", } if err := checkHCLKeys(list, valid); err != nil { @@ -121,6 +116,7 @@ func parseConfig(result *Config, list *ast.ObjectList) error { delete(m, "atlas") delete(m, "consul") delete(m, "vault") + delete(m, "tls") delete(m, "http_api_response_headers") // Decode the rest @@ -191,6 +187,13 @@ func parseConfig(result *Config, list *ast.ObjectList) error { } } + // Parse the TLS config + if o := list.Filter("tls"); len(o.Items) > 0 { + if err := parseTLSConfig(&result.TLSConfig, o); err != nil { + return multierror.Prefix(err, "tls ->") + } + } + // Parse out http_api_response_headers fields. These are in HCL as a list so // we need to iterate over them and merge them. if headersO := list.Filter("http_api_response_headers"); len(headersO.Items) > 0 { @@ -649,6 +652,41 @@ func parseConsulConfig(result **config.ConsulConfig, list *ast.ObjectList) error return nil } +func parseTLSConfig(result **TLSConfig, list *ast.ObjectList) error { + list = list.Elem() + if len(list.Items) > 1 { + return fmt.Errorf("only one 'tls' block allowed") + } + + // Get the TLS object + listVal := list.Items[0].Val + + valid := []string{ + "http_tls", + "rpc_tls", + "verify_server_hostname", + "ca_file", + "cert_file", + "key_file", + } + + if err := checkHCLKeys(listVal, valid); err != nil { + return err + } + + var m map[string]interface{} + if err := hcl.DecodeObject(&m, listVal); err != nil { + return err + } + + var tlsConfig TLSConfig + if err := mapstructure.WeakDecode(m, &tlsConfig); err != nil { + return err + } + *result = &tlsConfig + return nil +} + func parseVaultConfig(result **config.VaultConfig, list *ast.ObjectList) error { list = list.Elem() if len(list.Items) > 1 { diff --git a/command/agent/config_parse_test.go b/command/agent/config_parse_test.go index bdb4bd10d..2ef33f6f1 100644 --- a/command/agent/config_parse_test.go +++ b/command/agent/config_parse_test.go @@ -136,6 +136,14 @@ func TestConfig_Parse(t *testing.T) { TaskTokenTTL: "1s", Token: "12345", }, + TLSConfig: &TLSConfig{ + EnableHTTP: true, + EnableRPC: true, + VerifyServerHostname: true, + CAFile: "foo", + CertFile: "bar", + KeyFile: "pipe", + }, HTTPAPIResponseHeaders: map[string]string{ "Access-Control-Allow-Origin": "*", }, diff --git a/command/agent/http.go b/command/agent/http.go index 44050c849..02567da16 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -54,14 +54,14 @@ func NewHTTPServer(agent *Agent, config *Config, logOutput io.Writer) (*HTTPServ return nil, fmt.Errorf("failed to start HTTP listener: %v", err) } - if config.HttpTLS { + if config.TLSConfig.EnableHTTP { tlsConf := &tlsutil.Config{ - VerifyIncoming: true, + VerifyIncoming: false, VerifyOutgoing: true, - VerifyServerHostname: config.VerifyServerHostname, - CAFile: config.CAFile, - CertFile: config.CertFile, - KeyFile: config.KeyFile, + VerifyServerHostname: config.TLSConfig.VerifyServerHostname, + CAFile: config.TLSConfig.CAFile, + CertFile: config.TLSConfig.CertFile, + KeyFile: config.TLSConfig.KeyFile, ServerName: config.NodeName, } tlsConfig, err := tlsConf.IncomingTLSConfig() diff --git a/nomad/config.go b/nomad/config.go index 82be3e72a..212a1262f 100644 --- a/nomad/config.go +++ b/nomad/config.go @@ -193,6 +193,7 @@ type Config struct { // place, and a small jitter is applied to avoid a thundering herd. RPCHoldTimeout time.Duration + // Enable TLS for incoming RPC calls from Nomad clients RpcTLS bool // VerifyServerHostname is used to enable hostname verification of servers. This @@ -287,6 +288,7 @@ func DefaultConfig() *Config { return c } +// tlsConfig returns a TLSUtil Config based on the server configuration func (c *Config) tlsConfig() *tlsutil.Config { tlsConf := &tlsutil.Config{ VerifyIncoming: true, diff --git a/nomad/server.go b/nomad/server.go index 59694976e..68c02355a 100644 --- a/nomad/server.go +++ b/nomad/server.go @@ -234,7 +234,6 @@ func NewServer(config *Config, consulSyncer *consul.Syncer, logger *log.Logger) } // Initialize the RPC layer - // TODO: TLS... if err := s.setupRPC(tlsWrap); err != nil { s.Shutdown() s.logger.Printf("[ERR] nomad: failed to start RPC layer: %s", err) diff --git a/tlsutil/config.go b/tlsutil/config.go index 8d1ebc1bf..56c81cbe0 100644 --- a/tlsutil/config.go +++ b/tlsutil/config.go @@ -9,8 +9,7 @@ import ( "time" ) -// Wrapper is a variant of DCWrapper, where the DC is provided as -// a constant value. This is usually done by currying DCWrapper. +// Wrapper wraps a connection and enables TLS on it. type Wrapper func(conn net.Conn) (net.Conn, error) // Config used to create tls.Config @@ -126,7 +125,7 @@ func (c *Config) OutgoingTLSConfig() (*tls.Config, error) { return tlsConfig, nil } -// OutgoingTLSWrapper returns a a DCWrapper based on the OutgoingTLS +// OutgoingTLSWrapper returns a a Wrapper based on the OutgoingTLS // configuration. If hostname verification is on, the wrapper // will properly generate the dynamic server name for verification. func (c *Config) OutgoingTLSWrapper() (Wrapper, error) { diff --git a/tlsutil/config_test.go b/tlsutil/config_test.go index 712ded2b7..48cba972c 100644 --- a/tlsutil/config_test.go +++ b/tlsutil/config_test.go @@ -256,6 +256,7 @@ func TestConfig_outgoingWrapper_OK(t *testing.T) { } func TestConfig_outgoingWrapper_BadCert(t *testing.T) { + // TODO this test is currently hanging, need to investigate more. t.SkipNow() config := &Config{ CAFile: "../test/ca/root.cer",