Tests: Usermod rename user, change password, lock password#1622
Tests: Usermod rename user, change password, lock password#1622aborah-sudo wants to merge 4 commits intoshadow-maint:masterfrom
Conversation
aborah-sudo
changed the title
aborah-sudo
force-pushed
the
usermode
branch
7 times, most recently
from
1df807b to
83bcf72
Compare
aborah-sudo
force-pushed
the
usermode
branch
6 times, most recently
from
d06e12d to
0611ed0
Compare
ikerexxe
left a comment
ikerexxe
left a comment
There was a problem hiding this comment.
I see you've chosen passlib over my suggestion to use hashlib. Can you help me understand the technical reasoning behind this choice? I want to make sure I understand the requirements correctly.
In addition, several commit messages mention usermode, but that's a different binary that isn't provided by shadow. Do you mind updating the commit messages to usermod?
aborah-sudo
force-pushed
the
usermode
branch
4 times, most recently
from
7f9d5a6 to
e761230
Compare
|
I see we went back to the I also want to avoid Please implement this using |
aborah-sudo
force-pushed
the
usermode
branch
3 times, most recently
from
6d48bb4 to
7b7a520
Compare
Done |
ikerexxe
left a comment
ikerexxe
left a comment
There was a problem hiding this comment.
There's a critical issue with the current password hash generation approach that needs to be addressed. While the code that you implemented produces a string that looks like a SHA-512 crypt hash (starting with $6$), it's not actually a valid SHA-512 crypt hash according to the crypt(3) specification. The issue is that hashlib.sha512() performs a simple single-pass hash, whereas the SHA-512 crypt algorithm requires thousands of rounds of key stretching (typically 5000+ iterations) and uses a specific base64 encoding with a custom alphabet. Real SHA-512 crypt hashes have exactly 86 base64-encoded characters after the salt, not 128 hex characters.
The practical impact is that while usermod -p will accept and store your generated hash (since it doesn't validate the format), users would never be able to authenticate with passwords set this way because the system's authentication libraries expect real crypt-format hashes.
| shadow_entry = shadow.tools.getent.shadow("tuser1") | ||
| assert shadow_entry is not None, "User should be found" | ||
| assert shadow_entry.password is not None, "Password should not be None" | ||
| assert shadow_entry.password.startswith("!"), "Password should be locked with ! prefix" | ||
| assert shadow_entry.password == f"!{password_hash}", "Password should have ! prefix when locked" |
There was a problem hiding this comment.
This check is missing in the steps and expectedresults. You need to move some setup to steps and also add this check
That means i have to use passlib as i used before ? passlib is the officially recommended replacement for the crypt module. It provides cross-platform support for SHA-512 crypt hashes. |
Added passlib dependency for generating valid SHA-512 crypt hashes
This is the transformation to Python of the test located in `tests/usertools/01/10_usermod_rename_user_in_group.test` which checks that `usermod` can rename user who is member of a group
This is the transformation to Python of the test located in `tests/usertools/01/11_usermod_change_password.test` which checks that `usermod` can change user password
This is the transformation to Python of the test located in `tests/usertools/01/11_usermod_lock_password.test` which checks that `usermod` can lock user password
2 participants
No description provided.