Skip to content
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

securitypolicy: add security policy enforcer registration and defaults #1476

Merged
merged 2 commits into from
Aug 18, 2022

Conversation

anmaxvl
Copy link
Contributor

@anmaxvl anmaxvl commented Aug 8, 2022

Add enforcer registration logic and support for default enforcer.
The host can request which security policy enforcer to use with
supplied policy, if none supplied, GCS code tries to make a "guess"
as to which enforcer should be used: "allow all" or "default".
Default enforcer is set to StandardSecurityPolicyEnforcer unless
GCS is built with "rego" tag present. In that case, the default
enforcer will be set to RegoEnforcer.

New annotation has been added that allows callers to pick which
enforcer to use, e.g.

{
    ...
    "annotations": {
        "io.microsoft.virtualmachine.lcow.enforcer": "rego"
    },
    ...
}

Signed-off-by: Maksim An maksiman@microsoft.com

@anmaxvl anmaxvl requested a review from a team as a code owner August 8, 2022 20:06
@anmaxvl anmaxvl marked this pull request as draft August 8, 2022 20:18
@anmaxvl anmaxvl marked this pull request as ready for review August 8, 2022 20:32
@anmaxvl
Copy link
Contributor Author

anmaxvl commented Aug 9, 2022

@SeanTAllen @KenGordon @matajoh FYI

@SeanTAllen
Copy link
Contributor

This might create a lot of merge conflicts with the coming rego engine PR, can we do this PR after that one? I think the merge conflicts for this PR will be easier to fix and less likely to result in a large round of debugging.

@anmaxvl anmaxvl force-pushed the rego-enforcer-build-tags branch 2 times, most recently from b4181ae to 8ec3ab5 Compare August 10, 2022 16:45
@anmaxvl anmaxvl changed the title securitypolicy: add build tags for rego securitypolicy: add security policy enforcer registration and defaults Aug 10, 2022
@dcantah dcantah self-assigned this Aug 16, 2022
Copy link
Contributor

@helsaawy helsaawy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nits, lgtm

internal/protocol/guestresource/resources.go Outdated Show resolved Hide resolved
pkg/securitypolicy/securitypolicy.go Outdated Show resolved Hide resolved
@@ -21,6 +21,24 @@ import (
"github.com/Microsoft/hcsshim/pkg/annotations"
)

type CreateEnforcerFunc func(_ SecurityPolicyState, criMounts, criPrivilegedMounts []oci.Mount) (SecurityPolicyEnforcer, error)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can just define this as:

type CreateEnforcerFunc func(SecurityPolicyState, []oci.Mount, []oci.Mount) (SecurityPolicyEnforcer, error)

(Unless you want to keepthe names for regular vs privileged mounts)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, alternatively I can add a comment, but I think this way it's self explanatory.

pkg/securitypolicy/securitypolicyenforcer.go Outdated Show resolved Hide resolved
pkg/securitypolicy/securitypolicyenforcer.go Outdated Show resolved Hide resolved
Add enforcer registration logic and support for default enforcer.
The host can request which security policy enforcer to use with
supplied policy, if none supplied, GCS code tries to make a "guess"
as to which enforcer should be used: "allow all" or "default".
Default enforcer is set to `StandardSecurityPolicyEnforcer` unless
GCS is built with "rego" tag present. In that case, the default
enforcer will be set to `RegoEnforcer`.

New annotation has been added that allows callers to pick which
enforcer to use, e.g.
```pod.json
{
    ...
    "annotations": {
        "io.microsoft.virtualmachine.lcow.enforcer": "rego"
    },
    ...
}
```

Signed-off-by: Maksim An <maksiman@microsoft.com>
Signed-off-by: Maksim An <maksiman@microsoft.com>
Copy link
Contributor

@helsaawy helsaawy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if can merge even with the integration tests failing
(we probably should add continueOnErrorr: true to them until we fix the flaky-ness)

@dcantah
Copy link
Contributor

dcantah commented Aug 18, 2022

Somebody reported that the mingw issue was resolved for them on 1.19... I'm going to trial this

@anmaxvl anmaxvl merged commit 8351158 into microsoft:main Aug 18, 2022
@anmaxvl anmaxvl deleted the rego-enforcer-build-tags branch August 18, 2022 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants