From 5e85b5a090ec1d4f5d5b4c93a4faf9bae9dbb9cd Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Wed, 25 Oct 2017 13:59:08 -0400 Subject: [PATCH] add rpc_upgrade_mode as config option for tls upgrades --- command/agent/config-test-fixtures/basic.hcl | 1 + command/agent/config_parse.go | 1 + command/agent/config_parse_test.go | 1 + nomad/rpc.go | 8 +- nomad/rpc_test.go | 88 ++++++++++++++++++++ nomad/structs/config/tls.go | 5 ++ 6 files changed, 101 insertions(+), 3 deletions(-) diff --git a/command/agent/config-test-fixtures/basic.hcl b/command/agent/config-test-fixtures/basic.hcl index 188f085be..a96a1e431 100644 --- a/command/agent/config-test-fixtures/basic.hcl +++ b/command/agent/config-test-fixtures/basic.hcl @@ -152,6 +152,7 @@ tls { ca_file = "foo" cert_file = "bar" key_file = "pipe" + rpc_upgrade_mode = true verify_https_client = true } sentinel { diff --git a/command/agent/config_parse.go b/command/agent/config_parse.go index 25b25f8a2..3f42b2ae5 100644 --- a/command/agent/config_parse.go +++ b/command/agent/config_parse.go @@ -771,6 +771,7 @@ func parseTLSConfig(result **config.TLSConfig, list *ast.ObjectList) error { "http", "rpc", "verify_server_hostname", + "rpc_upgrade_mode", "ca_file", "cert_file", "key_file", diff --git a/command/agent/config_parse_test.go b/command/agent/config_parse_test.go index 15ad5dc16..1a824f93a 100644 --- a/command/agent/config_parse_test.go +++ b/command/agent/config_parse_test.go @@ -172,6 +172,7 @@ func TestConfig_Parse(t *testing.T) { CAFile: "foo", CertFile: "bar", KeyFile: "pipe", + RPCUpgradeMode: true, VerifyHTTPSClient: true, }, HTTPAPIResponseHeaders: map[string]string{ diff --git a/nomad/rpc.go b/nomad/rpc.go index 45efd57cb..47d22054d 100644 --- a/nomad/rpc.go +++ b/nomad/rpc.go @@ -100,9 +100,11 @@ func (s *Server) handleConn(conn net.Conn, isTLS bool) { // Enforce TLS if EnableRPC is set if s.config.TLSConfig.EnableRPC && !isTLS && RPCType(buf[0]) != rpcTLS { - s.logger.Printf("[WARN] nomad.rpc: Non-TLS connection attempted with RequireTLS set") - conn.Close() - return + if !s.config.TLSConfig.RPCUpgradeMode { + s.logger.Printf("[WARN] nomad.rpc: Non-TLS connection attempted with RequireTLS set") + conn.Close() + return + } } // Switch on the byte diff --git a/nomad/rpc_test.go b/nomad/rpc_test.go index d8890f34e..cc15a8aa3 100644 --- a/nomad/rpc_test.go +++ b/nomad/rpc_test.go @@ -3,10 +3,17 @@ package nomad import ( "net" "net/rpc" + "os" + "path" "testing" "time" + "github.com/hashicorp/net-rpc-msgpackrpc" + "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" ) // rpcClient is a test helper method to return a ClientCodec to use to make rpc @@ -84,3 +91,84 @@ func TestRPC_forwardRegion(t *testing.T) { t.Fatalf("err: %v", err) } } + +func TestRPC_PlaintextRPCSucceedsWhenInUpgradeMode(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, "node1") + c.TLSConfig = &config.TLSConfig{ + EnableRPC: true, + VerifyServerHostname: true, + CAFile: cafile, + CertFile: foocert, + KeyFile: fookey, + RPCUpgradeMode: true, + } + }) + defer s1.Shutdown() + + codec := rpcClient(t, s1) + + // Create the register request + node := mock.Node() + req := &structs.NodeRegisterRequest{ + Node: node, + WriteRequest: structs.WriteRequest{Region: "global"}, + } + + var resp structs.GenericResponse + err := msgpackrpc.CallWithCodec(codec, "Node.Register", req, &resp) + assert.Nil(err) + + // Check that heartbeatTimers has the heartbeat ID + _, ok := s1.heartbeatTimers[node.ID] + assert.True(ok) +} + +func TestRPC_PlaintextRPCFailsWhenNotInUpgradeMode(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, "node1") + c.TLSConfig = &config.TLSConfig{ + EnableRPC: true, + VerifyServerHostname: true, + CAFile: cafile, + CertFile: foocert, + KeyFile: fookey, + } + }) + defer s1.Shutdown() + + codec := rpcClient(t, s1) + + node := mock.Node() + req := &structs.NodeRegisterRequest{ + Node: node, + WriteRequest: structs.WriteRequest{Region: "global"}, + } + + var resp structs.GenericResponse + err := msgpackrpc.CallWithCodec(codec, "Node.Register", req, &resp) + assert.NotNil(err) + assert.Contains(err.Error(), "connection reset by peer") +} diff --git a/nomad/structs/config/tls.go b/nomad/structs/config/tls.go index 2baa76a07..1a2f40c35 100644 --- a/nomad/structs/config/tls.go +++ b/nomad/structs/config/tls.go @@ -29,6 +29,11 @@ type TLSConfig struct { // Must be provided to serve TLS connections. KeyFile string `mapstructure:"key_file"` + // RPCUpgradeMode should be enabled when a cluster ie being upgraded + // to TLS. Allows servers to accept both plaintext and TLS connections and + // should only be a temporary state. + RPCUpgradeMode bool `mapstructure:"rpc_upgrade_mode"` + // Verify connections to the HTTPS API VerifyHTTPSClient bool `mapstructure:"verify_https_client"` }