agent: sensible keyring error (#7272)

Fixes #7231. Before an agent would always emit a warning when there is
an encrypt key in the configuration and an existing keyring stored,
which is happening on restart.

Now it only emits that warning when the encrypt key from the
configuration is not part of the keyring.

agent/agent.go

@@ -1585,6 +1585,7 @@ func (a *Agent) setupBaseKeyrings(config *consul.Config) error {
 	fileLAN := filepath.Join(a.config.DataDir, SerfLANKeyring)
 	fileWAN := filepath.Join(a.config.DataDir, SerfWANKeyring)
 
+	var existingLANKeyring, existingWANKeyring bool
 	if a.config.EncryptKey == "" {
 		goto LOAD
 	}
@@ -1592,12 +1593,16 @@ func (a *Agent) setupBaseKeyrings(config *consul.Config) error {
 		if err := initKeyring(fileLAN, a.config.EncryptKey); err != nil {
 			return err
 		}
+	} else {
+		existingLANKeyring = true
 	}
 	if a.config.ServerMode && federationEnabled {
 		if _, err := os.Stat(fileWAN); err != nil {
 			if err := initKeyring(fileWAN, a.config.EncryptKey); err != nil {
 				return err
 			}
+		} else {
+			existingWANKeyring = true
 		}
 	}
 
@@ -1617,6 +1622,26 @@ LOAD:
 		}
 	}
 
+	// Only perform the following checks if there was an encrypt_key
+	// provided in the configuration.
+	if a.config.EncryptKey != "" {
+		msg := " keyring doesn't include key provided with -encrypt, using keyring"
+		if existingLANKeyring &&
+			keyringIsMissingKey(
+				config.SerfLANConfig.MemberlistConfig.Keyring,
+				a.config.EncryptKey,
+			) {
+			a.logger.Warn(msg, "keyring", "LAN")
+		}
+		if existingWANKeyring &&
+			keyringIsMissingKey(
+				config.SerfWANConfig.MemberlistConfig.Keyring,
+				a.config.EncryptKey,
+			) {
+			a.logger.Warn(msg, "keyring", "WAN")
+		}
+	}
+
 	return nil
 }
 

agent/config/builder.go

@@ -1106,16 +1106,6 @@ func (b *Builder) Validate(rt RuntimeConfig) error {
 		if _, err := decodeBytes(rt.EncryptKey); err != nil {
 			return fmt.Errorf("encrypt has invalid key: %s", err)
 		}
-		keyfileLAN := filepath.Join(rt.DataDir, SerfLANKeyring)
-		if _, err := os.Stat(keyfileLAN); err == nil {
-			b.warn("WARNING: LAN keyring exists but -encrypt given, using keyring")
-		}
-		if rt.ServerMode {
-			keyfileWAN := filepath.Join(rt.DataDir, SerfWANKeyring)
-			if _, err := os.Stat(keyfileWAN); err == nil {
-				b.warn("WARNING: WAN keyring exists but -encrypt given, using keyring")
-			}
-		}
 	}
 
 	// Check the data dir for signs of an un-migrated Consul 0.5.x or older

agent/config/runtime_test.go

@@ -2157,41 +2157,6 @@ func TestConfigFlagsAndEdgecases(t *testing.T) {
 			hcl:  []string{` encrypt = "this is not a valid key" `},
 			err:  "encrypt has invalid key: illegal base64 data at input byte 4",
 		},
-		{
-			desc: "encrypt given but LAN keyring exists",
-			args: []string{
-				`-data-dir=` + dataDir,
-			},
-			json: []string{`{ "encrypt": "pUqJrVyVRj5jsiYEkM/tFQYfWyJIv4s3XkvDwy7Cu5s=" }`},
-			hcl:  []string{` encrypt = "pUqJrVyVRj5jsiYEkM/tFQYfWyJIv4s3XkvDwy7Cu5s=" `},
-			patch: func(rt *RuntimeConfig) {
-				rt.EncryptKey = "pUqJrVyVRj5jsiYEkM/tFQYfWyJIv4s3XkvDwy7Cu5s="
-				rt.DataDir = dataDir
-			},
-			pre: func() {
-				writeFile(filepath.Join(dataDir, SerfLANKeyring), []byte("pUqJrVyVRj5jsiYEkM/tFQYfWyJIv4s3XkvDwy7Cu5s="))
-			},
-			warns: []string{`WARNING: LAN keyring exists but -encrypt given, using keyring`},
-		},
-		{
-			desc: "encrypt given but WAN keyring exists",
-			args: []string{
-				`-data-dir=` + dataDir,
-			},
-			json: []string{`{ "encrypt": "pUqJrVyVRj5jsiYEkM/tFQYfWyJIv4s3XkvDwy7Cu5s=", "server": true }`},
-			hcl:  []string{` encrypt = "pUqJrVyVRj5jsiYEkM/tFQYfWyJIv4s3XkvDwy7Cu5s=" server = true `},
-			patch: func(rt *RuntimeConfig) {
-				rt.EncryptKey = "pUqJrVyVRj5jsiYEkM/tFQYfWyJIv4s3XkvDwy7Cu5s="
-				rt.ServerMode = true
-				rt.LeaveOnTerm = false
-				rt.SkipLeaveOnInt = true
-				rt.DataDir = dataDir
-			},
-			pre: func() {
-				writeFile(filepath.Join(dataDir, SerfWANKeyring), []byte("pUqJrVyVRj5jsiYEkM/tFQYfWyJIv4s3XkvDwy7Cu5s="))
-			},
-			warns: []string{`WARNING: WAN keyring exists but -encrypt given, using keyring`},
-		},
 		{
 			desc: "multiple check files",
 			args: []string{

agent/keyring.go

@@ -1,6 +1,7 @@
 package agent
 
 import (
+	"bytes"
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
@@ -23,7 +24,7 @@ const (
 func initKeyring(path, key string) error {
 	var keys []string
 
-	if keyBytes, err := base64.StdEncoding.DecodeString(key); err != nil {
+	if keyBytes, err := decodeStringKey(key); err != nil {
 		return fmt.Errorf("Invalid key: %s", err)
 	} else if err := memberlist.ValidateKey(keyBytes); err != nil {
 		return fmt.Errorf("Invalid key: %s", err)
@@ -87,7 +88,7 @@ func loadKeyringFile(c *serf.Config) error {
 func loadKeyring(c *serf.Config, keys []string) error {
 	keysDecoded := make([][]byte, len(keys))
 	for i, key := range keys {
-		keyBytes, err := base64.StdEncoding.DecodeString(key)
+		keyBytes, err := decodeStringKey(key)
 		if err != nil {
 			return err
 		}
@@ -107,6 +108,10 @@ func loadKeyring(c *serf.Config, keys []string) error {
 	return nil
 }
 
+func decodeStringKey(key string) ([]byte, error) {
+	return base64.StdEncoding.DecodeString(key)
+}
+
 // keyringProcess is used to abstract away the semantic similarities in
 // performing various operations on the encryption keyring.
 func (a *Agent) keyringProcess(args *structs.KeyringRequest) (*structs.KeyringResponses, error) {
@@ -172,3 +177,18 @@ func parseKeyringRequest(req *structs.KeyringRequest, token string, relayFactor
 	req.Token = token
 	req.RelayFactor = relayFactor
 }
+
+// keyringIsMissingKey checks whether a key is part of a keyring. Returns true
+// if it is not included.
+func keyringIsMissingKey(keyring *memberlist.Keyring, key string) bool {
+	k1, err := decodeStringKey(key)
+	if err != nil {
+		return true
+	}
+	for _, k2 := range keyring.GetKeys() {
+		if bytes.Equal(k1, k2) {
+			return false
+		}
+	}
+	return true
+}

agent/keyring_test.go

@@ -334,3 +334,15 @@ func TestValidateLocalOnly(t *testing.T) {
 
 	require.Error(t, ValidateLocalOnly(true, false))
 }
+
+func TestAgent_KeyringIsMissingKey(t *testing.T) {
+	key1 := "tbLJg26ZJyJ9pK3qhc9jig=="
+	key2 := "4leC33rgtXKIVUr9Nr0snQ=="
+	decoded1, err := decodeStringKey(key1)
+	require.NoError(t, err)
+	keyring, err := memberlist.NewKeyring([][]byte{}, decoded1)
+	require.NoError(t, err)
+
+	require.True(t, keyringIsMissingKey(keyring, key2))
+	require.False(t, keyringIsMissingKey(keyring, key1))
+}