Bug 2078938

Summary: sudo wrapper breaks secure_path
Product: Red Hat Developer Toolset Reporter: Mohamed Akram <mohd.akram>
Component: devtoolset-metaAssignee: Marek Polacek <mpolacek>
Status: CLOSED CANTFIX QA Contact: Martin Cermak <mcermak>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: DTS 11.0 RHEL 7CC: fche, fweimer, jakub, mnewsome, ohudlick, sipoyare
Target Milestone: alpha   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2022-10-17 21:45:08 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
A patch to fix the issue
none
More comprehensive patch, remove redundant use of env
none
package-independent sudo wrapper none

Description Mohamed Akram 2022-04-26 14:19:03 UTC
Created attachment 1875101 [details]
A patch to fix the issue

Description of problem:

The sudo wrapper that comes with devtoolset forcibly overrides the PATH, ignoring any setting of secure_path.

Version-Release number of selected component (if applicable):
11.0

How reproducible:
Always

Steps to Reproduce:
1. Have the following in /etc/sudoers:

Defaults secure_path = /sbin:/bin:/usr/sbin:/usr/bin

2. Run:

PATH=/home/user/bin:/usr/bin scl enable devtoolset-11 -- sudo bash -c 'echo \$PATH'

Actual results:
/opt/rh/devtoolset-11/root/usr/bin:/home/user/bin:/usr/bin

Expected results:
/opt/rh/devtoolset-11/root/usr/bin:/sbin:/bin:/usr/sbin:/usr/bin

Additional info:
A possible patch is attached to fix this.

Comment 1 Mohamed Akram 2022-04-26 14:44:19 UTC
Created attachment 1875104 [details]
More comprehensive patch, remove redundant use of env

Comment 2 Mohamed Akram 2022-04-27 08:09:22 UTC
Created attachment 1875259 [details]
package-independent sudo wrapper

Comment 3 Marek Polacek 2022-10-17 21:45:08 UTC
Sorry it's taken so long to respond to this.

It's not clear to me what you're actually trying to do.  scl enable has to set PATH/LD_LIBRARY_PATH, so the patch cannot be accepted.
scl enable will make use of the toolset binaries.

Maybe you want to use "/usr/bin/sudo scl enable <...>" instead?

I'm afraid we can't really change how the script works, because it might break other uses.

Comment 4 Mohamed Akram 2022-10-18 08:03:36 UTC
I ran into this issue because a script I had broke once I started using a SCL. The binary I was running was in /usr/sbin - when using regular sudo, I could run the program just fine via eg. `sudo semanage`. When I enable an SCL, trying to run the same command afterward fails, because the wrapper script took the existing PATH from the current non-root user, which doesn't have /usr/sbin.

> It's not clear to me what you're actually trying to do.  scl enable has to set PATH/LD_LIBRARY_PATH, so the patch cannot be accepted.
> scl enable will make use of the toolset binaries.

My script allows that. Instead of passing through the entire environment (via `-E`) and walking over `secure_path`, which is unnecessary and dangerous, I simply re-enable all the SCLs again inside the sudo call (making sure to reset `X_SCLS=` beforehand in case the user passes `-E`, since the scl command checks it before enabling) so that it sets the paths up. I've tested it and it seems to work well.

> Maybe you want to use "/usr/bin/sudo scl enable <...>" instead?

That won't work because I am using a `/etc/profile.d/enabledevtoolset-11.sh` file to load the SCL automatically.

> I'm afraid we can't really change how the script works, because it might break other uses.

I think being able to run binaries in /usr/sbin is a fairly important use for sudo, since that's where most admin commands live. The current script is also quite problematic in how it bypasses security features of sudo without informing the user. If not fixed in devtoolset-11 to not break people's code relying on the wrong behavior, I believe it should be fixed in newer devtoolsets. I hope you can reopen this issue.