Properly lowercase policy names. (#3210)

Previously we lowercased names on ingress but not on lookup or delete
which could cause unexpected results. Now, just unilaterally lowercase
policy names on write and delete. On get, to avoid the performance hit
of always lowercasing when not necessary since it's in the critical
path, we have a minor optimization -- we check the LRU first before
normalizing. For tokens, because they're already normalized when adding
policies during creation, this should always work; it might just be
slower for API calls.

Fixes #3187

command/policy_write.go

@@ -37,7 +37,8 @@ func (c *PolicyWriteCommand) Run(args []string) int {
 		return 2
 	}
 
-	name := args[0]
+	// Policies are normalized to lowercase
+	name := strings.ToLower(args[0])
 	path := args[1]
 
 	// Read the policy

vault/acl_test.go

@@ -453,7 +453,7 @@ path "auth/token/create*" {
 `
 
 var aclPolicy = `
-name = "dev"
+name = "DeV"
 path "dev/*" {
 	policy = "sudo"
 }
@@ -482,7 +482,7 @@ path "foo/bar" {
 `
 
 var aclPolicy2 = `
-name = "ops"
+name = "OpS"
 path "dev/hide/*" {
 	policy = "deny"
 }

vault/logical_system.go

@@ -1846,7 +1846,7 @@ func (b *SystemBackend) handlePolicyRead(
 
 	return &logical.Response{
 		Data: map[string]interface{}{
-			"name":  name,
+			"name":  policy.Name,
 			"rules": policy.Raw,
 		},
 	}, nil
@@ -1873,8 +1873,9 @@ func (b *SystemBackend) handlePolicySet(
 		return handleError(err)
 	}
 
-	// Override the name
-	parse.Name = strings.ToLower(name)
+	if name != "" {
+		parse.Name = name
+	}
 
 	// Update the policy
 	if err := b.Core.policyStore.SetPolicy(parse); err != nil {

vault/logical_system_test.go

@@ -1239,8 +1239,12 @@ func TestSystemBackend_policyCRUD(t *testing.T) {
 		t.Fatalf("err: %v", err)
 	}
 
-	if resp != nil {
-		t.Fatalf("err: expected nil response, got %#v", *resp)
+	exp = map[string]interface{}{
+		"name":  "foo",
+		"rules": rules,
+	}
+	if !reflect.DeepEqual(resp.Data, exp) {
+		t.Fatalf("got: %#v expect: %#v", resp.Data, exp)
 	}
 
 	// List the policies

vault/policy_store.go

@@ -200,6 +200,8 @@ func (ps *PolicyStore) SetPolicy(p *Policy) error {
 	if p.Name == "" {
 		return fmt.Errorf("policy name missing")
 	}
+	// Policies are normalized to lower-case
+	p.Name = strings.ToLower(strings.TrimSpace(p.Name))
 	if strutil.StrListContains(immutablePolicies, p.Name) {
 		return fmt.Errorf("cannot update %s policy", p.Name)
 	}
@@ -230,6 +232,7 @@ func (ps *PolicyStore) setPolicyInternal(p *Policy) error {
 // GetPolicy is used to fetch the named policy
 func (ps *PolicyStore) GetPolicy(name string) (*Policy, error) {
 	defer metrics.MeasureSince([]string{"policy", "get_policy"}, time.Now())
+
 	if ps.lru != nil {
 		// Check for cached policy
 		if raw, ok := ps.lru.Get(name); ok {
@@ -237,6 +240,9 @@ func (ps *PolicyStore) GetPolicy(name string) (*Policy, error) {
 		}
 	}
 
+	// Policies are normalized to lower-case
+	name = strings.ToLower(strings.TrimSpace(name))
+
 	// Special case the root policy
 	if name == "root" {
 		p := &Policy{Name: "root"}
@@ -320,6 +326,9 @@ func (ps *PolicyStore) ListPolicies() ([]string, error) {
 // DeletePolicy is used to delete the named policy
 func (ps *PolicyStore) DeletePolicy(name string) error {
 	defer metrics.MeasureSince([]string{"policy", "delete_policy"}, time.Now())
+
+	// Policies are normalized to lower-case
+	name = strings.ToLower(strings.TrimSpace(name))
 	if strutil.StrListContains(immutablePolicies, name) {
 		return fmt.Errorf("cannot delete %s policy", name)
 	}

vault/policy_store_test.go

@@ -61,7 +61,7 @@ func TestPolicyStore_CRUD(t *testing.T) {
 
 func testPolicyStore_CRUD(t *testing.T, ps *PolicyStore) {
 	// Get should return nothing
-	p, err := ps.GetPolicy("dev")
+	p, err := ps.GetPolicy("Dev")
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
@@ -70,7 +70,7 @@ func testPolicyStore_CRUD(t *testing.T, ps *PolicyStore) {
 	}
 
 	// Delete should be no-op
-	err = ps.DeletePolicy("dev")
+	err = ps.DeletePolicy("deV")
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
@@ -92,7 +92,7 @@ func testPolicyStore_CRUD(t *testing.T, ps *PolicyStore) {
 	}
 
 	// Get should work
-	p, err = ps.GetPolicy("dev")
+	p, err = ps.GetPolicy("dEv")
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
@@ -110,13 +110,13 @@ func testPolicyStore_CRUD(t *testing.T, ps *PolicyStore) {
 	}
 
 	// Delete should be clear the entry
-	err = ps.DeletePolicy("dev")
+	err = ps.DeletePolicy("Dev")
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
 
 	// Get should fail
-	p, err = ps.GetPolicy("dev")
+	p, err = ps.GetPolicy("deV")
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}