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

rego: fix slightly incorrect sandbox and hugepage mounts enforcement #1625

Merged
merged 1 commit into from
Jan 28, 2023

Conversation

anmaxvl
Copy link
Contributor

@anmaxvl anmaxvl commented Jan 27, 2023

Sandbox and hugepage mounts come via CRI config in the form: sandbox://<absolute-path>, however the existing enforcement and tests expect it to be sandbox://<relative-path> which causes a problem during mount enforcement, when the sandbox prefix is replaced with an additional path separator in the end.

Additionally update policy tests.

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

@anmaxvl anmaxvl requested a review from a team as a code owner January 27, 2023 07:03
@@ -57,10 +59,10 @@ func securityPolicyFromContainers(
return base64.StdEncoding.EncodeToString([]byte(policyString)), nil
}

func sandboxSecurityPolicy(t *testing.T, policyType string, allowEnvironmentVariableDropping bool) string {
Copy link
Member

Choose a reason for hiding this comment

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

Why remove this test element? Couldn't both be varied, i.e. unencrypted scratch and env dropping?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just looking at the code, it seemed like there was a bad rebase. I don't sandboxSecurityPolicy is used anywhere with allow env var dropping... the only places where that parameter is used is in encrypted scratch tests.

Copy link
Member

@matajoh matajoh left a comment

Choose a reason for hiding this comment

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

Looks good.

Sandbox and hugepage mounts come via CRI config in the form:
`sandbox://<absolute-path>`, however the existing enforcement and tests
expect it to be `sandbox://<relative-path>` which causes a problem during
mount enforcement, when the sandbox prefix is replaced with an additional
path separator in the end.

Additionally update policy tests.

Signed-off-by: Maksim An <maksiman@microsoft.com>
@anmaxvl anmaxvl force-pushed the fix/sandbox-hugepage-dirs-rego branch from 29c1096 to 912f789 Compare January 27, 2023 23:42
@anmaxvl anmaxvl merged commit 97875f7 into microsoft:main Jan 28, 2023
@anmaxvl anmaxvl deleted the fix/sandbox-hugepage-dirs-rego branch January 28, 2023 01:48
princepereira pushed a commit to princepereira/hcsshim that referenced this pull request Aug 29, 2024
…icrosoft#1625)

Sandbox and hugepage mounts come via CRI config in the form:
`sandbox://<absolute-path>`, however the existing enforcement and tests
expect it to be `sandbox://<relative-path>` which causes a problem during
mount enforcement, when the sandbox prefix is replaced with an additional
path separator in the end.

Additionally update policy tests.

Signed-off-by: Maksim An <maksiman@microsoft.com>
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.

3 participants