Fix 1.5 regression that meant non-string values in the seal stanza wo…

…uld fail config parsing, preventing startup. (#9555)
hashicorp · Jul 23, 2020 · 34144e3 · 34144e3
1 parent 5269a51
commit 34144e3
Show file tree

Hide file tree

Showing 4 changed files with 124 additions and 30 deletions.
diff --git a/command/server/config_test.go b/command/server/config_test.go
@@ -49,3 +49,7 @@ func TestParseEntropy(t *testing.T) {
 func TestConfigRaftRetryJoin(t *testing.T) {
 	testConfigRaftRetryJoin(t)
 }
+
+func TestParseSeals(t *testing.T) {
+	testParseSeals(t)
+}
diff --git a/command/server/config_test_helpers.go b/command/server/config_test_helpers.go
@@ -2,6 +2,7 @@ package server
 
 import (
 	"fmt"
+	"github.com/stretchr/testify/require"
 	"strings"
 	"testing"
 	"time"
@@ -711,3 +712,60 @@ listener "tcp" {
 		t.Fatal(diff)
 	}
 }
+
+func testParseSeals(t *testing.T) {
+	config, err := LoadConfigFile("./test-fixtures/config_seals.hcl")
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	config.Listeners[0].RawConfig = nil
+
+	expected := &Config{
+		Storage: &Storage{
+			Type:   "consul",
+			Config: map[string]string{},
+		},
+		SharedConfig: &configutil.SharedConfig{
+			Listeners: []*configutil.Listener{
+				{
+					Type:    "tcp",
+					Address: "127.0.0.1:443",
+				},
+			},
+			Seals: []*configutil.KMS{
+				&configutil.KMS{
+					Type:    "pkcs11",
+					Purpose: []string{"many", "purposes"},
+					Config: map[string]string{
+						"lib":                    "/usr/lib/libcklog2.so",
+						"slot":                   "0.0",
+						"pin":                    "XXXXXXXX",
+						"key_label":              "HASHICORP",
+						"mechanism":              "0x1082",
+						"hmac_mechanism":         "0x0251",
+						"hmac_key_label":         "vault-hsm-hmac-key",
+						"default_hmac_key_label": "vault-hsm-hmac-key",
+						"generate_key":           "true",
+					},
+				},
+				&configutil.KMS{
+					Type:     "pkcs11",
+					Purpose:  []string{"single"},
+					Disabled: true,
+					Config: map[string]string{
+						"lib":                    "/usr/lib/libcklog2.so",
+						"slot":                   "0.0",
+						"pin":                    "XXXXXXXX",
+						"key_label":              "HASHICORP",
+						"mechanism":              "0x1082",
+						"hmac_mechanism":         "0x0251",
+						"hmac_key_label":         "vault-hsm-hmac-key",
+						"default_hmac_key_label": "vault-hsm-hmac-key",
+						"generate_key":           "true",
+					},
+				},
+			},
+		},
+	}
+	require.Equal(t, config, expected)
+}
diff --git a/command/server/test-fixtures/config_seals.hcl b/command/server/test-fixtures/config_seals.hcl
@@ -0,0 +1,34 @@
+listener "tcp" {
+  address = "127.0.0.1:443"
+}
+
+backend "consul" {
+}
+
+seal "pkcs11" {
+  purpose = "many,purposes"
+  lib = "/usr/lib/libcklog2.so"
+  slot = "0.0"
+  pin = "XXXXXXXX"
+  key_label = "HASHICORP"
+  mechanism = "0x1082"
+  hmac_mechanism = "0x0251"
+  hmac_key_label = "vault-hsm-hmac-key"
+  default_hmac_key_label = "vault-hsm-hmac-key"
+  generate_key = "true"
+}
+
+seal "pkcs11" {
+  purpose = "single"
+  disabled = "true"
+  lib = "/usr/lib/libcklog2.so"
+  slot = "0.0"
+  pin = "XXXXXXXX"
+  key_label = "HASHICORP"
+  mechanism = 0x1082
+  hmac_mechanism = 0x0251
+  hmac_key_label = "vault-hsm-hmac-key"
+  default_hmac_key_label = "vault-hsm-hmac-key"
+  generate_key = "true"
+}
+
diff --git a/internalshared/configutil/kms.go b/internalshared/configutil/kms.go
@@ -68,51 +68,49 @@ func parseKMS(result *[]*KMS, list *ast.ObjectList, blockName string, maxKMS int
 			key = item.Keys[0].Token.Value().(string)
 		}
 
-		// We first decode into a map[string]interface{} because purpose isn't
-		// necessarily a string. Then we migrate everything else over to
-		// map[string]string and error if it doesn't work.
-		var m map[string]interface{}
-		if err := hcl.DecodeObject(&m, item.Val); err != nil {
-			return multierror.Prefix(err, fmt.Sprintf("%s.%s:", blockName, key))
-		}
-
+		var disabled bool
 		var purpose []string
 		var err error
-		if v, ok := m["purpose"]; ok {
-			if purpose, err = parseutil.ParseCommaStringSlice(v); err != nil {
-				return multierror.Prefix(fmt.Errorf("unable to parse 'purpose' in kms type %q: %w", key, err), fmt.Sprintf("%s.%s:", blockName, key))
+		{
+			// We first decode into a map[string]interface{} because purpose isn't
+			// necessarily a string. Then we migrate everything else over to
+			// map[string]string and error if it doesn't work.
+			var m map[string]interface{}
+			if err := hcl.DecodeObject(&m, item.Val); err != nil {
+				return multierror.Prefix(err, fmt.Sprintf("%s.%s:", blockName, key))
 			}
-			for i, p := range purpose {
-				purpose[i] = strings.ToLower(p)
+
+			if v, ok := m["purpose"]; ok {
+				if purpose, err = parseutil.ParseCommaStringSlice(v); err != nil {
+					return multierror.Prefix(fmt.Errorf("unable to parse 'purpose' in kms type %q: %w", key, err), fmt.Sprintf("%s.%s:", blockName, key))
+				}
+				for i, p := range purpose {
+					purpose[i] = strings.ToLower(p)
+				}
 			}
-			delete(m, "purpose")
-		}
 
-		var disabled bool
-		if v, ok := m["disabled"]; ok {
-			disabled, err = parseutil.ParseBool(v)
-			if err != nil {
-				return multierror.Prefix(err, fmt.Sprintf("%s.%s:", blockName, key))
+			if v, ok := m["disabled"]; ok {
+				disabled, err = parseutil.ParseBool(v)
+				if err != nil {
+					return multierror.Prefix(err, fmt.Sprintf("%s.%s:", blockName, key))
+				}
 			}
-			delete(m, "disabled")
 		}
 
-		strMap := make(map[string]string, len(m))
-		for k, v := range m {
-			if vs, ok := v.(string); ok {
-				strMap[k] = vs
-			} else {
-				return multierror.Prefix(fmt.Errorf("unable to parse 'purpose' in kms type %q: value could not be parsed as string", key), fmt.Sprintf("%s.%s:", blockName, key))
-			}
+		var cfg map[string]string
+		if err := hcl.DecodeObject(&cfg, item.Val); err != nil {
+			return multierror.Prefix(err, fmt.Sprintf("%s.%s:", blockName, key))
 		}
+		delete(cfg, "purpose")
+		delete(cfg, "disabled")
 
 		seal := &KMS{
 			Type:     strings.ToLower(key),
 			Purpose:  purpose,
 			Disabled: disabled,
 		}
-		if len(strMap) > 0 {
-			seal.Config = strMap
+		if len(cfg) > 0 {
+			seal.Config = cfg
 		}
 		seals = append(seals, seal)
 	}