Re: usermod: lock/unlock local password
On Mon, Feb 11, 2013 at 10:11:25PM +0100, André Stöbe wrote: > Antoine Jacoutot wrote: > > This diff adds 2 new options to usermod(8): > > -U to unlock a user's password > > -Z to lock a user's password > > Today I was working with these two switches and really got confused. > I've tested the following with snapshots from Jan 11 and 5.3-beta. > > I've got a user with 13 asterisks in the password field as described in > passwd(5): > test:*:1002:1002::0:0:,,,:/home/test:/bin/ksh > > After locking the account with "usermod -Z test": > test:*:1002:1002::0:0:,,,:/home/test:/bin/ksh- > > After unlocking the account with "usermod -U test": > test::1002:1002::0:0:,,,:/home/test:/bin > > 1) The login shell is broken. Fixed. > 2) The password field consists of 12 asterisks. I'd expect it to be just > the same as it was before unlocking the account. This propably makes > security(8) complain, and more importantly, it never adds an asterisk > when locking but always removes an asterisk when unlocking, so the > account would be accessible without a password after some lock-unlock > cycles (at least the shell is still broken): > test::1002:1002::0:0:,,,:/home/test:/bin That's because contrary to other Unices, we lock an account by prefixing the pwd with `*' (others use `!'). I'll rework that part to make it more clever. Thanks for the report André. -- Antoine
Re: usermod: lock/unlock local password
On Mon, Feb 11, 2013 at 10:57:46PM +0100, Antoine Jacoutot wrote: > On Mon, Feb 11, 2013 at 10:11:25PM +0100, André Stöbe wrote: > > Antoine Jacoutot wrote: > > > This diff adds 2 new options to usermod(8): > > > -U to unlock a user's password > > > -Z to lock a user's password > > > > Today I was working with these two switches and really got confused. > > I've tested the following with snapshots from Jan 11 and 5.3-beta. > > > > I've got a user with 13 asterisks in the password field as described in > > passwd(5): > > test:*:1002:1002::0:0:,,,:/home/test:/bin/ksh > > > > After locking the account with "usermod -Z test": > > test:*:1002:1002::0:0:,,,:/home/test:/bin/ksh- > > > > After unlocking the account with "usermod -U test": > > test::1002:1002::0:0:,,,:/home/test:/bin > > > > 1) The login shell is broken. > > 2) The password field consists of 12 asterisks. I'd expect it to be just > > the same as it was before unlocking the account. This propably makes > > security(8) complain, and more importantly, it never adds an asterisk > > when locking but always removes an asterisk when unlocking, so the > > account would be accessible without a password after some lock-unlock > > cycles (at least the shell is still broken): > > test::1002:1002::0:0:,,,:/home/test:/bin > > > > Can't tell if this problem relates to users with normal password > > authentication. I did only test users with 13 asterisks in the password > > field. > > I'll have a look. OK, I was reading passwd(5) and now I'm asking myself - why the hell do daemons from ports have 13 asterisks in password field (base daemons just have single asterisk)? _tor:*:566:566:daemon:0:0:tor:/nonexistent:/sbin/nologin This is obviously not intended to be an account for logging in even via some "other authentication methods". jirib
Re: usermod: lock/unlock local password
On Mon, Feb 11, 2013 at 10:11:25PM +0100, André Stöbe wrote: > Antoine Jacoutot wrote: > > This diff adds 2 new options to usermod(8): > > -U to unlock a user's password > > -Z to lock a user's password > > Today I was working with these two switches and really got confused. > I've tested the following with snapshots from Jan 11 and 5.3-beta. > > I've got a user with 13 asterisks in the password field as described in > passwd(5): > test:*:1002:1002::0:0:,,,:/home/test:/bin/ksh > > After locking the account with "usermod -Z test": > test:*:1002:1002::0:0:,,,:/home/test:/bin/ksh- > > After unlocking the account with "usermod -U test": > test::1002:1002::0:0:,,,:/home/test:/bin > > 1) The login shell is broken. > 2) The password field consists of 12 asterisks. I'd expect it to be just > the same as it was before unlocking the account. This propably makes > security(8) complain, and more importantly, it never adds an asterisk > when locking but always removes an asterisk when unlocking, so the > account would be accessible without a password after some lock-unlock > cycles (at least the shell is still broken): > test::1002:1002::0:0:,,,:/home/test:/bin > > Can't tell if this problem relates to users with normal password > authentication. I did only test users with 13 asterisks in the password > field. I'll have a look. -- Antoine
Re: usermod: lock/unlock local password
Antoine Jacoutot wrote: > This diff adds 2 new options to usermod(8): > -U to unlock a user's password > -Z to lock a user's password Today I was working with these two switches and really got confused. I've tested the following with snapshots from Jan 11 and 5.3-beta. I've got a user with 13 asterisks in the password field as described in passwd(5): test:*:1002:1002::0:0:,,,:/home/test:/bin/ksh After locking the account with "usermod -Z test": test:*:1002:1002::0:0:,,,:/home/test:/bin/ksh- After unlocking the account with "usermod -U test": test::1002:1002::0:0:,,,:/home/test:/bin 1) The login shell is broken. 2) The password field consists of 12 asterisks. I'd expect it to be just the same as it was before unlocking the account. This propably makes security(8) complain, and more importantly, it never adds an asterisk when locking but always removes an asterisk when unlocking, so the account would be accessible without a password after some lock-unlock cycles (at least the shell is still broken): test::1002:1002::0:0:,,,:/home/test:/bin Can't tell if this problem relates to users with normal password authentication. I did only test users with 13 asterisks in the password field. Regards André
Re: usermod: lock/unlock local password
On Fri, Sep 14, 2012 at 08:59, Antoine Jacoutot wrote: > > Anyone? >> +/* lock the account */ >> +if (strncmp(pwp->pw_shell+strlen(pwp->pw_shell) - 1, > acctlock_str+strlen(acctlock_str) - 1, sizeof(acctlock_str) - 1) != 0) { This looks like a horifically complicated way to check if the last character is -. Can we simplify some?
Re: usermod: lock/unlock local password
On Tue, Sep 11, 2012 at 06:24:04PM +0200, Antoine Jacoutot wrote: > On Tue, Sep 11, 2012 at 04:46:57PM +0200, Antoine Jacoutot wrote: > > On Mon, Sep 10, 2012 at 05:01:13PM +0200, Antoine Jacoutot wrote: > > > Hi. > > > > > > This diff adds 2 new options to usermod(8): > > > -U to unlock a user's password > > > -Z to lock a user's password > > > > > > In effect locking/unlocking the password means to add a '!' in front of > > > the encrypted entry in master.passwd. > > > Note that this disable the _password_ not the account of course (you > > > could still connect using ssh+key for e.g.). > > > > > > That said, I have some use for it and would like to be able to have this > > > if at all possible. > > > Behavior is basically the same as Linux's usermod(8) except that I am > > > using -Z for locking the password (-Z is for SElinux in Linux land and > > > -L is used instead but we use it ourselves for the login class). > > > > Ok new diff that does something slightly different. > > Instead of putting a '!' in front of the password (which confused people), > > lock/unlock means appending/removing a dash to the user's login shell. > > While this is non standard behavior (but as I mentionned in a previous > > mail, usermod(8) is already not standard accross unices) I think it is > > better because: > > - account is locked, period (no local login, no remote login -- no surprise > > for tedu ;-) ) > > - no values are touched > > (e.g. one could disable an account by using 'usermod -e 1' but when he > > wants to re-enable the account he'll be forced to change the expiration > > time) > > New diff which also put a '*' in front of the encrypted password; i.e. both > pre and post-auth become locked (req. by Theo). > With some man page tweak from jmc. Anyone? > Index: user.c > === > RCS file: /cvs/src/usr.sbin/user/user.c,v > retrieving revision 1.90 > diff -u -r1.90 user.c > --- user.c29 Jan 2012 08:38:54 - 1.90 > +++ user.c11 Sep 2012 16:20:11 - > @@ -100,7 +100,9 @@ > F_UID = 0x0400, > F_USERNAME = 0x0800, > F_CLASS = 0x1000, > - F_SETSECGROUP = 0x4000 > + F_SETSECGROUP = 0x4000, > + F_ACCTLOCK = 0x8000, > + F_ACCTUNLOCK= 0x1 > }; > > #define CONFFILE "/etc/usermgmt.conf" > @@ -1339,11 +1341,17 @@ > struct group*grp; > const char *homedir; > charbuf[LINE_MAX]; > + characctlock_str[] = "-"; > + charpwlock_str[] = "*"; > + charpw_len[PasswordLength + 1]; > + charshell_len[MaxShellNameLen]; > size_t colonc, loginc; > size_t cc; > FILE*master; > charnewdir[MaxFileNameLen]; > char*colon; > + char*pw_tmp = NULL; > + char*shell_tmp = NULL; > int len; > int masterfd; > int ptmpfd; > @@ -1359,6 +1367,10 @@ > if (!is_local(login_name, _PATH_MASTERPASSWD)) { > errx(EXIT_FAILURE, "User `%s' must be a local user", > login_name); > } > + if (up != NULL) { > + if ((up->u_flags & (F_ACCTLOCK | F_ACCTUNLOCK)) && (pwp->pw_uid > == 0)) > + errx(EXIT_FAILURE, "(un)locking is not supported for > the `%s' account", pwp->pw_name); > + } > /* keep dir name in case we need it for '-m' */ > homedir = pwp->pw_dir; > > @@ -1410,6 +1422,48 @@ > pwp->pw_passwd = up->u_password; > } > } > + if (up->u_flags & F_ACCTLOCK) { > + /* lock the account */ > + if (strncmp(pwp->pw_shell+strlen(pwp->pw_shell) - 1, > acctlock_str+strlen(acctlock_str) - 1, sizeof(acctlock_str) - 1) != 0) { > + shell_tmp = malloc(strlen(pwp->pw_shell) + > sizeof(acctlock_str)); > + if (shell_tmp == NULL) { > + (void) close(ptmpfd); > + pw_abort(); > + errx(EXIT_FAILURE, "account lock: > cannot allocate memory"); > + } > + strlcpy(shell_tmp, pwp->pw_shell, > sizeof(shell_len)); > + strlcat(shell_tmp, acctlock_str, > sizeof(shell_len)); > + pwp->pw_shell = shell_tmp; > + } > + /* lock the password */ > + if (strncmp(pwp->pw_passwd, pwlock_str, > sizeof(pwlock_str)-1) != 0) { > + pw_tmp = malloc(strlen(pwp->pw_passwd) + > sizeof(pwlock_str)); > + if (pw_tmp == NULL) { > + (void) close(ptmpfd); > +
Re: usermod: lock/unlock local password
On Tue, Sep 11, 2012 at 04:46:57PM +0200, Antoine Jacoutot wrote: > On Mon, Sep 10, 2012 at 05:01:13PM +0200, Antoine Jacoutot wrote: > > Hi. > > > > This diff adds 2 new options to usermod(8): > > -U to unlock a user's password > > -Z to lock a user's password > > > > In effect locking/unlocking the password means to add a '!' in front of > > the encrypted entry in master.passwd. > > Note that this disable the _password_ not the account of course (you > > could still connect using ssh+key for e.g.). > > > > That said, I have some use for it and would like to be able to have this > > if at all possible. > > Behavior is basically the same as Linux's usermod(8) except that I am > > using -Z for locking the password (-Z is for SElinux in Linux land and > > -L is used instead but we use it ourselves for the login class). > > Ok new diff that does something slightly different. > Instead of putting a '!' in front of the password (which confused people), > lock/unlock means appending/removing a dash to the user's login shell. > While this is non standard behavior (but as I mentionned in a previous mail, > usermod(8) is already not standard accross unices) I think it is better > because: > - account is locked, period (no local login, no remote login -- no surprise > for tedu ;-) ) > - no values are touched > (e.g. one could disable an account by using 'usermod -e 1' but when he > wants to re-enable the account he'll be forced to change the expiration time) New diff which also put a '*' in front of the encrypted password; i.e. both pre and post-auth become locked (req. by Theo). With some man page tweak from jmc. Index: user.c === RCS file: /cvs/src/usr.sbin/user/user.c,v retrieving revision 1.90 diff -u -r1.90 user.c --- user.c 29 Jan 2012 08:38:54 - 1.90 +++ user.c 11 Sep 2012 16:20:11 - @@ -100,7 +100,9 @@ F_UID = 0x0400, F_USERNAME = 0x0800, F_CLASS = 0x1000, - F_SETSECGROUP = 0x4000 + F_SETSECGROUP = 0x4000, + F_ACCTLOCK = 0x8000, + F_ACCTUNLOCK= 0x1 }; #define CONFFILE "/etc/usermgmt.conf" @@ -1339,11 +1341,17 @@ struct group*grp; const char *homedir; charbuf[LINE_MAX]; + characctlock_str[] = "-"; + charpwlock_str[] = "*"; + charpw_len[PasswordLength + 1]; + charshell_len[MaxShellNameLen]; size_t colonc, loginc; size_t cc; FILE*master; charnewdir[MaxFileNameLen]; char*colon; + char*pw_tmp = NULL; + char*shell_tmp = NULL; int len; int masterfd; int ptmpfd; @@ -1359,6 +1367,10 @@ if (!is_local(login_name, _PATH_MASTERPASSWD)) { errx(EXIT_FAILURE, "User `%s' must be a local user", login_name); } + if (up != NULL) { + if ((up->u_flags & (F_ACCTLOCK | F_ACCTUNLOCK)) && (pwp->pw_uid == 0)) + errx(EXIT_FAILURE, "(un)locking is not supported for the `%s' account", pwp->pw_name); + } /* keep dir name in case we need it for '-m' */ homedir = pwp->pw_dir; @@ -1410,6 +1422,48 @@ pwp->pw_passwd = up->u_password; } } + if (up->u_flags & F_ACCTLOCK) { + /* lock the account */ + if (strncmp(pwp->pw_shell+strlen(pwp->pw_shell) - 1, acctlock_str+strlen(acctlock_str) - 1, sizeof(acctlock_str) - 1) != 0) { + shell_tmp = malloc(strlen(pwp->pw_shell) + sizeof(acctlock_str)); + if (shell_tmp == NULL) { + (void) close(ptmpfd); + pw_abort(); + errx(EXIT_FAILURE, "account lock: cannot allocate memory"); + } + strlcpy(shell_tmp, pwp->pw_shell, sizeof(shell_len)); + strlcat(shell_tmp, acctlock_str, sizeof(shell_len)); + pwp->pw_shell = shell_tmp; + } + /* lock the password */ + if (strncmp(pwp->pw_passwd, pwlock_str, sizeof(pwlock_str)-1) != 0) { + pw_tmp = malloc(strlen(pwp->pw_passwd) + sizeof(pwlock_str)); + if (pw_tmp == NULL) { + (void) close(ptmpfd); + pw_abort(); + errx(EXIT_FAILURE, "password lock: cannot allocate memory"); + } +
Re: usermod: lock/unlock local password
On Mon, Sep 10, 2012 at 05:01:13PM +0200, Antoine Jacoutot wrote: > Hi. > > This diff adds 2 new options to usermod(8): > -U to unlock a user's password > -Z to lock a user's password > > In effect locking/unlocking the password means to add a '!' in front of > the encrypted entry in master.passwd. > Note that this disable the _password_ not the account of course (you > could still connect using ssh+key for e.g.). > > That said, I have some use for it and would like to be able to have this > if at all possible. > Behavior is basically the same as Linux's usermod(8) except that I am > using -Z for locking the password (-Z is for SElinux in Linux land and > -L is used instead but we use it ourselves for the login class). Ok new diff that does something slightly different. Instead of putting a '!' in front of the password (which confused people), lock/unlock means appending/removing a dash to the user's login shell. While this is non standard behavior (but as I mentionned in a previous mail, usermod(8) is already not standard accross unices) I think it is better because: - account is locked, period (no local login, no remote login -- no surprise for tedu ;-) ) - no values are touched (e.g. one could disable an account by using 'usermod -e 1' but when he wants to re-enable the account he'll be forced to change the expiration time) Thanks to Stuart for the idea. Thoughts? Index: user.c === RCS file: /cvs/src/usr.sbin/user/user.c,v retrieving revision 1.90 diff -u -r1.90 user.c --- user.c 29 Jan 2012 08:38:54 - 1.90 +++ user.c 11 Sep 2012 13:38:12 - @@ -100,7 +100,9 @@ F_UID = 0x0400, F_USERNAME = 0x0800, F_CLASS = 0x1000, - F_SETSECGROUP = 0x4000 + F_SETSECGROUP = 0x4000, + F_ACCTLOCK = 0x8000, + F_ACCTUNLOCK= 0x1 }; #define CONFFILE "/etc/usermgmt.conf" @@ -1339,11 +1341,14 @@ struct group*grp; const char *homedir; charbuf[LINE_MAX]; + charlocked_str[] = "-"; + charshell_len[MaxShellNameLen]; size_t colonc, loginc; size_t cc; FILE*master; charnewdir[MaxFileNameLen]; char*colon; + char*shell_tmp = NULL; int len; int masterfd; int ptmpfd; @@ -1359,6 +1364,10 @@ if (!is_local(login_name, _PATH_MASTERPASSWD)) { errx(EXIT_FAILURE, "User `%s' must be a local user", login_name); } + if (up != NULL) { + if ((up->u_flags & (F_ACCTLOCK | F_ACCTUNLOCK)) && (login_name == 0)) + errx(EXIT_FAILURE, "(un)locking is not supported for the `%s' account", pwp->pw_name); + } /* keep dir name in case we need it for '-m' */ homedir = pwp->pw_dir; @@ -1410,6 +1419,35 @@ pwp->pw_passwd = up->u_password; } } + if (up->u_flags & F_ACCTLOCK) { + if (strncmp(pwp->pw_shell+strlen(pwp->pw_shell) - 1, locked_str+strlen(locked_str) - 1, sizeof(locked_str) - 1) == 0) { +warnx("account '%s' is already locked", pwp->pw_name); + } else { + shell_tmp = malloc(strlen(pwp->pw_shell) + sizeof(locked_str)); + if (shell_tmp == NULL) { + (void) close(ptmpfd); + pw_abort(); + errx(EXIT_FAILURE, "lock: cannot allocate memory"); + } + strlcpy(shell_tmp, pwp->pw_shell, sizeof(shell_len)); + strlcat(shell_tmp, locked_str, sizeof(shell_len)); + pwp->pw_shell = shell_tmp; + } + } + if (up->u_flags & F_ACCTUNLOCK) { + if (strncmp(pwp->pw_shell+strlen(pwp->pw_shell) - 1, locked_str+strlen(locked_str) - 1, sizeof(locked_str) - 1) != 0) { + warnx("account '%s' is not locked", pwp->pw_name); + } else { + shell_tmp = malloc(strlen(pwp->pw_shell) - sizeof(locked_str)); + if (shell_tmp == NULL) { + (void) close(ptmpfd); + pw_abort(); + errx(EXIT_FAILURE, "unlock: cannot allocate memory"); + } + strlcpy(shell_tmp, pwp->pw_shell, sizeof(shell_tmp) + 1); + pwp->pw_shell = shell_tmp; +
Re: usermod: lock/unlock local password
I like the direction this is going. I've implemented a shell equivalent in the past for one scenario, but having it in the tools directly definitely makes life easier. 1000 admins can implement it differently, but this at least provides consistent behavior and provides a mechanism for automation that is robust. I look forward to the next version with '-' added/removed to/from the users shell ;-) Thanks, Penned by Antoine Jacoutot on 20120910 10:01.13, we have: | Hi. | | This diff adds 2 new options to usermod(8): | -U to unlock a user's password | -Z to lock a user's password | | In effect locking/unlocking the password means to add a '!' in front of | the encrypted entry in master.passwd. | Note that this disable the _password_ not the account of course (you | could still connect using ssh+key for e.g.). | | That said, I have some use for it and would like to be able to have this | if at all possible. | Behavior is basically the same as Linux's usermod(8) except that I am | using -Z for locking the password (-Z is for SElinux in Linux land and | -L is used instead but we use it ourselves for the login class). | | Comments? | | | | | | Index: user.c | === | RCS file: /cvs/src/usr.sbin/user/user.c,v | retrieving revision 1.90 | diff -u -r1.90 user.c | --- user.c29 Jan 2012 08:38:54 - 1.90 | +++ user.c10 Sep 2012 15:00:21 - | @@ -100,7 +100,9 @@ | F_UID = 0x0400, | F_USERNAME = 0x0800, | F_CLASS = 0x1000, | - F_SETSECGROUP = 0x4000 | + F_SETSECGROUP = 0x4000, | + F_PWLOCK= 0x8000, | + F_PWUNLOCK = 0x1 | }; | | #define CONFFILE "/etc/usermgmt.conf" | @@ -1339,11 +1341,14 @@ | struct group*grp; | const char *homedir; | charbuf[LINE_MAX]; | + charlocked_str[] = "!"; | + charpw_len[PasswordLength + 1]; | size_t colonc, loginc; | size_t cc; | FILE*master; | charnewdir[MaxFileNameLen]; | char*colon; | + char*pw_tmp; | int len; | int masterfd; | int ptmpfd; | @@ -1359,6 +1364,9 @@ | if (!is_local(login_name, _PATH_MASTERPASSWD)) { | errx(EXIT_FAILURE, "User `%s' must be a local user", login_name); | } | + if ((up->u_flags & (F_PWLOCK | F_PWUNLOCK)) && (pwp->pw_uid == 0)) { | + errx(EXIT_FAILURE, "(un)locking is not supported for `%s'", pwp->pw_name); | + } | /* keep dir name in case we need it for '-m' */ | homedir = pwp->pw_dir; | | @@ -1410,6 +1418,29 @@ | pwp->pw_passwd = up->u_password; | } | } | + if (up->u_flags & F_PWLOCK) { | + if (strncmp(pwp->pw_passwd, locked_str, sizeof(locked_str)-1) == 0) { | + warnx("user '%s' is already locked", pwp->pw_name); | + } else { | + pw_tmp = malloc(strlen(pwp->pw_passwd) + sizeof(locked_str)); | + if (pw_tmp == NULL) { | + (void) close(ptmpfd); | + pw_abort(); | + errx(EXIT_FAILURE, "cannot allocate memory"); | + } | + strlcpy(pw_tmp, locked_str, sizeof(pw_len)); | + strlcat(pw_tmp, pwp->pw_passwd, sizeof(pw_len)); | + pwp->pw_passwd = pw_tmp; | + free (pw_tmp); | + } | + } | + if (up->u_flags & F_PWUNLOCK) { | + if (strncmp(pwp->pw_passwd, locked_str, sizeof(locked_str)-1) != 0) { | + warnx("user '%s' is not locked", pwp->pw_name); | + } else { | + pwp->pw_passwd += sizeof(locked_str)-1; | + } | + } | if (up->u_flags & F_UID) { | /* check uid isn't already allocated */ | if (!(up->u_flags & F_DUPUID) && getpwuid((uid_t)(up->u_uid)) != NULL) { | @@ -1617,7 +1648,7 @@ | "[-p password] [-r low..high]\n" | " [-s shell] [-u uid] user\n", prog); | } else if (strcmp(prog, "usermod") == 0) { | - (void) fprintf(stderr, "usage: %s [-mov] " | + (void) fprintf(stderr, "usage: %s [-UZmov] " | "[-c comment] [-d home-directory] [-e expiry-time]\n" | " [-f inactive-time] " | "[-G secondary-group[,group,...]]\n" | @@ -1788,7 +1819,7 @@ | free(u.u_primgrp); | u.u_primgrp = NULL; | have_new
Re: usermod: lock/unlock local password
On Tue, Sep 11, 2012 at 03:47:57AM -0400, Ted Unangst wrote: > On Mon, Sep 10, 2012 at 17:01, Antoine Jacoutot wrote: > > > In effect locking/unlocking the password means to add a '!' in front of > > the encrypted entry in master.passwd. > > Note that this disable the _password_ not the account of course (you > > could still connect using ssh+key for e.g.). > > I am very concerned that this violates the principle of least surprise. Stuart proposed that my diff appends/remove '-' from the user shell. I think this is a good idea, I'll send another diff later. -- Antoine
Re: usermod: lock/unlock local password
On 2012/09/11 03:47, Ted Unangst wrote: > On Mon, Sep 10, 2012 at 17:01, Antoine Jacoutot wrote: > > > In effect locking/unlocking the password means to add a '!' in front of > > the encrypted entry in master.passwd. > > Note that this disable the _password_ not the account of course (you > > could still connect using ssh+key for e.g.). > > I am very concerned that this violates the principle of least surprise. > This is already common enough that /usr/libexec/security checks for alternative access methods if the password is "disabled" (i.e. the crypted password is neither 13 chars long nor starts with $[0-9a-f]$) but the shell is valid.
Re: usermod: lock/unlock local password
On Tue, Sep 11, 2012 at 03:47:57AM -0400, Ted Unangst wrote: > On Mon, Sep 10, 2012 at 17:01, Antoine Jacoutot wrote: > > > In effect locking/unlocking the password means to add a '!' in front of > > the encrypted entry in master.passwd. > > Note that this disable the _password_ not the account of course (you > > could still connect using ssh+key for e.g.). > > I am very concerned that this violates the principle of least surprise. Well I can extend the man page accordingly if you want. But I mean it has the same result as using vipw(8). Disabling a password has always been different than disabling an account. -- Antoine
Re: usermod: lock/unlock local password
On Mon, Sep 10, 2012 at 17:01, Antoine Jacoutot wrote: > In effect locking/unlocking the password means to add a '!' in front of > the encrypted entry in master.passwd. > Note that this disable the _password_ not the account of course (you > could still connect using ssh+key for e.g.). I am very concerned that this violates the principle of least surprise.
Re: usermod: lock/unlock local password
On Tue, Sep 11, 2012 at 10:23:13AM +0300, Eugene Yunak wrote: > On 11 September 2012 09:37, Antoine Jacoutot wrote: > > On Tue, Sep 11, 2012 at 09:33:56AM +0300, Eugene Yunak wrote: > >> On 10 September 2012 18:01, Antoine Jacoutot wrote: > >> > Hi. > >> > > >> > This diff adds 2 new options to usermod(8): > >> > -U to unlock a user's password > >> > -Z to lock a user's password > >> > > >> > In effect locking/unlocking the password means to add a '!' in front of > >> > the encrypted entry in master.passwd. > >> > Note that this disable the _password_ not the account of course (you > >> > could still connect using ssh+key for e.g.). > >> > > >> > That said, I have some use for it and would like to be able to have this > >> > if at all possible. > >> > Behavior is basically the same as Linux's usermod(8) except that I am > >> > using -Z for locking the password (-Z is for SElinux in Linux land and > >> > -L is used instead but we use it ourselves for the login class). > >> > > >> > Comments? > >> > >> Hi, > >> > >> Isn't think better placed in passwd? > >> At least Linux and Solaris (since 5.6 i believe) have this as -l and > >> -u in passwd(1), > >> so this might be a better option to keep it consistent with other > >> systems. HP-UX > >> only implements -l; I haven't checked others. > > > > It is consistent; this is how usermod works in linux as well. > > Isn't it better to be consistent with most Unix systems and not just Linux? > The world is Linux-centric enough already and an OpenBSD should know it > better than anyone else ;) FreeBSD and NetBSD do the same (i.e. lock using usermod). I don't really care about HP-UX compatibility... and I don't understand your comment about "OpenBSD should know it better"; what is it you want exactly? As I said, I have a use for it using usermod(8). If you have a use for it with passwd(1) then provide a diff. Each Unix has a slightly different useradd/mod/del ... command you know. > >> OpenBSD passwd already uses -l to restrict passwd to local files only > >> though so > >> you would still need to use a different letter (as you do with > >> usermod) but at least > >> passwd(1) is where most unix admins would look for this option first. > > > > This diff is for the usermod part, not passwd; both are different things. > > I don't get it - how are they "different things"? Both manipulate shadow. And so does vipw(8). Look this is a diff for _usermod_. If you want to add flags to passwd(1), then just do so, I have no problem with it. -- Antoine
Re: usermod: lock/unlock local password
On 11 September 2012 09:37, Antoine Jacoutot wrote: > On Tue, Sep 11, 2012 at 09:33:56AM +0300, Eugene Yunak wrote: >> On 10 September 2012 18:01, Antoine Jacoutot wrote: >> > Hi. >> > >> > This diff adds 2 new options to usermod(8): >> > -U to unlock a user's password >> > -Z to lock a user's password >> > >> > In effect locking/unlocking the password means to add a '!' in front of >> > the encrypted entry in master.passwd. >> > Note that this disable the _password_ not the account of course (you >> > could still connect using ssh+key for e.g.). >> > >> > That said, I have some use for it and would like to be able to have this >> > if at all possible. >> > Behavior is basically the same as Linux's usermod(8) except that I am >> > using -Z for locking the password (-Z is for SElinux in Linux land and >> > -L is used instead but we use it ourselves for the login class). >> > >> > Comments? >> >> Hi, >> >> Isn't think better placed in passwd? >> At least Linux and Solaris (since 5.6 i believe) have this as -l and >> -u in passwd(1), >> so this might be a better option to keep it consistent with other >> systems. HP-UX >> only implements -l; I haven't checked others. > > It is consistent; this is how usermod works in linux as well. Isn't it better to be consistent with most Unix systems and not just Linux? The world is Linux-centric enough already and an OpenBSD should know it better than anyone else ;) >> OpenBSD passwd already uses -l to restrict passwd to local files only though >> so >> you would still need to use a different letter (as you do with >> usermod) but at least >> passwd(1) is where most unix admins would look for this option first. > > This diff is for the usermod part, not passwd; both are different things. I don't get it - how are they "different things"? Both manipulate shadow. -- The best the little guy can do is what the little guy does right
Re: usermod: lock/unlock local password
On Tue, Sep 11, 2012 at 09:33:56AM +0300, Eugene Yunak wrote: > On 10 September 2012 18:01, Antoine Jacoutot wrote: > > Hi. > > > > This diff adds 2 new options to usermod(8): > > -U to unlock a user's password > > -Z to lock a user's password > > > > In effect locking/unlocking the password means to add a '!' in front of > > the encrypted entry in master.passwd. > > Note that this disable the _password_ not the account of course (you > > could still connect using ssh+key for e.g.). > > > > That said, I have some use for it and would like to be able to have this > > if at all possible. > > Behavior is basically the same as Linux's usermod(8) except that I am > > using -Z for locking the password (-Z is for SElinux in Linux land and > > -L is used instead but we use it ourselves for the login class). > > > > Comments? > > > > > > > > > > > > Index: user.c > > === > > RCS file: /cvs/src/usr.sbin/user/user.c,v > > retrieving revision 1.90 > > diff -u -r1.90 user.c > > --- user.c 29 Jan 2012 08:38:54 - 1.90 > > +++ user.c 10 Sep 2012 15:00:21 - > > @@ -100,7 +100,9 @@ > > F_UID = 0x0400, > > F_USERNAME = 0x0800, > > F_CLASS = 0x1000, > > - F_SETSECGROUP = 0x4000 > > + F_SETSECGROUP = 0x4000, > > + F_PWLOCK= 0x8000, > > + F_PWUNLOCK = 0x1 > > }; > > > > #define CONFFILE "/etc/usermgmt.conf" > > @@ -1339,11 +1341,14 @@ > > struct group*grp; > > const char *homedir; > > charbuf[LINE_MAX]; > > + charlocked_str[] = "!"; > > + charpw_len[PasswordLength + 1]; > > size_t colonc, loginc; > > size_t cc; > > FILE*master; > > charnewdir[MaxFileNameLen]; > > char*colon; > > + char*pw_tmp; > > int len; > > int masterfd; > > int ptmpfd; > > @@ -1359,6 +1364,9 @@ > > if (!is_local(login_name, _PATH_MASTERPASSWD)) { > > errx(EXIT_FAILURE, "User `%s' must be a local user", > > login_name); > > } > > + if ((up->u_flags & (F_PWLOCK | F_PWUNLOCK)) && (pwp->pw_uid == 0)) { > > + errx(EXIT_FAILURE, "(un)locking is not supported for `%s'", > > pwp->pw_name); > > + } > > /* keep dir name in case we need it for '-m' */ > > homedir = pwp->pw_dir; > > > > @@ -1410,6 +1418,29 @@ > > pwp->pw_passwd = up->u_password; > > } > > } > > + if (up->u_flags & F_PWLOCK) { > > + if (strncmp(pwp->pw_passwd, locked_str, > > sizeof(locked_str)-1) == 0) { > > +warnx("user '%s' is already locked", > > pwp->pw_name); > > + } else { > > + pw_tmp = malloc(strlen(pwp->pw_passwd) + > > sizeof(locked_str)); > > + if (pw_tmp == NULL) { > > + (void) close(ptmpfd); > > + pw_abort(); > > + errx(EXIT_FAILURE, "cannot allocate > > memory"); > > + } > > + strlcpy(pw_tmp, locked_str, sizeof(pw_len)); > > + strlcat(pw_tmp, pwp->pw_passwd, > > sizeof(pw_len)); > > + pwp->pw_passwd = pw_tmp; > > + free (pw_tmp); > > + } > > + } > > + if (up->u_flags & F_PWUNLOCK) { > > + if (strncmp(pwp->pw_passwd, locked_str, > > sizeof(locked_str)-1) != 0) { > > + warnx("user '%s' is not locked", > > pwp->pw_name); > > + } else { > > + pwp->pw_passwd += sizeof(locked_str)-1; > > + } > > + } > > if (up->u_flags & F_UID) { > > /* check uid isn't already allocated */ > > if (!(up->u_flags & F_DUPUID) && > > getpwuid((uid_t)(up->u_uid)) != NULL) { > > @@ -1617,7 +1648,7 @@ > > "[-p password] [-r low..high]\n" > > " [-s shell] [-u uid] user\n", prog); > > } else if (strcmp(prog, "usermod") == 0) { > > - (void) fprintf(stderr, "usage: %s [-mov] " > > + (void) fprintf(stderr, "usage: %s [-UZmov] " > > "[-c comment] [-d home-directory] [-e expiry-time]\n" > > " [-f inactive-time] " > > "[-G secondary-group[,group,...]]\n" > > @@ -1788,7 +1819,7 @@ > > free(u.u_primgrp); > > u.
Re: usermod: lock/unlock local password
On 10 September 2012 18:01, Antoine Jacoutot wrote: > Hi. > > This diff adds 2 new options to usermod(8): > -U to unlock a user's password > -Z to lock a user's password > > In effect locking/unlocking the password means to add a '!' in front of > the encrypted entry in master.passwd. > Note that this disable the _password_ not the account of course (you > could still connect using ssh+key for e.g.). > > That said, I have some use for it and would like to be able to have this > if at all possible. > Behavior is basically the same as Linux's usermod(8) except that I am > using -Z for locking the password (-Z is for SElinux in Linux land and > -L is used instead but we use it ourselves for the login class). > > Comments? > > > > > > Index: user.c > === > RCS file: /cvs/src/usr.sbin/user/user.c,v > retrieving revision 1.90 > diff -u -r1.90 user.c > --- user.c 29 Jan 2012 08:38:54 - 1.90 > +++ user.c 10 Sep 2012 15:00:21 - > @@ -100,7 +100,9 @@ > F_UID = 0x0400, > F_USERNAME = 0x0800, > F_CLASS = 0x1000, > - F_SETSECGROUP = 0x4000 > + F_SETSECGROUP = 0x4000, > + F_PWLOCK= 0x8000, > + F_PWUNLOCK = 0x1 > }; > > #define CONFFILE "/etc/usermgmt.conf" > @@ -1339,11 +1341,14 @@ > struct group*grp; > const char *homedir; > charbuf[LINE_MAX]; > + charlocked_str[] = "!"; > + charpw_len[PasswordLength + 1]; > size_t colonc, loginc; > size_t cc; > FILE*master; > charnewdir[MaxFileNameLen]; > char*colon; > + char*pw_tmp; > int len; > int masterfd; > int ptmpfd; > @@ -1359,6 +1364,9 @@ > if (!is_local(login_name, _PATH_MASTERPASSWD)) { > errx(EXIT_FAILURE, "User `%s' must be a local user", > login_name); > } > + if ((up->u_flags & (F_PWLOCK | F_PWUNLOCK)) && (pwp->pw_uid == 0)) { > + errx(EXIT_FAILURE, "(un)locking is not supported for `%s'", > pwp->pw_name); > + } > /* keep dir name in case we need it for '-m' */ > homedir = pwp->pw_dir; > > @@ -1410,6 +1418,29 @@ > pwp->pw_passwd = up->u_password; > } > } > + if (up->u_flags & F_PWLOCK) { > + if (strncmp(pwp->pw_passwd, locked_str, > sizeof(locked_str)-1) == 0) { > +warnx("user '%s' is already locked", > pwp->pw_name); > + } else { > + pw_tmp = malloc(strlen(pwp->pw_passwd) + > sizeof(locked_str)); > + if (pw_tmp == NULL) { > + (void) close(ptmpfd); > + pw_abort(); > + errx(EXIT_FAILURE, "cannot allocate > memory"); > + } > + strlcpy(pw_tmp, locked_str, sizeof(pw_len)); > + strlcat(pw_tmp, pwp->pw_passwd, > sizeof(pw_len)); > + pwp->pw_passwd = pw_tmp; > + free (pw_tmp); > + } > + } > + if (up->u_flags & F_PWUNLOCK) { > + if (strncmp(pwp->pw_passwd, locked_str, > sizeof(locked_str)-1) != 0) { > + warnx("user '%s' is not locked", > pwp->pw_name); > + } else { > + pwp->pw_passwd += sizeof(locked_str)-1; > + } > + } > if (up->u_flags & F_UID) { > /* check uid isn't already allocated */ > if (!(up->u_flags & F_DUPUID) && > getpwuid((uid_t)(up->u_uid)) != NULL) { > @@ -1617,7 +1648,7 @@ > "[-p password] [-r low..high]\n" > " [-s shell] [-u uid] user\n", prog); > } else if (strcmp(prog, "usermod") == 0) { > - (void) fprintf(stderr, "usage: %s [-mov] " > + (void) fprintf(stderr, "usage: %s [-UZmov] " > "[-c comment] [-d home-directory] [-e expiry-time]\n" > " [-f inactive-time] " > "[-G secondary-group[,group,...]]\n" > @@ -1788,7 +1819,7 @@ > free(u.u_primgrp); > u.u_primgrp = NULL; > have_new_user = 0; > - while ((c = getopt(argc, argv, "G:L:S:c:d:e:f:g:l:mop:s:u:v")) != -1) > { > + while ((c = getopt(argc, argv, "G:L:S:UZc:d:e:f:g:l:mop:s:u:v")) != > -1) { > switch(c) { > case 'G': >
Re: usermod: lock/unlock local password
On 09/10/12 15:24, sven falempin wrote: 2012/9/10 Antoine Jacoutot In effect locking/unlocking the password means to add a '!' in front of the encrypted entry in master.passwd. Note that this disable the _password_ not the account of course (you could still connect using ssh+key for e.g.). noob remarks: it doesnt lock it modify the password (unexpected behavior) though every other login way (like keys) works. And curiously, that's exactly what he said in his original email.. -- Scott McEachern https://www.blackstaff.ca
Re: usermod: lock/unlock local password
2012/9/10 Antoine Jacoutot > Hi. > > This diff adds 2 new options to usermod(8): > -U to unlock a user's password > -Z to lock a user's password > > In effect locking/unlocking the password means to add a '!' in front of > the encrypted entry in master.passwd. > Note that this disable the _password_ not the account of course (you > could still connect using ssh+key for e.g.). > > That said, I have some use for it and would like to be able to have this > if at all possible. > Behavior is basically the same as Linux's usermod(8) except that I am > using -Z for locking the password (-Z is for SElinux in Linux land and > -L is used instead but we use it ourselves for the login class). > > Comments? > > > > noob remarks: it doesnt lock it modify the password (unexpected behavior) though every other login way (like keys) works. > > > Index: user.c > === > RCS file: /cvs/src/usr.sbin/user/user.c,v > retrieving revision 1.90 > diff -u -r1.90 user.c > --- user.c 29 Jan 2012 08:38:54 - 1.90 > +++ user.c 10 Sep 2012 15:00:21 - > @@ -100,7 +100,9 @@ > F_UID = 0x0400, > F_USERNAME = 0x0800, > F_CLASS = 0x1000, > - F_SETSECGROUP = 0x4000 > + F_SETSECGROUP = 0x4000, > + F_PWLOCK= 0x8000, > + F_PWUNLOCK = 0x1 > }; > > #define CONFFILE "/etc/usermgmt.conf" > @@ -1339,11 +1341,14 @@ > struct group*grp; > const char *homedir; > charbuf[LINE_MAX]; > + charlocked_str[] = "!"; > + charpw_len[PasswordLength + 1]; > size_t colonc, loginc; > size_t cc; > FILE*master; > charnewdir[MaxFileNameLen]; > char*colon; > + char*pw_tmp; > int len; > int masterfd; > int ptmpfd; > @@ -1359,6 +1364,9 @@ > if (!is_local(login_name, _PATH_MASTERPASSWD)) { > errx(EXIT_FAILURE, "User `%s' must be a local user", > login_name); > } > + if ((up->u_flags & (F_PWLOCK | F_PWUNLOCK)) && (pwp->pw_uid == 0)) > { > + errx(EXIT_FAILURE, "(un)locking is not supported for > `%s'", pwp->pw_name); > + } > /* keep dir name in case we need it for '-m' */ > homedir = pwp->pw_dir; > > @@ -1410,6 +1418,29 @@ > pwp->pw_passwd = up->u_password; > } > } > + if (up->u_flags & F_PWLOCK) { > + if (strncmp(pwp->pw_passwd, locked_str, > sizeof(locked_str)-1) == 0) { > +warnx("user '%s' is already locked", > pwp->pw_name); > + } else { > + pw_tmp = malloc(strlen(pwp->pw_passwd) + > sizeof(locked_str)); > + if (pw_tmp == NULL) { > + (void) close(ptmpfd); > + pw_abort(); > + errx(EXIT_FAILURE, "cannot > allocate memory"); > + } > + strlcpy(pw_tmp, locked_str, > sizeof(pw_len)); > + strlcat(pw_tmp, pwp->pw_passwd, > sizeof(pw_len)); > + pwp->pw_passwd = pw_tmp; > + free (pw_tmp); > + } > + } > + if (up->u_flags & F_PWUNLOCK) { > + if (strncmp(pwp->pw_passwd, locked_str, > sizeof(locked_str)-1) != 0) { > + warnx("user '%s' is not locked", > pwp->pw_name); > + } else { > + pwp->pw_passwd += sizeof(locked_str)-1; > + } > + } > if (up->u_flags & F_UID) { > /* check uid isn't already allocated */ > if (!(up->u_flags & F_DUPUID) && > getpwuid((uid_t)(up->u_uid)) != NULL) { > @@ -1617,7 +1648,7 @@ > "[-p password] [-r low..high]\n" > " [-s shell] [-u uid] user\n", prog); > } else if (strcmp(prog, "usermod") == 0) { > - (void) fprintf(stderr, "usage: %s [-mov] " > + (void) fprintf(stderr, "usage: %s [-UZmov] " > "[-c comment] [-d home-directory] [-e expiry-time]\n" > " [-f inactive-time] " > "[-G secondary-group[,group,...]]\n" > @@ -1788,7 +1819,7 @@ > free(u.u_primgrp); > u.u_primgrp = NULL; > have_new_user = 0; > - while ((c = getopt(argc, argv, "G:L:S:c:d:e:f:g:l:mop:s:u:v")) != > -1) { > + while ((c = getopt(argc, argv, "G:L:S:UZc:d:e:f:g:l:mop:s:u:v")
usermod: lock/unlock local password
Hi. This diff adds 2 new options to usermod(8): -U to unlock a user's password -Z to lock a user's password In effect locking/unlocking the password means to add a '!' in front of the encrypted entry in master.passwd. Note that this disable the _password_ not the account of course (you could still connect using ssh+key for e.g.). That said, I have some use for it and would like to be able to have this if at all possible. Behavior is basically the same as Linux's usermod(8) except that I am using -Z for locking the password (-Z is for SElinux in Linux land and -L is used instead but we use it ourselves for the login class). Comments? Index: user.c === RCS file: /cvs/src/usr.sbin/user/user.c,v retrieving revision 1.90 diff -u -r1.90 user.c --- user.c 29 Jan 2012 08:38:54 - 1.90 +++ user.c 10 Sep 2012 15:00:21 - @@ -100,7 +100,9 @@ F_UID = 0x0400, F_USERNAME = 0x0800, F_CLASS = 0x1000, - F_SETSECGROUP = 0x4000 + F_SETSECGROUP = 0x4000, + F_PWLOCK= 0x8000, + F_PWUNLOCK = 0x1 }; #define CONFFILE "/etc/usermgmt.conf" @@ -1339,11 +1341,14 @@ struct group*grp; const char *homedir; charbuf[LINE_MAX]; + charlocked_str[] = "!"; + charpw_len[PasswordLength + 1]; size_t colonc, loginc; size_t cc; FILE*master; charnewdir[MaxFileNameLen]; char*colon; + char*pw_tmp; int len; int masterfd; int ptmpfd; @@ -1359,6 +1364,9 @@ if (!is_local(login_name, _PATH_MASTERPASSWD)) { errx(EXIT_FAILURE, "User `%s' must be a local user", login_name); } + if ((up->u_flags & (F_PWLOCK | F_PWUNLOCK)) && (pwp->pw_uid == 0)) { + errx(EXIT_FAILURE, "(un)locking is not supported for `%s'", pwp->pw_name); + } /* keep dir name in case we need it for '-m' */ homedir = pwp->pw_dir; @@ -1410,6 +1418,29 @@ pwp->pw_passwd = up->u_password; } } + if (up->u_flags & F_PWLOCK) { + if (strncmp(pwp->pw_passwd, locked_str, sizeof(locked_str)-1) == 0) { +warnx("user '%s' is already locked", pwp->pw_name); + } else { + pw_tmp = malloc(strlen(pwp->pw_passwd) + sizeof(locked_str)); + if (pw_tmp == NULL) { + (void) close(ptmpfd); + pw_abort(); + errx(EXIT_FAILURE, "cannot allocate memory"); + } + strlcpy(pw_tmp, locked_str, sizeof(pw_len)); + strlcat(pw_tmp, pwp->pw_passwd, sizeof(pw_len)); + pwp->pw_passwd = pw_tmp; + free (pw_tmp); + } + } + if (up->u_flags & F_PWUNLOCK) { + if (strncmp(pwp->pw_passwd, locked_str, sizeof(locked_str)-1) != 0) { + warnx("user '%s' is not locked", pwp->pw_name); + } else { + pwp->pw_passwd += sizeof(locked_str)-1; + } + } if (up->u_flags & F_UID) { /* check uid isn't already allocated */ if (!(up->u_flags & F_DUPUID) && getpwuid((uid_t)(up->u_uid)) != NULL) { @@ -1617,7 +1648,7 @@ "[-p password] [-r low..high]\n" " [-s shell] [-u uid] user\n", prog); } else if (strcmp(prog, "usermod") == 0) { - (void) fprintf(stderr, "usage: %s [-mov] " + (void) fprintf(stderr, "usage: %s [-UZmov] " "[-c comment] [-d home-directory] [-e expiry-time]\n" " [-f inactive-time] " "[-G secondary-group[,group,...]]\n" @@ -1788,7 +1819,7 @@ free(u.u_primgrp); u.u_primgrp = NULL; have_new_user = 0; - while ((c = getopt(argc, argv, "G:L:S:c:d:e:f:g:l:mop:s:u:v")) != -1) { + while ((c = getopt(argc, argv, "G:L:S:UZc:d:e:f:g:l:mop:s:u:v")) != -1) { switch(c) { case 'G': while ((u.u_groupv[u.u_groupc] = strsep(&optarg, ",")) != NULL && @@ -1814,6 +1845,12 @@ } u.u_flags |= F_SETSECGROUP; break; + case 'U': + u.u_flags |= F_PWUNLOCK; +