Skip to content

Commit 5d9c784

Browse files
VAULT-35716 make allowed and denied_parameters compare lists (hashicorp#9478) (hashicorp#9524)
* make allowed and denied_parameters compare lists * change name of env var * add changelog * linter fixes and unnecessary code removal Co-authored-by: Bruno Oliveira de Souza <bruno.souza@hashicorp.com>
1 parent be36cf4 commit 5d9c784

File tree

3 files changed

+790
-24
lines changed

3 files changed

+790
-24
lines changed

changelog/_9478.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
```release-note:change
2+
policies: change list comparison to allowed_parameters and denied_parameters from "exact match" to "contains all"
3+
```

vault/acl.go

Lines changed: 59 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package vault
66
import (
77
"context"
88
"fmt"
9+
"os"
910
"reflect"
1011
"slices"
1112
"sort"
@@ -526,26 +527,25 @@ CHECK:
526527
return
527528
}
528529

529-
if len(permissions.DeniedParameters) == 0 {
530-
goto ALLOWED_PARAMETERS
531-
}
530+
useLegacyMatching := os.Getenv("VAULT_LEGACY_EXACT_MATCHING_ON_LIST") != ""
532531

533-
// Check if all parameters have been denied
534-
if _, ok := permissions.DeniedParameters["*"]; ok {
535-
return
536-
}
532+
if len(permissions.DeniedParameters) > 0 {
533+
// Check if all parameters have been denied
534+
if _, ok := permissions.DeniedParameters["*"]; ok {
535+
return
536+
}
537537

538-
for parameter, value := range req.Data {
539-
// Check if parameter has been explicitly denied
540-
if valueSlice, ok := permissions.DeniedParameters[strings.ToLower(parameter)]; ok {
541-
// If the value exists in denied values slice, deny
542-
if valueInParameterList(value, valueSlice) {
543-
return
538+
for parameter, value := range req.Data {
539+
// Check if parameter has been explicitly denied
540+
if valueSlice, ok := permissions.DeniedParameters[strings.ToLower(parameter)]; ok {
541+
// If the value exists in denied values slice, deny
542+
if valueInDeniedParameterList(value, valueSlice, useLegacyMatching) {
543+
return
544+
}
544545
}
545546
}
546547
}
547548

548-
ALLOWED_PARAMETERS:
549549
// If we don't have any allowed parameters set, allow
550550
if len(permissions.AllowedParameters) == 0 {
551551
ret.Allowed = true
@@ -565,9 +565,9 @@ CHECK:
565565
return
566566
}
567567

568-
// If the value doesn't exists in the allowed values slice,
568+
// If the value doesn't exist in the allowed values slice,
569569
// deny
570-
if ok && !valueInParameterList(value, valueSlice) {
570+
if ok && !valueInAllowedParameterList(value, valueSlice, useLegacyMatching) {
571571
return
572572
}
573573
}
@@ -812,16 +812,54 @@ func (c *Core) performPolicyChecksSinglePath(ctx context.Context, acl *ACL, te *
812812
return ret
813813
}
814814

815-
func valueInParameterList(v interface{}, list []interface{}) bool {
815+
func valueInAllowedParameterList(v interface{}, list []interface{}, useLegacyMatching bool) bool {
816816
// Empty list is equivalent to the item always existing in the list
817817
if len(list) == 0 {
818818
return true
819819
}
820820

821-
return valueInSlice(v, list)
821+
var oneByOneMissingMatch bool
822+
if vSlice, ok := v.([]interface{}); ok && !useLegacyMatching {
823+
// when not running in legacy mode, we run a relaxed check for slices that verifies if all
824+
// elements in the slice exist in the allowed list, as opposed to checking if the allowed
825+
// list contains a single element that matches the entire slice (but this whole-slice match
826+
// is still supported)
827+
for _, v := range vSlice {
828+
if !valueInParameterList(v, list) {
829+
oneByOneMissingMatch = true
830+
break
831+
}
832+
}
833+
834+
if !oneByOneMissingMatch {
835+
// no missing match means all elements in the slice were found in the allowed list, so allow it
836+
return true
837+
}
838+
}
839+
840+
return valueInParameterList(v, list)
822841
}
823842

824-
func valueInSlice(v interface{}, list []interface{}) bool {
843+
func valueInDeniedParameterList(v interface{}, list []interface{}, useLegacyMatching bool) bool {
844+
// Empty list is equivalent to the item always existing in the list
845+
if len(list) == 0 {
846+
return true
847+
}
848+
849+
if vSlice, ok := v.([]interface{}); ok && !useLegacyMatching {
850+
// The new behaviour is that if any value in the slice is in the denied list, we deny.
851+
// Always execute it in order to log a warning in case we find a breaking change while using the legacy mode.
852+
for _, v := range vSlice {
853+
if valueInParameterList(v, list) {
854+
return true
855+
}
856+
}
857+
}
858+
859+
return valueInParameterList(v, list)
860+
}
861+
862+
func valueInParameterList(v interface{}, list []interface{}) bool {
825863
for _, el := range list {
826864
if el == nil || v == nil {
827865
// It doesn't seem possible to set up a nil entry in the list, but it is possible
@@ -830,11 +868,8 @@ func valueInSlice(v interface{}, list []interface{}) bool {
830868
if el == v {
831869
return true
832870
}
833-
} else if reflect.TypeOf(el).String() == "string" && reflect.TypeOf(v).String() == "string" {
834-
item := el.(string)
835-
val := v.(string)
836-
837-
if strutil.GlobbedStringsMatch(item, val) {
871+
} else if elStr, ok := el.(string); ok {
872+
if vStr, ok := v.(string); ok && strutil.GlobbedStringsMatch(elStr, vStr) {
838873
return true
839874
}
840875
} else if reflect.DeepEqual(el, v) {

0 commit comments

Comments
 (0)