On Sun, Jan 14, 2018 at 10:45:55PM -0500, Drew DeVault wrote:
> These are useful for daemon users, etc.
> ---
> Sorry for the noise. Initially I just updated my original patch without
> much thought and sent it along, but when I tested it more, I found out
> that it was total dogshit. This patch actually works and (hopefully)
> doesn't introduce blatantly obvious security holes.
>
> libutil/passwd.c | 38 ++++++++++++++++++++++----------------
> login.c | 13 +++++++++----
> passwd.c | 28 +++++++++++++++++++++++-----
> passwd.h | 3 ++-
> 4 files changed, 56 insertions(+), 26 deletions(-)
>
> diff --git a/libutil/passwd.c b/libutil/passwd.c
> index 0798225..d60000c 100644
> --- a/libutil/passwd.c
> +++ b/libutil/passwd.c
> _AT_@ -14,10 +14,9 @@
> #include "../text.h"
> #include "../util.h"
>
> -/* Returns -1 on error, 0 for incorrect password
> - * and 1 if all went OK */
> -int
> -pw_check(const struct passwd *pw, const char *pass)
> +/* Returns NULL on error or a string pointer if all went OK */
> +const char *
> +pw_get(const struct passwd *pw)
> {
> char *cryptpass, *p;
> struct spwd *spw;
> _AT_@ -25,14 +24,7 @@ pw_check(const struct passwd *pw, const char *pass)
> p = pw->pw_passwd;
> if (p[0] == '!' || p[0] == '*') {
> weprintf("denied\n");
> - return -1;
> - }
> -
> - if (pw->pw_passwd[0] == '\0') {
> - if (pass[0] == '\0')
> - return 1;
> - weprintf("incorrect password\n");
> - return 0;
> + return NULL;
> }
>
> if (pw->pw_passwd[0] == 'x' && pw->pw_passwd[1] == '\0') {
> _AT_@ -43,21 +35,35 @@ pw_check(const struct passwd *pw, const char *pass)
> weprintf("getspnam: %s:", pw->pw_name);
> else
> weprintf("who are you?\n");
> - return -1;
> + return NULL;
> }
> p = spw->sp_pwdp;
> if (p[0] == '!' || p[0] == '*') {
> weprintf("denied\n");
> - return -1;
> + return NULL;
> }
> + return p;
> + }
> +}
> +
> +/* Returns -1 on error, 0 for incorrect password
> + * and 1 if all went OK */
> +int
> +pw_check(const char *hash, const char *pass)
> +{
> + char *cryptpass;
> +
> + if (!hash || hash[0] == '!' || hash[0] == '*') {
> + weprintf("denied\n");
> + return -1;
> }
>
> - cryptpass = crypt(pass, p);
> + cryptpass = crypt(pass, hash);
> if (!cryptpass) {
> weprintf("crypt:");
> return -1;
> }
> - if (strcmp(cryptpass, p) != 0) {
> + if (strcmp(cryptpass, hash) != 0) {
> weprintf("incorrect password\n");
> return 0;
> }
> diff --git a/login.c b/login.c
> index 25a59e4..3490632 100644
> --- a/login.c
> +++ b/login.c
> _AT_@ -107,11 +107,16 @@ main(int argc, char *argv[])
> /* Flush pending input */
> ioctl(0, TCFLSH, (void *)0);
>
> - pass = getpass("Password: ");
> - if (!pass)
> - eprintf("getpass:");
> - if (pw_check(pw, pass) <= 0)
> + const char *hash = pw_get(pw);
> + if (!hash)
> exit(1);
> + else if (*hash) {
> + pass = getpass("Password: ");
> + if (!pass)
> + eprintf("getpass:");
> + if (pw_check(hash, pass) <= 0)
> + exit(1);
> + }
>
> tty = ttyname(0);
> if (!tty)
> diff --git a/passwd.c b/passwd.c
> index 52b70a8..cd70c64 100644
> --- a/passwd.c
> +++ b/passwd.c
> _AT_@ -164,9 +164,12 @@ main(int argc, char *argv[])
> struct passwd *pw;
> struct spwd *spw = NULL;
> FILE *fp = NULL;
> - int r = -1, status = 1;
> + int r = -1, status = 1, fdel = 0;
>
> ARGBEGIN {
> + case 'd':
> + fdel = 1;
> + break;
> default:
> usage();
> } ARGEND;
> _AT_@ -216,6 +219,14 @@ main(int argc, char *argv[])
> prevhash = pw->pw_passwd;
> }
>
> + printf("%s\n", prevhash);
> + if (!*prevhash) {
> + if (pw->pw_uid == getuid())
> + goto newpass;
> + else
> + eprintf("denied\n");
> + }
> +
> printf("Changing password for %s\n", pw->pw_name);
> inpass = getpass("Old password: ");
> if (!inpass)
> _AT_@ -230,10 +241,15 @@ main(int argc, char *argv[])
> eprintf("incorrect password\n");
>
> newpass:
> - inpass = getpass("Enter new password: ");
> + if (fdel) {
> + cryptpass3 = estrdup("");
> + goto writepass;
> + } else {
> + inpass = getpass("Enter new password: ");
> + }
> if (!inpass)
> eprintf("getpass:");
> - if (inpass[0] == '\0')
> + if (inpass[0] == '\0' && !fdel)
> eprintf("no password supplied\n");
> p = crypt(inpass, prevhash);
> if (!p)
> _AT_@ -249,10 +265,11 @@ newpass:
> /* Flush pending input */
> ioctl(0, TCFLSH, (void *)0);
>
> - inpass = getpass("Retype new password: ");
> + if (!fdel)
> + inpass = getpass("Retype new password: ");
> if (!inpass)
> eprintf("getpass:");
> - if (inpass[0] == '\0')
> + if (inpass[0] == '\0' && !fdel)
> eprintf("no password supplied\n");
> p = crypt(inpass, salt);
> if (!p)
> _AT_@ -261,6 +278,7 @@ newpass:
> if (strcmp(cryptpass2, cryptpass3) != 0)
> eprintf("passwords don't match\n");
>
> +writepass:
> fp = spw_get_file(pw->pw_name);
> if (fp) {
> r = spw_write_file(fp, spw, cryptpass3);
> diff --git a/passwd.h b/passwd.h
> index ec8286b..f39a826 100644
> --- a/passwd.h
> +++ b/passwd.h
> _AT_@ -1,4 +1,5 @@
> /* See LICENSE file for copyright and license details. */
> /* passwd.c */
> -int pw_check(const struct passwd *, const char *);
> +const char *pw_get(const struct passwd *);
> +int pw_check(const char *, const char *);
> int pw_init(void);
> --
> 2.15.0
>
>
Hi,
This looks too complicated for what it should do to me.
You also added functionality (like a -d) option. This should be separated in
a different patch.
Please pay more attention.
--
Kind regards,
Hiltjo
Received on Mon Jan 15 2018 - 20:31:21 CET