From 070e632c549f54dac86ce37e66544b990efc8faa Mon Sep 17 00:00:00 2001
From: Chelsea Holland Komlo <me@chelseakomlo.com>
Date: Thu, 2 Nov 2017 10:36:06 -0400
Subject: [PATCH] allow clients to reload TLS configuration on the fly

---
 client/client.go         | 20 +++++++++++++++
 client/client_test.go    | 53 ++++++++++++++++++++++++++++++++++++++++
 command/agent/agent.go   | 10 +++++++-
 command/agent/command.go |  5 ++--
 4 files changed, 85 insertions(+), 3 deletions(-)

diff --git a/client/client.go b/client/client.go
index 34869bc5268f..99ab63414130 100644
--- a/client/client.go
+++ b/client/client.go
@@ -302,6 +302,26 @@ func NewClient(cfg *config.Config, consulCatalog consul.CatalogAPI, consulServic
 	return c, nil
 }
 
+// ReloadTLSConnectoins allows a client to reload RPC connections if the
+// client's TLS configuration changes from plaintext to TLS
+func (c *Client) ReloadTLSConnections() error {
+	c.configLock.Lock()
+	defer c.configLock.Unlock()
+
+	if c.config.TLSConfig.EnableRPC {
+		tw, err := c.config.TLSConfiguration().OutgoingTLSWrapper()
+		if err != nil {
+			return err
+		}
+		c.connPool.ReloadTLS(tw)
+	} else {
+		// enable downgrades
+		c.connPool.ReloadTLS(nil)
+	}
+
+	return nil
+}
+
 // init is used to initialize the client and perform any setup
 // needed before we begin starting its various components.
 func (c *Client) init() error {
diff --git a/client/client_test.go b/client/client_test.go
index 3492557f7f9f..d9f771e8baba 100644
--- a/client/client_test.go
+++ b/client/client_test.go
@@ -2,6 +2,7 @@ package client
 
 import (
 	"fmt"
+	"io"
 	"io/ioutil"
 	"log"
 	"math/rand"
@@ -460,6 +461,58 @@ func TestClient_MixedTLS(t *testing.T) {
 	)
 }
 
+func TestClient_ReloadTLS(t *testing.T) {
+	t.Parallel()
+	t.Skip("depends on closing non-tls channels on ReloadTLSConnections")
+	assert := assert.New(t)
+
+	s1, addr := testServer(t, func(c *nomad.Config) {
+		c.Region = "dc1"
+	})
+	defer s1.Shutdown()
+	testutil.WaitForLeader(t, s1.RPC)
+
+	const (
+		cafile  = "../helper/tlsutil/testdata/ca.pem"
+		foocert = "../helper/tlsutil/testdata/nomad-foo.pem"
+		fookey  = "../helper/tlsutil/testdata/nomad-foo-key.pem"
+	)
+
+	c1 := testClient(t, func(c *config.Config) {
+		c.Servers = []string{addr}
+		c.TLSConfig = &nconfig.TLSConfig{
+			EnableHTTP:           true,
+			EnableRPC:            true,
+			VerifyServerHostname: true,
+			CAFile:               cafile,
+			CertFile:             foocert,
+			KeyFile:              fookey,
+		}
+	})
+	defer c1.Shutdown()
+
+	err := c1.ReloadTLSConnections()
+	assert.Nil(err)
+
+	req := structs.NodeSpecificRequest{
+		NodeID:       c1.Node().ID,
+		QueryOptions: structs.QueryOptions{Region: "dc1"},
+	}
+	var out structs.SingleNodeResponse
+	testutil.AssertUntil(100*time.Millisecond,
+		func() (bool, error) {
+			err := c1.RPC("Node.GetNode", &req, &out)
+			if err != io.EOF {
+				return false, fmt.Errorf("client RPC succeeded when it should have failed:\n%+v", out)
+			}
+			return true, nil
+		},
+		func(err error) {
+			t.Fatalf(err.Error())
+		},
+	)
+}
+
 // TestClient_BadTLS asserts that when a client and server are running with TLS
 // enabled -- but their certificates are signed by different CAs -- they're
 // unable to communicate.
diff --git a/command/agent/agent.go b/command/agent/agent.go
index ef171245aade..735c115aed17 100644
--- a/command/agent/agent.go
+++ b/command/agent/agent.go
@@ -733,6 +733,7 @@ func (a *Agent) Reload(newConfig *Config) error {
 
 	if !a.config.TLSConfig.IsEmpty() && !newConfig.TLSConfig.IsEmpty() {
 		a.logger.Println("[INFO] Updating agent's existing TLS configuration \n\n")
+
 		// Handle errors in loading the new certificate files.
 		// This is just a TLS configuration reload, we don't need to refresh
 		// existing network connections
@@ -741,6 +742,7 @@ func (a *Agent) Reload(newConfig *Config) error {
 
 	if a.config.TLSConfig.IsEmpty() && !newConfig.TLSConfig.IsEmpty() {
 		a.logger.Println("[INFO] Moving from plaintext configuration to TLS \n\n")
+
 		// compeltely reload the agent's TLS configuration. This means the agent
 		// is moving from plaintext to TLS connections.
 		// This does not handle errors in loading the new TLS configuration
@@ -756,7 +758,13 @@ func (a *Agent) Reload(newConfig *Config) error {
 	if s := a.Server(); s != nil {
 		err := s.ReloadTLSConnections()
 		if err != nil {
-			a.logger.Printf("[WARN] agent: Issue reloading the server's TLS Configuration, consider a full system restart: %v", err.Error())
+			a.logger.Printf("[ERR] agent: Issue reloading the server's TLS Configuration, consider a full system restart: %v", err.Error())
+			return err
+		}
+	} else if c := a.Client(); c != nil {
+		err := c.ReloadTLSConnections()
+		if err != nil {
+			a.logger.Printf("[ERR] agent: Issue reloading the client's TLS Configuration, consider a full system restart: %v", err.Error())
 			return err
 		}
 	}
diff --git a/command/agent/command.go b/command/agent/command.go
index ee121eb2ba49..650aad8e2114 100644
--- a/command/agent/command.go
+++ b/command/agent/command.go
@@ -15,6 +15,9 @@ import (
 	"syscall"
 	"time"
 
+	flaghelper "github.com/hashicorp/nomad/helper/flag-helpers"
+	gatedwriter "github.com/hashicorp/nomad/helper/gated-writer"
+
 	metrics "github.com/armon/go-metrics"
 	"github.com/armon/go-metrics/circonus"
 	"github.com/armon/go-metrics/datadog"
@@ -23,8 +26,6 @@ import (
 	checkpoint "github.com/hashicorp/go-checkpoint"
 	gsyslog "github.com/hashicorp/go-syslog"
 	"github.com/hashicorp/logutils"
-	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"