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

Implement reading password from pipe/stdin #927

Merged
merged 2 commits into from
Feb 1, 2024

Conversation

thalman
Copy link
Contributor

@thalman thalman commented Jan 25, 2024

New option --stdin/-s is available for root user. It is useful
for automation/setup and it makes shadow utils passwd more versatile.

Add alternative function to agetpass for reading password
from stdin or pipe.

Signed-off-by: Tomas Halman <tomas@halman.net>
src/passwd.c Outdated Show resolved Hide resolved
@stoeckmann
Copy link
Contributor

It is useful for automation/setup and it makes shadow utils passwd more versatile.

Which use case does this cover compared to what newusers has to offer? The advantage of newusers is that it's not setuid. Pretty much "root only" already.

@thalman
Copy link
Contributor Author

thalman commented Jan 26, 2024

Hmm, I found that my PR does not work correctly with PAM. Let me investigate it first.

Copy link
Collaborator

@alejandro-colomar alejandro-colomar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't remove the -s comment in this PR. (Never mind; fixed in master.)

@alejandro-colomar alejandro-colomar dismissed their stale review January 29, 2024 15:05

Already fixed in master

New option --stdin/-t is available for root user. It is useful
for automation/setup and it makes shadow utils passwd more versatile.

Signed-off-by: Tomas Halman <tomas@halman.net>
@thalman
Copy link
Contributor Author

thalman commented Jan 29, 2024

It is useful for automation/setup and it makes shadow utils passwd more versatile.

Which use case does this cover compared to what newusers has to offer? The advantage of newusers is that it's not setuid. Pretty much "root only" already.

It is useful in automation and tests and compatibility.

Distributions like Fedora and RHEL (and clones) uses another implementation of passwd that has --stdin option.
Currently I'm trying to push passwd from shadow utils to Fedora as a replacement for that other implementation.

To make transition easier for Fedora/RHEL users, I would like to have this option implemented in passwd utility.

But yes you are right, this can be achieved in many different ways.

@thalman
Copy link
Contributor Author

thalman commented Jan 29, 2024

Please don't remove the -s comment in this PR. (Never mind; fixed in master.)

Removed from PR

@thalman thalman marked this pull request as ready for review January 29, 2024 15:28
Copy link
Collaborator

@ikerexxe ikerexxe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for the patch and your hard work here!

@stoeckmann
Copy link
Contributor

Which use case does this cover compared to what newusers has to offer? The advantage of newusers is that it's not setuid. Pretty much "root only" already.

But yes you are right, this can be achieved in many different ways.

Just to add to my initial remark: The solution with newusers has a critical issue: Passwords cannot contain colons because they are field separators. So I'm not a fan of newusers anymore at all.

Thank you for clarification!

@hallyn hallyn merged commit 49001ca into shadow-maint:master Feb 1, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants