Repository navigation
firstboot: Add --root-shell option and tighten up passwd/shadow handling - #16496
Conversation
bdfe4e2 to
77e4173
Compare
poettering
left a comment
poettering
left a comment
There was a problem hiding this comment.
man page update missing
i wonder if we should make a stricter check: i.e. validate that the selected shell is actually available in the image. Or maybe it shouldn#t be a hard check, but a warning only. i.e. a chase_symlinks() check placed at an appropriate place
poettering
added
the
reviewed/needs-rework 🔨
8819e07 to
b3dcf83
Compare
|
Addressed the comments and switched the order of the two commits. First comes the refactoring, then the new feature. Should be a lot more logical to follow. |
|
man page update still missing, see earlier review |
|
(looks good codewise otherwise) |
There are a lot of edge cases that the current implementation doesn't handle, especially in cases where one of passwd/shadow exists and the other doesn't exist. For example, if --root-password is specified, we will write /etc/shadow but won't add a root entry to /etc/passwd if there is none. To fix some of these issues, we constrain systemd-firstboot to only modify /etc/passwd and /etc/shadow if both do not exist already (or --force) is specified. On top of that, we calculate all necessary information for both passwd and shadow upfront so we can take it all into account when writing the actual files. If no root password options are given --force is specified or both files do not exist, we lock the root account for security purposes.
b3dcf83 to
28900a1
Compare
|
btw, did you see my earlier comments that we probably should at least warn if the specified shell doesn't exist in the image? i.e. a quick call to chase_symlinks() with the specified shell and an error message if it cannot be found? |
|
Yes I did, it was getting late yesterday so I didn't add it yet. Should I modify valid_shell to do the chase_symlinks call or will that break other stuff? |
|
I'd keep it separate. valid_shell should not go to disk I think. it should be a superficial check just checking if the value makes any sense at all without matching it up with reality on some system. |
yuwata
removed
the
reviewed/needs-rework 🔨
4 participants
The first commit adds --root-shell and related options to configure the shell for the root user.
Second commit tries to tighten up the whole passwd/shadow story a bit more. There are a lot of edge cases when we're modifying both files since they generally should be modified in lockstep so I tried to reduce the number of possibles scenarios a bit by either modifying neither or modifying both but nothing inbetween that.
I also added some logic so that if we have to add the root entries to passwd and shadow ourselves and no root password is provided by the user, we lock the root account so as to not cause security issues by accident.
Tested manually and via my modified mkosi that uses systemd-firstboot. Maybe we should eventually add a firstboot test to make it a bit harder to accidentally break stuff.
Ideally this could still go in for v246 since it makes stuff a bit more strict than before (if we want this in the first place of course).