-
Notifications
You must be signed in to change notification settings - Fork 272
Remove the standard JSON enforcer and the JSON policy parsing in the Rego enforcer #2539
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Remove the standard JSON enforcer and the JSON policy parsing in the Rego enforcer #2539
Conversation
c8be83c
to
64da836
Compare
// Return an user-friendly error message if we get a JSON policy. | ||
// Previously such policy was supported, but we currently only | ||
// support Rego. | ||
return nil, fmt.Errorf("JSON policy is not supported.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need this. JSON policies only existed for dev purposes. We never shipped a product that supported them. There was a small overlap of time after the first release when the confcom toolling could produce them and they would have loaded.
} | ||
// createAllowAllEnforcer creates and returns OpenDoorSecurityPolicyEnforcer | ||
// instance. The provided base64EncodedPolicy must be either empty or contain | ||
// exactly the field "allow_all" set to true. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"exactly the field "allow_all" set to true" We don't want that anymore. They can use a long hand allow all policy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this and the above comment addressed
// this is either a Rego policy or malformed JSON | ||
code = string(rawPolicy) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you run the tests with this change? This change is needed and is used by tests and in enforcements. The handles are created based on osAwareMarshalRego
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Running go test -tags functional,rego ./pkg/securitypolicy/
on Linux, all tests pass. Not tried Windows yet but I think the security policy tests are mostly platform agnostic?
I also don't think any of the test should try and give the rego enforcer a JSON policy - if they need to turn some JSON into Rego, the test ought to call osAwareMarshalRego or MarshalPolicy themselves (which I think is already how it works?), then pass the resulting Rego to the Rego enforcer.
I also can't find any invocation of CreateSecurityPolicyEnforcer or createRegoEnforcer in the tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but I think the security policy tests are mostly platform agnostic?
Just realized that I missed the fact that there is now regopolicy_linux/regopolicy_windows_test.go
(Although I think technically, unless the tests are generating actual CIM hashes, it should in theory be able to run on Linux too?)
Tried running it on Windows, all pass as well
64da836
to
165fb45
Compare
e102141
to
75a8b68
Compare
This commit removes the long deprecated standard JSON enforcer - all confidential containers now has to either use Rego (the default), or the open_door enforcer if provided with an empty policy or a policy that is `{"allow_all": true}` (both case checked against host data). Note that the host can still choose either rego or open_door, and this is not measured into host data, but the policy check in createOpenDoorEnforcer ensures that if the policy is a rego policy, trying to use an open_door enforcer will error, leaving the enforcer at the default (which for confidential is a deny-everything ClosedDoorSecurityPolicyEnforcer). Signed-off-by: Tingmao Wang <tingmaowang@microsoft.com>
This removes the ability to send in a standard JSON policy to the Rego enforcer. Previously it would translate it into the equivalent Rego policy (the format is different from the `containers := [...]` definition in Rego), but we need to stop supporting this. Signed-off-by: Tingmao Wang <tingmaowang@microsoft.com>
For the open door enforcer to be used, policy must be empty (i.e. the case in non-confidential containers). Signed-off-by: Tingmao Wang <tingmaowang@microsoft.com>
Suggested-by: Ken Gordon <kegordo@microsoft.com> Signed-off-by: Tingmao Wang <tingmaowang@microsoft.com>
Signed-off-by: Tingmao Wang <tingmaowang@microsoft.com>
75a8b68
to
217484d
Compare
This removes two kinds of JSON policies, both of which have never been supported for customer usage:
containers
declaration, but in another custom format) to the Rego enforcer.Note that this in itself is not a security vulnerability against any deployments using normal Rego policy, as the host data in the attestation report generated form a container with such a policy would be the hash of a JSON structure, thus such a report would not e.g. pass existing key release policy enforcement.