Skip to content
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

Add SETENV support #932

Merged
merged 8 commits into from
Dec 18, 2024
Merged

Add SETENV support #932

merged 8 commits into from
Dec 18, 2024

Conversation

squell
Copy link
Member

@squell squell commented Dec 10, 2024

Closes #760

Also Closes #546, since we add parsing+warning support for things like NOEXEC and TIMEOUT.

This does not add support for --preserve-env or --preserve-env=VAR. I think the former needs some more discussion since it is the same as disabling env_reset. The latter is not controversial for us but better part of a separate PR.

This can be reviewed commit-by-commit if desired.

src/sudo/env/environment.rs Outdated Show resolved Hide resolved
@squell squell force-pushed the feature-setenv-parse branch 3 times, most recently from 79fc2cf to 8043767 Compare December 10, 2024 16:54
@squell squell force-pushed the feature-setenv-parse branch from 8043767 to d18d098 Compare December 17, 2024 22:37
@squell squell force-pushed the feature-setenv-parse branch from d18d098 to 62ac2bb Compare December 17, 2024 22:53
@squell squell mentioned this pull request Dec 17, 2024
@squell squell marked this pull request as ready for review December 17, 2024 23:21
@squell squell force-pushed the feature-setenv-parse branch 2 times, most recently from 6275390 to fe123d2 Compare December 17, 2024 23:28
@squell squell marked this pull request as draft December 17, 2024 23:28
@squell squell force-pushed the feature-setenv-parse branch 4 times, most recently from 032d90b to ca13518 Compare December 18, 2024 00:14
@squell squell force-pushed the feature-setenv-parse branch from f1dd0df to 61a983f Compare December 18, 2024 00:23
@squell squell force-pushed the feature-setenv-parse branch from 611ce26 to a5880af Compare December 18, 2024 00:25
@squell squell marked this pull request as ready for review December 18, 2024 00:31
@squell
Copy link
Member Author

squell commented Dec 18, 2024

Note that this implementation is slightly different from ogsudo, as indicated by the (non-)compliance test case in can_surprisingly_be_set_from_commandline that is explicitly ignored. Both sudo-rs and sudo-c treat (NO)SETENV like (NO)PASSWD; that is, it acts like an on/off switch for a single line and can apply to multiple commands following it.

However, there is an exception in that ALL has an implicit SETENV unless it is explicitly marked as NOSETENV. In ogsudo, this exception applies to any ALL, even one that appears to be 'ruled' by a more distant NOSETENV like so:

... NOSETENV: /path/to/some/command, ALL

sudo-rs will not allow environment control in this case--this is more predictable/logical (it says "NOSETENV", right?) and is also the 'fail safe' condition (this will cause noticeable errors if someone relies on this behaviour in sudo-c, instead of silently granting more privileges to users than might be expected). The above line is in fact a dubious configuration in any case since the ALL will subsume the previous /path/to/command.

@squell squell added enhancement New feature or request C-parser Parser/AST C-checker Permission checking logic labels Dec 18, 2024
@squell squell merged commit e731b54 into main Dec 18, 2024
13 checks passed
@squell squell deleted the feature-setenv-parse branch December 18, 2024 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-checker Permission checking logic C-parser Parser/AST enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SETENV/NOSETENV option Parse unknown tags in sudoers file even if we do not know them
2 participants