Skip to content

add SUDO_HOME (fixes #928) #1087

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

raviksharma
Copy link

Here is I how tested the feature.

ravi@dragon0:~/code/raviksharma/sudo-rs$ cargo build --release
   Compiling libc v0.2.171
   Compiling sudo-rs v0.2.5 (/home/ravi/code/raviksharma/sudo-rs)
   Compiling glob v0.3.2
   Compiling log v0.4.27
    Finished `release` profile [optimized] target(s) in 7.58s
ravi@dragon0:~/code/raviksharma/sudo-rs$ sudo chown root:root target/release/sudo
ravi@dragon0:~/code/raviksharma/sudo-rs$ sudo chmod 4755 target/release/sudo
ravi@dragon0:~/code/raviksharma/sudo-rs$ ./target/release/sudo env|grep SUDO_*
SUDO_COMMAND=/usr/bin/env
SUDO_GID=1000
SUDO_HOME=/home/ravi
SUDO_UID=1000
SUDO_USER=ravi
ravi@dragon0:~/code/raviksharma/sudo-rs$ echo $HOME
/home/ravi
ravi@dragon0:~/code/raviksharma/sudo-rs$ id
uid=1000(ravi) gid=1000(ravi) ...

@raviksharma raviksharma marked this pull request as draft April 28, 2025 14:57
@squell squell linked an issue Apr 28, 2025 that may be closed by this pull request
@squell
Copy link
Member

squell commented Apr 28, 2025

I'm guessing (without having even looked at the code) that the compliance-test-og is failing simply because SUDO_HOME isn't in the sudo that we're testing against. Several fix opportunities:

  • One fix would be to move the integration tests to the e2e-tests for now. Not the nicest, perhaps, because then we don't see the correspondence with original sudo.

  • Last: it's of course possible in the test to not just ask if we're running "original_sudo", but also what version of original sudo and only run the test if it's higher than the ogsudo version that introduced this.

I'd also be curious if someone has an objection to adding SUDO_HOME (@rnijveld?).

Edit after looking at the code changes Note that otherwise code changes LGTM 👍

@squell squell added the C-exec Execution component (interfacing with OS) label Apr 28, 2025
@rnijveld
Copy link
Collaborator

rnijveld commented May 2, 2025

In principle I'm not against this feature, but I would like to know what the problem to be solved is (criteria 3 of our contributing guidelines), and in what way this is a common problem (criteria 4). I don't believe we need to worry about the other points in there, I don't see any security concerns here myself and this implementation is straightforward and doesn't add any dependencies. Would you be able to answer this?

@squell
Copy link
Member

squell commented May 2, 2025

Also see sudo-project/sudo#358 which perhaps has a bit of a rationale

@raviksharma
Copy link
Author

I see sudo-compliance-tests is running ogsudo version 1.19.13 from bookwork and the SUDO_HOME was introduced in 1.9.16. Let me see what I can do.

@squell squell added freeze Temporarily blocked from merging pending a release and removed freeze Temporarily blocked from merging pending a release labels May 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-exec Execution component (interfacing with OS)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Original sudo introduced SUDO_HOME
3 participants