luxolus
/
open-vault
2
0
Fork 0
Code Issues 1 Pull Requests Releases Activity

Clone policy permissions and then use existing values rather than policy values for modifications (#2826) 

Should fix #2804
This commit is contained in:
Jeff Mitchell 2017-06-07 13:49:51 -04:00 committed by GitHub
parent f6d48312d8
commit ab5014534e
3 changed files with 99 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

62
vault/acl.go
View File

@ -5,6 +5,7 @@ import (
"strings"
"github.com/armon/go-radix"
"github.com/hashicorp/errwrap"
"github.com/hashicorp/vault/helper/strutil"
"github.com/hashicorp/vault/logical"
)
@ -51,7 +52,11 @@ func NewACL(policies []*Policy) (*ACL, error) {
// Check for an existing policy
raw, ok := tree.Get(pc.Prefix)
if !ok {
tree.Insert(pc.Prefix, pc.Permissions)
clonedPerms, err := pc.Permissions.Clone()
if err != nil {
return nil, errwrap.Wrapf("error cloning ACL permissions: {{err}}", err)
}
tree.Insert(pc.Prefix, clonedPerms)
continue
}
@ -66,15 +71,15 @@ func NewACL(policies []*Policy) (*ACL, error) {
case pc.Permissions.CapabilitiesBitmap&DenyCapabilityInt > 0:
// If this new policy explicitly denies, only save the deny value
pc.Permissions.CapabilitiesBitmap = DenyCapabilityInt
pc.Permissions.AllowedParameters = nil
pc.Permissions.DeniedParameters = nil
existingPerms.CapabilitiesBitmap = DenyCapabilityInt
existingPerms.AllowedParameters = nil
existingPerms.DeniedParameters = nil
goto INSERT
default:
// Insert the capabilities in this new policy into the existing
// value
pc.Permissions.CapabilitiesBitmap = existingPerms.CapabilitiesBitmap | pc.Permissions.CapabilitiesBitmap
existingPerms.CapabilitiesBitmap = existingPerms.CapabilitiesBitmap | pc.Permissions.CapabilitiesBitmap
}
// Note: In these stanzas, we're preferring minimum lifetimes. So
@ -85,59 +90,58 @@ func NewACL(policies []*Policy) (*ACL, error) {
// If we have an existing max, and we either don't have a current
// max, or the current is greater than the previous, use the
// existing.
if existingPerms.MaxWrappingTTL > 0 &&
(pc.Permissions.MaxWrappingTTL == 0 ||
existingPerms.MaxWrappingTTL < pc.Permissions.MaxWrappingTTL) {
pc.Permissions.MaxWrappingTTL = existingPerms.MaxWrappingTTL
if pc.Permissions.MaxWrappingTTL > 0 &&
(existingPerms.MaxWrappingTTL == 0 ||
pc.Permissions.MaxWrappingTTL < existingPerms.MaxWrappingTTL) {
existingPerms.MaxWrappingTTL = pc.Permissions.MaxWrappingTTL
}
// If we have an existing min, and we either don't have a current
// min, or the current is greater than the previous, use the
// existing
if existingPerms.MinWrappingTTL > 0 &&
(pc.Permissions.MinWrappingTTL == 0 ||
existingPerms.MinWrappingTTL < pc.Permissions.MinWrappingTTL) {
pc.Permissions.MinWrappingTTL = existingPerms.MinWrappingTTL
if pc.Permissions.MinWrappingTTL > 0 &&
(existingPerms.MinWrappingTTL == 0 ||
pc.Permissions.MinWrappingTTL < existingPerms.MinWrappingTTL) {
existingPerms.MinWrappingTTL = pc.Permissions.MinWrappingTTL
}
if len(existingPerms.AllowedParameters) > 0 {
if pc.Permissions.AllowedParameters == nil {
pc.Permissions.AllowedParameters = existingPerms.AllowedParameters
if len(pc.Permissions.AllowedParameters) > 0 {
if existingPerms.AllowedParameters == nil {
existingPerms.AllowedParameters = pc.Permissions.AllowedParameters
} else {
for key, value := range existingPerms.AllowedParameters {
pcValue, ok := pc.Permissions.AllowedParameters[key]
for key, value := range pc.Permissions.AllowedParameters {
pcValue, ok := existingPerms.AllowedParameters[key]
// If an empty array exist it should overwrite any other
// value.
if len(value) == 0 || (ok && len(pcValue) == 0) {
pc.Permissions.AllowedParameters[key] = []interface{}{}
existingPerms.AllowedParameters[key] = []interface{}{}
} else {
// Merge the two maps, appending values on key conflict.
pc.Permissions.AllowedParameters[key] = append(value, pc.Permissions.AllowedParameters[key]...)
existingPerms.AllowedParameters[key] = append(value, existingPerms.AllowedParameters[key]...)
}
}
}
}
if len(existingPerms.DeniedParameters) > 0 {
if pc.Permissions.DeniedParameters == nil {
pc.Permissions.DeniedParameters = existingPerms.DeniedParameters
if len(pc.Permissions.DeniedParameters) > 0 {
if existingPerms.DeniedParameters == nil {
existingPerms.DeniedParameters = pc.Permissions.DeniedParameters
} else {
for key, value := range existingPerms.DeniedParameters {
pcValue, ok := pc.Permissions.DeniedParameters[key]
for key, value := range pc.Permissions.DeniedParameters {
pcValue, ok := existingPerms.DeniedParameters[key]
// If an empty array exist it should overwrite any other
// value.
if len(value) == 0 || (ok && len(pcValue) == 0) {
pc.Permissions.DeniedParameters[key] = []interface{}{}
existingPerms.DeniedParameters[key] = []interface{}{}
} else {
// Merge the two maps, appending values on key conflict.
pc.Permissions.DeniedParameters[key] = append(value, pc.Permissions.DeniedParameters[key]...)
existingPerms.DeniedParameters[key] = append(value, existingPerms.DeniedParameters[key]...)
}
}
}
}
INSERT:
tree.Insert(pc.Prefix, pc.Permissions)
tree.Insert(pc.Prefix, existingPerms)
}
}

32
vault/acl_test.go
View File

@ -2,6 +2,7 @@ package vault
import (
"reflect"
"sync"
"testing"
"time"
@ -245,7 +246,7 @@ func TestACL_PolicyMerge(t *testing.T) {
{"allow/all1", nil, nil, map[string][]interface{}{"*": []interface{}{}, "test": []interface{}{}, "test1": []interface{}{"foo"}}, nil},
{"deny/all", nil, nil, nil, map[string][]interface{}{"*": []interface{}{}, "test": []interface{}{}}},
{"deny/all1", nil, nil, nil, map[string][]interface{}{"*": []interface{}{}, "test": []interface{}{}}},
{"value/merge", nil, nil, map[string][]interface{}{"test": []interface{}{1, 2, 3, 4}}, map[string][]interface{}{"test": []interface{}{1, 2, 3, 4}}},
{"value/merge", nil, nil, map[string][]interface{}{"test": []interface{}{3, 4, 1, 2}}, map[string][]interface{}{"test": []interface{}{3, 4, 1, 2}}},
{"value/empty", nil, nil, map[string][]interface{}{"empty": []interface{}{}}, map[string][]interface{}{"empty": []interface{}{}}},
}
@ -415,6 +416,35 @@ func TestACL_ValuePermissions(t *testing.T) {
}
}
// NOTE: this test doesn't catch any races ATM
func TestACL_CreationRace(t *testing.T) {
policy, err := Parse(valuePermissionsPolicy)
if err != nil {
t.Fatalf("err: %v", err)
}
var wg sync.WaitGroup
stopTime := time.Now().Add(20 * time.Second)
for i := 0; i < 50; i++ {
wg.Add(1)
go func() {
defer wg.Done()
for {
if time.Now().After(stopTime) {
return
}
_, err := NewACL([]*Policy{policy})
if err != nil {
t.Fatalf("err: %v", err)
}
}
}()
}
wg.Wait()
}
var tokenCreationPolicy = `
name = "tokenCreation"
path "auth/token/create*" {

35
vault/policy.go
View File

@ -11,6 +11,7 @@ import (
"github.com/hashicorp/hcl"
"github.com/hashicorp/hcl/hcl/ast"
"github.com/hashicorp/vault/helper/parseutil"
"github.com/mitchellh/copystructure"
)
const (
@ -84,6 +85,40 @@ type Permissions struct {
DeniedParameters map[string][]interface{}
}
func (p *Permissions) Clone() (*Permissions, error) {
ret := &Permissions{
CapabilitiesBitmap: p.CapabilitiesBitmap,
MinWrappingTTL: p.MinWrappingTTL,
MaxWrappingTTL: p.MaxWrappingTTL,
}
switch {
case p.AllowedParameters == nil:
case len(p.AllowedParameters) == 0:
ret.AllowedParameters = make(map[string][]interface{})
default:
clonedAllowed, err := copystructure.Copy(p.AllowedParameters)
if err != nil {
return nil, err
}
ret.AllowedParameters = clonedAllowed.(map[string][]interface{})
}
switch {
case p.DeniedParameters == nil:
case len(p.DeniedParameters) == 0:
ret.DeniedParameters = make(map[string][]interface{})
default:
clonedDenied, err := copystructure.Copy(p.DeniedParameters)
if err != nil {
return nil, err
}
ret.DeniedParameters = clonedDenied.(map[string][]interface{})
}
return ret, nil
}
// Parse is used to parse the specified ACL rules into an
// intermediary set of policies, before being compiled into
// the ACL