Skip to content

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

Merged
anmaxvl merged 1 commit intomicrosoft:mainfrom
anmaxvl:fix/sandbox-hugepage-dirs-rego
Jan 28, 2023
Merged

rego: fix slightly incorrect sandbox and hugepage mounts enforcement#1625
anmaxvl merged 1 commit intomicrosoft:mainfrom
anmaxvl:fix/sandbox-hugepage-dirs-rego

Conversation

@anmaxvl
Copy link
Copy Markdown
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 [email protected]

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

func sandboxSecurityPolicy(t *testing.T, policyType string, allowEnvironmentVariableDropping bool) string {
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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 <[email protected]>
@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 <[email protected]>
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