WIP: 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,13 +362,13 @@ 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,
 		CAFile:               c.TLSConfig.CAFile,
 		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,15 +19,15 @@ 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"
 	"github.com/hashicorp/nomad/nomad/deploymentwatcher"
 	"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()
 }