From 6825bd731673558e8ffc0819c14c1c9e37f01cee Mon Sep 17 00:00:00 2001 From: Tingmao Wang Date: Wed, 17 Sep 2025 21:57:05 +0100 Subject: [PATCH 1/9] regopolicyinterpreter: Add Set metadata (in additional to string maps) In implementing the fragment parameters feature, we need a way to store, for each issuer and feed tuple, which parameters should be used for fragments matching it. Consider this list of fragments, which a parent fragment might define: fragments := [ { "feed": "mcr.microsoft.com/maa/enclavehost", "issuer": "did:x509:0:sha256:...", "includes": [ "containers", "fragments" ], "minimum_svn": "1", "parameters": { "region": "australiacentral2", "cloud": "Public" } }, // ... ] When we load the fragment containing this, we need to iterate through its data..fragments array, and for each entry, append the parameters object to an array that is keyed by the issuer and feed, since we can have multiple such fragment import entries for the same issuer and feed, but with different parameters. This basically means that we need the equivalent of the following code in Rego, which I've failed to come up with a way to write: for f in fragment_fragments { issuer = data.metadata.issuers[f.issuer] or {} feed = issuer.feeds[f.feed] or {} feed.parameters = feed.parameters or [] feed.parameters = array_append(feed.parameters, f.parameters) issuer.feeds[f.feed] = feed data.metadata.issuers[f.issuer] = issuer } While we can dynamically lookup or store based on a key that is from a variable access, and we can send multiple metadata updates, those updates cannot express the semantic of "merging" objects or arrays. To make this simpler, we introduce a new metadata data type - "set", which supports storing an unordered list of arbitrary objects (which themselves might be either a string, or an object containing key-value pairs), that can be inserted to via metadata operations one at a time. This means that we can simply append to the metadata update operations array once per each fragment import statement, and the outcome would be the union of all the parameter objects. This is also very easy to query in Rego, given an issuer and feed: parameters := [ fp.parameters | fp = data.metadata.fragment_parameters[_] fp.issuer == input.issuer fp.feed == input.feed ] It is likely that the existing code that extracts included containers/fragments from a fragment can be simplified by using this feature, but that is outside of the scope of this PR. Signed-off-by: Tingmao Wang --- .../regopolicyinterpreter.go | 261 +++++++++++----- .../regopolicyinterpreter_test.go | 284 +++++++++++++++++- internal/regopolicyinterpreter/test.rego | 35 ++- pkg/securitypolicy/regopolicy_linux_test.go | 4 +- 4 files changed, 507 insertions(+), 77 deletions(-) diff --git a/internal/regopolicyinterpreter/regopolicyinterpreter.go b/internal/regopolicyinterpreter/regopolicyinterpreter.go index 6e316f9b41..8584f49156 100644 --- a/internal/regopolicyinterpreter/regopolicyinterpreter.go +++ b/internal/regopolicyinterpreter/regopolicyinterpreter.go @@ -8,6 +8,8 @@ import ( "fmt" "log" "os" + "reflect" + "slices" "sync" "github.com/open-policy-agent/opa/ast" @@ -61,12 +63,28 @@ type RegoModule struct { /* See README for more details on Metadata */ -type regoMetadata map[string]map[string]interface{} +// This is conceptually a map[string](regoMetadataMap|regoMetadataSet) +type regoMetadata map[string]interface{} const metadataRootKey = "metadata" const metadataOperationsKey = "metadata" type regoMetadataAction string +type regoMetadataType string + +type regoMetadataMap map[string]interface{} + +// This uses a pointer to a slice so that we can update it after getting a +// reference +type regoMetadataSet []interface{} + +const ( + // A string to anything map + metadataTypeMap regoMetadataType = "map" + + // A list of anything that we treat as a set + metadataTypeSet regoMetadataType = "set" +) const ( metadataAdd regoMetadataAction = "add" @@ -76,9 +94,13 @@ const ( type regoMetadataOperation struct { Action regoMetadataAction `json:"action"` - Name string `json:"name"` - Key string `json:"key"` - Value interface{} `json:"value"` + + // Defaults to map + Type regoMetadataType `json:"type"` + + Name string `json:"name"` + Key string `json:"key"` + Value interface{} `json:"value"` } // The result from a policy query @@ -121,22 +143,44 @@ func copyValue(value interface{}) (interface{}, error) { return valueCopy, nil } -// deep copy for regoMetadata. -// We cannot use copyObject for this due to the fact that map[string]interface{} -// is a concrete type and a map of it cannot be used as a map of interface{}. -func copyRegoMetadata(value regoMetadata) (regoMetadata, error) { - valueJSON, err := json.Marshal(value) - if err != nil { - return nil, err - } - - var valueCopy regoMetadata - err = json.Unmarshal(valueJSON, &valueCopy) - if err != nil { - return nil, err +// deep copy for regoMetadata, preserves inner regoMetadataMap/Set types +func copyRegoMetadata(metadata regoMetadata) (regoMetadata, error) { + metadataCopy := make(regoMetadata) + for key, val := range metadata { + switch v := val.(type) { + case regoMetadataMap: + var copyVal regoMetadataMap + valueJSON, err := json.Marshal(v) + if err != nil { + return nil, err + } + err = json.Unmarshal(valueJSON, ©Val) + if err != nil { + return nil, err + } + metadataCopy[key] = copyVal + case regoMetadataSet: + var copyVal regoMetadataSet + valueJSON, err := json.Marshal(v) + if err != nil { + return nil, err + } + err = json.Unmarshal(valueJSON, ©Val) + if err != nil { + return nil, err + } + metadataCopy[key] = copyVal + default: + // We technically shouldn't reach here + copyVal, err := copyValue(v) + if err != nil { + return nil, err + } + metadataCopy[key] = copyVal + } } - return valueCopy, nil + return metadataCopy, nil } // NewRegoPolicyInterpreter creates a new RegoPolicyInterpreter, using the code provided. @@ -228,29 +272,56 @@ func (r *RegoPolicyInterpreter) UpdateData(key string, value interface{}) error } } -// GetMetadata retrieves a copy of a single metadata item from the policy. -func (r *RegoPolicyInterpreter) GetMetadata(name string, key string) (interface{}, error) { - r.dataAndModulesMutex.Lock() - defer r.dataAndModulesMutex.Unlock() - +// Does not make any copies, caller must hold dataAndModulesMutex. +// Returns the zero value of T if the metadata item does not exist. +func _getMetadata[T any](r *RegoPolicyInterpreter, name string) (T, error) { metadataRoot, ok := r.data[metadataRootKey].(regoMetadata) if !ok { - return nil, errors.New("illegal interpreter state: invalid metadata object type") + return *new(T), errors.New("illegal interpreter state: invalid metadata object type") } if metadata, ok := metadataRoot[name]; ok { - if value, ok := metadata[key]; ok { - value, err := copyValue(value) //nolint:govet // shadow - if err != nil { - return nil, fmt.Errorf("unable to copy value: %w", err) - } + value, ok := metadata.(T) + if !ok { + return *new(T), fmt.Errorf("metadata %s has the wrong type (wanted %T, but saved type was %T)", name, value, metadata) + } + return value, nil + } - return value, nil - } else { - return nil, fmt.Errorf("value not found in %s for key %s", name, key) + return *new(T), nil +} + +func _setMetadata[T any](r *RegoPolicyInterpreter, name string, value T) error { + metadataRoot, ok := r.data[metadataRootKey].(regoMetadata) + if !ok { + return errors.New("illegal interpreter state: invalid metadata object type") + } + metadataRoot[name] = value + return nil +} + +// GetMetadata retrieves a copy of a single metadata item from the policy. +func (r *RegoPolicyInterpreter) GetMetadataMapValue(name string, key string) (interface{}, error) { + r.dataAndModulesMutex.Lock() + defer r.dataAndModulesMutex.Unlock() + + metadata, err := _getMetadata[regoMetadataMap](r, name) + if err != nil { + return nil, err + } + if metadata == nil { + return nil, fmt.Errorf("value not found in %s for key %s (map has not been initialized)", name, key) + } + + if value, ok := metadata[key]; ok { + value, err := copyValue(value) //nolint:govet // shadow + if err != nil { + return nil, fmt.Errorf("unable to copy value: %w", err) } + + return value, nil } else { - return nil, fmt.Errorf("metadata not found for name %s", name) + return nil, fmt.Errorf("value not found in %s for key %s", name, key) } } @@ -287,6 +358,16 @@ func newRegoMetadataOperation(operation interface{}) (*regoMetadataOperation, er if !ok { return nil, errors.New("unable to load metadata object") } + dataType, ok := data["type"] + if !ok { + metadataOp.Type = metadataTypeMap + } else { + dataTypeStr, ok := dataType.(string) + if !ok { + return nil, errors.New("unable to load metadata type") + } + metadataOp.Type = regoMetadataType(dataTypeStr) + } metadataOp.Name, ok = data["name"].(string) if !ok { return nil, errors.New("unable to load metadata name") @@ -296,29 +377,27 @@ func newRegoMetadataOperation(operation interface{}) (*regoMetadataOperation, er return nil, errors.New("unable to load metadata action") } metadataOp.Action = regoMetadataAction(action) - metadataOp.Key, ok = data["key"].(string) - if !ok { - return nil, errors.New("unable to load metadata key") - } - if metadataOp.Action != metadataRemove { - metadataOp.Value, ok = data["value"] + var hasKey, hasValue bool + key, hasKey := data["key"] + if hasKey { + metadataOp.Key, ok = key.(string) if !ok { - return nil, errors.New("unable to load metadata value") + return nil, errors.New("unable to load metadata key") } } - return &metadataOp, nil -} + metadataOp.Value, hasValue = data["value"] + + if (metadataOp.Action != metadataRemove || metadataOp.Type == metadataTypeSet) && !hasValue { + return nil, errors.New("missing metadata value") + } -func (m regoMetadata) getOrCreate(name string) map[string]interface{} { - if metadata, ok := m[name]; ok { - return metadata + if metadataOp.Type == metadataTypeMap && !hasKey { + return nil, errors.New("missing metadata key") } - metadata := make(map[string]interface{}) - m[name] = metadata - return metadata + return &metadataOp, nil } func (r *RegoPolicyInterpreter) UpdateOSType(os string) error { @@ -327,6 +406,7 @@ func (r *RegoPolicyInterpreter) UpdateOSType(os string) error { ops := []*regoMetadataOperation{ { Action: metadataAdd, + Type: metadataTypeMap, Name: "operatingsystem", Key: "ostype", Value: os, @@ -335,32 +415,66 @@ func (r *RegoPolicyInterpreter) UpdateOSType(os string) error { return r.updateMetadata(ops) } +// dataAndModulesMutex must be held before calling this func (r *RegoPolicyInterpreter) updateMetadata(ops []*regoMetadataOperation) error { - // dataAndModulesMutex must be held before calling this - - metadataRoot, ok := r.data[metadataRootKey].(regoMetadata) - if !ok { - return errors.New("illegal interpreter state: invalid metadata object type") - } - for _, op := range ops { - metadata := metadataRoot.getOrCreate(op.Name) - switch op.Action { - case metadataAdd: - if _, ok := metadata[op.Key]; ok { - return fmt.Errorf("cannot add metadata value, key %s[%s] already exists", op.Name, op.Key) - } else { - metadata[op.Key] = op.Value + switch op.Type { + case metadataTypeMap: + metadata, err := _getMetadata[regoMetadataMap](r, op.Name) + if err != nil { + return fmt.Errorf("unable to get metadata: %w", err) + } + if metadata == nil { + metadata = make(regoMetadataMap) } + switch op.Action { + case metadataAdd: + if _, ok := metadata[op.Key]; ok { + return fmt.Errorf("cannot add metadata value, key %s[%s] already exists", op.Name, op.Key) + } else { + metadata[op.Key] = op.Value + } + + case metadataUpdate: + metadata[op.Key] = op.Value - case metadataUpdate: - metadata[op.Key] = op.Value + case metadataRemove: + delete(metadata, op.Key) - case metadataRemove: - delete(metadata, op.Key) + default: + return fmt.Errorf("unrecognized metadata action: %s for map", op.Action) + } + if err := _setMetadata[regoMetadataMap](r, op.Name, metadata); err != nil { + return fmt.Errorf("unable to set metadata: %w", err) + } + case metadataTypeSet: + metadata, err := _getMetadata[regoMetadataSet](r, op.Name) + if err != nil { + return fmt.Errorf("unable to get metadata: %w", err) + } + if metadata == nil { + metadata = make(regoMetadataSet, 0) + } + switch op.Action { + case metadataAdd: + if !slices.ContainsFunc(metadata, func(e interface{}) bool { + return reflect.DeepEqual(e, op.Value) + }) { + metadata = append(metadata, op.Value) + } + case metadataRemove: + metadata = slices.DeleteFunc(metadata, func(e interface{}) bool { + return reflect.DeepEqual(e, op.Value) + }) + default: + return fmt.Errorf("unrecognized metadata action: %s for set", op.Action) + } + if err := _setMetadata[regoMetadataSet](r, op.Name, metadata); err != nil { + return fmt.Errorf("unable to set metadata: %w", err) + } default: - return fmt.Errorf("unrecognized metadata action: %s", op.Action) + return fmt.Errorf("unrecognized metadata type: %s", op.Type) } } @@ -513,6 +627,19 @@ func (r RegoQueryResult) Object(key string) (map[string]interface{}, error) { } } +// Array attempts to interpret the result value as an array. +func (r RegoQueryResult) Array(key string) ([]interface{}, error) { + if value, ok := r[key]; ok { + if arr, ok := value.([]interface{}); ok { + return arr, nil + } else { + return nil, fmt.Errorf("value for '%s' is not an array", key) + } + } else { + return nil, fmt.Errorf("unable to find value for key '%s'", key) + } +} + // Bool attempts to interpret a result value as a boolean. func (r RegoQueryResult) Bool(key string) (bool, error) { if value, ok := r[key]; ok { diff --git a/internal/regopolicyinterpreter/regopolicyinterpreter_test.go b/internal/regopolicyinterpreter/regopolicyinterpreter_test.go index 534b87e892..58703073d4 100644 --- a/internal/regopolicyinterpreter/regopolicyinterpreter_test.go +++ b/internal/regopolicyinterpreter/regopolicyinterpreter_test.go @@ -7,6 +7,7 @@ import ( "math/rand" "os" "reflect" + "slices" "strconv" "testing" "testing/quick" @@ -87,7 +88,7 @@ func Test_copyRegoMetadata(t *testing.T) { for name, origObject := range orig { if copyObject, ok := copy[name]; ok { - if !assertObjectsEqual(origObject, copyObject) { + if !assertMetadataValueEqual(t, origObject, copyObject) { t.Errorf("original and copy differ on key %s", name) return false } @@ -555,6 +556,147 @@ func Test_Module(t *testing.T) { } +func Test_Set(t *testing.T) { + rego, err := setupRego() + if err != nil { + t.Fatal(err) + } + + f := func(name metadataName, valuesToInsert testArray, nonExistantValue testArray) bool { + uniqueValuesToInsert := make([]interface{}, 0, len(valuesToInsert)) + for _, v := range valuesToInsert { + if !slices.ContainsFunc(uniqueValuesToInsert, func(e interface{}) bool { + return reflect.DeepEqual(e, v) + }) { + uniqueValuesToInsert = append(uniqueValuesToInsert, v) + } + } + + for _, v := range uniqueValuesToInsert { + contains, err := setContains(rego, name, v) + + if err != nil { + t.Errorf("error checking if set contains value %v: %v", v, err) + return false + } + if contains { + t.Errorf("set unexpectedly contains value %v before we added it", v) + return false + } + if err = setAdd(rego, name, v); err != nil { + t.Errorf("error adding value %v to set: %v", v, err) + return false + } + contains, err = setContains(rego, name, v) + if err != nil { + t.Errorf("error checking if set contains value %v: %v", v, err) + return false + } + if !contains { + t.Errorf("set does not contain value %v after we added it", v) + return false + } + } + + set, err := getSet(rego, name) + if err != nil { + t.Errorf("error retrieving set: %v", err) + return false + } + + if !assertArraysEqualsUnordered(uniqueValuesToInsert, set) { + t.Errorf("set does not match inserted values: %v != %v", set, uniqueValuesToInsert) + assertArraysEqualsUnordered(uniqueValuesToInsert, set) + return false + } + + for _, v := range nonExistantValue { + if slices.ContainsFunc(valuesToInsert, func(e interface{}) bool { + return reflect.DeepEqual(e, v) + }) { + continue + } + + contains, err := setContains(rego, name, v) + if err != nil { + t.Errorf("error checking if set contains value %v: %v", v, err) + return false + } + if contains { + t.Errorf("set unexpectedly contains value %v", v) + return false + } + if err = setRemove(rego, name, v); err != nil { + t.Errorf("error removing non-existant value %v from set: %v", v, err) + return false + } + } + + set, err = getSet(rego, name) + if err != nil { + t.Errorf("error retrieving set: %v", err) + return false + } + + if !assertArraysEqualsUnordered(uniqueValuesToInsert, set) { + t.Errorf("set does not match inserted values after removing non-existant values: %v != %v", set, uniqueValuesToInsert) + return false + } + + for _, v := range uniqueValuesToInsert { + err = setAdd(rego, name, v) + if err != nil { + t.Errorf("error adding value %v to set: %v", v, err) + return false + } + } + + set, err = getSet(rego, name) + if err != nil { + t.Errorf("error retrieving set: %v", err) + return false + } + + if !assertArraysEqualsUnordered(uniqueValuesToInsert, set) { + t.Errorf("set does not match inserted values after inserting duplicate values: %v != %v", set, uniqueValuesToInsert) + return false + } + + for _, v := range uniqueValuesToInsert { + err = setRemove(rego, name, v) + if err != nil { + t.Errorf("error removing value %v from set: %v", v, err) + return false + } + contains, err := setContains(rego, name, v) + if err != nil { + t.Errorf("error checking if set contains value %v: %v", v, err) + return false + } + if contains { + t.Errorf("set still contains value %v after we removed it", v) + return false + } + } + + set, err = getSet(rego, name) + if err != nil { + t.Errorf("error retrieving set: %v", err) + return false + } + + if len(set) != 0 { + t.Errorf("set is not empty after removing all values: %v", set) + return false + } + return true + } + + if err := quick.Check(f, &quick.Config{MaxCount: 100, Rand: testRand}); err != nil { + t.Errorf("quick.Check failed: %v", err) + } +} + // fixtures func setupRego() (*RegoPolicyInterpreter, error) { @@ -656,9 +798,9 @@ const ( ) func generateValue(r *rand.Rand, depth int) interface{} { - choices := []testValueType{testValueArray, testValueString, testValueFloat, testValueBool, testValueNull} + choices := []testValueType{testValueString, testValueBool, testValueNull} if depth < maxObjectDepth { - choices = append(choices, testValueObject) + choices = append(choices, testValueObject, testValueArray) } switch choices[r.Intn(len(choices))] { @@ -671,8 +813,8 @@ func generateValue(r *rand.Rand, depth int) interface{} { case testValueString: return randString(r) - case testValueFloat: - return r.Float64() + // case testValueFloat: + // return r.Float64() case testValueBool: return r.Intn(2) == 1 @@ -710,6 +852,11 @@ func (testValue) Generate(r *rand.Rand, _ int) reflect.Value { return reflect.ValueOf(value) } +func (testArray) Generate(r *rand.Rand, _ int) reflect.Value { + value := generateArray(r, 0) + return reflect.ValueOf(value) +} + func (testObject) Generate(r *rand.Rand, _ int) reflect.Value { value := generateObject(r, 0) return reflect.ValueOf(value) @@ -717,10 +864,15 @@ func (testObject) Generate(r *rand.Rand, _ int) reflect.Value { func (testRegoMetadata) Generate(r *rand.Rand, _ int) reflect.Value { numObjects := r.Intn(maxNumberOfFields) + numSets := r.Intn(maxNumberOfFields) metadata := make(testRegoMetadata) for i := 0; i < numObjects; i++ { name := uniqueString(r) - metadata[name] = generateObject(r, 0) + metadata[name] = regoMetadataMap(generateObject(r, 0)) + } + for i := 0; i < numSets; i++ { + name := uniqueString(r) + metadata[name] = regoMetadataSet(generateArray(r, 0)) } return reflect.ValueOf(metadata) } @@ -829,8 +981,76 @@ func computeGap(r *RegoPolicyInterpreter, name metadataName, expected int) error return nil } +func setAdd(r *RegoPolicyInterpreter, name metadataName, value interface{}) error { + input := map[string]interface{}{"name": string(name), "value": value} + result, err := r.Query("data.test.setAdd", input) + if err != nil { + return fmt.Errorf("received error when trying to query rego: %w", err) + } + + success, err := result.Bool("success") + if err != nil { + return errors.New("Expected result.success to be a bool") + } + + if !success { + return errors.New("set_add query failed unexpectedly") + } + + return nil +} + +func setRemove(r *RegoPolicyInterpreter, name metadataName, value interface{}) error { + input := map[string]interface{}{"name": string(name), "value": value} + result, err := r.Query("data.test.setRemove", input) + if err != nil { + return fmt.Errorf("received error when trying to query rego: %w", err) + } + + success, err := result.Bool("success") + if err != nil { + return errors.New("Expected result.success to be a bool") + } + + if !success { + return errors.New("set_remove query failed unexpectedly") + } + + return nil +} + +func setContains(r *RegoPolicyInterpreter, name metadataName, value interface{}) (bool, error) { + input := map[string]interface{}{"name": string(name), "value": value} + result, err := r.Query("data.test.setContains", input) + if err != nil { + return false, fmt.Errorf("received error when trying to query rego: %w", err) + } + + contains, err := result.Bool("result") + if err != nil { + return false, errors.New("Expected result.result to be a bool") + } + + return contains, nil +} + +func getSet(r *RegoPolicyInterpreter, name metadataName) ([]interface{}, error) { + input := map[string]interface{}{"name": string(name)} + result, err := r.Query("data.test.getSet", input) + if err != nil { + return nil, fmt.Errorf("received error when trying to query rego: %w", err) + } + + set, err := result.Array("result") + if err != nil { + return nil, errors.New("Expected result.result to be an array") + } + + return set, nil +} + func assertListEqual(r *RegoPolicyInterpreter, name metadataName, key string, expectedValues []int) error { - rawValues, err := r.GetMetadata(string(name), key) + rawValues, err := r.GetMetadataMapValue(string(name), key) if err != nil { return fmt.Errorf("unable to get metadata list %s: %w", name, err) } @@ -888,6 +1108,10 @@ func uniqueString(r *rand.Rand) string { } func assertValuesEqual(lhs interface{}, rhs interface{}) bool { + if reflect.DeepEqual(lhs, rhs) { + return true + } + if lhsObject, ok := lhs.(testObject); ok { if rhsObject, ok := rhs.(testObject); ok { return assertObjectsEqual(lhsObject, rhsObject) @@ -953,6 +1177,33 @@ func assertArraysEqual(lhs testArray, rhs testArray) bool { return true } +func assertArraysEqualsUnordered(lhs testArray, rhs testArray) bool { + if len(lhs) != len(rhs) { + return false + } + + if assertArraysEqual(lhs, rhs) { + return true + } + + used := make([]bool, len(rhs)) + for _, lhsValue := range lhs { + rhsIndex := -1 + for i, rhsValue := range rhs { + if !used[i] && assertValuesEqual(lhsValue, rhsValue) { + rhsIndex = i + break + } + } + if rhsIndex == -1 { + return false + } + used[rhsIndex] = true + } + + return true +} + func assertObjectsEqual(lhs testObject, rhs testObject) bool { if len(lhs) != len(rhs) { return false @@ -970,3 +1221,22 @@ func assertObjectsEqual(lhs testObject, rhs testObject) bool { return true } + +func assertMetadataValueEqual(t *testing.T, lhs interface{}, rhs interface{}) bool { + if lhsMap, ok := lhs.(regoMetadataMap); ok { + if rhsMap, ok := rhs.(regoMetadataMap); ok { + return assertObjectsEqual(testObject(lhsMap), testObject(rhsMap)) + } else { + return false + } + } else if lhsSet, ok := lhs.(regoMetadataSet); ok { + if rhsSet, ok := rhs.(regoMetadataSet); ok { + return assertArraysEqualsUnordered(testArray(lhsSet), testArray(rhsSet)) + } else { + return false + } + } else { + t.Errorf("lhs (type %v) passed to assertMetadataValueEqual is not a valid rego metadata value", reflect.TypeOf(lhs)) + return false + } +} diff --git a/internal/regopolicyinterpreter/test.rego b/internal/regopolicyinterpreter/test.rego index db6082e8c2..275b1bd3af 100644 --- a/internal/regopolicyinterpreter/test.rego +++ b/internal/regopolicyinterpreter/test.rego @@ -109,7 +109,40 @@ compute_gap := {"result": result, "metadata": [removeGreater, removeLesser]} { "name": input.name, "action": "remove", "key": "lesser" - } + } } subtract := data.module.subtract + +setAdd := {"success": true, "metadata": [addSet]} { + addSet := { + "name": input.name, + "type": "set", + "action": "add", + "value": { + "value": input.value + } + } +} + +setRemove := {"success": true, "metadata": [removeSet]} { + removeSet := { + "name": input.name, + "type": "set", + "action": "remove", + "value": { + "value": input.value + } + } +} + +default setContains := {"result": false} +setContains := {"result": true} { + data.metadata[input.name][_].value == input.value +} + +default getSet := {"result": []} +getSet := {"result": result} { + s := data.metadata[input.name] + result := [item.value | item := s[_]] +} diff --git a/pkg/securitypolicy/regopolicy_linux_test.go b/pkg/securitypolicy/regopolicy_linux_test.go index 51da87a18c..10f489ac1f 100644 --- a/pkg/securitypolicy/regopolicy_linux_test.go +++ b/pkg/securitypolicy/regopolicy_linux_test.go @@ -4524,7 +4524,7 @@ func expectFragmentNotLoaded(t *testing.T, policy *regoEnforcer, issuer, feed st t.Errorf("fragment module is present") return false } - mtdIssuer, err := policy.rego.GetMetadata("issuers", issuer) + mtdIssuer, err := policy.rego.GetMetadataMapValue("issuers", issuer) if err != nil && !strings.Contains(err.Error(), "value not found") && !strings.Contains(err.Error(), "metadata not found for name issuers") { t.Errorf("unexpected error when checking issuer metadata: %v", err) @@ -5094,7 +5094,7 @@ mount_device := data.fragment.mount_device t.Fatalf("unable to mount device: %v", err) } - if test, err := policy.rego.GetMetadata("custom", key); err == nil { + if test, err := policy.rego.GetMetadataMapValue("custom", key); err == nil { if test != value { t.Error("incorrect metadata value stored by fragment") } From 0fb3eba1c12ef3b6d19f04b950dea2189ec16437 Mon Sep 17 00:00:00 2001 From: Tingmao Wang Date: Fri, 19 Sep 2025 14:02:37 +0100 Subject: [PATCH 2/9] framework.rego: Refactor policy_fragments to remove code duplication and improve comments Signed-off-by: Tingmao Wang --- pkg/securitypolicy/framework.rego | 44 ++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/pkg/securitypolicy/framework.rego b/pkg/securitypolicy/framework.rego index 76e5b048a0..c22f625800 100644 --- a/pkg/securitypolicy/framework.rego +++ b/pkg/securitypolicy/framework.rego @@ -1186,6 +1186,28 @@ extract_fragment_includes(includes) := fragment { } } +# data.metadata.issuers is a map of maps that contains information loaded from fragments: +# { +# "did:issuer_1...": { +# "feeds": { +# "feed1": [ +# { +# // The extracted "includes" for a fragment with this issuer and feed, e.g.: +# "containers": [...], +# "fragments": [...], +# }, +# // if multiple fragments with the same issuer and feed exists, they go here +# ] +# } +# } +# } +# +# Rules like candidate_containers and candidate_fragments will read this map to +# gather all the allowed containers / nested fragments. +# +# This map does not contain any containers / fragments allowed by the top-level +# policy itself. The candidate_* rules need to combine both sources. + issuer_exists(iss) { data.metadata.issuers[iss] } @@ -1216,25 +1238,21 @@ update_issuer(includes) := issuer { issuer := {"feeds": {input.feed: [extract_fragment_includes(includes)]}} } -default candidate_fragments := [] +# The policy might not define the fragments variable, in which case we default +# to [] to prevent breaking other rules. +default policy_fragments := [] -candidate_fragments := fragments { +policy_fragments := pf { semver.compare(policy_framework_version, version) == 0 - - policy_fragments := [f | f := data.policy.fragments[_]] - fragment_fragments := [f | - feed := data.metadata.issuers[_].feeds[_] - fragment := feed[_] - f := fragment.fragments[_] - ] - - fragments := array.concat(policy_fragments, fragment_fragments) + pf := data.policy.fragments } -candidate_fragments := fragments { +policy_fragments := pf { semver.compare(policy_framework_version, version) < 0 + pf := apply_defaults("fragment", data.policy.fragments, policy_framework_version) +} - policy_fragments := apply_defaults("fragment", data.policy.fragments, policy_framework_version) +candidate_fragments := fragments { fragment_fragments := [f | feed := data.metadata.issuers[_].feeds[_] fragment := feed[_] From 04b3dc88ebb949f5130606a657c556480fd60073 Mon Sep 17 00:00:00 2001 From: Tingmao Wang Date: Thu, 30 Oct 2025 09:00:00 +0000 Subject: [PATCH 3/9] rego: Support env rules with separate name and value patterns and matching strategies This is to make it easier to parameterize environment rules. Currently, name and value for an environment rule are actually combined into one "pattern" field, and there is only one strategy for the combined pattern. This presents a problem when a fragment wants to delegate the decision of e.g. whether to match the value (but only the value, not the key) with a regex or with a fixed string. We split "pattern" and "strategy" out into "name", "name_strategy", "value" and "value_strategy" in order to allow more flexibility when fragment exposes env-var parameters. Signed-off-by: Tingmao Wang --- pkg/securitypolicy/framework.rego | 63 +++++++++++++++++++++++++++---- 1 file changed, 55 insertions(+), 8 deletions(-) diff --git a/pkg/securitypolicy/framework.rego b/pkg/securitypolicy/framework.rego index c22f625800..da7b383ce6 100644 --- a/pkg/securitypolicy/framework.rego +++ b/pkg/securitypolicy/framework.rego @@ -234,21 +234,68 @@ command_ok(command) { } } -env_ok(pattern, "string", value) { +# A env rule can be of two form: +# { +# "pattern": "name=value", +# "strategy": "string" | "re2" +# } +# or +# { +# "name": "name_pattern", +# "name_strategy": "string" | "re2", +# "value": "value_pattern", +# "value_strategy": "string" | "re2" +# } + +# env_pattern_ok(pattern, strategy, value) tests whether the given string +# pattern matches the input value. + +env_pattern_ok(pattern, "string", value) { pattern == value } -env_ok(pattern, "re2", value) { +env_pattern_ok(pattern, "re2", value) { regex.match(anchor_pattern(pattern), value) } +# env_rule_ok accepts both forms of env rules described above, and matches it +# against the given env string (of form name=value). + +env_rule_ok(rule, env) { + pattern := object.get(rule, "pattern", null) + strategy := object.get(rule, "strategy", null) + pattern != null + strategy != null + env_pattern_ok(pattern, strategy, env) +} + +env_rule_ok(rule, env) { + rule_name := object.get(rule, "name", null) + name_strategy := object.get(rule, "name_strategy", null) + rule_value := object.get(rule, "value", null) + value_strategy := object.get(rule, "value_strategy", null) + rule_name != null + name_strategy != null + rule_value != null + value_strategy != null + + # Split the env into name and value (value can contain '=', name cannot) + eq_idx := indexof(env, "=") + eq_idx >= 0 + env_name := substring(env, 0, eq_idx) + env_value := substring(env, eq_idx + 1, -1) + + env_pattern_ok(rule_name, name_strategy, env_name) + env_pattern_ok(rule_value, value_strategy, env_value) +} + rule_ok(rule, env) { not rule.required } rule_ok(rule, env) { rule.required - env_ok(rule.pattern, rule.strategy, env) + env_rule_ok(rule, env) } envList_ok(env_rules, envList) { @@ -259,7 +306,7 @@ envList_ok(env_rules, envList) { every env in envList { some rule in env_rules - env_ok(rule.pattern, rule.strategy, env) + env_rule_ok(rule, env) } } @@ -267,7 +314,7 @@ valid_envs_subset(env_rules) := envs { envs := {env | some env in input.envList some rule in env_rules - env_ok(rule.pattern, rule.strategy, env) + env_rule_ok(rule, env) } } @@ -1602,14 +1649,14 @@ env_matches(env) { input.rule in ["create_container", "exec_in_container"] some container in data.metadata.matches[input.containerID] some rule in container.env_rules - env_ok(rule.pattern, rule.strategy, env) + env_rule_ok(rule, env) } env_matches(env) { input.rule in ["exec_external"] some process in candidate_external_processes some rule in process.env_rules - env_ok(rule.pattern, rule.strategy, env) + env_rule_ok(rule, env) } errors[envError] { @@ -1627,7 +1674,7 @@ errors[envError] { env_rule_matches(rule) { some env in input.envList - env_ok(rule.pattern, rule.strategy, env) + env_rule_ok(rule, env) } errors["missing required environment variable"] { From cd89b069a3fa0621fdf6661135ccc4bfe84b977a Mon Sep 17 00:00:00 2001 From: Tingmao Wang Date: Wed, 29 Oct 2025 15:39:31 +0000 Subject: [PATCH 4/9] regopolicy_test: Add tests for name/value env rules Signed-off-by: Tingmao Wang --- pkg/securitypolicy/rego_utils_test.go | 100 ++++++++++++------- pkg/securitypolicy/regopolicy_linux_test.go | 27 ++--- pkg/securitypolicy/securitypolicy.go | 8 ++ pkg/securitypolicy/securitypolicy_marshal.go | 6 +- 4 files changed, 90 insertions(+), 51 deletions(-) diff --git a/pkg/securitypolicy/rego_utils_test.go b/pkg/securitypolicy/rego_utils_test.go index 6afe7de6cc..3736d54f6a 100644 --- a/pkg/securitypolicy/rego_utils_test.go +++ b/pkg/securitypolicy/rego_utils_test.go @@ -44,31 +44,32 @@ const ( maxPlan9MountIndex = 16 // variables that influence generated test fixtures - minStringLength = 10 - maxContainersInGeneratedConstraints = 32 - maxLayersInGeneratedContainer = 32 - maxGeneratedCommandLength = 128 - maxGeneratedCommandArgs = 12 - maxGeneratedEnvironmentVariables = 16 - maxGeneratedEnvironmentVariableRuleLength = 64 - maxGeneratedEnvironmentVariableRules = 8 - maxGeneratedFragmentNamespaceLength = 32 - maxGeneratedMountTargetLength = 256 - maxGeneratedVersion = 10 - rootHashLength = 64 - maxGeneratedMounts = 4 - maxGeneratedMountSourceLength = 32 - maxGeneratedMountDestinationLength = 32 - maxGeneratedMountOptions = 5 - maxGeneratedMountOptionLength = 32 - maxGeneratedExecProcesses = 4 - maxGeneratedWorkingDirLength = 128 - maxSignalNumber = 64 - maxGeneratedNameLength = 8 - maxGeneratedGroupNames = 4 - maxGeneratedCapabilities = 12 - maxGeneratedCapabilitesLength = 24 - maxWindowsSignalLength = 64 + minStringLength = 10 + maxContainersInGeneratedConstraints = 32 + maxLayersInGeneratedContainer = 32 + maxGeneratedCommandLength = 128 + maxGeneratedCommandArgs = 12 + maxGeneratedEnvironmentVariables = 16 + maxGeneratedEnvironmentVariableNameLength = 31 + maxGeneratedEnvironmentVariableValueLength = 32 + maxGeneratedEnvironmentVariableRules = 8 + maxGeneratedFragmentNamespaceLength = 32 + maxGeneratedMountTargetLength = 256 + maxGeneratedVersion = 10 + rootHashLength = 64 + maxGeneratedMounts = 4 + maxGeneratedMountSourceLength = 32 + maxGeneratedMountDestinationLength = 32 + maxGeneratedMountOptions = 5 + maxGeneratedMountOptionLength = 32 + maxGeneratedExecProcesses = 4 + maxGeneratedWorkingDirLength = 128 + maxSignalNumber = 64 + maxGeneratedNameLength = 8 + maxGeneratedGroupNames = 4 + maxGeneratedCapabilities = 12 + maxGeneratedCapabilitesLength = 24 + maxWindowsSignalLength = 64 // additional consts // the standard enforcer tests don't do anything with the encoded policy // string. this const exists to make that explicit @@ -2232,7 +2233,7 @@ func (*SecurityPolicy) Generate(r *rand.Rand, _ int) reflect.Value { for j := 0; j < numEnvRules; j++ { rule := EnvRuleConfig{ Strategy: "string", - Rule: randVariableString(r, maxGeneratedEnvironmentVariableRuleLength), + Rule: generateRandomEnvironmentVariable(r), Required: false, } c.EnvRules.Elements[strconv.Itoa(j)] = rule @@ -2428,9 +2429,18 @@ func generateEnvironmentVariableRules(r *rand.Rand) []EnvRuleConfig { numArgs := atLeastOneAtMost(r, maxGeneratedEnvironmentVariableRules) for i := 0; i < int(numArgs); i++ { - rule := EnvRuleConfig{ - Strategy: "string", - Rule: randVariableString(r, maxGeneratedEnvironmentVariableRuleLength), + var rule EnvRuleConfig + rule.UseNameValue = randBool(r) + name := randVariableString(r, maxGeneratedEnvironmentVariableNameLength) + value := randVariableString(r, maxGeneratedEnvironmentVariableValueLength) + if rule.UseNameValue { + rule.Name = name + rule.NameStrategy = EnvVarRuleString + rule.Value = value + rule.ValueStrategy = EnvVarRuleString + } else { + rule.Rule = fmt.Sprintf("%s=%s", name, value) + rule.Strategy = EnvVarRuleString } rules = append(rules, rule) } @@ -2438,6 +2448,12 @@ func generateEnvironmentVariableRules(r *rand.Rand) []EnvRuleConfig { return rules } +func generateRandomEnvironmentVariable(r *rand.Rand) string { + name := randVariableString(r, maxGeneratedEnvironmentVariableNameLength) + value := randVariableString(r, maxGeneratedEnvironmentVariableValueLength) + return fmt.Sprintf("%s=%s", name, value) +} + func generateExecProcesses(r *rand.Rand) []containerExecProcess { var processes []containerExecProcess @@ -2507,15 +2523,26 @@ func generateEnvironmentVariables(r *rand.Rand) []string { numVars := atLeastOneAtMost(r, maxGeneratedEnvironmentVariables) for i := 0; i < int(numVars); i++ { - variable := randVariableString(r, maxGeneratedEnvironmentVariableRuleLength) + variable := generateRandomEnvironmentVariable(r) envVars = append(envVars, variable) } return envVars } -func generateNeverMatchingEnvironmentVariable(r *rand.Rand) string { - return randString(r, maxGeneratedEnvironmentVariableRuleLength+1) +func envRuleToStr(rule EnvRuleConfig) string { + if rule.UseNameValue { + if strings.Contains(rule.Name, "=") { + panic(fmt.Sprintf("expected env rule name %q to not contain '='", rule.Name)) + } + return fmt.Sprintf("%s=%s", rule.Name, rule.Value) + } else { + return rule.Rule + } +} + +func hasRegexInRule(rule EnvRuleConfig) bool { + return rule.Strategy == EnvVarRuleRegex || rule.NameStrategy == EnvVarRuleRegex || rule.ValueStrategy == EnvVarRuleRegex } func buildEnvironmentVariablesFromEnvRules(rules []EnvRuleConfig, r *rand.Rand) []string { @@ -2533,8 +2560,8 @@ func buildEnvironmentVariablesFromEnvRules(rules []EnvRuleConfig, r *rand.Rand) // tests for _, rule := range rules { if rule.Required { - if rule.Strategy != EnvVarRuleRegex { - vars = append(vars, rule.Rule) + if !hasRegexInRule(rule) { + vars = append(vars, envRuleToStr(rule)) } numberOfMatches-- } @@ -2562,9 +2589,8 @@ func buildEnvironmentVariablesFromEnvRules(rules []EnvRuleConfig, r *rand.Rand) } } - // include it if it's not regex - if rules[anIndex].Strategy != EnvVarRuleRegex { - vars = append(vars, rules[anIndex].Rule) + if !hasRegexInRule(rules[anIndex]) { + vars = append(vars, envRuleToStr(rules[anIndex])) usedIndexes[anIndex] = struct{}{} } numberOfMatches-- diff --git a/pkg/securitypolicy/regopolicy_linux_test.go b/pkg/securitypolicy/regopolicy_linux_test.go index 10f489ac1f..8cb1a7c7e1 100644 --- a/pkg/securitypolicy/regopolicy_linux_test.go +++ b/pkg/securitypolicy/regopolicy_linux_test.go @@ -8,7 +8,6 @@ import ( "encoding/json" "errors" "fmt" - "math/rand" "os" "path" "path/filepath" @@ -1233,7 +1232,8 @@ func Test_Rego_EnforceEnvironmentVariablePolicy_NotAllMatches(t *testing.T) { return false } - envList := append(tc.envList, generateNeverMatchingEnvironmentVariable(testRand)) + // Generate a new random env var that will not be in the allowed list + envList := append(tc.envList, generateRandomEnvironmentVariable(testRand)) _, _, _, err = tc.policy.EnforceCreateContainerPolicy(p.ctx, tc.sandboxID, tc.containerID, tc.argList, envList, tc.workingDir, tc.mounts, false, tc.noNewPrivileges, tc.user, tc.groups, tc.umask, tc.capabilities, tc.seccomp) // not getting an error means something is broken @@ -1241,7 +1241,8 @@ func Test_Rego_EnforceEnvironmentVariablePolicy_NotAllMatches(t *testing.T) { return false } - return assertDecisionJSONContains(t, err, "invalid env list", envList[0]) + anyKeyInConstraints := strings.Split(envList[0], "=")[0] + return assertDecisionJSONContains(t, err, "invalid env list", anyKeyInConstraints) } if err := quick.Check(f, &quick.Config{MaxCount: 50, Rand: testRand}); err != nil { @@ -1481,7 +1482,11 @@ func Test_Rego_EnforceCreateContainer(t *testing.T) { _, _, _, err = tc.policy.EnforceCreateContainerPolicy(p.ctx, tc.sandboxID, tc.containerID, tc.argList, tc.envList, tc.workingDir, tc.mounts, false, tc.noNewPrivileges, tc.user, tc.groups, tc.umask, tc.capabilities, tc.seccomp) // getting an error means something is broken - return err == nil + if err != nil { + t.Error(err) + return false + } + return true } if err := quick.Check(f, &quick.Config{MaxCount: 50, Rand: testRand}); err != nil { @@ -3020,13 +3025,9 @@ exec_external := { "env_list": ["%s"] }` - generateEnv := func(r *rand.Rand) string { - return randVariableString(r, maxGeneratedEnvironmentVariableRuleLength) - } - generateEnvs := func(envSet stringSet) []string { numVars := atLeastOneAtMost(testRand, maxGeneratedEnvironmentVariableRules) - return envSet.randUniqueArray(testRand, generateEnv, numVars) + return envSet.randUniqueArray(testRand, generateRandomEnvironmentVariable, numVars) } testFunc := func(gc *generatedConstraints) bool { @@ -3184,7 +3185,7 @@ func Test_Rego_EnforceEnvironmentVariablePolicy_MissingRequired(t *testing.T) { // add a rule to re2 match requiredRule := EnvRuleConfig{ Strategy: "string", - Rule: randVariableString(testRand, maxGeneratedEnvironmentVariableRuleLength), + Rule: generateRandomEnvironmentVariable(testRand), Required: true, } @@ -6181,7 +6182,7 @@ func Test_Rego_Enforce_CreateContainer_RequiredEnvMissingHasErrorMessage(t *test container := selectContainerFromContainerList(constraints.containers, testRand) requiredRule := EnvRuleConfig{ Strategy: "string", - Rule: randVariableString(testRand, maxGeneratedEnvironmentVariableRuleLength), + Rule: generateRandomEnvironmentVariable(testRand), Required: true, } @@ -6435,7 +6436,7 @@ func Test_Rego_EnforceCreateContainer_RetryEverything(t *testing.T) { func Test_Rego_ExecInContainerPolicy_RequiredEnvMissingHasErrorMessage(t *testing.T) { constraints := generateConstraints(testRand, 1) container := selectContainerFromContainerList(constraints.containers, testRand) - neededEnv := randVariableString(testRand, maxGeneratedEnvironmentVariableRuleLength) + neededEnv := generateRandomEnvironmentVariable(testRand) requiredRule := EnvRuleConfig{ Strategy: "string", Rule: neededEnv, @@ -6481,7 +6482,7 @@ func Test_Rego_ExecInContainerPolicy_RequiredEnvMissingHasErrorMessage(t *testin func Test_Rego_ExecExternalProcessPolicy_RequiredEnvMissingHasErrorMessage(t *testing.T) { constraints := generateConstraints(testRand, 1) process := generateExternalProcess(testRand) - neededEnv := randVariableString(testRand, maxGeneratedEnvironmentVariableRuleLength) + neededEnv := generateRandomEnvironmentVariable(testRand) requiredRule := EnvRuleConfig{ Strategy: "string", Rule: neededEnv, diff --git a/pkg/securitypolicy/securitypolicy.go b/pkg/securitypolicy/securitypolicy.go index f1d761d439..ebc439ba51 100644 --- a/pkg/securitypolicy/securitypolicy.go +++ b/pkg/securitypolicy/securitypolicy.go @@ -107,6 +107,14 @@ type EnvRuleConfig struct { Strategy EnvVarRule `json:"strategy" toml:"strategy"` Rule string `json:"rule" toml:"rule"` Required bool `json:"required" toml:"required"` + + // If UseNameValue is true, the marshalled Rego will use rules with name and + // value separately, and ignore .Rule and .Strategy. + UseNameValue bool + Name string + NameStrategy EnvVarRule + Value string + ValueStrategy EnvVarRule } type IDNameConfig struct { diff --git a/pkg/securitypolicy/securitypolicy_marshal.go b/pkg/securitypolicy/securitypolicy_marshal.go index 665dc9e4f0..01adc59cb9 100644 --- a/pkg/securitypolicy/securitypolicy_marshal.go +++ b/pkg/securitypolicy/securitypolicy_marshal.go @@ -370,7 +370,11 @@ func writeCommand(builder *strings.Builder, command []string, indent string) { } func (e EnvRuleConfig) marshalRego() string { - return fmt.Sprintf("{\"pattern\": `%s`, \"strategy\": \"%s\", \"required\": %v}", e.Rule, e.Strategy, e.Required) + if e.UseNameValue { + return fmt.Sprintf("{\"name\": `%s`, \"name_strategy\": \"%s\", \"value\": `%s`, \"value_strategy\": \"%s\", \"required\": %v}", e.Name, e.NameStrategy, e.Value, e.ValueStrategy, e.Required) + } else { + return fmt.Sprintf("{\"pattern\": `%s`, \"strategy\": \"%s\", \"required\": %v}", e.Rule, e.Strategy, e.Required) + } } type envRuleArray []EnvRuleConfig From 3a955527a585096106ecc5b63533ba27926e4093 Mon Sep 17 00:00:00 2001 From: Tingmao Wang Date: Fri, 19 Sep 2025 14:06:33 +0100 Subject: [PATCH 5/9] rego: Implement fragment parameters This commit implements the support for the "parameters" feature for fragments. This is achieved by having the framework extract the parameters object from fragment import statements, and storing them in a set, tagged with the applicable (issuer, feed) pair. On future fragment loads, if the fragment's issuer and feed pair matches with any previous entry from this set, the parameters will be passed to the fragment via an automatically injected object added to the end of the fragment's Rego source (__fragment_parameters). (Note that in reality, we need to combine this set with the import statements defined in the main policy. c.f. candidate_fragments. This is done in fragment_parameters_for) In order for fragments to use this parameter object, it is expected that all fragments will now have an additional "stub" inserted at runtime to define the parameter() function, which will, under the hood, reference a "hidden" variable __fragment_parameters, also inserted dynamically at runtime, containing the actual parameter values. If multiple fragment parameters are specified, all combinations of values are considered "allowed", and therefore the fragment injection is repeated, passing in different parameter combinations, one for each combination. This means that if a fragment defines, for example: containers := [ { ... "env_rules": [ { "name": "SOME_KEY", "name_strategy": "string", "value": parameter("my_param"), "value_strategy": "string" } ] } ] and we have two fragment import statement for this fragment, e.g.: fragments := [ { "feed": ..., "issuer": ..., "minimum_svn": ..., "parameters": { "my_param": "value1" } }, { "feed": ..., "issuer": ..., "minimum_svn": ..., "parameters": { "my_param": "value2" } } ] Then the container can be started with either SOME_KEY=value1 or SOME_KEY=value2. Signed-off-by: Tingmao Wang --- Changes: - Fix missing [_] - fix wrong usage of defer - Inject fragment parameter function definitions at runtime: We must inject this at runtime so as to avoid having to support this exact implementation (eg __fragment_parameters[name]) of fetching the parameters forever (as it will essentially be hard-coded in the generated policy). - Move the actual implementation of parameter() into the framework so that fragments doesn't have to say 'import future.keywords.every', 'import future.keywords.in'. - Use indirection with default when accessing fragment.parameters to not break older fragments. If we don't do this, loading a fragment without the parameters definition would fail due to "unsafe" rego variable references. - Fix broken tests caused by the new extract_parameter framework function Signed-off-by: Tingmao Wang --- pkg/securitypolicy/fragment_definition.rego | 5 + pkg/securitypolicy/framework.rego | 99 +++++++++++++- pkg/securitypolicy/regopolicy_linux_test.go | 14 ++ .../securitypolicyenforcer_rego.go | 129 ++++++++++++++++-- 4 files changed, 236 insertions(+), 11 deletions(-) create mode 100644 pkg/securitypolicy/fragment_definition.rego diff --git a/pkg/securitypolicy/fragment_definition.rego b/pkg/securitypolicy/fragment_definition.rego new file mode 100644 index 0000000000..e217452fad --- /dev/null +++ b/pkg/securitypolicy/fragment_definition.rego @@ -0,0 +1,5 @@ +default __fragment_parameters_metadata := {} +__fragment_parameters_metadata := data[input.namespace].parameters { + data[input.namespace].parameters +} +parameter(name) := data.framework.extract_parameter(name, __fragment_parameters, __fragment_parameters_metadata) diff --git a/pkg/securitypolicy/framework.rego b/pkg/securitypolicy/framework.rego index da7b383ce6..4bd1ab04c6 100644 --- a/pkg/securitypolicy/framework.rego +++ b/pkg/securitypolicy/framework.rego @@ -1299,6 +1299,65 @@ policy_fragments := pf { pf := apply_defaults("fragment", data.policy.fragments, policy_framework_version) } +# data.metadata.fragment_parameters is a set of {issuer, feed, parameters} +# objects, representing possible parameters for nested fragments. (There can be +# duplicate issuer and feeds, All possible parameters will be tried on load of +# the respective fragment.) +# [ +# { +# "issuer": "did:issuer_1...", +# "feed": "feed1", +# "parameters": { +# "foo": "foo_standard", +# "bar": "bar_standard", +# } +# }, +# { +# "issuer": "did:issuer_1...", +# "feed": "feed1", +# "parameters": { +# "foo": "foo_premium", +# "bar": "bar_premium", +# } +# }, +# { +# "issuer": "did:issuer_2...", +# "feed": "feed2", +# "parameters": { +# ... +# } +# } +# ] +# +# This set does not contains any parameters specified by the top-level policy. +# Readers must combine both sources. +# +# Note that although both the issuers map and the fragment_parameters set are +# updated during fragment load, issuers represents the information extracted +# from _already loaded_ fragments (and hence it will only contain (issuer, feed) +# pairs which the host has injected a fragment for). The fragment_parameters +# set represents what parameters to use when a fragment is loaded later on, so +# it contains (issuer, feed) pairs for not-yet-loaded fragments. + +default fragment_parameters_for(_, _) := [] + +fragment_parameters_for(iss, feed) := params { + params_nested := [ + p.parameters + | p := data.metadata.fragment_parameters[_] + p.issuer == iss + p.feed == feed + ] + params_policy := [ + p.parameters + | p := policy_fragments[_] + p.issuer == iss + p.feed == feed + p.parameters + ] + params := array.concat(params_nested, params_policy) +} + candidate_fragments := fragments { fragment_fragments := [f | feed := data.metadata.issuers[_].feeds[_] @@ -1335,13 +1394,14 @@ default load_fragment := {"allowed": false} # point we can check the SVN defined in the fragment is valid, and if # successful, add the fragment to the metadata. -load_fragment := {"allowed": true} { +load_fragment := {"allowed": true, "parameters": possibleParams} { not input.fragment_loaded some fragment in candidate_fragments fragment_issuer_feed_ok(fragment) + possibleParams := fragment_parameters_for(fragment.issuer, fragment.feed) } -load_fragment := {"metadata": [updateIssuer], "add_module": add_module, "allowed": true} { +load_fragment := {"metadata": array.concat([updateIssuer], updateParameters), "add_module": add_module, "allowed": true} { input.fragment_loaded some fragment in candidate_fragments fragment_issuer_feed_ok(fragment) @@ -1355,6 +1415,22 @@ load_fragment := {"metadata": [updateIssuer], "add_module": add_module, "allowed "value": issuer, } + updateParameters := [ + { + "name": "fragment_parameters", + "type": "set", + "action": "add", + "value": fp, + } + | fragment := fragment_fragments[_] + fragment.parameters + fp := { + "issuer": fragment.issuer, + "feed": fragment.feed, + "parameters": fragment.parameters + } + ] + add_module := "namespace" in fragment.includes } @@ -1508,6 +1584,16 @@ registry_changes := {"allowed": true} { } } +# This is a helper function that will be used by the parameter() function +# injected into fragments, and is not otherwise intended to be called by user +# directly. + +extract_parameter(name, fragment_parameters_obj, parameters_metadata) := fragment_parameters_obj[name] { + name in object.keys(fragment_parameters_obj) +} else := parameters_metadata[name]["default"] { + "default" in object.keys(parameters_metadata[name]) +} + reason := { "errors": errors, "error_objects": error_objects @@ -2396,6 +2482,15 @@ check_fragment(raw_fragment, framework_version) := fragment { "feed": raw_fragment.feed, "minimum_svn": raw_fragment.minimum_svn, "includes": raw_fragment.includes, + + # The "parameters" field was added in 0.3.2, but we really do not want + # to silently ignore it if is provided in a policy mistakenly using an + # older framework_version, since it is restrictive. Therefore, instead + # of doing a check_fragment_parameters function which returns {} if the + # policy's framework_version is lower, we simply do an object.get to + # default it, but set the value if it exists. + "parameters": object.get(raw_fragment, "parameters", {}), + # Additional fields need to have default logic applied } } diff --git a/pkg/securitypolicy/regopolicy_linux_test.go b/pkg/securitypolicy/regopolicy_linux_test.go index 8cb1a7c7e1..0c22005636 100644 --- a/pkg/securitypolicy/regopolicy_linux_test.go +++ b/pkg/securitypolicy/regopolicy_linux_test.go @@ -4561,6 +4561,7 @@ enforcement_point_info := { "default_results": {"allowed": true}, "use_framework": true } +default extract_parameter(_, _, _) := "" `, fragment.info.minimumSVN, frameworkVersion) err = tc.policy.LoadFragment(p.ctx, fragment.info.issuer, fragment.info.feed, code) @@ -5124,6 +5125,8 @@ load_fragment := {"allowed": true, "add_module": true} data.framework.load_fragment := {"allowed": true, "add_module": true} input.issuer := "%s" data.framework.input.issuer := "%s" +default extract_parameter(_, _, _) := "" +data.framework.extract_parameter(a, b, c) := extract_parameter(a, b, c) `, fragment.info.minimumSVN, frameworkVersion, expectedIssuer, expectedIssuer) err = tc.policy.LoadFragment(p.ctx, actualIssuer, fragment.info.feed, code) @@ -5174,6 +5177,8 @@ enforcement_point_info := { "use_framework": true } data.framework.load_fragment := load_fragment +default extract_parameter(_, _, _) := "" +data.framework.extract_parameter(a, b, c) := extract_parameter(a, b, c) `, fragment.constraints.svn, frameworkVersion) err = tc.policy.LoadFragment(p.ctx, fragment.info.issuer, fragment.info.feed, code) @@ -7016,6 +7021,15 @@ func Test_Rego_Fragment_FrameworkSVN(t *testing.T) { fragmentConstraints.svn = mustIncrementSVN(gc.fragments[0].minimumSVN) code := fragmentConstraints.toFragment().marshalRego() + // Simulate what the actual fragment loading code does. We need to add this + // definition even if there are no arguments and the main fragment code does + // not use the parameter functions, otherwise it will fail Rego compilation + // with unsafe variable error. + code, err = getRegoWithParameterDefinitions(code, make(map[string]interface{})) + if err != nil { + t.Fatalf("Error adding parameter definitions to fragment rego: %v", err) + } + policy.rego.AddModule(fragmentConstraints.namespace, &rpi.RegoModule{ Namespace: fragmentConstraints.namespace, Feed: gc.fragments[0].feed, diff --git a/pkg/securitypolicy/securitypolicyenforcer_rego.go b/pkg/securitypolicy/securitypolicyenforcer_rego.go index 5e196ebd9a..28e18dd7c3 100644 --- a/pkg/securitypolicy/securitypolicyenforcer_rego.go +++ b/pkg/securitypolicy/securitypolicyenforcer_rego.go @@ -4,6 +4,7 @@ package securitypolicy import ( + "bytes" "context" _ "embed" "encoding/base64" @@ -21,6 +22,7 @@ import ( rpi "github.com/Microsoft/hcsshim/internal/regopolicyinterpreter" oci "github.com/opencontainers/runtime-spec/specs-go" "github.com/pkg/errors" + "github.com/sirupsen/logrus" ) const regoEnforcerName = "rego" @@ -41,6 +43,11 @@ const invalidPolicyMessage = "Security policy is not valid. Please check securit const noReasonMessage = "Security policy is either not valid or did not provide a reason for denial. Please check security policy or re-generate with tooling." const noAPIVersionError = "policy does not define api_version" +// Rego code injected at runtime to fragments to support parameters. +// +//go:embed fragment_definition.rego +var fragmentDefinitionRego string + // RegoEnforcer is a stub implementation of a security policy, which will be // based on [Rego] policy language. The detailed implementation will be // introduced in the subsequent PRs and documentation updated accordingly. @@ -1088,6 +1095,33 @@ func parseNamespace(rego string) (string, error) { return namespace, nil } +// Inject __fragment_parameters object definition and parameter function +// definitions into the Rego code. +// +// This function adds the injected object and functions to the end of the +// provided Rego code, and returns completely the modified Rego. +// +// Order doesn't matter in Rego, but we add it to the end to avoid changing line +// numbers, in case there is a syntax error in the original Rego code that needs +// to be reported. +func getRegoWithParameterDefinitions(rego string, parameters map[string]interface{}) (string, error) { + var buffer bytes.Buffer + buffer.WriteString(rego) + buffer.WriteString("\n") + parametersObjectJson, err := json.Marshal(parameters) + if err != nil { + return "", errors.Errorf("unable to marshal parameters object: %v", err) + } + buffer.WriteString( + fmt.Sprintf( + "__fragment_parameters := %s\n%s\n", + string(parametersObjectJson), + fragmentDefinitionRego, + ), + ) + return buffer.String(), nil +} + func (policy *regoEnforcer) LoadFragment(ctx context.Context, issuer string, feed string, rego string) error { namespace, err := parseNamespace(rego) if err != nil { @@ -1109,12 +1143,32 @@ func (policy *regoEnforcer) LoadFragment(ctx context.Context, issuer string, fee } // Check that the fragment is signed by the expected issuer before loading - // its Rego code. - _, err = policy.enforce(ctx, "load_fragment", input) + // its Rego code. This step also gives us a chance for Rego to pass any + // parameters object(s) declared in the fragment import statement to us. + res, err := policy.enforce(ctx, "load_fragment", input) if err != nil { return err } + parameters := make([]map[string]interface{}, 0) + _, hasParameters := res["parameters"] + + // Older policies which overrides load_fragment with their own code might + // not produce result.parameters + if hasParameters { + gotParameters, err := res.Array("parameters") + if err != nil { + return errors.Wrapf(err, "unable to get parameters from load_fragment result") + } + for _, gotParams := range gotParameters { + params, ok := gotParams.(map[string]interface{}) + if !ok { + return fmt.Errorf("parameters must be an object, got %T", gotParams) + } + parameters = append(parameters, params) + } + } + // At this point we need to add the fragment code as a new Rego module in // order for the framework (or any user defined policies) to check the SVN, // and potentially other information defined by its Rego code. We've already @@ -1122,17 +1176,74 @@ func (policy *regoEnforcer) LoadFragment(ctx context.Context, issuer string, fee // to load (won't override framework or other built-in modules). Once we // added the module, we must make sure the module is removed if we return // with error (or if add_module returned from Rego is false). - policy.rego.AddModule(fragment.ID(), fragment) + input["fragment_loaded"] = true - results, err := policy.enforce(ctx, "load_fragment", input) - if err != nil { - policy.rego.RemoveModule(fragment.ID()) - return err + if len(parameters) == 0 { + // We still want to load the fragment even if no parameters are defined. + // We apply a default of {} in check_fragment, so we shouldn't get here + // unless the policy overrides the load_fragment enforcement point with + // its own implementation. + parameters = append(parameters, make(map[string]interface{})) } - addModule, _ := results.Bool("add_module") - if !addModule { + // We load the module once for each possible parameter combinations, in + // order to capture all allowed container configurations. + for _, params := range parameters { + parameterKeys := make([]string, 0, len(params)) + for k := range params { + parameterKeys = append(parameterKeys, k) + } + log.G(ctx).WithFields(logrus.Fields{ + "namespace": namespace, + // Don't actually print the parameters, since they might be + // sensitive. + "parameterKeys": strings.Join(parameterKeys, ","), + }).Debugf("Loading fragment module with parameters") + + // We want to add the parameter functions regardless of whether any + // parameters are actually provided by the parent policy or not, to + // avoid undefined rules in case the fragment uses the parameter + // functions. + patchedRego, err := getRegoWithParameterDefinitions(rego, params) + if err != nil { + return fmt.Errorf("unable to patch fragment rego: %w", err) + } + + log.G(ctx).WithFields(logrus.Fields{ + "originalLength": len(rego), + "patchedLength": len(patchedRego), + }).Debug("Injected parameters object to fragment rego") + + newFragment := &rpi.RegoModule{ + Issuer: issuer, + Feed: feed, + Code: patchedRego, + Namespace: namespace, + } + policy.rego.AddModule(fragment.ID(), newFragment) + + // The module must be removed by the end of this iteration (or when we + // return with error), unless add_module in the result is true (in which + // case we make sure we only ever add one module) + + results, err := policy.enforce(ctx, "load_fragment", input) + if err != nil { + policy.rego.RemoveModule(fragment.ID()) + return err + } + + addModule, _ := results.Bool("add_module") + if addModule { + if len(parameters) > 1 { + policy.rego.RemoveModule(fragment.ID()) + return errors.New("Fragment cannot include namespace if multiple possible parameter combinations are defined") + } + // len(parameters) == 1, the loop would not run again anyway. We do + // this so we skip the RemoveModule below. + return nil + } + policy.rego.RemoveModule(fragment.ID()) } From 62b8bda9b785a4f33d5fe8e726336343931ecd6a Mon Sep 17 00:00:00 2001 From: Tingmao Wang Date: Tue, 4 Nov 2025 10:05:38 +0000 Subject: [PATCH 6/9] rego: Increment framework version to 0.5.0 Signed-off-by: Tingmao Wang --- pkg/securitypolicy/version_framework | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/securitypolicy/version_framework b/pkg/securitypolicy/version_framework index 267577d47e..8f0916f768 100644 --- a/pkg/securitypolicy/version_framework +++ b/pkg/securitypolicy/version_framework @@ -1 +1 @@ -0.4.1 +0.5.0 From db11eda6e6acf25f2aa1892849e1fdefb600016f Mon Sep 17 00:00:00 2001 From: Tingmao Wang Date: Tue, 4 Nov 2025 18:30:52 +0000 Subject: [PATCH 7/9] rego: Fix missing error message when starting container with mismatching env list If every element in the input env list matches some env rule in some container, but there is no container that matches the entire env list, we currently deny correctly but returns no error message. Signed-off-by: Tingmao Wang --- pkg/securitypolicy/framework.rego | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/pkg/securitypolicy/framework.rego b/pkg/securitypolicy/framework.rego index 4bd1ab04c6..8250936897 100644 --- a/pkg/securitypolicy/framework.rego +++ b/pkg/securitypolicy/framework.rego @@ -1854,6 +1854,27 @@ errors["missing required environment variable"] { count(processes) > 0 } +# All environment variables matches some rule in some container, but there are +# no containers with exactly the given combination of rules (i.e. for every +# container, there is at least one mismatching rule). +errors["invalid env list"] { + input.rule in ["create_container"] + + every container in data.metadata.matches[input.containerID] { + noNewPrivileges_ok(container.no_new_privileges) + user_ok(container.user) + privileged_ok(container.allow_elevated) + workingDirectory_ok(container.working_dir) + command_ok(container.command) + mountList_ok(container.mounts, container.allow_elevated) + + some env_in in input.envList + every rule in container.env_rules { + not env_rule_ok(rule, env_in) + } + } +} + default workingDirectory_matches := false workingDirectory_matches { From 697019337baa0b459ab125347ab546b1be4b5d43 Mon Sep 17 00:00:00 2001 From: Tingmao Wang Date: Tue, 4 Nov 2025 14:29:42 +0000 Subject: [PATCH 8/9] regopolicy_test: Add tests for fragment parameters This will test for scenarios like different fragments using the same parameter name, multiple parameter combinations, correct parameter passing for nested fragments, and make sure container creation is denied if the parameter and the given environment / command doesn't match. Signed-off-by: Tingmao Wang --- .../_container_common.rego.inc | 28 + .../env_rule_param.rego | 66 ++ .../env_rule_param_another_fragment.rego | 35 + .../nested_fragment.rego | 39 + .../nested_importer.rego | 41 + .../nested_importer_2.rego | 41 + .../param_on_command.rego | 32 + .../simple_env_rule_param.rego | 35 + pkg/securitypolicy/regopolicy_linux_test.go | 725 ++++++++++++++++++ pkg/securitypolicy/securitypolicy_internal.go | 1 + pkg/securitypolicy/securitypolicy_marshal.go | 14 +- 11 files changed, 1055 insertions(+), 2 deletions(-) create mode 100644 pkg/securitypolicy/fragment_test_policies/_container_common.rego.inc create mode 100644 pkg/securitypolicy/fragment_test_policies/env_rule_param.rego create mode 100644 pkg/securitypolicy/fragment_test_policies/env_rule_param_another_fragment.rego create mode 100644 pkg/securitypolicy/fragment_test_policies/nested_fragment.rego create mode 100644 pkg/securitypolicy/fragment_test_policies/nested_importer.rego create mode 100644 pkg/securitypolicy/fragment_test_policies/nested_importer_2.rego create mode 100644 pkg/securitypolicy/fragment_test_policies/param_on_command.rego create mode 100644 pkg/securitypolicy/fragment_test_policies/simple_env_rule_param.rego diff --git a/pkg/securitypolicy/fragment_test_policies/_container_common.rego.inc b/pkg/securitypolicy/fragment_test_policies/_container_common.rego.inc new file mode 100644 index 0000000000..9872e5f1ef --- /dev/null +++ b/pkg/securitypolicy/fragment_test_policies/_container_common.rego.inc @@ -0,0 +1,28 @@ + "allow_elevated": false, + "allow_stdio_access": true, + "capabilities": { + "ambient": [], + "bounding": [], + "effective": [], + "inheritable": [], + "permitted": [] + }, + "id": "container_id:tag", + "name": "sidecarcontainer", + "no_new_privileges": false, + "seccomp_profile_sha256": "", + "signals": [], + "user": { + "group_idnames": [ + { + "pattern": "", + "strategy": "any" + } + ], + "umask": "0022", + "user_idname": { + "pattern": "", + "strategy": "any" + } + }, + "working_dir": "/", diff --git a/pkg/securitypolicy/fragment_test_policies/env_rule_param.rego b/pkg/securitypolicy/fragment_test_policies/env_rule_param.rego new file mode 100644 index 0000000000..d4fb074bc1 --- /dev/null +++ b/pkg/securitypolicy/fragment_test_policies/env_rule_param.rego @@ -0,0 +1,66 @@ +package fragment + +svn := "1" +framework_version := "0.5.0" + +parameters := { + "env_param": { + "default": { + "value": "default_value", + "value_strategy": "string" + } + }, + "env_param_nodefault": { + }, + "env_string_param": { + "default": "default_string_value" + }, + "env_string_param_nodefault": { + } +} + +containers := [ + { + @@CONTAINER_COMMON@@ + "command": [ + "init" + ], + "env_rules": [ + { + "name": "ENV_VAR_FIXED", + "name_strategy": "string", + "value": "fixed_value", + "value_strategy": "string" + }, + { + "name": "ENV_VAR_PARAMETER", + "name_strategy": "string", + "value": parameter("env_param").value, + "value_strategy": parameter("env_param").value_strategy + }, + { + "name": "ENV_VAR_PARAMETER_NODEFAULT", + "name_strategy": "string", + "value": parameter("env_param_nodefault").value, + "value_strategy": parameter("env_param_nodefault").value_strategy + }, + { + "name": "ENV_STRING_PARAM", + "name_strategy": "string", + "value": parameter("env_string_param"), + "value_strategy": "string" + }, + { + "name": "ENV_STRING_PARAM_NODEFAULT", + "name_strategy": "string", + "value": parameter("env_string_param_nodefault"), + "value_strategy": "string" + } + ], + "exec_processes": [], + "layers": [ + "0000000000000000000000000000000000000000000000000000000000000000" + ], + "mounts": [] + } +] diff --git a/pkg/securitypolicy/fragment_test_policies/env_rule_param_another_fragment.rego b/pkg/securitypolicy/fragment_test_policies/env_rule_param_another_fragment.rego new file mode 100644 index 0000000000..6c2f89619f --- /dev/null +++ b/pkg/securitypolicy/fragment_test_policies/env_rule_param_another_fragment.rego @@ -0,0 +1,35 @@ +package fragment + +svn := "1" +framework_version := "0.5.0" + +parameters := { + "env_param": { + "default": { + "value": ".+", + "value_strategy": "re2" + } + } +} + +containers := [ + { + @@CONTAINER_COMMON@@ + "command": [ + "init" + ], + "env_rules": [ + { + "name": "ENV_VAR_PARAMETER", + "name_strategy": "string", + "value": parameter("env_param").value, + "value_strategy": parameter("env_param").value_strategy + } + ], + "exec_processes": [], + "layers": [ + "1111111111111111111111111111111111111111111111111111111111111111" + ], + "mounts": [] + } +] diff --git a/pkg/securitypolicy/fragment_test_policies/nested_fragment.rego b/pkg/securitypolicy/fragment_test_policies/nested_fragment.rego new file mode 100644 index 0000000000..71a3b1cb7e --- /dev/null +++ b/pkg/securitypolicy/fragment_test_policies/nested_fragment.rego @@ -0,0 +1,39 @@ +package fragment + +svn := "1" +framework_version := "0.5.0" + +parameters := { + "l1_param": {}, + "l2_param": { + "default": "l2param_default" + } +} + +containers := [ + { + @@CONTAINER_COMMON@@ + "command": [ + "init" + ], + "env_rules": [ + { + "name": "L1_PARAM", + "name_strategy": "string", + "value": parameter("l1_param"), + "value_strategy": "string" + }, + { + "name": "L2_PARAM", + "name_strategy": "string", + "value": parameter("l2_param"), + "value_strategy": "string" + } + ], + "exec_processes": [], + "layers": [ + "2222222222222222222222222222222222222222222222222222222222222222" + ], + "mounts": [] + } +] diff --git a/pkg/securitypolicy/fragment_test_policies/nested_importer.rego b/pkg/securitypolicy/fragment_test_policies/nested_importer.rego new file mode 100644 index 0000000000..2e9ea09db8 --- /dev/null +++ b/pkg/securitypolicy/fragment_test_policies/nested_importer.rego @@ -0,0 +1,41 @@ +package fragment + +svn := "1" +framework_version := "0.5.0" + +parameters := { + "l1_param": { + "default": "l1param_default" + } +} + +fragments := [ + { + "feed": "nested_fragment", + "includes": [ + "containers", + "fragments" + ], + "issuer": "nested:issuer", + "minimum_svn": "1", + "parameters": { + "l1_param": parameter("l1_param"), + "l2_param": "l2param_from_l1_1" + } + }, + { + "feed": "nested_fragment", + "includes": [ + "containers", + "fragments" + ], + "issuer": "nested:issuer", + "minimum_svn": "1", + "parameters": { + "l1_param": parameter("l1_param"), + "l2_param": "l2param_from_l1_2" + } + } +] + +containers := [] diff --git a/pkg/securitypolicy/fragment_test_policies/nested_importer_2.rego b/pkg/securitypolicy/fragment_test_policies/nested_importer_2.rego new file mode 100644 index 0000000000..90ef51f86c --- /dev/null +++ b/pkg/securitypolicy/fragment_test_policies/nested_importer_2.rego @@ -0,0 +1,41 @@ +package fragment + +svn := "1" +framework_version := "0.5.0" + +parameters := { + "l1_param": { + "default": "l1param_default_2" + } +} + +fragments := [ + { + "feed": "nested_fragment", + "includes": [ + "containers", + "fragments" + ], + "issuer": "nested:issuer", + "minimum_svn": "1", + "parameters": { + "l1_param": parameter("l1_param"), + "l2_param": "l2param_from_l1_1" + } + }, + { + "feed": "nested_fragment", + "includes": [ + "containers", + "fragments" + ], + "issuer": "nested:issuer", + "minimum_svn": "1", + "parameters": { + "l1_param": parameter("l1_param"), + "l2_param": "l2param_from_l1_2" + } + } +] + +containers := [] diff --git a/pkg/securitypolicy/fragment_test_policies/param_on_command.rego b/pkg/securitypolicy/fragment_test_policies/param_on_command.rego new file mode 100644 index 0000000000..3603c5c593 --- /dev/null +++ b/pkg/securitypolicy/fragment_test_policies/param_on_command.rego @@ -0,0 +1,32 @@ +package fragment + +svn := "1" +framework_version := "0.5.0" + +parameters := { + "command_param": { + "default": [ + "init" + ] + } +} + +containers := [ + { + @@CONTAINER_COMMON@@ + "command": parameter("command_param"), + "env_rules": [ + { + "name": "MY_ENV", + "name_strategy": "string", + "value": "1", + "value_strategy": "string" + } + ], + "exec_processes": [], + "layers": [ + "0000000000000000000000000000000000000000000000000000000000000000" + ], + "mounts": [] + } +] diff --git a/pkg/securitypolicy/fragment_test_policies/simple_env_rule_param.rego b/pkg/securitypolicy/fragment_test_policies/simple_env_rule_param.rego new file mode 100644 index 0000000000..665bd0dbc0 --- /dev/null +++ b/pkg/securitypolicy/fragment_test_policies/simple_env_rule_param.rego @@ -0,0 +1,35 @@ +package fragment + +svn := "1" +framework_version := "0.5.0" + +parameters := { + "env_param": { + "default": { + "value": ".+", + "value_strategy": "re2" + } + } +} + +containers := [ + { + @@CONTAINER_COMMON@@ + "command": [ + "init" + ], + "env_rules": [ + { + "name": "ENV_VAR_PARAMETER", + "name_strategy": "string", + "value": parameter("env_param").value, + "value_strategy": parameter("env_param").value_strategy + } + ], + "exec_processes": [], + "layers": [ + "0000000000000000000000000000000000000000000000000000000000000000" + ], + "mounts": [] + } +] diff --git a/pkg/securitypolicy/regopolicy_linux_test.go b/pkg/securitypolicy/regopolicy_linux_test.go index 0c22005636..4c90095ade 100644 --- a/pkg/securitypolicy/regopolicy_linux_test.go +++ b/pkg/securitypolicy/regopolicy_linux_test.go @@ -4,6 +4,8 @@ package securitypolicy import ( + _ "embed" + "context" "encoding/json" "errors" @@ -5310,6 +5312,488 @@ func Test_Rego_LoadFragment_BadIssuer_MustNotTryToLoadRego_Compat_0_10_0(t *test } } +func Test_Rego_LoadFragment_Container_FragmentParameters_Simple(t *testing.T) { + p := setupRegoFragmentParameterTest(t, []fragmentCodeAndParameters{ + { + fragmentCode: simpleEnvRuleParamPolicyCode, + possibleParams: []mapOfAny{ + { + "env_param": mapOfAny{ + "value": "allowed.value", + "value_strategy": "string", + }, + }, + }, + }, + }, false) + + fragmentParameterTestCheckOneEnv(t, p, []string{ + "ENV_VAR_PARAMETER=allowed.value", + }, []string{ + "ENV_VAR_PARAMETER=denied.value", + "ENV_VAR_PARAMETER=allowed_value", + "ENV_VAR_PARAMETER=", + "ENV_VAR_PARAMETER", + }) +} + +func Test_Rego_LoadFragment_FragmentParameters_MultiplePossibilities(t *testing.T) { + p := setupRegoFragmentParameterTest(t, []fragmentCodeAndParameters{ + { + fragmentCode: simpleEnvRuleParamPolicyCode, + possibleParams: []mapOfAny{ + { + "env_param": mapOfAny{ + "value": "allowed.value.1", + "value_strategy": "string", + }, + }, + { + "env_param": mapOfAny{ + "value": "allowed.value.2", + "value_strategy": "string", + }, + }, + }, + }, + }, false) + + fragmentParameterTestCheckOneEnv(t, p, []string{ + "ENV_VAR_PARAMETER=allowed.value.1", + "ENV_VAR_PARAMETER=allowed.value.2", + }, []string{ + "ENV_VAR_PARAMETER=denied.value.1", + "ENV_VAR_PARAMETER=allowed_value.2", + "ENV_VAR_PARAMETER=allowed.value.3", + "ENV_VAR_PARAMETER=", + "ENV_VAR_PARAMETER", + }) +} + +func Test_Rego_LoadFragment_FragmentParameters_Default(t *testing.T) { + p := setupRegoFragmentParameterTest(t, []fragmentCodeAndParameters{ + { + fragmentCode: simpleEnvRuleParamPolicyCode, + possibleParams: []mapOfAny{ + {}, + }, + }, + }, false) + + fragmentParameterTestCheckOneEnv(t, p, []string{ + "ENV_VAR_PARAMETER=default_is_allow_all_non_empty", + }, []string{ + "ANOTHER_ENV=should.deny", + "ENV_VAR_PARAMETER=", + }) +} + +// Test passing parameters not defined in the fragment. Current behaviour is +// ignore but this could be changed in the future to be more strict. +func Test_Rego_LoadFragment_FragmentParameters_UndefinedParameters(t *testing.T) { + p := setupRegoFragmentParameterTest(t, []fragmentCodeAndParameters{ + { + fragmentCode: simpleEnvRuleParamPolicyCode, + possibleParams: []mapOfAny{ + { + "bogus_param": "some_value", + }, + }, + }, + }, false) + + fragmentParameterTestCheckOneEnv(t, p, []string{ + "ENV_VAR_PARAMETER=default_is_allow_all_non_empty", + }, []string{ + "ANOTHER_ENV=should.deny", + "ENV_VAR_PARAMETER=", + }) +} + +func Test_Rego_LoadFragment_FragmentParameters_SpecialChars(t *testing.T) { + p := setupRegoFragmentParameterTest(t, []fragmentCodeAndParameters{ + { + fragmentCode: simpleEnvRuleParamPolicyCode, + possibleParams: []mapOfAny{ + { + "env_param": mapOfAny{ + "value": "!@#$%^&*( )_+-=[]{};'\\:\"|,./<>?\n\r\t\x01", + "value_strategy": "string", + }, + }, + }, + }, + }, false) + + fragmentParameterTestCheckOneEnv(t, p, []string{ + "ENV_VAR_PARAMETER=!@#$%^&*( )_+-=[]{};'\\:\"|,./<>?\n\r\t\x01", + }, []string{ + "ENV_VAR_PARAMETER=denied.value", + "ENV_VAR_PARAMETER=?>?\n\r\t\x01\n", + }) +} + +func Test_Rego_LoadFragment_FragmentParameters_Empty(t *testing.T) { + p := setupRegoFragmentParameterTest(t, []fragmentCodeAndParameters{ + { + fragmentCode: simpleEnvRuleParamPolicyCode, + possibleParams: []mapOfAny{ + { + "env_param": mapOfAny{ + "value": "", + "value_strategy": "string", + }, + }, + }, + }, + }, false) + + fragmentParameterTestCheckOneEnv(t, p, []string{ + "ENV_VAR_PARAMETER=", + }, []string{ + "ENV_VAR_PARAMETER=denied.value", + "ENV_VAR_PARAMETER==", + "ENV_VAR_PARAMETER=\n", + "ENV_VAR_PARAMETER=.", + "\nENV_VAR_PARAMETER=", + "ENV_VAR_PARAMETER", + }) +} + +func Test_Rego_LoadFragment_FragmentParameters_MultipleParams(t *testing.T) { + p := setupRegoFragmentParameterTest(t, []fragmentCodeAndParameters{ + { + fragmentCode: envRuleParamPolicyCode, + possibleParams: []mapOfAny{ + { + "env_param_nodefault": mapOfAny{ + "value": "aaa", + "value_strategy": "string", + }, + "env_string_param_nodefault": "bbb", + }, + }, + }, + }, false) + correctArray := []string{ + "ENV_VAR_FIXED=fixed_value", + "ENV_VAR_PARAMETER=default_value", + "ENV_VAR_PARAMETER_NODEFAULT=aaa", + "ENV_STRING_PARAM=default_string_value", + "ENV_STRING_PARAM_NODEFAULT=bbb", + } + wrongArrays := make([][]string, 0) + + // wrong value + for idxToChange := range correctArray { + wrongArray := make([]string, 0, len(correctArray)) + for idx, val := range correctArray { + if idx == idxToChange { + wrongArray = append(wrongArray, val+"_wrong") + } else { + wrongArray = append(wrongArray, val) + } + } + wrongArrays = append(wrongArrays, wrongArray) + } + + fragmentParameterTestCheckMultipleEnv(t, p, [][]string{correctArray}, wrongArrays) +} + +func Test_Rego_LoadFragment_FragmentParameters_MultipleParams_MultipleChoices(t *testing.T) { + p := setupRegoFragmentParameterTest(t, []fragmentCodeAndParameters{ + { + fragmentCode: envRuleParamPolicyCode, + possibleParams: []mapOfAny{ + { + "env_param_nodefault": mapOfAny{ + "value": "aaa", + "value_strategy": "string", + }, + "env_string_param_nodefault": "bbb", + "env_param": mapOfAny{ + "value": "ccc", + "value_strategy": "string", + }, + "env_string_param": "ddd", + }, + { + "env_param_nodefault": mapOfAny{ + "value": "111", + "value_strategy": "string", + }, + "env_string_param_nodefault": "222", + "env_param": mapOfAny{ + "value": "333", + "value_strategy": "string", + }, + "env_string_param": "444", + }, + }, + }, + }, false) + + correctArray1 := []string{ + "ENV_VAR_FIXED=fixed_value", + "ENV_VAR_PARAMETER=ccc", + "ENV_VAR_PARAMETER_NODEFAULT=aaa", + "ENV_STRING_PARAM=ddd", + "ENV_STRING_PARAM_NODEFAULT=bbb", + } + correctArray2 := []string{ + "ENV_VAR_FIXED=fixed_value", + "ENV_VAR_PARAMETER=333", + "ENV_VAR_PARAMETER_NODEFAULT=111", + "ENV_STRING_PARAM=444", + "ENV_STRING_PARAM_NODEFAULT=222", + } + correctArrays := [][]string{correctArray1, correctArray2} + wrongArrays := make([][]string, 0) + + // wrong value + for _, correctArray := range correctArrays { + for idxToChange := range correctArray { + wrongArray := make([]string, 0, len(correctArray)) + for idx, val := range correctArray { + if idx == idxToChange { + wrongArray = append(wrongArray, val+"_wrong") + } else { + wrongArray = append(wrongArray, val) + } + } + wrongArrays = append(wrongArrays, wrongArray) + } + } + + // wrong combination 1 + invalidCombination := make([]string, 0, len(correctArray1)) + for i := range correctArray1 { + if i%2 == 0 { + invalidCombination = append(invalidCombination, correctArray1[i]) + } else { + invalidCombination = append(invalidCombination, correctArray2[i]) + } + } + wrongArrays = append(wrongArrays, invalidCombination) + + // wrong combination 2 + invalidCombination = make([]string, 0, len(correctArray1)) + for i := range correctArray1 { + if i%2 == 1 { + invalidCombination = append(invalidCombination, correctArray1[i]) + } else { + invalidCombination = append(invalidCombination, correctArray2[i]) + } + } + wrongArrays = append(wrongArrays, invalidCombination) + + fragmentParameterTestCheckMultipleEnv(t, p, correctArrays, wrongArrays) +} + +func Test_Rego_LoadFragment_FragmentParameters_TwoFragments_CommonParameterName(t *testing.T) { + p := setupRegoFragmentParameterTest(t, []fragmentCodeAndParameters{ + { + fragmentCode: simpleEnvRuleParamPolicyCode, + possibleParams: []mapOfAny{ + { + "env_param": mapOfAny{ + "value": "value1", + "value_strategy": "string", + }, + }, + }, + }, + { + fragmentCode: envRuleParamAnotherFragmentPolicyCode, + possibleParams: []mapOfAny{ + { + "env_param": mapOfAny{ + "value": "value2", + "value_strategy": "string", + }, + }, + }, + }, + }, false) + + fragmentParameterTestCheckOneEnvWithLayer(t, p, []string{paramTestImageBaseLayer}, []string{ + "ENV_VAR_PARAMETER=value1", + }, []string{ + "ENV_VAR_PARAMETER=value2", + }) + + fragmentParameterTestCheckOneEnvWithLayer(t, p, []string{paramTestImageLayer1}, []string{ + "ENV_VAR_PARAMETER=value2", + }, []string{ + "ENV_VAR_PARAMETER=value1", + }) +} + +func Test_Rego_LoadFragment_FragmentParameters_Nested(t *testing.T) { + p := setupRegoFragmentParameterTest(t, []fragmentCodeAndParameters{ + { + fragmentCode: nestedImporterPolicyCode, + possibleParams: []mapOfAny{ + {}, + }, + }, + { + fragmentCode: nestedImporter2PolicyCode, + possibleParams: []mapOfAny{ + {}, + }, + }, + { + fragmentCode: nestedImporter2PolicyCode, + possibleParams: []mapOfAny{ + { + "l1_param": "l1_value_3", + }, + }, + }, + }, true) + + err := p.LoadFragment(context.Background(), "nested:issuer", "nested_fragment", paramTestTemplateFragmentCode(nestedFragmentPolicyCode)) + if err != nil { + t.Fatalf("unable to load nested fragment: %v", err) + } + + fragmentParameterTestCheckMultipleEnvWithLayer(t, p, []string{paramTestImageLayer2}, [][]string{ + { + "L1_PARAM=l1param_default", + "L2_PARAM=l2param_from_l1_1", + }, + { + "L1_PARAM=l1param_default", + "L2_PARAM=l2param_from_l1_2", + }, + { + "L1_PARAM=l1param_default_2", + "L2_PARAM=l2param_from_l1_1", + }, + { + "L1_PARAM=l1param_default_2", + "L2_PARAM=l2param_from_l1_2", + }, + { + "L1_PARAM=l1_value_3", + "L2_PARAM=l2param_from_l1_1", + }, + { + "L1_PARAM=l1_value_3", + "L2_PARAM=l2param_from_l1_2", + }, + }, [][]string{ + { + "L1_PARAM=l1_invalid", + "L2_PARAM=l2param_from_l1_2", + }, + { + "L1_PARAM=l1_value_3", + "L2_PARAM=l2_invalid", + }, + }) + + err = fragmentParameterTestCreateContainer(p, []string{ + "L1_PARAM=l1param_default", + "L2_PARAM=l2param_from_l1_1", + }, []string{"init"}, []string{paramTestImageLayer1}, nil) + assertDecisionJSONContains(t, err, "deviceHash not found") + assertDecisionJSONDoesNotContain(t, err, "invalid env list") + + p = setupRegoFragmentParameterTest(t, []fragmentCodeAndParameters{ + { + fragmentCode: nestedImporterPolicyCode, + possibleParams: []mapOfAny{ + {}, + }, + }, + }, false) + + err = p.LoadFragment(context.Background(), "nested:issuer", "nested_fragment", paramTestTemplateFragmentCode(nestedFragmentPolicyCode)) + if err == nil { + t.Fatal("expected error when loading nested fragment when parent fragment does not include fragments") + } +} + +func Test_Rego_LoadFragment_FragmentParameters_ParamOnCommand(t *testing.T) { + p := setupRegoFragmentParameterTest(t, []fragmentCodeAndParameters{ + { + fragmentCode: paramOnCommandPolicyCode, + possibleParams: []mapOfAny{ + { + "command_param": []string{"custom_command"}, + }, + }, + }, + }, false) + + err := fragmentParameterTestCreateContainer(p, []string{ + "MY_ENV=1", + }, []string{"custom_command"}, []string{paramTestImageBaseLayer}, []string{ + "MY_ENV=1", + }) + if err != nil { + t.Fatalf("unexpected error creating container with custom command: %v", err) + } + + err = fragmentParameterTestCreateContainer(p, []string{ + "MY_ENV=1", + }, []string{"invalid_command"}, []string{paramTestImageBaseLayer}, nil) + assertDecisionJSONContains(t, err, "invalid command") + + p = setupRegoFragmentParameterTest(t, []fragmentCodeAndParameters{ + { + fragmentCode: paramOnCommandPolicyCode, + possibleParams: []mapOfAny{ + {}, + }, + }, + }, false) + + err = fragmentParameterTestCreateContainer(p, []string{ + "MY_ENV=1", + }, []string{"custom_command"}, []string{paramTestImageBaseLayer}, nil) + assertDecisionJSONContains(t, err, "invalid command") + + err = fragmentParameterTestCreateContainer(p, []string{ + "MY_ENV=1", + }, []string{"init"}, []string{paramTestImageBaseLayer}, nil) + if err != nil { + t.Fatalf("unexpected error creating container with default command: %v", err) + } + + p = setupRegoFragmentParameterTest(t, []fragmentCodeAndParameters{ + { + fragmentCode: paramOnCommandPolicyCode, + possibleParams: []mapOfAny{ + { + "command_param": []string{ + "sleep", + "infinity", + }, + }, + }, + }, + }, false) + + err = fragmentParameterTestCreateContainer(p, []string{ + "MY_ENV=1", + }, []string{"sleep", "infinity"}, []string{paramTestImageBaseLayer}, []string{ + "MY_ENV=1", + }) + if err != nil { + t.Fatalf("unexpected error creating container with custom command: %v", err) + } + + err = fragmentParameterTestCreateContainer(p, []string{ + "MY_ENV=1", + }, []string{"bash"}, []string{paramTestImageBaseLayer}, nil) + assertDecisionJSONContains(t, err, "invalid command") +} + func Test_Rego_Scratch_Mount_Policy(t *testing.T) { for _, tc := range []struct { unencryptedAllowed bool @@ -7878,3 +8362,244 @@ func Test_Rego_SandboxSysfsCarveOut_PrivilegedRequestDenied(t *testing.T) { } assertDecisionJSONContains(t, err, "privileged escalation not allowed") } + +//go:embed fragment_test_policies/_container_common.rego.inc +var containerCommonCode string + +//go:embed fragment_test_policies/simple_env_rule_param.rego +var simpleEnvRuleParamPolicyCode string + +//go:embed fragment_test_policies/env_rule_param.rego +var envRuleParamPolicyCode string + +//go:embed fragment_test_policies/env_rule_param_another_fragment.rego +var envRuleParamAnotherFragmentPolicyCode string + +//go:embed fragment_test_policies/nested_importer.rego +var nestedImporterPolicyCode string + +//go:embed fragment_test_policies/nested_importer_2.rego +var nestedImporter2PolicyCode string + +//go:embed fragment_test_policies/nested_fragment.rego +var nestedFragmentPolicyCode string + +//go:embed fragment_test_policies/param_on_command.rego +var paramOnCommandPolicyCode string + +const paramTestImageBaseLayer = "0000000000000000000000000000000000000000000000000000000000000000" +const paramTestImageLayer1 = "1111111111111111111111111111111111111111111111111111111111111111" +const paramTestImageLayer2 = "2222222222222222222222222222222222222222222222222222222222222222" + +func paramTestTemplateFragmentCode(code string) string { + return strings.ReplaceAll(code, "@@CONTAINER_COMMON@@", containerCommonCode) +} + +type mapOfAny map[string]interface{} + +type fragmentCodeAndParameters struct { + fragmentCode string + possibleParams []mapOfAny +} + +func setupRegoFragmentParameterTest( + t *testing.T, + fragmentsToLoad []fragmentCodeAndParameters, + allowSubfragments bool, +) (p *regoEnforcer) { + fragmentImports := make([]*fragment, 0) + + includes := []string{ + "containers", + } + if allowSubfragments { + includes = append(includes, "fragments") + } + topFragmentIssuer := testDataGenerator.uniqueFragmentIssuer() + topFragmentFeedFmt := "feed_%d" + + for i_fragment, f := range fragmentsToLoad { + for _, params := range f.possibleParams { + importWithParams := fragment{ + issuer: topFragmentIssuer, + feed: fmt.Sprintf(topFragmentFeedFmt, i_fragment), + minimumSVN: "1", + includes: includes, + parameters: params, + } + fragmentImports = append(fragmentImports, &importWithParams) + } + } + + gc := &generatedConstraints{ + fragments: fragmentImports, + ctx: context.Background(), + } + securityPolicy := gc.toPolicy() + defaultMounts := generateMounts(testRand) + privilegedMounts := generateMounts(testRand) + + policy, err := newRegoPolicy(securityPolicy.marshalRego(), + toOCIMounts(defaultMounts), + toOCIMounts(privilegedMounts), testOSType) + if err != nil { + t.Fatalf("failed to create rego policy: %v", err) + } + + for i_fragment, f := range fragmentsToLoad { + err = policy.LoadFragment(gc.ctx, topFragmentIssuer, fmt.Sprintf(topFragmentFeedFmt, i_fragment), paramTestTemplateFragmentCode(f.fragmentCode)) + } + if err != nil { + t.Fatalf("failed to load fragment: %v", err) + } + + return policy +} + +func fragmentParameterTestCreateContainer( + policy *regoEnforcer, + envList []string, + command []string, + layers []string, + expectedEnvListAfterEnforcement []string, +) (err error) { + sandboxID := testDataGenerator.uniqueSandboxID() + containerID := testDataGenerator.uniqueContainerID() + ctx := context.Background() + scratchDisk := getScratchDiskMountTarget(sandboxID) + + err = policy.EnforceRWDeviceMountPolicy(ctx, scratchDisk, true, true, "xfs") + if err != nil { + return fmt.Errorf("error mounting scratch disk: %v", err) + } + + layerPaths := make([]string, len(layers)) + for i, layerHash := range layers { + layerPath := testDataGenerator.uniqueLayerMountTarget() + err = policy.EnforceDeviceMountPolicy(ctx, layerPath, layerHash) + if err != nil { + return fmt.Errorf("error mounting layer %s: %v", layerHash, err) + } + layerPaths[len(layers)-i-1] = layerPath + } + + overlayTarget := getOverlayMountTarget(containerID) + + // see NOTE_TESTCOPY + err = policy.EnforceOverlayMountPolicy(ctx, containerID, copyStrings(layerPaths), overlayTarget) + if err != nil { + return fmt.Errorf("error mounting filesystem: %v", err) + } + + envToKeep, _, _, err := policy.EnforceCreateContainerPolicy( + ctx, + sandboxID, + containerID, + copyStrings(command), + copyStrings(envList), + "/", + []oci.Mount{}, + false, + false, + IDName{ + ID: "0", + }, + []IDName{ + {ID: "0"}, + }, + "0022", + &oci.LinuxCapabilities{}, + "", + ) + if err != nil { + return err + } + if expectedEnvListAfterEnforcement != nil { + if !areStringArraysEqual(envToKeep, expectedEnvListAfterEnforcement) { + return fmt.Errorf("environment variables after enforcement do not match expected values.\nExpected: %v\nActual: %v", + expectedEnvListAfterEnforcement, + envToKeep) + } + } + return nil +} + +func fragmentParameterTestCheckOneEnv( + t *testing.T, + p *regoEnforcer, + expectAllowEnvs []string, + expectDenyEnvs []string, +) { + t.Helper() + + fragmentParameterTestCheckOneEnvWithLayer(t, p, []string{paramTestImageBaseLayer}, expectAllowEnvs, expectDenyEnvs) +} + +func fragmentParameterTestCheckMultipleEnv( + t *testing.T, + p *regoEnforcer, + expectAllowEnvs [][]string, + expectDenyEnvs [][]string, +) { + t.Helper() + + fragmentParameterTestCheckMultipleEnvWithLayer(t, p, []string{paramTestImageBaseLayer}, expectAllowEnvs, expectDenyEnvs) +} + +func fragmentParameterTestCheckMultipleEnvWithLayer( + t *testing.T, + p *regoEnforcer, + layerHashs []string, + expectAllowEnvs [][]string, + expectDenyEnvs [][]string, +) { + t.Helper() + + for _, envs := range expectAllowEnvs { + err := fragmentParameterTestCreateContainer(p, envs, []string{"init"}, layerHashs, envs) + if err != nil { + t.Errorf("expected to allow env list %v, but got error: %v", envs, err) + } + } + + for _, envs := range expectDenyEnvs { + err := fragmentParameterTestCreateContainer(p, envs, []string{"init"}, layerHashs, nil) + if err == nil { + t.Errorf("expected to deny env list %v, but got no error", envs) + continue + } + assertDecisionJSONContains(t, err, "invalid env list") + } +} + +func fragmentParameterTestCheckOneEnvWithLayer( + t *testing.T, + p *regoEnforcer, + layerHashs []string, + expectAllowEnvs []string, + expectDenyEnvs []string, +) { + t.Helper() + + for _, env := range expectAllowEnvs { + err := fragmentParameterTestCreateContainer(p, []string{ + env, + }, []string{"init"}, layerHashs, []string{ + env, + }) + if err != nil { + t.Errorf("expected to allow env %q, but got error: %v", env, err) + } + } + + for _, env := range expectDenyEnvs { + err := fragmentParameterTestCreateContainer(p, []string{ + env, + }, []string{"init"}, layerHashs, nil) + if err == nil { + t.Errorf("expected to deny env %q, but got no error", env) + continue + } + assertDecisionJSONContains(t, err, "invalid env list") + } +} diff --git a/pkg/securitypolicy/securitypolicy_internal.go b/pkg/securitypolicy/securitypolicy_internal.go index c736fb58ed..da759a4925 100644 --- a/pkg/securitypolicy/securitypolicy_internal.go +++ b/pkg/securitypolicy/securitypolicy_internal.go @@ -242,6 +242,7 @@ type fragment struct { feed string minimumSVN string includes []string + parameters map[string]interface{} } func (c *Container) toInternal() (*securityPolicyContainer, error) { diff --git a/pkg/securitypolicy/securitypolicy_marshal.go b/pkg/securitypolicy/securitypolicy_marshal.go index 01adc59cb9..8fa32b7099 100644 --- a/pkg/securitypolicy/securitypolicy_marshal.go +++ b/pkg/securitypolicy/securitypolicy_marshal.go @@ -571,8 +571,18 @@ func addExternalProcesses(builder *strings.Builder, processes []*externalProcess func (f fragment) marshalRego() string { includes := stringArray(f.includes).marshalRego() - return fmt.Sprintf(`{"issuer": "%s", "feed": "%s", "minimum_svn": "%s", "includes": %s}`, - f.issuer, f.feed, f.minimumSVN, includes) + + if len(f.parameters) == 0 { + return fmt.Sprintf(`{"issuer": "%s", "feed": "%s", "minimum_svn": "%s", "includes": %s}`, + f.issuer, f.feed, f.minimumSVN, includes) + } + + paramsJson, err := json.Marshal(f.parameters) + if err != nil { + panic(fmt.Errorf("failed to marshal fragment parameters object to JSON: %w", err)) + } + return fmt.Sprintf(`{"issuer": "%s", "feed": "%s", "minimum_svn": "%s", "includes": %s, "parameters": %s}`, + f.issuer, f.feed, f.minimumSVN, includes, string(paramsJson)) } func addFragments(builder *strings.Builder, fragments []*fragment) { From 8eba843fa9c83f5908348cc48c408dbb59733a26 Mon Sep 17 00:00:00 2001 From: Tingmao Wang Date: Wed, 26 Nov 2025 12:51:37 +0000 Subject: [PATCH 9/9] Rename the parameters metadata block to parameters_api Name still provisional, might change later. Signed-off-by: Tingmao Wang --- pkg/securitypolicy/fragment_definition.rego | 4 ++-- pkg/securitypolicy/fragment_test_policies/env_rule_param.rego | 2 +- .../env_rule_param_another_fragment.rego | 2 +- .../fragment_test_policies/nested_fragment.rego | 2 +- .../fragment_test_policies/nested_importer.rego | 2 +- .../fragment_test_policies/nested_importer_2.rego | 2 +- .../fragment_test_policies/param_on_command.rego | 2 +- .../fragment_test_policies/simple_env_rule_param.rego | 2 +- 8 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/securitypolicy/fragment_definition.rego b/pkg/securitypolicy/fragment_definition.rego index e217452fad..027f7f341d 100644 --- a/pkg/securitypolicy/fragment_definition.rego +++ b/pkg/securitypolicy/fragment_definition.rego @@ -1,5 +1,5 @@ default __fragment_parameters_metadata := {} -__fragment_parameters_metadata := data[input.namespace].parameters { - data[input.namespace].parameters +__fragment_parameters_metadata := data[input.namespace].parameters_api { + data[input.namespace].parameters_api } parameter(name) := data.framework.extract_parameter(name, __fragment_parameters, __fragment_parameters_metadata) diff --git a/pkg/securitypolicy/fragment_test_policies/env_rule_param.rego b/pkg/securitypolicy/fragment_test_policies/env_rule_param.rego index d4fb074bc1..f906da5f60 100644 --- a/pkg/securitypolicy/fragment_test_policies/env_rule_param.rego +++ b/pkg/securitypolicy/fragment_test_policies/env_rule_param.rego @@ -3,7 +3,7 @@ package fragment svn := "1" framework_version := "0.5.0" -parameters := { +parameters_api := { "env_param": { "default": { "value": "default_value", diff --git a/pkg/securitypolicy/fragment_test_policies/env_rule_param_another_fragment.rego b/pkg/securitypolicy/fragment_test_policies/env_rule_param_another_fragment.rego index 6c2f89619f..1b651ac5b4 100644 --- a/pkg/securitypolicy/fragment_test_policies/env_rule_param_another_fragment.rego +++ b/pkg/securitypolicy/fragment_test_policies/env_rule_param_another_fragment.rego @@ -3,7 +3,7 @@ package fragment svn := "1" framework_version := "0.5.0" -parameters := { +parameters_api := { "env_param": { "default": { "value": ".+", diff --git a/pkg/securitypolicy/fragment_test_policies/nested_fragment.rego b/pkg/securitypolicy/fragment_test_policies/nested_fragment.rego index 71a3b1cb7e..1a8e83b806 100644 --- a/pkg/securitypolicy/fragment_test_policies/nested_fragment.rego +++ b/pkg/securitypolicy/fragment_test_policies/nested_fragment.rego @@ -3,7 +3,7 @@ package fragment svn := "1" framework_version := "0.5.0" -parameters := { +parameters_api := { "l1_param": {}, "l2_param": { "default": "l2param_default" diff --git a/pkg/securitypolicy/fragment_test_policies/nested_importer.rego b/pkg/securitypolicy/fragment_test_policies/nested_importer.rego index 2e9ea09db8..936f4e410a 100644 --- a/pkg/securitypolicy/fragment_test_policies/nested_importer.rego +++ b/pkg/securitypolicy/fragment_test_policies/nested_importer.rego @@ -3,7 +3,7 @@ package fragment svn := "1" framework_version := "0.5.0" -parameters := { +parameters_api := { "l1_param": { "default": "l1param_default" } diff --git a/pkg/securitypolicy/fragment_test_policies/nested_importer_2.rego b/pkg/securitypolicy/fragment_test_policies/nested_importer_2.rego index 90ef51f86c..8c32eeb838 100644 --- a/pkg/securitypolicy/fragment_test_policies/nested_importer_2.rego +++ b/pkg/securitypolicy/fragment_test_policies/nested_importer_2.rego @@ -3,7 +3,7 @@ package fragment svn := "1" framework_version := "0.5.0" -parameters := { +parameters_api := { "l1_param": { "default": "l1param_default_2" } diff --git a/pkg/securitypolicy/fragment_test_policies/param_on_command.rego b/pkg/securitypolicy/fragment_test_policies/param_on_command.rego index 3603c5c593..a792a27eeb 100644 --- a/pkg/securitypolicy/fragment_test_policies/param_on_command.rego +++ b/pkg/securitypolicy/fragment_test_policies/param_on_command.rego @@ -3,7 +3,7 @@ package fragment svn := "1" framework_version := "0.5.0" -parameters := { +parameters_api := { "command_param": { "default": [ "init" diff --git a/pkg/securitypolicy/fragment_test_policies/simple_env_rule_param.rego b/pkg/securitypolicy/fragment_test_policies/simple_env_rule_param.rego index 665bd0dbc0..04b38bb8cb 100644 --- a/pkg/securitypolicy/fragment_test_policies/simple_env_rule_param.rego +++ b/pkg/securitypolicy/fragment_test_policies/simple_env_rule_param.rego @@ -3,7 +3,7 @@ package fragment svn := "1" framework_version := "0.5.0" -parameters := { +parameters_api := { "env_param": { "default": { "value": ".+",