-
Notifications
You must be signed in to change notification settings - Fork 185
build.sh: allow group write on /usr/bin #4410
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
base: main
Are you sure you want to change the base?
Conversation
Allow group write permissions on /usr/bin because in upstream project's CI we want to overwrite binaries for testing. The dir is owned by root:root and CI runs in openshift as a user that is a member of the `root` (GID: 0) group. See coreos/coreos-installer#1716
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request adds group-write permissions to /usr/bin to facilitate testing in a CI environment. However, this change introduces a critical security vulnerability by allowing any process with root group privileges to modify system binaries, potentially leading to privilege escalation. My review includes a critical comment explaining the risks and suggesting a safer alternative, such as using a separate directory for test binaries and modifying the PATH.
| # Allow group write permissions on /usr/bin because in upstream | ||
| # project's CI we want to overwrite binaries for testing. The dir is | ||
| # owned by root:root and CI runs in openshift as a user that is a | ||
| # member of the `root` (GID: 0) group. | ||
| # See https://github.com/coreos/coreos-installer/pull/1716 | ||
| chmod g+w /usr/bin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Granting group-write permissions to /usr/bin introduces a critical security vulnerability. Any process running with GID 0 (as is common in OpenShift for non-root users) can modify or replace any system binary in that directory. This could be exploited for privilege escalation if a compromised process replaces a commonly used executable (like ls, sudo, or curl) with a malicious version.
While this is for a CI environment, it's a dangerous practice that violates the principle of least privilege. A safer alternative for overriding binaries for testing would be:
- Create a separate directory (e.g.,
/opt/testing-bin). - Grant write permissions to that directory.
- In your CI test environment, prepend this directory to the
PATHenvironment variable (e.g.,export PATH=/opt/testing-bin:$PATH).
This approach allows you to override binaries without altering the permissions of critical system directories.
We opened up the permissions when building the COSA container [1] so this isn't necessary any longer with a few adjustments here. [1] coreos/coreos-assembler#4410
|
so here we are opening up permissions on @HuijingHei maybe we could be more targeted in what we copy over in ostree CI? What we're currently copying is quite exhaustive: Details |
We opened up the permissions when building the COSA container [1] so this isn't necessary any longer with a few adjustments here. [1] coreos/coreos-assembler#4410
We opened up the permissions when building the COSA container [1] so this isn't necessary any longer with a few adjustments here. [1] coreos/coreos-assembler#4410
Agree with you, I have no better workaround for this, how about |
Allow group write permissions on /usr/bin because in upstream project's CI we want to overwrite binaries for testing. The dir is owned by root:root and CI runs in openshift as a user that is a member of the
root(GID: 0) group.See coreos/coreos-installer#1716