search

ostreedev / ostree-rs-ext

Rust library with higher level APIs on top of the core ostree API
Apache License 2.0
72 stars 22 forks source link

Ensure SELinux label policy is applied when merging image layers #626

Closed jeamland closed 2 months ago

jeamland commented 2 months ago

This has some bearing on #388.

When importing a container image via ostree container image pull ..., care is taken to maintain SELinux label policy when importing the layers, vis:

https://github.com/ostreedev/ostree-rs-ext/blob/9a4743a657ffe0435018d9720c6df80a486ca0f1/lib/src/container/store.rs#L869-L876

and then in write_tar itself:

https://github.com/ostreedev/ostree-rs-ext/blob/9a4743a657ffe0435018d9720c6df80a486ca0f1/lib/src/tar/write.rs#L335-L343

However when it comes time to merge these layers with the base commit, we fail to apply the SELinux policy of the base commit:

https://github.com/ostreedev/ostree-rs-ext/blob/9a4743a657ffe0435018d9720c6df80a486ca0f1/lib/src/container/store.rs#L973-L985

This change adds a call to ostree::RepoCommitModifier::set_sepolicy_from_commit() to ensure that the SELinux labelling is consistent with the base commit.

cgwalters commented 2 months ago

The merge shouldn't introduce any new files or directories though; we should always be taking the underlying object. So I think this will just unnecessarily force us to recompute the labels without fixing the core bug from #388 right?

jeamland commented 2 months ago

So the relationship to #388 may not be there but there is a noticeable difference in result nonetheless.

tl;dr: The merge commit seems to relabel files based on the context in which the merge commit happens rather than the policy of the base commit. Working shown in (possibly overenthusiastic) detail below.

The scenario I'm working with here is that I've got an ostree commit created using rpm-ostree compose tree. I've exported this with ostree container encapsulate, then used the resulting container image to create a new container image, in this case installing another rpm (I chose telnet) not present in the original. I've then pulled this image back in to the original ostree repo using ostree container image pull. The ostree container image pull is being done inside a container.

When I run this process without this patch, I see an oddly large number of changes between the base commit and the commit based off the new container image:

# ostree diff --repo /tmp/test_repo base_commit ostree/container/image/oci-archive_3A__2F_customisation__test/layered_2E_tar | wc -l
6886

Examining this more closely we see some additions we expect:

# ostree diff --repo /tmp/test_repo base_commit ostree/container/image/oci-archive_3A__2F_customisation__test/layered_2E_tar | grep telnet
M    /usr/etc/selinux/targeted/active/modules/100/telnet
A    /usr/bin/telnet
A    /usr/share/doc/telnet
A    /usr/share/doc/telnet/README
A    /usr/share/man/man1/telnet.1.gz

But we also see a bunch of file modifications that seem unnecessary, to pick one example out of this case a whole bunch of zoneinfo files have changed:

# ostree diff --repo /tmp/test_repo base_commit ostree/container/image/oci-archive_3A__2F_customisation__test/layered_2E_tar | grep /usr/share/zoneinfo
M    /usr/share/zoneinfo
M    /usr/share/zoneinfo/Africa
M    /usr/share/zoneinfo/America
M    /usr/share/zoneinfo/America/Argentina
M    /usr/share/zoneinfo/America/Indiana
M    /usr/share/zoneinfo/America/Kentucky
M    /usr/share/zoneinfo/America/North_Dakota
M    /usr/share/zoneinfo/Antarctica
M    /usr/share/zoneinfo/Arctic
M    /usr/share/zoneinfo/Asia
M    /usr/share/zoneinfo/Atlantic
M    /usr/share/zoneinfo/Australia
M    /usr/share/zoneinfo/Brazil
M    /usr/share/zoneinfo/Canada
M    /usr/share/zoneinfo/Chile
M    /usr/share/zoneinfo/Etc
M    /usr/share/zoneinfo/Europe
M    /usr/share/zoneinfo/Indian
M    /usr/share/zoneinfo/Mexico
M    /usr/share/zoneinfo/Pacific
M    /usr/share/zoneinfo/US
[...]

Digging in to these we see that the only change is in the metadata, and the only change there is to the label:

# ostree ls -dXC --repo /tmp/test_repo base_commit /usr/share/zoneinfo/posix/Australia
d00755 0 0      0 4d0c29a0ee66111181a3b003c64a2aad3be8c0d59c4a7270be3dae58c5bb29ed 832a63bf3be2edb1d2306f191f01d86ca7bb0c69ba2b13cd711e6f1f53ec8572 { [(b'security.selinux', b'system_u:object_r:locale_t:s0')] } /usr/share/zoneinfo/posix/Australia
# ostree ls -dXC --repo  /tmp/test_repo ostree/container/image/oci-archive_3A__2F_customisation__test/layered_2E_tar /usr/share/zoneinfo/posix/Australia
d00755 0 0      0 4d0c29a0ee66111181a3b003c64a2aad3be8c0d59c4a7270be3dae58c5bb29ed 2b7ec15cc38eb013c1dc5db23a7879aaa9a21d8d4f3fbd69c7563a38743af64e { [(b'security.selinux', b'system_u:object_r:container_file_t:s0:c240,c499')] } /usr/share/zoneinfo/posix/Australia

So the next question is when did the label change happen? Luckily we can examine the individual layer commits. I see four of those and examining each we see:

# ostree ls -dXC --repo /tmp/test_repo ostree/container/blob/sha256_3A_017ac54d6797e1c173c7e20ee61ae24b701e2493d146db3f45aa918dffeb0347 /usr/share/zoneinfo/posix/Australia
error: Inspecting path '/usr/share/zoneinfo/posix/Australia': No such file or directory: /usr/share/zoneinfo
# ostree ls -dXC --repo /tmp/test_repo ostree/container/blob/sha256_3A_3ddaa625f2f100642534cba90880a57d31eb3cf0bf1f2e6d2c23988729f18049 /usr/share/zoneinfo/posix/Australia
error: Inspecting path '/usr/share/zoneinfo/posix/Australia': No such file or directory: /usr/share
# ostree ls -dXC --repo /tmp/test_repo ostree/container/blob/sha256_3A_525cd6a36ca994a49be414fdaf46457fc970f1f092aeec9f8160f76bf57c1124 /usr/share/zoneinfo/posix/Australia
error: Inspecting path '/usr/share/zoneinfo/posix/Australia': No such file or directory: /usr/share
# ostree ls -dXC --repo /tmp/test_repo ostree/container/blob/sha256_3A_b0bba405443017b77512bf3b28e217d894c56de48ad11e71e115d8e453b24127 /usr/share/zoneinfo/posix/Australia
d00755 0 0      0 4d0c29a0ee66111181a3b003c64a2aad3be8c0d59c4a7270be3dae58c5bb29ed 832a63bf3be2edb1d2306f191f01d86ca7bb0c69ba2b13cd711e6f1f53ec8572 { [(b'security.selinux', b'system_u:object_r:locale_t:s0')] } /usr/share/zoneinfo/posix/Australia

So the only place that directory shows up is in that last layer and with the correct label and, confirming a suspicion:

# ostree rev-parse --repo /tmp/test_repo ostree/container/blob/sha256_3A_b0bba405443017b77512bf3b28e217d894c56de48ad11e71e115d8e453b24127
1ce09beeae1185fcf8fbd850c7ce3ec309616ec39848e1351d5debf3942f631f
# ostree rev-parse --repo /tmp/test_repo base_commit
1ce09beeae1185fcf8fbd850c7ce3ec309616ec39848e1351d5debf3942f631f

Yep, it's the same as the base commit. So the only operation that touched that label was the merge commit.

Doing the process again but with the patch to add the SELinux policy during the merge commit:

# ostree diff --repo /tmp/test_repo base_commit ostree/container/image/oci-archive_3A__2F_customisation__test/layered_2E_tar | wc -l
30

That seems more like what I expected, as does this:

# ostree ls -dXC --repo /tmp/test_repo ostree/container/image/oci-archive_3A__2F_customisation__test/layered_2E_tar /usr/share/zoneinfo/posix/Australia
d00755 0 0      0 4d0c29a0ee66111181a3b003c64a2aad3be8c0d59c4a7270be3dae58c5bb29ed 832a63bf3be2edb1d2306f191f01d86ca7bb0c69ba2b13cd711e6f1f53ec8572 { [(b'security.selinux', b'system_u:object_r:locale_t:s0')] } /usr/share/zoneinfo/posix/Australia