Update logic to fix a few edge cases:

This commit is contained in:
Brian Kassouf 2017-02-16 15:20:11 -08:00
parent 8d880f5181
commit 136730cb01
2 changed files with 56 additions and 28 deletions

View File

@ -2,6 +2,7 @@ package vault
import (
"reflect"
"strings"
"github.com/armon/go-radix"
"github.com/hashicorp/vault/logical"
@ -259,41 +260,51 @@ CHECK:
// Only check parameter permissions for operations that can modify
// parameters.
if op == logical.UpdateOperation || op == logical.DeleteOperation || op == logical.CreateOperation {
if len(req.Data) == 0 {
return true, sudo
}
if len(permissions.DeniedParameters) == 0 {
goto ALLOWED_PARAMETERS
}
// Check if all parameters have been denied
if _, ok := permissions.DeniedParameters["*"]; ok {
return false, sudo
}
_, allowedAll := permissions.AllowedParameters["*"]
if len(permissions.DeniedParameters) == 0 && allowedAll {
return true, sudo
}
for parameter, value := range req.Data {
// Check if parameter has been explictly denied
if valueSlice, ok := permissions.DeniedParameters[parameter]; ok {
if valueSlice, ok := permissions.DeniedParameters[strings.ToLower(parameter)]; ok {
// If the value exists in denied values slice, deny
if valueInParameterList(value, valueSlice) {
return false, sudo
}
// If the value doesn't exist in the denied values slice,
// continue
continue
}
}
ALLOWED_PARAMETERS:
// If we don't have any allowed parameters set, allow
if len(permissions.AllowedParameters) == 0 {
return true, sudo
}
_, allowedAll := permissions.AllowedParameters["*"]
if len(permissions.AllowedParameters) == 1 && allowedAll {
return true, sudo
}
for parameter, value := range req.Data {
valueSlice, ok := permissions.AllowedParameters[strings.ToLower(parameter)]
// Requested parameter is not in allowed list
if !ok && !allowedAll {
return false, sudo
}
// Specfic parameters have been allowed
if len(permissions.AllowedParameters) > 0 && !allowedAll {
// Requested parameter is not in allowed list
valueSlice, ok := permissions.AllowedParameters[parameter]
if !ok {
return false, sudo
}
// If the value doesn't exists in the allowed values slice,
// deny
if !valueInParameterList(value, valueSlice) {
return false, sudo
}
// If the value doesn't exists in the allowed values slice,
// deny
if ok && !valueInParameterList(value, valueSlice) {
return false, sudo
}
}
}
@ -302,7 +313,7 @@ CHECK:
}
func valueInParameterList(v interface{}, list []interface{}) bool {
if len(list) == 0 || valueInSlice("*", list) {
if len(list) == 0 {
return true
}

View File

@ -1,6 +1,7 @@
package vault
import (
"fmt"
"reflect"
"testing"
@ -337,9 +338,10 @@ func TestACL_ValuePermissions(t *testing.T) {
{"foo/bar", []string{"allow"}, []interface{}{"good"}, true},
{"foo/baz", []string{"allow"}, []interface{}{"good"}, true},
{"foo/baz", []string{"deny"}, []interface{}{"bad"}, false},
{"foo/baz", []string{"deny"}, []interface{}{"good"}, true},
{"foo/baz", []string{"deny"}, []interface{}{"good"}, false},
{"foo/baz", []string{"allow", "deny"}, []interface{}{"good", "bad"}, false},
{"foo/baz", []string{"deny", "allow"}, []interface{}{"good", "bad"}, false},
{"foo/baz", []string{"deny", "allow"}, []interface{}{"bad", "good"}, false},
{"foo/baz", []string{"allow"}, []interface{}{"bad"}, false},
{"foo/baz", []string{"neither"}, []interface{}{"bad"}, false},
{"fizz/buzz", []string{"allow_multi"}, []interface{}{"good"}, true},
@ -348,14 +350,18 @@ func TestACL_ValuePermissions(t *testing.T) {
{"fizz/buzz", []string{"allow_multi"}, []interface{}{"bad"}, false},
{"fizz/buzz", []string{"allow_multi", "allow"}, []interface{}{"good1", "good"}, true},
{"fizz/buzz", []string{"deny_multi"}, []interface{}{"bad2"}, false},
{"fizz/buzz", []string{"deny_multi", "allow_multi"}, []interface{}{"good", "good2"}, true},
{"fizz/buzz", []string{"deny_multi", "allow_multi"}, []interface{}{"good", "good2"}, false},
// {"test/types", []string{"array"}, []interface{}{[1]string{"good"}}, true},
{"test/types", []string{"map"}, []interface{}{map[string]interface{}{"good": "one"}}, true},
{"test/types", []string{"map"}, []interface{}{map[string]interface{}{"bad": "one"}}, false},
{"test/types", []string{"int"}, []interface{}{1}, true},
{"test/types", []string{"int"}, []interface{}{3}, false},
{"test/types", []string{"bool"}, []interface{}{false}, false},
{"test/types", []string{"bool"}, []interface{}{true}, true},
{"test/types", []string{"bool"}, []interface{}{false}, true},
{"test/types", []string{"bool"}, []interface{}{true}, false},
{"test/star", []string{"anything"}, []interface{}{true}, true},
{"test/star", []string{"foo"}, []interface{}{true}, true},
{"test/star", []string{"bar"}, []interface{}{false}, true},
{"test/star", []string{"bar"}, []interface{}{true}, false},
}
for _, tc := range tcases {
@ -365,6 +371,7 @@ func TestACL_ValuePermissions(t *testing.T) {
}
for _, op := range toperations {
request.Operation = op
fmt.Println(tc)
allowed, _ := acl.AllowOperation(&request)
if allowed != tc.allowed {
t.Fatalf("bad: case %#v: %v", tc, allowed)
@ -644,9 +651,19 @@ path "test/types" {
allowed_parameters = {
"map" = [{"good" = "one"}]
"int" = [1, 2]
"bool" = [false]
}
denied_parameters = {
}
}
path "test/star" {
policy = "write"
allowed_parameters = {
"*" = []
"foo" = []
"bar" = [false]
}
denied_parameters = {
"bool" = [false]
}
}
`