Page MenuHomeFreeBSD

bsdinstall: allow setting the root password via env variables
ClosedPublic

Authored by brd on Jun 24 2022, 1:48 PM.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

brd requested review of this revision.Jun 24 2022, 1:48 PM
This revision is now accepted and ready to land.Jun 24 2022, 4:04 PM

You could probably simplify it by just an if/elif/else instead of having to indent a separate if/else in the first else block, but, I don't think it is a big deal

Reviewed by: allanjude

kithrup_mac.com added a subscriber: kithrup_mac.com.
kithrup_mac.com added inline comments.
usr.sbin/bsdinstall/scripts/rootpass
36–42

This can just be

if [ -z "$ROOTPASS_ENC" -a -z "$ROOTPASS_PLAIN" ]; then
dteske requested changes to this revision.Jun 29 2022, 4:16 AM

Sorry for being late to the party

usr.sbin/bsdinstall/scripts/rootpass
36–42

Actually, it can be just:

if [ -z "$ROOTPASS_ENC$ROOTPASS_PLAIN" ]; then

As an aside …

You would want to avoid passing greater than 4 args to [ because many (all?) implementations only combinatorically guarantee syntactically correct results for handling up to 4 arguments (that for example passing 20 arguments, chaining many “-a” arguments, is not deterministically tested whereas there are test units for every possible combination up to — and including — 4 arguments). So using && was actually more correct from a portability standpoint, and I cannot condone the change to use -a.

However, the whole discussion of && vs [ … -a … ] is registered moot when you realize that you can just expand both variables into a single string and then test the results. If either string is non-NULL, the desired results are achieved without having to resort to using either && or -a

This revision now requires changes to proceed.Jun 29 2022, 4:16 AM
0mp requested changes to this revision.Jun 30 2022, 9:58 AM
0mp added a subscriber: 0mp.

Please include a patch for the manual page as well explaining the new variables.

usr.sbin/bsdinstall/scripts/rootpass
36–42

Ooooh clever and optimized, I like it.

Address feedback from allanjude@

brd marked 3 inline comments as done.Jul 8 2022, 5:29 PM
brd added inline comments.
usr.sbin/bsdinstall/scripts/rootpass
36–42

Also see the BUGS section of test(1)..

I chose to go with the way Allan suggested since it looks cleaner to me.

brd marked an inline comment as done.

Add man page update

LGTM, thanks!

One last nit that you may want to change is to use printf instead of echo. echo may behave in an undesired way if the root password is starting with a dash for example.

jilles added inline comments.
usr.sbin/bsdinstall/scripts/rootpass
36–38

Perhaps this echo won't cause any problems (nobody sets their root password to -e and -n, and nobody is going to run this with a shell with different echo behaviour?) but something like printf '%s\n' "$ROOTPASS_PLAIN" is more obviously correct.

An alternative is something like

pw -R $BSDINSTALL_CHROOT usermod root -h 0 <<END
$ROOTPASS_PLAIN
END

but that has the ugliness of a here-document.

usr.sbin/bsdinstall/scripts/rootpass
36–38

I tried to break it, and could not:

> cat test.sh    
#!/bin/sh

echo -nooo
> sh -x ./test.sh
+ echo -nooo
-nooo

Try #2:

> cat test.sh 
#!/bin/sh

echo -nooo lkjsldjf
> sh -x ./test.sh 
+ echo -nooo lkjsldjf
-nooo lkjsldjf

Anything else I should try?

FWIW, I'd switch to printf '%s' "$ROOTPASS" on general principles, to avoid the possibility of silliness.

In D35588#813713, @0mp wrote:

LGTM, thanks!

One last nit that you may want to change is to use printf instead of echo. echo may behave in an undesired way if the root password is starting with a dash for example.

*dons former infosec hat* Agree. Even if it looks OK now, who knows what you didn't consider or what someone will figure in 10 or 20 years?

pauamma requested changes to this revision.Tue, Jul 19, 4:00 AM
pauamma added inline comments.
usr.sbin/bsdinstall/bsdinstall.8
345–348

Need to say which is used if they're both present and give the format of ROOTPASS_ENC. Salted hash?

This revision now requires changes to proceed.Tue, Jul 19, 4:00 AM
usr.sbin/bsdinstall/bsdinstall.8
345–348

Did the first half, but I am not sure this is the right place to do the latter half.. Perhaps best to just Xr pw(8)?

Switch to printf(1) and prefer the encrypted one if both are set

usr.sbin/bsdinstall/bsdinstall.8
345–348

If by that you mean something like

in the format expected by
.Xr pw 8
.Fl H Ar 0

(markup not guaranteed correct), WFM.

Add man page link to pw(8)

This revision was not accepted when it landed; it landed in state Needs Review.Fri, Aug 5, 3:11 PM
This revision was automatically updated to reflect the committed changes.