diff --git a/CHANGELOG.md b/CHANGELOG.md index c4755426751c..debe81925089 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,9 +3,11 @@ __BACKWARDS INCOMPATIBILITIES:__ * config: Nomad no longer parses Atlas configuration stanzas. Atlas has been deprecated since earlier this year. If you have an Atlas stanza in your - config file it will have to be removed. + config file it will have to be removed. IMPROVEMENTS: + * core: Allow operators to reload TLS certificate and key files via SIGHUP + [GH-3479] * core: Allow agents to be run in `rpc_upgrade_mode` when migrating a cluster to TLS rather than changing `heartbeat_grace` * api: Allocations now track and return modify time in addition to create time diff --git a/client/client.go b/client/client.go index 21ce75353edb..22c13cdfe6ad 100644 --- a/client/client.go +++ b/client/client.go @@ -17,7 +17,7 @@ import ( "github.com/boltdb/bolt" consulapi "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/lib" - "github.com/hashicorp/go-multierror" + multierror "github.com/hashicorp/go-multierror" "github.com/hashicorp/nomad/client/allocdir" "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/client/driver" @@ -369,6 +369,11 @@ func (c *Client) Leave() error { return nil } +// GetConfig returns the config of the client for testing purposes only +func (c *Client) GetConfig() *config.Config { + return c.config +} + // Datacenter returns the datacenter for the given client func (c *Client) Datacenter() string { return c.config.Node.Datacenter diff --git a/client/config/config.go b/client/config/config.go index eebe68723e2c..1f89e4033c6b 100644 --- a/client/config/config.go +++ b/client/config/config.go @@ -356,6 +356,7 @@ func (c *Config) TLSConfiguration() *tlsutil.Config { CAFile: c.TLSConfig.CAFile, CertFile: c.TLSConfig.CertFile, KeyFile: c.TLSConfig.KeyFile, + KeyLoader: c.TLSConfig.GetKeyLoader(), } return tlsConf } diff --git a/command/agent/agent.go b/command/agent/agent.go index 60907a0a8c34..c66bdaeb2410 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -43,7 +43,9 @@ const ( // scheduled to, and are responsible for interfacing with // servers to run allocations. type Agent struct { - config *Config + config *Config + configLock sync.Mutex + logger *log.Logger logOutput io.Writer @@ -724,6 +726,40 @@ func (a *Agent) Stats() map[string]map[string]string { return stats } +// Reload handles configuration changes for the agent. Provides a method that +// is easier to unit test, as this action is invoked via SIGHUP. +func (a *Agent) Reload(newConfig *Config) error { + a.configLock.Lock() + defer a.configLock.Unlock() + + if newConfig.TLSConfig != nil { + + // TODO(chelseakomlo) In a later PR, we will introduce the ability to reload + // TLS configuration if the agent is not running with TLS enabled. + if a.config.TLSConfig != nil { + // Reload the certificates on the keyloader and on success store the + // updated TLS config. It is important to reuse the same keyloader + // as this allows us to dynamically reload configurations not only + // on the Agent but on the Server and Client too (they are + // referencing the same keyloader). + keyloader := a.config.TLSConfig.GetKeyLoader() + _, err := keyloader.LoadKeyPair(newConfig.TLSConfig.CertFile, newConfig.TLSConfig.KeyFile) + if err != nil { + return err + } + a.config.TLSConfig = newConfig.TLSConfig + a.config.TLSConfig.KeyLoader = keyloader + } + } + + return nil +} + +// GetConfigCopy creates a replica of the agent's config, excluding locks +func (a *Agent) GetConfig() *Config { + return a.config +} + // setupConsul creates the Consul client and starts its main Run loop. func (a *Agent) setupConsul(consulConfig *config.ConsulConfig) error { apiConf, err := consulConfig.ApiConfig() diff --git a/command/agent/agent_test.go b/command/agent/agent_test.go index 0c73294c2b90..5dc8d8ce737e 100644 --- a/command/agent/agent_test.go +++ b/command/agent/agent_test.go @@ -22,7 +22,7 @@ func tmpDir(t testing.TB) string { return dir } -func TestAgent_RPCPing(t *testing.T) { +func TestAgent_RPC_Ping(t *testing.T) { t.Parallel() agent := NewTestAgent(t, t.Name(), nil) defer agent.Shutdown() @@ -559,3 +559,190 @@ func TestAgent_HTTPCheckPath(t *testing.T) { t.Errorf("expected client check path to be %q but found %q", expected, check.Path) } } + +// This test asserts that the keyloader embedded in the TLS config is shared +// across the Agent, Server, and Client. This is essential for certificate +// reloading to work. +func TestServer_Reload_TLS_Shared_Keyloader(t *testing.T) { + t.Parallel() + assert := assert.New(t) + + // We will start out with a bad cert and then reload with a good one. + const ( + cafile = "../../helper/tlsutil/testdata/ca.pem" + foocert = "../../helper/tlsutil/testdata/nomad-bad.pem" + fookey = "../../helper/tlsutil/testdata/nomad-bad-key.pem" + foocert2 = "../../helper/tlsutil/testdata/nomad-foo.pem" + fookey2 = "../../helper/tlsutil/testdata/nomad-foo-key.pem" + ) + + agent := NewTestAgent(t, t.Name(), func(c *Config) { + c.TLSConfig = &sconfig.TLSConfig{ + EnableHTTP: true, + EnableRPC: true, + VerifyServerHostname: true, + CAFile: cafile, + CertFile: foocert, + KeyFile: fookey, + } + }) + defer agent.Shutdown() + + originalKeyloader := agent.Config.TLSConfig.GetKeyLoader() + originalCert, err := originalKeyloader.GetOutgoingCertificate(nil) + assert.NotNil(originalKeyloader) + if assert.Nil(err) { + assert.NotNil(originalCert) + } + + // Switch to the correct certificates and reload + newConfig := &Config{ + TLSConfig: &sconfig.TLSConfig{ + EnableHTTP: true, + EnableRPC: true, + VerifyServerHostname: true, + CAFile: cafile, + CertFile: foocert2, + KeyFile: fookey2, + }, + } + + assert.Nil(agent.Reload(newConfig)) + assert.Equal(agent.Config.TLSConfig.CertFile, newConfig.TLSConfig.CertFile) + assert.Equal(agent.Config.TLSConfig.KeyFile, newConfig.TLSConfig.KeyFile) + assert.Equal(agent.Config.TLSConfig.GetKeyLoader(), originalKeyloader) + + // Assert is passed through on the server correctly + if assert.NotNil(agent.server.GetConfig().TLSConfig) { + serverKeyloader := agent.server.GetConfig().TLSConfig.GetKeyLoader() + assert.Equal(serverKeyloader, originalKeyloader) + newCert, err := serverKeyloader.GetOutgoingCertificate(nil) + assert.Nil(err) + assert.NotEqual(originalCert, newCert) + } + + // Assert is passed through on the client correctly + if assert.NotNil(agent.client.GetConfig().TLSConfig) { + clientKeyloader := agent.client.GetConfig().TLSConfig.GetKeyLoader() + assert.Equal(clientKeyloader, originalKeyloader) + newCert, err := clientKeyloader.GetOutgoingCertificate(nil) + assert.Nil(err) + assert.NotEqual(originalCert, newCert) + } +} + +func TestServer_Reload_TLS_Certificate(t *testing.T) { + t.Parallel() + assert := assert.New(t) + + const ( + cafile = "../../helper/tlsutil/testdata/ca.pem" + foocert = "../../helper/tlsutil/testdata/nomad-bad.pem" + fookey = "../../helper/tlsutil/testdata/nomad-bad-key.pem" + foocert2 = "../../helper/tlsutil/testdata/nomad-foo.pem" + fookey2 = "../../helper/tlsutil/testdata/nomad-foo-key.pem" + ) + + agentConfig := &Config{ + TLSConfig: &sconfig.TLSConfig{ + EnableHTTP: true, + EnableRPC: true, + VerifyServerHostname: true, + CAFile: cafile, + CertFile: foocert, + KeyFile: fookey, + }, + } + + agent := &Agent{ + config: agentConfig, + } + + newConfig := &Config{ + TLSConfig: &sconfig.TLSConfig{ + EnableHTTP: true, + EnableRPC: true, + VerifyServerHostname: true, + CAFile: cafile, + CertFile: foocert2, + KeyFile: fookey2, + }, + } + + originalKeyloader := agentConfig.TLSConfig.GetKeyLoader() + assert.NotNil(originalKeyloader) + + err := agent.Reload(newConfig) + assert.Nil(err) + assert.Equal(agent.config.TLSConfig.CertFile, newConfig.TLSConfig.CertFile) + assert.Equal(agent.config.TLSConfig.KeyFile, newConfig.TLSConfig.KeyFile) + assert.Equal(agent.config.TLSConfig.GetKeyLoader(), originalKeyloader) +} + +func TestServer_Reload_TLS_Certificate_Invalid(t *testing.T) { + t.Parallel() + assert := assert.New(t) + + const ( + cafile = "../../helper/tlsutil/testdata/ca.pem" + foocert = "../../helper/tlsutil/testdata/nomad-bad.pem" + fookey = "../../helper/tlsutil/testdata/nomad-bad-key.pem" + foocert2 = "invalid_cert_path" + fookey2 = "invalid_key_path" + ) + + agentConfig := &Config{ + TLSConfig: &sconfig.TLSConfig{ + EnableHTTP: true, + EnableRPC: true, + VerifyServerHostname: true, + CAFile: cafile, + CertFile: foocert, + KeyFile: fookey, + }, + } + + agent := &Agent{ + config: agentConfig, + } + + newConfig := &Config{ + TLSConfig: &sconfig.TLSConfig{ + EnableHTTP: true, + EnableRPC: true, + VerifyServerHostname: true, + CAFile: cafile, + CertFile: foocert2, + KeyFile: fookey2, + }, + } + + err := agent.Reload(newConfig) + assert.NotNil(err) + assert.NotEqual(agent.config.TLSConfig.CertFile, newConfig.TLSConfig.CertFile) + assert.NotEqual(agent.config.TLSConfig.KeyFile, newConfig.TLSConfig.KeyFile) +} + +func Test_GetConfig(t *testing.T) { + assert := assert.New(t) + + agentConfig := &Config{ + Telemetry: &Telemetry{}, + Client: &ClientConfig{}, + Server: &ServerConfig{}, + ACL: &ACLConfig{}, + Ports: &Ports{}, + Addresses: &Addresses{}, + AdvertiseAddrs: &AdvertiseAddrs{}, + Vault: &sconfig.VaultConfig{}, + Consul: &sconfig.ConsulConfig{}, + Sentinel: &sconfig.SentinelConfig{}, + } + + agent := &Agent{ + config: agentConfig, + } + + actualAgentConfig := agent.GetConfig() + assert.Equal(actualAgentConfig, agentConfig) +} diff --git a/command/agent/command.go b/command/agent/command.go index cd704b697a70..9966b278cb08 100644 --- a/command/agent/command.go +++ b/command/agent/command.go @@ -23,8 +23,8 @@ import ( checkpoint "github.com/hashicorp/go-checkpoint" gsyslog "github.com/hashicorp/go-syslog" "github.com/hashicorp/logutils" - "github.com/hashicorp/nomad/helper/flag-helpers" - "github.com/hashicorp/nomad/helper/gated-writer" + flaghelper "github.com/hashicorp/nomad/helper/flag-helpers" + gatedwriter "github.com/hashicorp/nomad/helper/gated-writer" "github.com/hashicorp/nomad/nomad/structs/config" "github.com/hashicorp/nomad/version" "github.com/mitchellh/cli" @@ -529,11 +529,11 @@ func (c *Command) Run(args []string) int { go c.retryJoin(config) // Wait for exit - return c.handleSignals(config) + return c.handleSignals() } // handleSignals blocks until we get an exit-causing signal -func (c *Command) handleSignals(config *Config) int { +func (c *Command) handleSignals() int { signalCh := make(chan os.Signal, 4) signal.Notify(signalCh, os.Interrupt, syscall.SIGTERM, syscall.SIGHUP, syscall.SIGPIPE) @@ -557,17 +557,15 @@ WAIT: // Check if this is a SIGHUP if sig == syscall.SIGHUP { - if conf := c.handleReload(config); conf != nil { - *config = *conf - } + c.handleReload() goto WAIT } // Check if we should do a graceful leave graceful := false - if sig == os.Interrupt && config.LeaveOnInt { + if sig == os.Interrupt && c.agent.GetConfig().LeaveOnInt { graceful = true - } else if sig == syscall.SIGTERM && config.LeaveOnTerm { + } else if sig == syscall.SIGTERM && c.agent.GetConfig().LeaveOnTerm { graceful = true } @@ -599,12 +597,12 @@ WAIT: } // handleReload is invoked when we should reload our configs, e.g. SIGHUP -func (c *Command) handleReload(config *Config) *Config { +func (c *Command) handleReload() { c.Ui.Output("Reloading configuration...") newConf := c.readConfig() if newConf == nil { c.Ui.Error(fmt.Sprintf("Failed to reload configs")) - return config + return } // Change the log level @@ -617,7 +615,13 @@ func (c *Command) handleReload(config *Config) *Config { minLevel, c.logFilter.Levels)) // Keep the current log level - newConf.LogLevel = config.LogLevel + newConf.LogLevel = c.agent.GetConfig().LogLevel + } + + // Reloads configuration for an agent running in both client and server mode + err := c.agent.Reload(newConf) + if err != nil { + c.agent.logger.Printf("[ERR] agent: failed to reload the config: %v", err) } if s := c.agent.Server(); s != nil { @@ -630,8 +634,6 @@ func (c *Command) handleReload(config *Config) *Config { } } } - - return newConf } // setupTelemetry is used ot setup the telemetry sub-systems diff --git a/command/agent/config.go b/command/agent/config.go index 5a65df2e3214..d4407acf0864 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -680,8 +680,7 @@ func (c *Config) Merge(b *Config) *Config { // Apply the TLS Config if result.TLSConfig == nil && b.TLSConfig != nil { - tlsConfig := *b.TLSConfig - result.TLSConfig = &tlsConfig + result.TLSConfig = b.TLSConfig.Copy() } else if b.TLSConfig != nil { result.TLSConfig = result.TLSConfig.Merge(b.TLSConfig) } diff --git a/command/agent/config_parse.go b/command/agent/config_parse.go index 3b4c5a5d8898..6c63a008dfdb 100644 --- a/command/agent/config_parse.go +++ b/command/agent/config_parse.go @@ -8,7 +8,7 @@ import ( "path/filepath" "time" - "github.com/hashicorp/go-multierror" + multierror "github.com/hashicorp/go-multierror" "github.com/hashicorp/hcl" "github.com/hashicorp/hcl/hcl/ast" "github.com/hashicorp/nomad/helper" @@ -749,6 +749,7 @@ func parseTLSConfig(result **config.TLSConfig, list *ast.ObjectList) error { if err := mapstructure.WeakDecode(m, &tlsConfig); err != nil { return err } + *result = &tlsConfig return nil } diff --git a/command/agent/http.go b/command/agent/http.go index 891bdf48f10d..1499eab6d4e2 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -14,7 +14,7 @@ import ( "time" "github.com/NYTimes/gziphandler" - "github.com/elazarl/go-bindata-assetfs" + assetfs "github.com/elazarl/go-bindata-assetfs" "github.com/hashicorp/nomad/helper/tlsutil" "github.com/hashicorp/nomad/nomad/structs" "github.com/rs/cors" @@ -75,6 +75,7 @@ func NewHTTPServer(agent *Agent, config *Config) (*HTTPServer, error) { CAFile: config.TLSConfig.CAFile, CertFile: config.TLSConfig.CertFile, KeyFile: config.TLSConfig.KeyFile, + KeyLoader: config.TLSConfig.GetKeyLoader(), } tlsConfig, err := tlsConf.IncomingTLSConfig() if err != nil { diff --git a/command/agent/http_test.go b/command/agent/http_test.go index 20dd7b46abae..5d4004c18e13 100644 --- a/command/agent/http_test.go +++ b/command/agent/http_test.go @@ -529,6 +529,108 @@ func checkIndex(resp *httptest.ResponseRecorder) error { return nil } +func TestHTTP_VerifyHTTPSClient_AfterConfigReload(t *testing.T) { + t.Parallel() + assert := assert.New(t) + + const ( + cafile = "../../helper/tlsutil/testdata/ca.pem" + foocert = "../../helper/tlsutil/testdata/nomad-bad.pem" + fookey = "../../helper/tlsutil/testdata/nomad-bad-key.pem" + foocert2 = "../../helper/tlsutil/testdata/nomad-foo.pem" + fookey2 = "../../helper/tlsutil/testdata/nomad-foo-key.pem" + ) + + agentConfig := &Config{ + TLSConfig: &config.TLSConfig{ + EnableHTTP: true, + VerifyHTTPSClient: true, + CAFile: cafile, + CertFile: foocert, + KeyFile: fookey, + }, + } + + newConfig := &Config{ + TLSConfig: &config.TLSConfig{ + EnableHTTP: true, + VerifyHTTPSClient: true, + CAFile: cafile, + CertFile: foocert2, + KeyFile: fookey2, + }, + } + + s := makeHTTPServer(t, func(c *Config) { + c.TLSConfig = agentConfig.TLSConfig + }) + defer s.Shutdown() + + // Make an initial request that should fail. + // Requests that specify a valid hostname, CA cert, and client + // certificate succeed. + tlsConf := &tls.Config{ + ServerName: "client.regionFoo.nomad", + RootCAs: x509.NewCertPool(), + GetClientCertificate: func(*tls.CertificateRequestInfo) (*tls.Certificate, error) { + c, err := tls.LoadX509KeyPair(foocert, fookey) + if err != nil { + return nil, err + } + return &c, nil + }, + } + + // HTTPS request should succeed + httpsReqURL := fmt.Sprintf("https://%s/v1/agent/self", s.Agent.config.AdvertiseAddrs.HTTP) + + cacertBytes, err := ioutil.ReadFile(cafile) + assert.Nil(err) + tlsConf.RootCAs.AppendCertsFromPEM(cacertBytes) + + transport := &http.Transport{TLSClientConfig: tlsConf} + client := &http.Client{Transport: transport} + req, err := http.NewRequest("GET", httpsReqURL, nil) + assert.Nil(err) + + // Check that we get an error that the certificate isn't valid for the + // region we are contacting. + _, err = client.Do(req) + assert.Contains(err.Error(), "certificate is valid for") + + // Reload the TLS configuration== + assert.Nil(s.Agent.Reload(newConfig)) + + // Requests that specify a valid hostname, CA cert, and client + // certificate succeed. + tlsConf = &tls.Config{ + ServerName: "client.regionFoo.nomad", + RootCAs: x509.NewCertPool(), + GetClientCertificate: func(*tls.CertificateRequestInfo) (*tls.Certificate, error) { + c, err := tls.LoadX509KeyPair(foocert2, fookey2) + if err != nil { + return nil, err + } + return &c, nil + }, + } + + cacertBytes, err = ioutil.ReadFile(cafile) + assert.Nil(err) + tlsConf.RootCAs.AppendCertsFromPEM(cacertBytes) + + transport = &http.Transport{TLSClientConfig: tlsConf} + client = &http.Client{Transport: transport} + req, err = http.NewRequest("GET", httpsReqURL, nil) + assert.Nil(err) + + resp, err := client.Do(req) + if assert.Nil(err) { + resp.Body.Close() + assert.Equal(resp.StatusCode, 200) + } +} + // getIndex parses X-Nomad-Index func getIndex(t *testing.T, resp *httptest.ResponseRecorder) uint64 { header := resp.Header().Get("X-Nomad-Index") diff --git a/helper/tlsutil/config.go b/helper/tlsutil/config.go index f1a175d446bb..3de5b925a7f2 100644 --- a/helper/tlsutil/config.go +++ b/helper/tlsutil/config.go @@ -7,6 +7,8 @@ import ( "io/ioutil" "net" "time" + + "github.com/hashicorp/nomad/nomad/structs/config" ) // RegionSpecificWrapper is used to invoke a static Region and turns a @@ -60,6 +62,9 @@ type Config struct { // KeyFile is used to provide a TLS key that is used for serving TLS connections. // Must be provided to serve TLS connections. KeyFile string + + // KeyLoader dynamically reloads TLS configuration. + KeyLoader *config.KeyLoader } // AppendCA opens and parses the CA file and adds the certificates to @@ -82,21 +87,27 @@ func (c *Config) AppendCA(pool *x509.CertPool) error { return nil } -// KeyPair is used to open and parse a certificate and key file -func (c *Config) KeyPair() (*tls.Certificate, error) { +// LoadKeyPair is used to open and parse a certificate and key file +func (c *Config) LoadKeyPair() (*tls.Certificate, error) { if c.CertFile == "" || c.KeyFile == "" { return nil, nil } - cert, err := tls.LoadX509KeyPair(c.CertFile, c.KeyFile) + + if c.KeyLoader == nil { + return nil, fmt.Errorf("No Keyloader object to perform LoadKeyPair") + } + + cert, err := c.KeyLoader.LoadKeyPair(c.CertFile, c.KeyFile) if err != nil { return nil, fmt.Errorf("Failed to load cert/key pair: %v", err) } - return &cert, err + return cert, err } // OutgoingTLSConfig generates a TLS configuration for outgoing // requests. It will return a nil config if this configuration should -// not use TLS for outgoing connections. +// not use TLS for outgoing connections. Provides a callback to +// fetch certificates, allowing for reloading on the fly. func (c *Config) OutgoingTLSConfig() (*tls.Config, error) { // If VerifyServerHostname is true, that implies VerifyOutgoing if c.VerifyServerHostname { @@ -125,12 +136,12 @@ func (c *Config) OutgoingTLSConfig() (*tls.Config, error) { return nil, err } - // Add cert/key - cert, err := c.KeyPair() + cert, err := c.LoadKeyPair() if err != nil { return nil, err } else if cert != nil { - tlsConfig.Certificates = []tls.Certificate{*cert} + tlsConfig.GetCertificate = c.KeyLoader.GetOutgoingCertificate + tlsConfig.GetClientCertificate = c.KeyLoader.GetClientCertificate } return tlsConfig, nil @@ -236,11 +247,11 @@ func (c *Config) IncomingTLSConfig() (*tls.Config, error) { } // Add cert/key - cert, err := c.KeyPair() + cert, err := c.LoadKeyPair() if err != nil { return nil, err } else if cert != nil { - tlsConfig.Certificates = []tls.Certificate{*cert} + tlsConfig.GetCertificate = c.KeyLoader.GetOutgoingCertificate } // Check if we require verification diff --git a/helper/tlsutil/config_test.go b/helper/tlsutil/config_test.go index 8a526317bd11..ffc3f54ec6c1 100644 --- a/helper/tlsutil/config_test.go +++ b/helper/tlsutil/config_test.go @@ -8,7 +8,9 @@ import ( "net" "testing" + "github.com/hashicorp/nomad/nomad/structs/config" "github.com/hashicorp/yamux" + "github.com/stretchr/testify/assert" ) const ( @@ -46,9 +48,11 @@ func TestConfig_CACertificate_Valid(t *testing.T) { } } -func TestConfig_KeyPair_None(t *testing.T) { - conf := &Config{} - cert, err := conf.KeyPair() +func TestConfig_LoadKeyPair_None(t *testing.T) { + conf := &Config{ + KeyLoader: &config.KeyLoader{}, + } + cert, err := conf.LoadKeyPair() if err != nil { t.Fatalf("err: %v", err) } @@ -57,12 +61,13 @@ func TestConfig_KeyPair_None(t *testing.T) { } } -func TestConfig_KeyPair_Valid(t *testing.T) { +func TestConfig_LoadKeyPair_Valid(t *testing.T) { conf := &Config{ - CertFile: foocert, - KeyFile: fookey, + CertFile: foocert, + KeyFile: fookey, + KeyLoader: &config.KeyLoader{}, } - cert, err := conf.KeyPair() + cert, err := conf.LoadKeyPair() if err != nil { t.Fatalf("err: %v", err) } @@ -138,28 +143,25 @@ func TestConfig_OutgoingTLS_VerifyHostname(t *testing.T) { } func TestConfig_OutgoingTLS_WithKeyPair(t *testing.T) { + assert := assert.New(t) + conf := &Config{ VerifyOutgoing: true, CAFile: cacert, CertFile: foocert, KeyFile: fookey, - } - tls, err := conf.OutgoingTLSConfig() - if err != nil { - t.Fatalf("err: %v", err) - } - if tls == nil { - t.Fatalf("expected config") - } - if len(tls.RootCAs.Subjects()) != 1 { - t.Fatalf("expect root cert") - } - if !tls.InsecureSkipVerify { - t.Fatalf("should skip verification") - } - if len(tls.Certificates) != 1 { - t.Fatalf("expected client cert") - } + KeyLoader: &config.KeyLoader{}, + } + tlsConf, err := conf.OutgoingTLSConfig() + assert.Nil(err) + assert.NotNil(tlsConf) + assert.Equal(len(tlsConf.RootCAs.Subjects()), 1) + assert.True(tlsConf.InsecureSkipVerify) + + clientHelloInfo := &tls.ClientHelloInfo{} + cert, err := tlsConf.GetCertificate(clientHelloInfo) + assert.Nil(err) + assert.NotNil(cert) } func startTLSServer(config *Config) (net.Conn, chan error) { @@ -206,6 +208,7 @@ func TestConfig_outgoingWrapper_OK(t *testing.T) { KeyFile: fookey, VerifyServerHostname: true, VerifyOutgoing: true, + KeyLoader: &config.KeyLoader{}, } client, errc := startTLSServer(config) @@ -274,6 +277,7 @@ func TestConfig_wrapTLS_OK(t *testing.T) { CertFile: foocert, KeyFile: fookey, VerifyOutgoing: true, + KeyLoader: &config.KeyLoader{}, } client, errc := startTLSServer(config) @@ -300,9 +304,10 @@ func TestConfig_wrapTLS_OK(t *testing.T) { func TestConfig_wrapTLS_BadCert(t *testing.T) { serverConfig := &Config{ - CAFile: cacert, - CertFile: badcert, - KeyFile: badkey, + CAFile: cacert, + CertFile: badcert, + KeyFile: badkey, + KeyLoader: &config.KeyLoader{}, } client, errc := startTLSServer(serverConfig) @@ -335,11 +340,14 @@ func TestConfig_wrapTLS_BadCert(t *testing.T) { } func TestConfig_IncomingTLS(t *testing.T) { + assert := assert.New(t) + conf := &Config{ VerifyIncoming: true, CAFile: cacert, CertFile: foocert, KeyFile: fookey, + KeyLoader: &config.KeyLoader{}, } tlsC, err := conf.IncomingTLSConfig() if err != nil { @@ -354,9 +362,11 @@ func TestConfig_IncomingTLS(t *testing.T) { if tlsC.ClientAuth != tls.RequireAndVerifyClientCert { t.Fatalf("should not skip verification") } - if len(tlsC.Certificates) != 1 { - t.Fatalf("expected client cert") - } + + clientHelloInfo := &tls.ClientHelloInfo{} + cert, err := tlsC.GetCertificate(clientHelloInfo) + assert.Nil(err) + assert.NotNil(cert) } func TestConfig_IncomingTLS_MissingCA(t *testing.T) { @@ -364,6 +374,7 @@ func TestConfig_IncomingTLS_MissingCA(t *testing.T) { VerifyIncoming: true, CertFile: foocert, KeyFile: fookey, + KeyLoader: &config.KeyLoader{}, } _, err := conf.IncomingTLSConfig() if err == nil { diff --git a/nomad/config.go b/nomad/config.go index a96345b847a7..bc7f715c9d23 100644 --- a/nomad/config.go +++ b/nomad/config.go @@ -312,10 +312,10 @@ func DefaultConfig() *Config { ConsulConfig: config.DefaultConsulConfig(), VaultConfig: config.DefaultVaultConfig(), RPCHoldTimeout: 5 * time.Second, + StatsCollectionInterval: 1 * time.Minute, TLSConfig: &config.TLSConfig{}, ReplicationBackoff: 30 * time.Second, SentinelGCInterval: 30 * time.Second, - StatsCollectionInterval: 1 * time.Minute, } // Enable all known schedulers by default @@ -348,13 +348,13 @@ func DefaultConfig() *Config { // tlsConfig returns a TLSUtil Config based on the server configuration func (c *Config) tlsConfig() *tlsutil.Config { - tlsConf := &tlsutil.Config{ + return &tlsutil.Config{ VerifyIncoming: true, VerifyOutgoing: true, VerifyServerHostname: c.TLSConfig.VerifyServerHostname, CAFile: c.TLSConfig.CAFile, CertFile: c.TLSConfig.CertFile, KeyFile: c.TLSConfig.KeyFile, + KeyLoader: c.TLSConfig.GetKeyLoader(), } - return tlsConf } diff --git a/nomad/rpc.go b/nomad/rpc.go index 4b0dbf828e0a..828ee0c94c0a 100644 --- a/nomad/rpc.go +++ b/nomad/rpc.go @@ -11,10 +11,10 @@ import ( "strings" "time" - "github.com/armon/go-metrics" + metrics "github.com/armon/go-metrics" "github.com/hashicorp/consul/lib" memdb "github.com/hashicorp/go-memdb" - "github.com/hashicorp/net-rpc-msgpackrpc" + msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc" "github.com/hashicorp/nomad/nomad/state" "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/raft" diff --git a/nomad/server.go b/nomad/server.go index afdffce77f2b..0189584732e7 100644 --- a/nomad/server.go +++ b/nomad/server.go @@ -19,7 +19,7 @@ import ( consulapi "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/lib" - "github.com/hashicorp/go-multierror" + multierror "github.com/hashicorp/go-multierror" lru "github.com/hashicorp/golang-lru" "github.com/hashicorp/nomad/command/agent/consul" "github.com/hashicorp/nomad/helper/tlsutil" @@ -27,7 +27,7 @@ import ( "github.com/hashicorp/nomad/nomad/state" "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/raft" - "github.com/hashicorp/raft-boltdb" + raftboltdb "github.com/hashicorp/raft-boltdb" "github.com/hashicorp/serf/serf" ) diff --git a/nomad/structs/config/tls.go b/nomad/structs/config/tls.go index 322e5581db76..ad8010b4bf05 100644 --- a/nomad/structs/config/tls.go +++ b/nomad/structs/config/tls.go @@ -1,5 +1,11 @@ package config +import ( + "crypto/tls" + "fmt" + "sync" +) + // TLSConfig provides TLS related configuration type TLSConfig struct { @@ -25,6 +31,11 @@ type TLSConfig struct { // Must be provided to serve TLS connections. CertFile string `mapstructure:"cert_file"` + // KeyLoader is a helper to dynamically reload TLS configuration + KeyLoader *KeyLoader + + keyloaderLock sync.Mutex + // 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"` @@ -38,9 +49,101 @@ type TLSConfig struct { VerifyHTTPSClient bool `mapstructure:"verify_https_client"` } +type KeyLoader struct { + cacheLock sync.Mutex + certificate *tls.Certificate +} + +// LoadKeyPair reloads the TLS certificate based on the specified certificate +// and key file. If successful, stores the certificate for further use. +func (k *KeyLoader) LoadKeyPair(certFile, keyFile string) (*tls.Certificate, error) { + k.cacheLock.Lock() + defer k.cacheLock.Unlock() + + // Allow downgrading + if certFile == "" && keyFile == "" { + k.certificate = nil + return nil, nil + } + + cert, err := tls.LoadX509KeyPair(certFile, keyFile) + if err != nil { + return nil, fmt.Errorf("Failed to load cert/key pair: %v", err) + } + + k.certificate = &cert + return k.certificate, nil +} + +// GetOutgoingCertificate fetches the currently-loaded certificate when +// accepting a TLS connection. This currently does not consider information in +// the ClientHello and only returns the certificate that was last loaded. +func (k *KeyLoader) GetOutgoingCertificate(*tls.ClientHelloInfo) (*tls.Certificate, error) { + k.cacheLock.Lock() + defer k.cacheLock.Unlock() + return k.certificate, nil +} + +// GetClientCertificate fetches the currently-loaded certificate when the Server +// requests a certificate from the caller. This currently does not consider +// information in the ClientHello and only returns the certificate that was last +// loaded. +func (k *KeyLoader) GetClientCertificate(*tls.CertificateRequestInfo) (*tls.Certificate, error) { + k.cacheLock.Lock() + defer k.cacheLock.Unlock() + return k.certificate, nil +} + +func (k *KeyLoader) Copy() *KeyLoader { + if k == nil { + return nil + } + + new := KeyLoader{} + new.certificate = k.certificate + return &new +} + +// GetKeyLoader returns the keyloader for a TLSConfig object. If the keyloader +// has not been initialized, it will first do so. +func (t *TLSConfig) GetKeyLoader() *KeyLoader { + t.keyloaderLock.Lock() + defer t.keyloaderLock.Unlock() + + // If the keyloader has not yet been initialized, do it here + if t.KeyLoader == nil { + t.KeyLoader = &KeyLoader{} + } + return t.KeyLoader +} + +// Copy copies the fields of TLSConfig to another TLSConfig object. Required as +// to not copy mutexes between objects. +func (t *TLSConfig) Copy() *TLSConfig { + if t == nil { + return t + } + + new := &TLSConfig{} + new.EnableHTTP = t.EnableHTTP + new.EnableRPC = t.EnableRPC + new.VerifyServerHostname = t.VerifyServerHostname + new.CAFile = t.CAFile + new.CertFile = t.CertFile + + t.keyloaderLock.Lock() + new.KeyLoader = t.KeyLoader.Copy() + t.keyloaderLock.Unlock() + + new.KeyFile = t.KeyFile + new.RPCUpgradeMode = t.RPCUpgradeMode + new.VerifyHTTPSClient = t.VerifyHTTPSClient + return new +} + // Merge is used to merge two TLS configs together func (t *TLSConfig) Merge(b *TLSConfig) *TLSConfig { - result := *t + result := t.Copy() if b.EnableHTTP { result.EnableHTTP = true @@ -63,5 +166,5 @@ func (t *TLSConfig) Merge(b *TLSConfig) *TLSConfig { if b.VerifyHTTPSClient { result.VerifyHTTPSClient = true } - return &result + return result }