From 85307a95dd7bf38ceb77ea45300b97b3b8750ecb 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 | 262 ++++++++++++---- .../regopolicyinterpreter_test.go | 285 +++++++++++++++++- internal/regopolicyinterpreter/test.rego | 35 ++- pkg/securitypolicy/regopolicy_linux_test.go | 4 +- 4 files changed, 509 insertions(+), 77 deletions(-) diff --git a/internal/regopolicyinterpreter/regopolicyinterpreter.go b/internal/regopolicyinterpreter/regopolicyinterpreter.go index 6e316f9b41..ff06a71ca1 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,17 @@ 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 { + var dataTypeStr string + 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 +378,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 +407,7 @@ func (r *RegoPolicyInterpreter) UpdateOSType(os string) error { ops := []*regoMetadataOperation{ { Action: metadataAdd, + Type: metadataTypeMap, Name: "operatingsystem", Key: "ostype", Value: os, @@ -335,32 +416,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 +628,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..90c7a12073 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,23 @@ func assertObjectsEqual(lhs testObject, rhs testObject) bool { return true } + +func assertMetadataValueEqual(t *testing.T, lhs interface{}, rhs interface{}) bool { + t.Helper() + 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 448944a48e..3eab7b910f 100644 --- a/pkg/securitypolicy/regopolicy_linux_test.go +++ b/pkg/securitypolicy/regopolicy_linux_test.go @@ -4557,7 +4557,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) @@ -5127,7 +5127,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 aaa4c1762d02173cdb357f3a180019cbf6598f80 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 2aaa40770d..8d8c3a6ae3 100644 --- a/pkg/securitypolicy/framework.rego +++ b/pkg/securitypolicy/framework.rego @@ -1204,6 +1204,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] } @@ -1234,25 +1256,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 bfcd57c7bd5668253a06a9c96a18aa37e2cb92cf 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 8d8c3a6ae3..a20af2b08e 100644 --- a/pkg/securitypolicy/framework.rego +++ b/pkg/securitypolicy/framework.rego @@ -242,21 +242,68 @@ command_ok(command) { } } -env_ok(pattern, "string", value) { +# An env rule can be of two forms: +# { +# "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) { @@ -267,7 +314,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) } } @@ -275,7 +322,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) } } @@ -1624,14 +1671,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] { @@ -1649,7 +1696,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 08fd5c45a5e160e771f54de873865f533138466b 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/regopolicy_windows_test.go | 11 +- pkg/securitypolicy/securitypolicy.go | 8 ++ pkg/securitypolicy/securitypolicy_marshal.go | 6 +- 5 files changed, 93 insertions(+), 59 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 3eab7b910f..ec8c46f0f5 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 { @@ -3053,13 +3058,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 { @@ -3217,7 +3218,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, } @@ -6214,7 +6215,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, } @@ -6468,7 +6469,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, @@ -6514,7 +6515,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/regopolicy_windows_test.go b/pkg/securitypolicy/regopolicy_windows_test.go index 33b49a64f8..eaa8fa85b5 100644 --- a/pkg/securitypolicy/regopolicy_windows_test.go +++ b/pkg/securitypolicy/regopolicy_windows_test.go @@ -7,7 +7,6 @@ import ( "context" _ "embed" "fmt" - "math/rand" "strconv" "strings" "testing" @@ -89,7 +88,7 @@ func Test_Rego_EnforceEnvironmentVariablePolicy_NotAllMatches_Windows(t *testing return false } - envList := append(tc.envList, generateNeverMatchingEnvironmentVariable(testRand)) + envList := append(tc.envList, generateRandomEnvironmentVariable(testRand)) _, _, _, err = tc.policy.EnforceCreateContainerPolicyV2(p.ctx, tc.containerID, tc.argList, envList, tc.workingDir, tc.mounts, tc.user, nil) @@ -570,13 +569,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 *generatedWindowsConstraints) bool { @@ -729,7 +724,7 @@ func Test_Rego_EnforceEnvironmentVariablePolicy_MissingRequired_Windows(t *testi // add a rule to re2 match requiredRule := EnvRuleConfig{ Strategy: "string", - Rule: randVariableString(testRand, maxGeneratedEnvironmentVariableRuleLength), + Rule: generateRandomEnvironmentVariable(testRand), Required: true, } 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 39f4b9cdd61d39cd54ad3d0f74af0e075b6d05eb 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 a20af2b08e..9713e1b355 100644 --- a/pkg/securitypolicy/framework.rego +++ b/pkg/securitypolicy/framework.rego @@ -1317,6 +1317,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[_] @@ -1353,13 +1412,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) @@ -1373,6 +1433,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 } @@ -1526,6 +1602,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 @@ -2424,6 +2510,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 ec8c46f0f5..5224bfa18d 100644 --- a/pkg/securitypolicy/regopolicy_linux_test.go +++ b/pkg/securitypolicy/regopolicy_linux_test.go @@ -4594,6 +4594,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) @@ -5157,6 +5158,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) @@ -5207,6 +5210,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) @@ -7049,6 +7054,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 20127bf4f2..46a5b00de4 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. @@ -1131,6 +1138,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 { @@ -1152,12 +1186,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 @@ -1165,17 +1219,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 8c0bcff8e23649c817842b75f6e681385cf43014 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 2b7c5ae018..8f0916f768 100644 --- a/pkg/securitypolicy/version_framework +++ b/pkg/securitypolicy/version_framework @@ -1 +1 @@ -0.4.2 +0.5.0 From 1ce52ddef93a12b66061138bb765cdc8150e9a24 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 9713e1b355..155e664bd2 100644 --- a/pkg/securitypolicy/framework.rego +++ b/pkg/securitypolicy/framework.rego @@ -1876,6 +1876,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 92e65f734b9364e3a1b74abebbb6409ae3c9c98b 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 5224bfa18d..0209800da5 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" @@ -5343,6 +5345,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 @@ -7942,3 +8426,244 @@ func Test_Rego_EnforceMountBlockDevicePolicy_OpenDoor(t *testing.T) { t.Errorf("open door should allow unmount_blockdev, got: %v", err) } } + +//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..1cdcb8177f 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 dbbdb1f352f4bea4f47ed7d515ee69e39e93c8ae 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": ".+",