syscontainers: set SELinux label for files copied to the host #1143
Conversation
|
@jeremyeder this should also fix the issue we were discussing last week with files copied to the host not getting the right label |
Atomic/rpm_host_install.py
Outdated
| @@ -25,6 +26,10 @@ def copyfile(src, dest): | |||
| return True | |||
| else: | |||
| shutil.copy2(src, dest) | |||
There was a problem hiding this comment.
It's more robust to use setfscreatecon() to atomically create the file with the right context; see what ostree-sepolicy.c does.
There was a problem hiding this comment.
thanks for the hint, I'll give it a try and update the patch. I'll need to check the "copy2" doesn't override it.
Atomic/rpm_host_install.py
Outdated
| @@ -52,7 +57,7 @@ def file_checksum(path, blocksize=(1<<20)): | |||
| return h.hexdigest() | |||
|
|
|||
| @staticmethod | |||
| def rm_add_files_to_host(old_installed_files_checksum, exports, prefix="/", files_template=None, values=None, rename_files=None): | |||
| def rm_add_files_to_host(old_installed_files_checksum, exports, prefix="/", files_template=None, values=None, rename_files=None, files_content=None): | |||
There was a problem hiding this comment.
hi, quick question, is files_content used? it seems to fail the pylint check when running normal make for upstream repo(forgot to mention it yesterday)
[root@localhost atomic]# make
/usr/bin/python setup.py build
running build
running build_py
copying Atomic/rpm_host_install.py -> build/lib/Atomic
running build_scripts
/usr/bin/python -m pylint --disable=all --enable=E --enable=W --additional-builtins=_ *.py atomic Atomic tests/unit/*.py -d=no-absolute-import,print-statement,no-absolute-import,bad-builtin,catching-non-exception,raising-non-exception
No config file found, using default configuration
************* Module Atomic.rpm_host_install
W: 60,133: Unused argument 'files_content' (unused-argument)
--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, -0.00)
make: *** [Makefile:41: pylint-check] Error 4
There was a problem hiding this comment.
thanks to have noticed it. It is a piece from another patch I was working on, I dropped it in a fixup patch
giuseppe
force-pushed
the
syscontainers-set-label
branch
from
fadcd8d to
412e981
Compare
giuseppe
changed the title
|
I've changed the implementation to use @peterbaouoft could you please verify if the firewalld system container still work with the new implementation and that the new files get a correct label? |
Atomic/rpm_host_install.py
Outdated
| @@ -74,7 +79,14 @@ def rm_add_files_to_host(old_installed_files_checksum, exports, prefix="/", file | |||
| # if there is a directory hostfs/ under exports, copy these files to the host file system. | |||
| hostfs = os.path.join(exports, "hostfs") | |||
| new_installed_files_checksum = {} | |||
| if os.path.exists(hostfs): | |||
| if not os.path.exists(hostfs): | |||
| return new_isntalled_files_checksum | |||
There was a problem hiding this comment.
could you please verify if the firewalld system container still work with the new implementation and that the new files get a correct label?
sure thing =)
Tho, seems like a typo here, shouldn't it be new_installed_files_checksum?
+ make pylint-check
python3 -m pylint --disable=all --enable=E --enable=W --additional-builtins=_ *.py atomic Atomic tests/unit/*.py -d=no-absolute-import,print-statement,no-absolute-import,bad-builtin,catching-non-exception,raising-non-exception
No config file found, using default configuration
************* Module Atomic.rpm_host_install
E: 83,19: Undefined variable 'new_isntalled_files_checksum' (undefined-variable)
There was a problem hiding this comment.
:/ yes thanks. Fixed in a newer version
giuseppe
force-pushed
the
syscontainers-set-label
branch
3 times, most recently
from
774cb72 to
64b8eee
Compare
giuseppe
changed the title
giuseppe
force-pushed
the
syscontainers-set-label
branch
7 times, most recently
from
e65ff3e to
7ea2756
Compare
Signed-off-by: Giuseppe Scrivano <[email protected]>
Signed-off-by: Giuseppe Scrivano <[email protected]>
giuseppe
force-pushed
the
syscontainers-set-label
branch
from
7ea2756 to
4f9e964
Compare
|
@peterbaouoft from my tests here, it solves the issue. Have you had a chance to verify this on your end? |
|
@giuseppe , currently checking. The last time steve and I verified the first stage However, recently I checked on the host I had, I am unable to run EDIT: Thanks for the fix. This patch fixes the problem we saw earlier for selinux issue, sadly not fixing |
|
@rhatdan are you fine with this PR? |
|
Well this will cause the target to be labeled correctly and all of the content to have that label. Any content created under the label may or may not be labeled correctly. So if you are copying a single file this will be fine, if you are copying a directory it could be a problem. |
|
we copy each file separately so that selabel_lookup_raw+setfscreatecon_raw is done for each file. Doesn't that solve the problem with the directories labelling? |
|
Yes that will solve the problem. |
|
LGTM |
|
📌 Commit 4f9e964 has been approved by |
Signed-off-by: Giuseppe Scrivano <[email protected]> Closes: #1143 Approved by: rhatdan
|
☀️ Test successful - status-papr |
Signed-off-by: Giuseppe Scrivano <[email protected]> Closes: projectatomic#1143 Approved by: rhatdan
Signed-off-by: Giuseppe Scrivano <[email protected]> Closes: projectatomic#1143 Approved by: rhatdan
Signed-off-by: Giuseppe Scrivano [email protected]