Parse policy HCL syntax and keys

vault/policy.go

@@ -4,7 +4,9 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/hashicorp/go-multierror"
 	"github.com/hashicorp/hcl"
+	"github.com/hashicorp/hcl/hcl/ast"
 )
 
 const (
@@ -50,13 +52,13 @@ var (
 // an ACL configuration.
 type Policy struct {
 	Name  string              `hcl:"name"`
-	Paths []*PathCapabilities `hcl:"path,expand"`
+	Paths []*PathCapabilities `hcl:"-"`
 	Raw   string
 }
 
-// Capability represents a policy for a path in the namespace
+// PathCapabilities represents a policy for a path in the namespace.
 type PathCapabilities struct {
-	Prefix             string `hcl:",key"`
+	Prefix             string
 	Policy             string
 	Capabilities       []string
 	CapabilitiesBitmap uint32 `hcl:"-"`
@@ -67,16 +69,65 @@ type PathCapabilities struct {
 // intermediary set of policies, before being compiled into
 // the ACL
 func Parse(rules string) (*Policy, error) {
-	// Decode the rules
+	// Parse the rules
+	root, err := hcl.Parse(rules)
+	if err != nil {
+		return nil, fmt.Errorf("Failed to parse policy: %s", err)
+	}
+
+	// Top-level item should be the object list
+	list, ok := root.Node.(*ast.ObjectList)
+	if !ok {
+		return nil, fmt.Errorf("Failed to parse policy: does not contain a root object")
+	}
+
+	// Check for invalid top-level keys
+	valid := []string{
+		"name",
+		"path",
+	}
+	if err := checkHCLKeys(list, valid); err != nil {
+		return nil, fmt.Errorf("Failed to parse policy: %s", err)
+	}
+
+	// Create the initial policy and store the raw text of the rules
 	p := &Policy{Raw: rules}
-	if err := hcl.Decode(p, rules); err != nil {
-		return nil, fmt.Errorf("Failed to parse ACL rules: %v", err)
+	if err := hcl.DecodeObject(&p, list); err != nil {
+		return nil, fmt.Errorf("Failed to parse policy: %s", err)
+	}
+
+	if o := list.Filter("path"); len(o.Items) > 0 {
+		if err := parsePaths(p, o); err != nil {
+			return nil, fmt.Errorf("Failed to parse policy: %s", err)
+		}
 	}
 
-	// Validate the path policy
-	for _, pc := range p.Paths {
-		// Strip a leading '/' as paths in Vault start after the / in the API
-		// path
+	return p, nil
+}
+
+func parsePaths(result *Policy, list *ast.ObjectList) error {
+	paths := make([]*PathCapabilities, 0, len(list.Items))
+	for _, item := range list.Items {
+		key := "path"
+		if len(item.Keys) > 0 {
+			key = item.Keys[0].Token.Value().(string)
+		}
+
+		valid := []string{
+			"policy",
+			"capabilities",
+		}
+		if err := checkHCLKeys(item.Val, valid); err != nil {
+			return multierror.Prefix(err, fmt.Sprintf("path %q:", key))
+		}
+
+		var pc PathCapabilities
+		pc.Prefix = key
+		if err := hcl.DecodeObject(&pc, item.Val); err != nil {
+			return multierror.Prefix(err, fmt.Sprintf("path %q:", key))
+		}
+
+		// Strip a leading '/' as paths in Vault start after the / in the API path
 		if len(pc.Prefix) > 0 && pc.Prefix[0] == '/' {
 			pc.Prefix = pc.Prefix[1:]
 		}
@@ -88,15 +139,19 @@ func Parse(rules string) (*Policy, error) {
 		}
 
 		// Map old-style policies into capabilities
-		switch pc.Policy {
-		case OldDenyPathPolicy:
-			pc.Capabilities = []string{DenyCapability}
-		case OldReadPathPolicy:
-			pc.Capabilities = append(pc.Capabilities, []string{ReadCapability, ListCapability}...)
-		case OldWritePathPolicy:
-			pc.Capabilities = append(pc.Capabilities, []string{CreateCapability, ReadCapability, UpdateCapability, DeleteCapability, ListCapability}...)
-		case OldSudoPathPolicy:
-			pc.Capabilities = append(pc.Capabilities, []string{CreateCapability, ReadCapability, UpdateCapability, DeleteCapability, ListCapability, SudoCapability}...)
+		if len(pc.Policy) > 0 {
+			switch pc.Policy {
+			case OldDenyPathPolicy:
+				pc.Capabilities = []string{DenyCapability}
+			case OldReadPathPolicy:
+				pc.Capabilities = append(pc.Capabilities, []string{ReadCapability, ListCapability}...)
+			case OldWritePathPolicy:
+				pc.Capabilities = append(pc.Capabilities, []string{CreateCapability, ReadCapability, UpdateCapability, DeleteCapability, ListCapability}...)
+			case OldSudoPathPolicy:
+				pc.Capabilities = append(pc.Capabilities, []string{CreateCapability, ReadCapability, UpdateCapability, DeleteCapability, ListCapability, SudoCapability}...)
+			default:
+				return fmt.Errorf("path %q: invalid policy '%s'", key, pc.Policy)
+			}
 		}
 
 		// Initialize the map
@@ -111,11 +166,43 @@ func Parse(rules string) (*Policy, error) {
 			case CreateCapability, ReadCapability, UpdateCapability, DeleteCapability, ListCapability, SudoCapability:
 				pc.CapabilitiesBitmap |= cap2Int[cap]
 			default:
-				return nil, fmt.Errorf("Invalid capability: %#v", pc)
+				return fmt.Errorf("path %q: invalid capability '%s'", key, cap)
 			}
 		}
 
 	PathFinished:
+
+		paths = append(paths, &pc)
 	}
-	return p, nil
+
+	result.Paths = paths
+	return nil
+}
+
+func checkHCLKeys(node ast.Node, valid []string) error {
+	var list *ast.ObjectList
+	switch n := node.(type) {
+	case *ast.ObjectList:
+		list = n
+	case *ast.ObjectType:
+		list = n.List
+	default:
+		return fmt.Errorf("cannot check HCL keys of type %T", n)
+	}
+
+	validMap := make(map[string]struct{}, len(valid))
+	for _, v := range valid {
+		validMap[v] = struct{}{}
+	}
+
+	var result error
+	for _, item := range list.Items {
+		key := item.Keys[0].Token.Value().(string)
+		if _, ok := validMap[key]; !ok {
+			result = multierror.Append(result, fmt.Errorf(
+				"invalid key '%s' on line %d", key, item.Assign.Line))
+		}
+	}
+
+	return result
 }

vault/policy_test.go

@@ -1,19 +1,51 @@
 package vault
 
 import (
-	"fmt"
 	"reflect"
+	"strings"
 	"testing"
 )
 
+var rawPolicy = strings.TrimSpace(`
+# Developer policy
+name = "dev"
+
+# Deny all paths by default
+path "*" {
+	policy = "deny"
+}
+
+# Allow full access to staging
+path "stage/*" {
+	policy = "sudo"
+}
+
+# Limited read privilege to production
+path "prod/version" {
+	policy = "read"
+}
+
+# Read access to foobar
+# Also tests stripping of leading slash
+path "/foo/bar" {
+	policy = "read"
+}
+
+# Add capabilities for creation and sudo to foobar
+# This will be separate; they are combined when compiled into an ACL
+path "foo/bar" {
+	capabilities = ["create", "sudo"]
+}
+`)
+
 func TestPolicy_Parse(t *testing.T) {
 	p, err := Parse(rawPolicy)
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
 
 	if p.Name != "dev" {
-		t.Fatalf("bad: %#v", p)
+		t.Fatalf("bad name: %q", p.Name)
 	}
 
 	expect := []*PathCapabilities{
@@ -48,46 +80,71 @@ func TestPolicy_Parse(t *testing.T) {
 			}, CreateCapabilityInt | SudoCapabilityInt, false},
 	}
 	if !reflect.DeepEqual(p.Paths, expect) {
-		ret := fmt.Sprintf("bad:\nexpected:\n")
-		for _, v := range expect {
-			ret = fmt.Sprintf("%s\n%#v", ret, *v)
-		}
-		ret = fmt.Sprintf("%s\n\ngot:\n", ret)
-		for _, v := range p.Paths {
-			ret = fmt.Sprintf("%s\n%#v", ret, *v)
-		}
-		t.Fatalf("%s\n", ret)
+		t.Errorf("expected \n\n%#v\n\n to be \n\n%#v\n\n", p.Paths, expect)
 	}
 }
 
-var rawPolicy = `
-# Developer policy
-name = "dev"
+func TestPolicy_ParseBadRoot(t *testing.T) {
+	_, err := Parse(strings.TrimSpace(`
+name = "test"
+bad  = "foo"
+nope = "yes"
+`))
+	if err == nil {
+		t.Fatalf("expected error")
+	}
 
-# Deny all paths by default
-path "*" {
-	policy = "deny"
+	if !strings.Contains(err.Error(), "invalid key 'bad' on line 2") {
+		t.Errorf("bad error: %q", err)
+	}
+
+	if !strings.Contains(err.Error(), "invalid key 'nope' on line 3") {
+		t.Errorf("bad error: %q", err)
+	}
 }
 
-# Allow full access to staging
-path "stage/*" {
-	policy = "sudo"
+func TestPolicy_ParseBadPath(t *testing.T) {
+	_, err := Parse(strings.TrimSpace(`
+path "/" {
+	capabilities = ["read"]
+	capabilites  = ["read"]
 }
+`))
+	if err == nil {
+		t.Fatalf("expected error")
+	}
 
-# Limited read privilege to production
-path "prod/version" {
-	policy = "read"
+	if !strings.Contains(err.Error(), "invalid key 'capabilites' on line 3") {
+		t.Errorf("bad error: %s", err)
+	}
 }
 
-# Read access to foobar
-# Also tests stripping of leading slash
-path "/foo/bar" {
-	policy = "read"
+func TestPolicy_ParseBadPolicy(t *testing.T) {
+	_, err := Parse(strings.TrimSpace(`
+path "/" {
+	policy = "banana"
 }
+`))
+	if err == nil {
+		t.Fatalf("expected error")
+	}
 
-# Add capabilities for creation and sudo to foobar
-# This will be separate; they are combined when compiled into an ACL
-path "foo/bar" {
-	capabilities = ["create", "sudo"]
+	if !strings.Contains(err.Error(), `path "/": invalid policy 'banana'`) {
+		t.Errorf("bad error: %s", err)
+	}
+}
+
+func TestPolicy_ParseBadCapabilities(t *testing.T) {
+	_, err := Parse(strings.TrimSpace(`
+path "/" {
+	capabilities = ["read", "banana"]
+}
+`))
+	if err == nil {
+		t.Fatalf("expected error")
+	}
+
+	if !strings.Contains(err.Error(), `path "/": invalid capability 'banana'`) {
+		t.Errorf("bad error: %s", err)
+	}
 }
-`