Prevents disabling gossip keyring file from disabling gossip encryption. (#3278)

agent/agent.go

@@ -794,10 +794,9 @@ func (a *Agent) consulConfig() (*consul.Config, error) {
 	// Setup the loggers
 	base.LogOutput = a.LogOutput
 
-	if !a.config.DisableKeyringFile {
+	// This will set up the LAN keyring, as well as the WAN for servers.
-		if err := a.setupKeyrings(base); err != nil {
+	if err := a.setupKeyrings(base); err != nil {
-			return nil, fmt.Errorf("Failed to configure keyring: %v", err)
+		return nil, fmt.Errorf("Failed to configure keyring: %v", err)
-		}
 	}
 
 	return base, nil
@@ -1026,6 +1025,26 @@ func (a *Agent) setupNodeID(config *Config) error {
 
 // setupKeyrings is used to initialize and load keyrings during agent startup
 func (a *Agent) setupKeyrings(config *consul.Config) error {
+	// If the keyring file is disabled then just poke the provided key
+	// into the in-memory keyring.
+	if a.config.DisableKeyringFile {
+		if a.config.EncryptKey == "" {
+			return nil
+		}
+
+		keys := []string{a.config.EncryptKey}
+		if err := loadKeyring(config.SerfLANConfig, keys); err != nil {
+			return err
+		}
+		if a.config.Server {
+			if err := loadKeyring(config.SerfWANConfig, keys); err != nil {
+				return err
+			}
+		}
+		return nil
+	}
+
+	// Otherwise, we need to deal with the keyring files.
 	fileLAN := filepath.Join(a.config.DataDir, SerfLANKeyring)
 	fileWAN := filepath.Join(a.config.DataDir, SerfWANKeyring)
 
@@ -1061,7 +1080,6 @@ LOAD:
 		}
 	}
 
-	// Success!
 	return nil
 }
 

agent/keyring.go

@@ -69,19 +69,22 @@ func loadKeyringFile(c *serf.Config) error {
 		return err
 	}
 
-	// Read in the keyring file data
 	keyringData, err := ioutil.ReadFile(c.KeyringFile)
 	if err != nil {
 		return err
 	}
 
-	// Decode keyring JSON
 	keys := make([]string, 0)
 	if err := json.Unmarshal(keyringData, &keys); err != nil {
 		return err
 	}
 
-	// Decode base64 values
+	return loadKeyring(c, keys)
+}
+
+// loadKeyring takes a list of base64-encoded strings and installs them in the
+// given Serf's keyring.
+func loadKeyring(c *serf.Config, keys []string) error {
 	keysDecoded := make([][]byte, len(keys))
 	for i, key := range keys {
 		keyBytes, err := base64.StdEncoding.DecodeString(key)
@@ -91,20 +94,16 @@ func loadKeyringFile(c *serf.Config) error {
 		keysDecoded[i] = keyBytes
 	}
 
-	// Guard against empty keyring
 	if len(keysDecoded) == 0 {
-		return fmt.Errorf("no keys present in keyring file: %s", c.KeyringFile)
+		return fmt.Errorf("no keys present in keyring: %s", c.KeyringFile)
 	}
 
-	// Create the keyring
 	keyring, err := memberlist.NewKeyring(keysDecoded, keysDecoded[0])
 	if err != nil {
 		return err
 	}
 
 	c.MemberlistConfig.Keyring = keyring
-
-	// Success!
 	return nil
 }
 

agent/keyring_test.go

@@ -1,6 +1,8 @@
 package agent
 
 import (
+	"bytes"
+	"encoding/base64"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -9,69 +11,225 @@ import (
 	"testing"
 
 	"github.com/hashicorp/consul/testutil"
+	"github.com/hashicorp/memberlist"
 )
 
+func checkForKey(key string, keyring *memberlist.Keyring) error {
+	rk, err := base64.StdEncoding.DecodeString(key)
+	if err != nil {
+		return err
+	}
+
+	pk := keyring.GetPrimaryKey()
+	if !bytes.Equal(rk, pk) {
+		return fmt.Errorf("got %q want %q", pk, rk)
+	}
+	return nil
+}
+
 func TestAgent_LoadKeyrings(t *testing.T) {
 	t.Parallel()
 	key := "tbLJg26ZJyJ9pK3qhc9jig=="
 
 	// Should be no configured keyring file by default
-	a1 := NewTestAgent(t.Name(), nil)
+	t.Run("no keys", func(t *testing.T) {
-	defer a1.Shutdown()
+		a1 := NewTestAgent(t.Name(), nil)
+		defer a1.Shutdown()
 
-	c1 := a1.Config.ConsulConfig
+		c1 := a1.Config.ConsulConfig
-	if c1.SerfLANConfig.KeyringFile != "" {
+		if c1.SerfLANConfig.KeyringFile != "" {
-		t.Fatalf("bad: %#v", c1.SerfLANConfig.KeyringFile)
+			t.Fatalf("bad: %#v", c1.SerfLANConfig.KeyringFile)
-	}
+		}
-	if c1.SerfLANConfig.MemberlistConfig.Keyring != nil {
+		if c1.SerfLANConfig.MemberlistConfig.Keyring != nil {
-		t.Fatalf("keyring should not be loaded")
+			t.Fatalf("keyring should not be loaded")
-	}
+		}
-	if c1.SerfWANConfig.KeyringFile != "" {
+		if c1.SerfWANConfig.KeyringFile != "" {
-		t.Fatalf("bad: %#v", c1.SerfLANConfig.KeyringFile)
+			t.Fatalf("bad: %#v", c1.SerfLANConfig.KeyringFile)
-	}
+		}
-	if c1.SerfWANConfig.MemberlistConfig.Keyring != nil {
+		if c1.SerfWANConfig.MemberlistConfig.Keyring != nil {
-		t.Fatalf("keyring should not be loaded")
+			t.Fatalf("keyring should not be loaded")
-	}
+		}
+	})
 
 	// Server should auto-load LAN and WAN keyring files
-	a2 := &TestAgent{Name: t.Name(), Key: key}
+	t.Run("server with keys", func(t *testing.T) {
-	a2.Start()
+		a2 := &TestAgent{Name: t.Name(), Key: key}
-	defer a2.Shutdown()
+		a2.Start()
+		defer a2.Shutdown()
 
-	c2 := a2.Config.ConsulConfig
+		c2 := a2.Config.ConsulConfig
-	if c2.SerfLANConfig.KeyringFile == "" {
+		if c2.SerfLANConfig.KeyringFile == "" {
-		t.Fatalf("should have keyring file")
+			t.Fatalf("should have keyring file")
-	}
+		}
-	if c2.SerfLANConfig.MemberlistConfig.Keyring == nil {
+		if c2.SerfLANConfig.MemberlistConfig.Keyring == nil {
-		t.Fatalf("keyring should be loaded")
+			t.Fatalf("keyring should be loaded")
-	}
+		}
-	if c2.SerfWANConfig.KeyringFile == "" {
+		if err := checkForKey(key, c2.SerfLANConfig.MemberlistConfig.Keyring); err != nil {
-		t.Fatalf("should have keyring file")
+			t.Fatalf("err: %v", err)
-	}
+		}
-	if c2.SerfWANConfig.MemberlistConfig.Keyring == nil {
+		if c2.SerfWANConfig.KeyringFile == "" {
-		t.Fatalf("keyring should be loaded")
+			t.Fatalf("should have keyring file")
-	}
+		}
+		if c2.SerfWANConfig.MemberlistConfig.Keyring == nil {
+			t.Fatalf("keyring should be loaded")
+		}
+		if err := checkForKey(key, c2.SerfWANConfig.MemberlistConfig.Keyring); err != nil {
+			t.Fatalf("err: %v", err)
+		}
+	})
 
 	// Client should auto-load only the LAN keyring file
-	cfg3 := TestConfig()
+	t.Run("client with keys", func(t *testing.T) {
-	cfg3.Server = false
+		cfg3 := TestConfig()
-	a3 := &TestAgent{Name: t.Name(), Config: cfg3, Key: key}
+		cfg3.Server = false
-	a3.Start()
+		a3 := &TestAgent{Name: t.Name(), Config: cfg3, Key: key}
-	defer a3.Shutdown()
+		a3.Start()
+		defer a3.Shutdown()
 
-	c3 := a3.Config.ConsulConfig
+		c3 := a3.Config.ConsulConfig
-	if c3.SerfLANConfig.KeyringFile == "" {
+		if c3.SerfLANConfig.KeyringFile == "" {
-		t.Fatalf("should have keyring file")
+			t.Fatalf("should have keyring file")
-	}
+		}
-	if c3.SerfLANConfig.MemberlistConfig.Keyring == nil {
+		if c3.SerfLANConfig.MemberlistConfig.Keyring == nil {
-		t.Fatalf("keyring should be loaded")
+			t.Fatalf("keyring should be loaded")
-	}
+		}
-	if c3.SerfWANConfig.KeyringFile != "" {
+		if err := checkForKey(key, c3.SerfLANConfig.MemberlistConfig.Keyring); err != nil {
-		t.Fatalf("bad: %#v", c3.SerfWANConfig.KeyringFile)
+			t.Fatalf("err: %v", err)
-	}
+		}
-	if c3.SerfWANConfig.MemberlistConfig.Keyring != nil {
+		if c3.SerfWANConfig.KeyringFile != "" {
-		t.Fatalf("keyring should not be loaded")
+			t.Fatalf("bad: %#v", c3.SerfWANConfig.KeyringFile)
-	}
+		}
+		if c3.SerfWANConfig.MemberlistConfig.Keyring != nil {
+			t.Fatalf("keyring should not be loaded")
+		}
+	})
+}
+
+func TestAgent_InmemKeyrings(t *testing.T) {
+	t.Parallel()
+	key := "tbLJg26ZJyJ9pK3qhc9jig=="
+
+	// Should be no configured keyring file by default
+	t.Run("no keys", func(t *testing.T) {
+		a1 := NewTestAgent(t.Name(), nil)
+		defer a1.Shutdown()
+
+		c1 := a1.Config.ConsulConfig
+		if c1.SerfLANConfig.KeyringFile != "" {
+			t.Fatalf("bad: %#v", c1.SerfLANConfig.KeyringFile)
+		}
+		if c1.SerfLANConfig.MemberlistConfig.Keyring != nil {
+			t.Fatalf("keyring should not be loaded")
+		}
+		if c1.SerfWANConfig.KeyringFile != "" {
+			t.Fatalf("bad: %#v", c1.SerfLANConfig.KeyringFile)
+		}
+		if c1.SerfWANConfig.MemberlistConfig.Keyring != nil {
+			t.Fatalf("keyring should not be loaded")
+		}
+	})
+
+	// Server should auto-load LAN and WAN keyring
+	t.Run("server with keys", func(t *testing.T) {
+		cfg2 := TestConfig()
+		cfg2.EncryptKey = key
+		cfg2.DisableKeyringFile = true
+
+		a2 := &TestAgent{Name: t.Name(), Config: cfg2}
+		a2.Start()
+		defer a2.Shutdown()
+
+		c2 := a2.Config.ConsulConfig
+		if c2.SerfLANConfig.KeyringFile != "" {
+			t.Fatalf("should not have keyring file")
+		}
+		if c2.SerfLANConfig.MemberlistConfig.Keyring == nil {
+			t.Fatalf("keyring should be loaded")
+		}
+		if err := checkForKey(key, c2.SerfLANConfig.MemberlistConfig.Keyring); err != nil {
+			t.Fatalf("err: %v", err)
+		}
+		if c2.SerfWANConfig.KeyringFile != "" {
+			t.Fatalf("should not have keyring file")
+		}
+		if c2.SerfWANConfig.MemberlistConfig.Keyring == nil {
+			t.Fatalf("keyring should be loaded")
+		}
+		if err := checkForKey(key, c2.SerfWANConfig.MemberlistConfig.Keyring); err != nil {
+			t.Fatalf("err: %v", err)
+		}
+	})
+
+	// Client should auto-load only the LAN keyring
+	t.Run("client with keys", func(t *testing.T) {
+		cfg3 := TestConfig()
+		cfg3.EncryptKey = key
+		cfg3.DisableKeyringFile = true
+		cfg3.Server = false
+		a3 := &TestAgent{Name: t.Name(), Config: cfg3}
+		a3.Start()
+		defer a3.Shutdown()
+
+		c3 := a3.Config.ConsulConfig
+		if c3.SerfLANConfig.KeyringFile != "" {
+			t.Fatalf("should not have keyring file")
+		}
+		if c3.SerfLANConfig.MemberlistConfig.Keyring == nil {
+			t.Fatalf("keyring should be loaded")
+		}
+		if err := checkForKey(key, c3.SerfLANConfig.MemberlistConfig.Keyring); err != nil {
+			t.Fatalf("err: %v", err)
+		}
+		if c3.SerfWANConfig.KeyringFile != "" {
+			t.Fatalf("bad: %#v", c3.SerfWANConfig.KeyringFile)
+		}
+		if c3.SerfWANConfig.MemberlistConfig.Keyring != nil {
+			t.Fatalf("keyring should not be loaded")
+		}
+	})
+
+	// Any keyring files should be ignored
+	t.Run("ignore files", func(t *testing.T) {
+		dir := testutil.TempDir(t, "consul")
+		defer os.RemoveAll(dir)
+
+		badKey := "unUzC2X3JgMKVJlZna5KVg=="
+		if err := initKeyring(filepath.Join(dir, SerfLANKeyring), badKey); err != nil {
+			t.Fatalf("err: %v", err)
+		}
+		if err := initKeyring(filepath.Join(dir, SerfWANKeyring), badKey); err != nil {
+			t.Fatalf("err: %v", err)
+		}
+
+		cfg4 := TestConfig()
+		cfg4.EncryptKey = key
+		cfg4.DisableKeyringFile = true
+		cfg4.DataDir = dir
+
+		a4 := &TestAgent{Name: t.Name(), Config: cfg4}
+		a4.Start()
+		defer a4.Shutdown()
+
+		c4 := a4.Config.ConsulConfig
+		if c4.SerfLANConfig.KeyringFile != "" {
+			t.Fatalf("should not have keyring file")
+		}
+		if c4.SerfLANConfig.MemberlistConfig.Keyring == nil {
+			t.Fatalf("keyring should be loaded")
+		}
+		if err := checkForKey(key, c4.SerfLANConfig.MemberlistConfig.Keyring); err != nil {
+			t.Fatalf("err: %v", err)
+		}
+		if c4.SerfWANConfig.KeyringFile != "" {
+			t.Fatalf("should not have keyring file")
+		}
+		if c4.SerfWANConfig.MemberlistConfig.Keyring == nil {
+			t.Fatalf("keyring should be loaded")
+		}
+		if err := checkForKey(key, c4.SerfWANConfig.MemberlistConfig.Keyring); err != nil {
+			t.Fatalf("err: %v", err)
+		}
+	})
 }
 
 func TestAgent_InitKeyring(t *testing.T) {