Re: usermod: lock/unlock local password

2013-02-15 Thread Antoine Jacoutot
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

2013-02-11 Thread Jiri B
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

2013-02-11 Thread Antoine Jacoutot
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

2013-02-11 Thread André Stöbe
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

2012-09-14 Thread Ted Unangst
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

2012-09-14 Thread Antoine Jacoutot
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

2012-09-11 Thread Antoine Jacoutot
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

2012-09-11 Thread Antoine Jacoutot
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

2012-09-11 Thread Todd T. Fries
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

2012-09-11 Thread Antoine Jacoutot
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

2012-09-11 Thread Stuart Henderson
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

2012-09-11 Thread Antoine Jacoutot
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

2012-09-11 Thread Ted Unangst
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

2012-09-11 Thread Antoine Jacoutot
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

2012-09-11 Thread Eugene Yunak
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

2012-09-10 Thread Antoine Jacoutot
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

2012-09-10 Thread Eugene Yunak
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

2012-09-10 Thread Scott McEachern

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-09-10 Thread sven falempin
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")