Allow server TLS configuration to be reloaded via SIGHUP

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
@@ -57,6 +59,10 @@ type Config struct {
 	// Must be provided to serve TLS connections.
 	CertFile string
 
+	// Stores a TLS certificate that has been loaded given the information in the
+	// configuration. This can be updated via config.Reload()
+	Certificate *tls.Certificate
+
 	// KeyFile is used to provide a TLS key that is used for serving TLS connections.
 	// Must be provided to serve TLS connections.
 	KeyFile string
@@ -82,21 +88,32 @@ 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) {
+// Update syncs a new TLS config to a previously-created TLS config helper
+func (c *Config) Update(newConfig *config.TLSConfig) {
+	c.CAFile = newConfig.CAFile
+	c.CertFile = newConfig.CertFile
+	c.KeyFile = newConfig.KeyFile
+}
+
+// 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 err != nil {
 		return nil, fmt.Errorf("Failed to load cert/key pair: %v", err)
 	}
-	return &cert, err
+
+	c.Certificate = &cert
+	return c.Certificate, nil
 }
 
 // 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,17 +142,20 @@ 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.getOutgoingCertificate
 	}
 
 	return tlsConfig, nil
 }
 
+func (c *Config) getOutgoingCertificate(*tls.ClientHelloInfo) (*tls.Certificate, error) {
+	return c.Certificate, nil
+}
+
 // 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.
@@ -236,11 +256,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.getOutgoingCertificate
 	}
 
 	// Check if we require verification

helper/tlsutil/config_test.go

@@ -8,7 +8,9 @@ import (
 	"net"
 	"testing"
 
+	c "github.com/hashicorp/nomad/nomad/structs/config"
 	"github.com/hashicorp/yamux"
+	"github.com/stretchr/testify/assert"
 )
 
 const (
@@ -46,9 +48,9 @@ func TestConfig_CACertificate_Valid(t *testing.T) {
 	}
 }
 
-func TestConfig_KeyPair_None(t *testing.T) {
+func TestConfig_LoadKeyPair_None(t *testing.T) {
 	conf := &Config{}
-	cert, err := conf.KeyPair()
+	cert, err := conf.LoadKeyPair()
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
@@ -57,12 +59,12 @@ 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,
 	}
-	cert, err := conf.KeyPair()
+	cert, err := conf.LoadKeyPair()
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
@@ -144,20 +146,27 @@ func TestConfig_OutgoingTLS_WithKeyPair(t *testing.T) {
 		CertFile:       foocert,
 		KeyFile:        fookey,
 	}
-	tls, err := conf.OutgoingTLSConfig()
+	tlsConf, err := conf.OutgoingTLSConfig()
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
-	if tls == nil {
+	if tlsConf == nil {
 		t.Fatalf("expected config")
 	}
-	if len(tls.RootCAs.Subjects()) != 1 {
+	if len(tlsConf.RootCAs.Subjects()) != 1 {
 		t.Fatalf("expect root cert")
 	}
-	if !tls.InsecureSkipVerify {
+	if !tlsConf.InsecureSkipVerify {
 		t.Fatalf("should skip verification")
 	}
-	if len(tls.Certificates) != 1 {
+
+	clientHelloInfo := &tls.ClientHelloInfo{}
+	cert, err := tlsConf.GetCertificate(clientHelloInfo)
+	// TODO add asert package
+	if err != nil {
+		t.Fatalf("expected no error")
+	}
+	if cert == nil {
 		t.Fatalf("expected client cert")
 	}
 }
@@ -335,6 +344,8 @@ func TestConfig_wrapTLS_BadCert(t *testing.T) {
 }
 
 func TestConfig_IncomingTLS(t *testing.T) {
+	assert := assert.New(t)
+
 	conf := &Config{
 		VerifyIncoming: true,
 		CAFile:         cacert,
@@ -354,9 +365,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) {
@@ -401,3 +414,41 @@ func TestConfig_IncomingTLS_NoVerify(t *testing.T) {
 		t.Fatalf("unexpected client cert")
 	}
 }
+
+func TestUpdate_NoData(t *testing.T) {
+	assert := assert.New(t)
+
+	conf := &Config{
+		VerifyIncoming: true,
+		CertFile:       foocert,
+		KeyFile:        fookey,
+	}
+
+	newConf := &c.TLSConfig{
+		CertFile: "",
+		KeyFile:  "",
+	}
+
+	conf.Update(newConf)
+	assert.Equal(conf.CertFile, "")
+	assert.Equal(conf.KeyFile, "")
+}
+
+func TestUpdate(t *testing.T) {
+	assert := assert.New(t)
+
+	conf := &Config{
+		VerifyIncoming: true,
+		CertFile:       foocert,
+		KeyFile:        fookey,
+	}
+
+	newConf := &c.TLSConfig{
+		CertFile: "path_to_certfile",
+		KeyFile:  "path_to_keyfile",
+	}
+
+	conf.Update(newConf)
+	assert.Equal(conf.CertFile, "path_to_certfile")
+	assert.Equal(conf.KeyFile, "path_to_keyfile")
+}

nomad/config.go

@@ -6,6 +6,7 @@ import (
 	"net"
 	"os"
 	"runtime"
+	"sync"
 	"time"
 
 	"github.com/hashicorp/memberlist"
@@ -57,6 +58,8 @@ type Config struct {
 	// must be handled via `atomic.*Int32()` calls.
 	BootstrapExpect int32
 
+	configLock sync.RWMutex
+
 	// DataDir is the directory to store our state in
 	DataDir string
 
@@ -229,6 +232,9 @@ type Config struct {
 	// TLSConfig holds various TLS related configurations
 	TLSConfig *config.TLSConfig
 
+	// tlsConfigHelper provides utility functions and a pointer to the TLS config
+	tlsConfigHelper *tlsutil.Config
+
 	// ACLEnabled controls if ACL enforcement and management is enabled.
 	ACLEnabled bool
 
@@ -259,6 +265,27 @@ func (c *Config) CheckVersion() error {
 	return nil
 }
 
+func (c *Config) SetTLSConfig(newTLSConfig *config.TLSConfig) error {
+	if newTLSConfig == nil {
+		return fmt.Errorf("no new tls configuration to reload")
+	}
+
+	c.configLock.Lock()
+	c.TLSConfig.Merge(newTLSConfig)
+	c.configLock.Unlock()
+
+	if c.tlsConfigHelper == nil {
+		return nil
+	}
+
+	// TODO can the TLSConfigHelper just have a TLSConfigCopy rather than copying
+	// fields?
+	c.tlsConfigHelper.Update(c.TLSConfig)
+	_, err := c.tlsConfigHelper.LoadKeyPair()
+
+	return err
+}
+
 // DefaultConfig returns the default configuration
 func DefaultConfig() *Config {
 	hostname, err := os.Hostname()
@@ -335,7 +362,7 @@ func DefaultConfig() *Config {
 
 // tlsConfig returns a TLSUtil Config based on the server configuration
 func (c *Config) tlsConfig() *tlsutil.Config {
-	tlsConf := &tlsutil.Config{
+	c.tlsConfigHelper = &tlsutil.Config{
 		VerifyIncoming:       true,
 		VerifyOutgoing:       true,
 		VerifyServerHostname: c.TLSConfig.VerifyServerHostname,
@@ -343,5 +370,5 @@ func (c *Config) tlsConfig() *tlsutil.Config {
 		CertFile:             c.TLSConfig.CertFile,
 		KeyFile:              c.TLSConfig.KeyFile,
 	}
-	return tlsConf
+	return c.tlsConfigHelper
 }

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"

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"
 )
 
@@ -512,6 +512,12 @@ func (s *Server) Reload(config *Config) error {
 		}
 	}
 
+	if s.config != nil && config.TLSConfig != nil {
+		if err := s.config.SetTLSConfig(config.TLSConfig); err != nil {
+			multierror.Append(&mErr, err)
+		}
+	}
+
 	return mErr.ErrorOrNil()
 }
 

nomad/server_test.go

@@ -13,12 +13,15 @@ import (
 	"time"
 
 	"github.com/hashicorp/consul/lib/freeport"
+	msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc"
 	"github.com/hashicorp/nomad/command/agent/consul"
+	"github.com/hashicorp/nomad/helper/tlsutil"
 	"github.com/hashicorp/nomad/helper/uuid"
 	"github.com/hashicorp/nomad/nomad/mock"
 	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/hashicorp/nomad/nomad/structs/config"
 	"github.com/hashicorp/nomad/testutil"
+	"github.com/stretchr/testify/assert"
 )
 
 var (
@@ -276,3 +279,50 @@ func TestServer_Reload_Vault(t *testing.T) {
 		t.Fatalf("Vault client should be running")
 	}
 }
+
+func TestServer_Reload_TLS(t *testing.T) {
+	t.Parallel()
+	assert := assert.New(t)
+
+	const (
+		cafile  = "../helper/tlsutil/testdata/ca.pem"
+		foocert = "../helper/tlsutil/testdata/nomad-foo.pem"
+		fookey  = "../helper/tlsutil/testdata/nomad-foo-key.pem"
+	)
+	dir := tmpDir(t)
+	defer os.RemoveAll(dir)
+	s1 := testServer(t, func(c *Config) {
+		c.DataDir = path.Join(dir, "nodeA")
+	})
+	defer s1.Shutdown()
+
+	codec := rpcClient(t, s1)
+
+	// assert that the server started in plaintext mode
+	assert.Equal(s1.config.TLSConfig.CertFile, "")
+
+	newTLSConfig := &config.TLSConfig{
+		EnableHTTP:           true,
+		EnableRPC:            true,
+		VerifyServerHostname: true,
+		CAFile:               cafile,
+		CertFile:             foocert,
+		KeyFile:              fookey,
+	}
+
+	config := s1.config
+	config.tlsConfigHelper = &tlsutil.Config{}
+
+	config.TLSConfig = newTLSConfig
+
+	err := s1.Reload(config)
+	assert.Nil(err)
+
+	// assert our server is now configured with the correct TLS configuration
+	assert.Equal(s1.config.TLSConfig.CertFile, foocert)
+
+	arg := struct{}{}
+	var out struct{}
+	err = msgpackrpc.CallWithCodec(codec, "Status.Ping", arg, &out)
+	assert.NotNil(err)
+}