etcupdate: check passwords after resolving conflicts

[oshogbo]

Summary

Recently we have changed a default shell for a root user.
We got some notification from users that after etcupdate the root
password is not set. This is caused by wrongly resolving conflicts
with etcupdate(1).

This change verify if the password is still set after resolving
conflicts. If the password was not set or still is this change
doesn't have any effect.

[oshogbo]

Style.

[emaste]

usr.sbin/etcupdate/etcupdate.sh

874

we just list whether the user had a password or not, not the password itself, right?
is there value in extending to no passwd, *, and a password?

896

Not sure what "distantly" means here.

[oshogbo]

Fix typo in comment, spotted by @emaste.

[oshogbo]

usr.sbin/etcupdate/etcupdate.sh

874

Yes exactly, we just check if password is there or not. The * is treated as password.

I had a version to distinguish those tree values. Then I was start wondering if we care if somebody changed from * to password or from password to *.
At the end I decided that this doesn't matters, but when you raised this question I started to wonder again.

896

Auto correct - sorry - should be "mistakenly".

[jhb]

usr.sbin/etcupdate/etcupdate.sh

896

Do you mean "Check if user accidentally removed a password"? English doesn't normally use double negatives and this currently implies to me the mistake is failing to remove passwords, not failing to preserve existing passwords.

[oshogbo]

Yeach, I totaly forgot about this part @jhb.

Elminate double negation.

Thanks!

[jhb]

usr.sbin/etcupdate/etcupdate.sh

874

I actually suspect you can do a stronger assertion which is that the password field shouldn't change as a result of etcupdate. That might be harder though as you want to avoid storing the hashes in env vars which is what that would otherwise require. If temp files were an option I'd be tempted to instead just pipe off the first two fields of both files to temp files and use 'comm' to check for any differences and then pipe the comm output to cut/awk to get just the first column for a list of users whose password had changed?

[oshogbo]

usr.sbin/etcupdate/etcupdate.sh

874

I quite afraid of tmpfiles and the env vars.
Maybe we can store all the data inside the awk?
Still if awk will crash we can leak hashes but it think it's more robust.
Something like:

awk '
BEGIN {
	FS = ":"
}

{
	if ($1 in user) {
		if (user[$1] != $2) {
			print $1 " changed"
		}
	} else {
		user[$1] = $2;
	}

}' ORIG_PASSWD NEW_PASSWD

This still need some, work, like handling comments or duplicates. What do you think?

[oshogbo]

usr.sbin/etcupdate/etcupdate.sh

874

s/it think it's more robust./I think it's more robust./

[oshogbo]

Ping.

[jhb]

usr.sbin/etcupdate/etcupdate.sh

927

Note that a panic fails the entire etcupdate and we might be part-way through a merge here. Perhaps this should be a warning instead?