Add TLS certificate and worker tag reloading on SIGHUP (ICU-1142) (ICU-1143)

CHANGELOG.md

@@ -2,7 +2,7 @@
 
 Canonical reference for changes, improvements, and bugfixes for Boundary.
 
-## Next
+## Next (Unreleased)
 
 ### Changes/Deprecations
 
@@ -22,6 +22,16 @@ All of these changes are from [PR
 * cli: All errors, including API errors, are now written to `stderr`. Previously
   in the default table format, API errors would be written to `stdout`.
 
+### New and Improved
+
+* server: Officially support reloading TLS parameters on `SIGHUP`. (This likely
+  worked before but wasn't fully tested.)
+  ([PR](https://github.com/hashicorp/boundary/pull/959))
+* server: On `SIGHUP`, [worker
+  tags](https://www.boundaryproject.io/docs/configuration/worker#tags) will be
+  re-parsed and new values used
+  ([PR](https://github.com/hashicorp/boundary/pull/959))
+
 ## 0.1.7 (2021/02/16)
 
 *Note* This release fixes an upgrade issue affecting users on Postgres 11

internal/cmd/base/listener.go

@@ -56,10 +56,10 @@ func NewListener(l *configutil.Listener, logger hclog.Logger, ui cli.Ui) (*alpnm
 		return nil, nil, nil, fmt.Errorf("unknown listener type: %q", l.Type)
 	}
 
-	var purpose string
-	if len(l.Purpose) == 1 {
-		purpose = l.Purpose[0]
+	if len(l.Purpose) != 1 {
+		return nil, nil, nil, fmt.Errorf("Expected single listener purpose, found %d", len(l.Purpose))
 	}
+	purpose := l.Purpose[0]
 
 	switch purpose {
 	case "cluster":

internal/cmd/base/servers.go

@@ -9,10 +9,12 @@ import (
 	"io"
 	"net"
 	"os"
+	"os/signal"
 	"sort"
 	"strconv"
 	"strings"
 	"sync"
+	"syscall"
 
 	"github.com/armon/go-metrics"
 	"github.com/hashicorp/boundary/globals"
@@ -269,25 +271,26 @@ func (b *Server) SetupListeners(ui cli.Ui, config *configutil.SharedConfig, allo
 	defer b.ReloadFuncsLock.Unlock()
 
 	for i, lnConfig := range config.Listeners {
-		var purpose string
-		for _, p := range lnConfig.Purpose {
-			purpose = strings.ToLower(p)
-			if !strutil.StrListContains(allowedPurposes, purpose) {
-				return fmt.Errorf("Unknown listener purpose %q", purpose)
-			}
+		if len(lnConfig.Purpose) != 1 {
+			return fmt.Errorf("Invalid size of listener purposes (%d)", len(lnConfig.Purpose))
 		}
-
-		// Override for now
-		// TODO: Way to configure
-		lnConfig.TLSCipherSuites = []uint16{
-			// 1.3
-			tls.TLS_AES_128_GCM_SHA256,
-			tls.TLS_AES_256_GCM_SHA384,
-			tls.TLS_CHACHA20_POLY1305_SHA256,
-			// 1.2
-			tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
-			tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
-			tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305,
+		purpose := strings.ToLower(lnConfig.Purpose[0])
+		if !strutil.StrListContains(allowedPurposes, purpose) {
+			return fmt.Errorf("Unknown listener purpose %q", purpose)
+		}
+		lnConfig.Purpose[0] = purpose
+
+		if lnConfig.TLSCipherSuites == nil {
+			lnConfig.TLSCipherSuites = []uint16{
+				// 1.3
+				tls.TLS_AES_128_GCM_SHA256,
+				tls.TLS_AES_256_GCM_SHA384,
+				tls.TLS_CHACHA20_POLY1305_SHA256,
+				// 1.2
+				tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
+				tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
+				tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305,
+			}
 		}
 
 		lnMux, props, reloadFunc, err := NewListener(lnConfig, b.Logger, ui)
@@ -320,9 +323,9 @@ func (b *Server) SetupListeners(ui cli.Ui, config *configutil.SharedConfig, allo
 		}
 
 		if reloadFunc != nil {
-			relSlice := b.ReloadFuncs["listener|"+lnConfig.Type]
+			relSlice := b.ReloadFuncs["listeners"]
 			relSlice = append(relSlice, reloadFunc)
-			b.ReloadFuncs["listener|"+lnConfig.Type] = relSlice
+			b.ReloadFuncs["listeners"] = relSlice
 		}
 
 		if lnConfig.MaxRequestSize == 0 {
@@ -708,3 +711,20 @@ func (b *Server) SetupWorkerPublicAddress(conf *config.Config, flagValue string)
 	conf.Worker.PublicAddr = net.JoinHostPort(host, port)
 	return nil
 }
+
+// MakeSighupCh returns a channel that can be used for SIGHUP
+// reloading. This channel will send a message for every
+// SIGHUP received.
+func MakeSighupCh() chan struct{} {
+	resultCh := make(chan struct{})
+
+	signalCh := make(chan os.Signal, 4)
+	signal.Notify(signalCh, syscall.SIGHUP)
+	go func() {
+		for {
+			<-signalCh
+			resultCh <- struct{}{}
+		}
+	}()
+	return resultCh
+}

internal/cmd/commands.go

@@ -1,10 +1,6 @@
 package cmd
 
 import (
-	"os"
-	"os/signal"
-	"syscall"
-
 	"github.com/hashicorp/boundary/internal/cmd/base"
 	"github.com/hashicorp/boundary/internal/cmd/commands/accountscmd"
 	"github.com/hashicorp/boundary/internal/cmd/commands/authenticate"
@@ -37,14 +33,14 @@ func initCommands(ui, serverCmdUi cli.Ui, runOpts *RunOptions) {
 		"server": func() (cli.Command, error) {
 			return &server.Command{
 				Server:    base.NewServer(base.NewCommand(serverCmdUi)),
-				SighupCh:  MakeSighupCh(),
+				SighupCh:  base.MakeSighupCh(),
 				SigUSR2Ch: MakeSigUSR2Ch(),
 			}, nil
 		},
 		"dev": func() (cli.Command, error) {
 			return &dev.Command{
 				Server:    base.NewServer(base.NewCommand(serverCmdUi)),
-				SighupCh:  MakeSighupCh(),
+				SighupCh:  base.MakeSighupCh(),
 				SigUSR2Ch: MakeSigUSR2Ch(),
 			}, nil
 		},
@@ -767,20 +763,3 @@ func initCommands(ui, serverCmdUi cli.Ui, runOpts *RunOptions) {
 		},
 	}
 }
-
-// MakeSighupCh returns a channel that can be used for SIGHUP
-// reloading. This channel will send a message for every
-// SIGHUP received.
-func MakeSighupCh() chan struct{} {
-	resultCh := make(chan struct{})
-
-	signalCh := make(chan os.Signal, 4)
-	signal.Notify(signalCh, syscall.SIGHUP)
-	go func() {
-		for {
-			<-signalCh
-			resultCh <- struct{}{}
-		}
-	}()
-	return resultCh
-}

internal/cmd/commands/server/listener_reload_test.go

@@ -0,0 +1,163 @@
+// +build !hsm
+
+// NOTE on the NOTE: This is from Vault, but that doesn't mean it's not valid
+// going forward with us.
+//
+// NOTE: we can't use this with HSM. We can't set testing mode on and it's not
+// safe to use env vars since that provides an attack vector in the real world.
+package server
+
+import (
+	"crypto/tls"
+	"crypto/x509"
+	"fmt"
+	"io/ioutil"
+	"os"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/hashicorp/boundary/internal/cmd/config"
+	"github.com/mitchellh/cli"
+	"github.com/stretchr/testify/require"
+)
+
+const reloadConfig = `
+disable_mlock = true
+
+telemetry {
+	prometheus_retention_time = "24h"
+	disable_hostname = true
+}
+
+controller {
+	name = "dev-controller"
+	description = "A default controller created in dev mode"
+	database {
+		url = "%s"
+	}
+}
+
+kms "aead" {
+	purpose = "root"
+	aead_type = "aes-gcm"
+	key = "%s"
+	key_id = "global_root"
+}
+
+kms "aead" {
+	purpose = "worker-auth"
+	aead_type = "aes-gcm"
+	key = "%s"
+	key_id = "global_worker-auth"
+}
+
+kms "aead" {
+	purpose = "recovery"
+	aead_type = "aes-gcm"
+	key = "%s"
+	key_id = "global_recovery"
+}
+
+listener "tcp" {
+	purpose = "api"
+	tls_cert_file = "%s/bundle.pem"
+	tls_key_file = "%s/bundle.pem"
+	cors_enabled = true
+	cors_allowed_origins = ["*"]
+}
+
+listener "tcp" {
+	purpose = "cluster"
+}
+
+listener "tcp" {
+	purpose = "proxy"
+}
+`
+
+func TestServer_ReloadListener(t *testing.T) {
+	t.Parallel()
+	require := require.New(t)
+	wg := &sync.WaitGroup{}
+
+	wd, _ := os.Getwd()
+	wd += "/test-fixtures/reload/"
+
+	td, err := ioutil.TempDir("", "boundary-test-")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(td)
+
+	controllerKey, workerAuthKey, recoveryKey := config.DevKeyGeneration()
+
+	cmd := testServerCommand(t, controllerKey)
+
+	defer func() {
+		if cmd.DevDatabaseCleanupFunc != nil {
+			require.NoError(cmd.DevDatabaseCleanupFunc())
+		}
+	}()
+	require.NoError(err)
+
+	// Setup initial certs
+	inBytes, err := ioutil.ReadFile(wd + "bundle1.pem")
+	require.NoError(err)
+	require.NoError(ioutil.WriteFile(td+"/bundle.pem", inBytes, 0777))
+
+	relHcl := fmt.Sprintf(reloadConfig, cmd.DatabaseUrl, controllerKey, workerAuthKey, recoveryKey, td, td)
+	require.NoError(ioutil.WriteFile(td+"/reload.hcl", []byte(relHcl), 0777))
+
+	// Populate CA pool
+	inBytes, _ = ioutil.ReadFile(td + "/bundle.pem")
+	certPool := x509.NewCertPool()
+	require.True(certPool.AppendCertsFromPEM(inBytes))
+
+	wg.Add(1)
+	args := []string{"-config", td + "/reload.hcl"}
+	go func() {
+		defer wg.Done()
+		if code := cmd.Run(args); code != 0 {
+			output := cmd.UI.(*cli.MockUi).ErrorWriter.String() + cmd.UI.(*cli.MockUi).OutputWriter.String()
+			t.Errorf("got a non-zero exit status: %s", output)
+		}
+	}()
+
+	testCertificateSerial := func(serial string) {
+		conn, err := tls.Dial("tcp", "127.0.0.1:9200", &tls.Config{
+			RootCAs: certPool,
+		})
+		require.NoError(err)
+		defer conn.Close()
+
+		require.NoError(conn.Handshake())
+		ser := conn.ConnectionState().PeerCertificates[0].SerialNumber.String()
+		require.Equal(ser, serial)
+	}
+
+	select {
+	case <-cmd.startedCh:
+	case <-time.After(15 * time.Second):
+		t.Fatalf("timeout")
+	}
+
+	testCertificateSerial("142541707881583626546634262782315760343015820827")
+
+	inBytes, err = ioutil.ReadFile(wd + "bundle2.pem")
+	require.NoError(err)
+	require.NoError(ioutil.WriteFile(td+"/bundle.pem", inBytes, 0777))
+
+	cmd.SighupCh <- struct{}{}
+	select {
+	case <-cmd.reloadedCh:
+	case <-time.After(5 * time.Second):
+		t.Fatalf("timeout")
+	}
+
+	testCertificateSerial("193080739105342897219784862820114567438786419504")
+
+	close(cmd.ShutdownCh)
+
+	wg.Wait()
+}