Skip to content

Commit

Permalink
Allow server TLS configuration to be reloaded via SIGHUP
Browse files Browse the repository at this point in the history
  • Loading branch information
chelseakomlo committed Oct 31, 2017
1 parent 2d77197 commit d37050a
Show file tree
Hide file tree
Showing 6 changed files with 181 additions and 27 deletions.
38 changes: 29 additions & 9 deletions helper/tlsutil/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
75 changes: 63 additions & 12 deletions helper/tlsutil/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand Down Expand Up @@ -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")
}
}
Expand Down Expand Up @@ -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,
Expand All @@ -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) {
Expand Down Expand Up @@ -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")
}
31 changes: 29 additions & 2 deletions nomad/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"net"
"os"
"runtime"
"sync"
"time"

"github.com/hashicorp/memberlist"
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
}
4 changes: 2 additions & 2 deletions nomad/rpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
10 changes: 8 additions & 2 deletions nomad/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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()
}

Expand Down
Loading

0 comments on commit d37050a

Please sign in to comment.