Fix policy enforcement to handle identical layers.#1441
Merged
anmaxvl merged 4 commits intomicrosoft:masterfrom Jun 28, 2022
Merged
Fix policy enforcement to handle identical layers.#1441anmaxvl merged 4 commits intomicrosoft:masterfrom
anmaxvl merged 4 commits intomicrosoft:masterfrom
Conversation
Contributor
Author
Contributor
|
For the record, I had an idea for how to address this and Maksim's was much better than mine. I was to stuck in thinking about changing the way it current worked rather than stepping back and going "wait, what is the best way to address this". I heartily endorse this PR. Let's ship this! |
Contributor
|
FYI @matajoh |
dcantah
reviewed
Jun 24, 2022
37d0904 to
364e112
Compare
helsaawy
approved these changes
Jun 27, 2022
Contributor
helsaawy
left a comment
There was a problem hiding this comment.
Minor nits, but LGTM overall
kevpar
reviewed
Jun 27, 2022
| EncodedSecurityPolicy: encoded, | ||
| Containers: containers, | ||
| Devices: devices, | ||
| Devices: map[string]string{}, |
Member
There was a problem hiding this comment.
Can you add a comment on what is being stored in this map?
Contributor
Author
There was a problem hiding this comment.
applied the docs patch, let me know if this works.
71e0016 to
6d853d5
Compare
Read-only container layer enforcement currently tracks which layers
have been mounted for each container. The state is being tracked
by maintaining a 2D slice of device targets and each slice represents
a potential overlay FS that can be used for a given container.
Index `i` in the Devices slice corresponds to container at index `i`
in the policy. For example for containers with the following hashes:
```
[
[hash1, hash2, hash3],
[hash1, hash4],
]
```
The corresponding Devices slice will look something like:
```
[
[/mnt/target1, /mnt/target2, /mnt/target3],
[/mnt/target1, /mnt/target4],
]
```
Each individual slice then corresponds to a potential overlay fs:
- overlay1: `[/mnt/target1, /mnt/target2, /mnt/target3]`
- overlay2: `[/mnt/target1, /mnt/target4]`
The issue arises when container contains duplicate layers like:
```
[
[hash1, hash2, hash2],
[hash1, hash4],
]
```
The potential overlays will be computed as following:
```
[
[/mnt/target1, /mnt/target2, /mnt/target2],
[/mnt/target1, /mnt/target4],
]
```
Instead of:
```
[
[/mnt/target1, /mnt/target2, /mnt/target3],
[/mnt/target1, /mnt/target4],
]
```
The issue is reproducable by the following Dockerfile:
```
FROM ubuntu:latest
COPY script.sh .
RUN chmod +x script.sh
```
where `script.sh` already has executable permission flag set.
To address the issue, the logic to track currently mounted devices
and enforcing the overlay has been updated.
Instead of tracking potential overlays, we track the devices that
have been mounted and their hashes:
```
{
/mnt/target1: hash1,
/mnt/target2: hash2,
/mnt/target3: hash2,
}
```
During overlay, we map the mount targets to the hashes and check
the resulting hash chain against the ones in the policy.
Signed-off-by: Maksim An <[email protected]>
Signed-off-by: Maksim An <[email protected]>
6d853d5 to
f6b45cc
Compare
…ounted. Signed-off-by: Maksim An <[email protected]>
Signed-off-by: Maksim An <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Read-only container layer enforcement currently tracks which layers
have been mounted for each container. The state is being tracked
by maintaining a 2D slice of device targets and each slice represents
a potential overlay FS that can be used for a given container.
Index
iin the Devices slice corresponds to container at indexiin the policy. For example for containers with the following hashes:
The corresponding Devices slice will look something like:
Each individual slice then corresponds to a potential overlay fs:
[/mnt/target1, /mnt/target2, /mnt/target3][/mnt/target1, /mnt/target4]The issue arises when container contains duplicate layers like:
The potential overlays will be computed as following:
Instead of:
The issue is reproducable by the following Dockerfile:
where
script.shalready has executable permission flag set.To address the issue, the logic to track currently mounted devices
and enforcing the overlay has been updated.
Instead of tracking potential overlays, we track the devices that
have been mounted and their hashes:
During overlay, we map the mount targets to the hashes and check
the resulting hash chain against the ones in the policy.
Signed-off-by: Maksim An [email protected]