Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 18 Nov 1998 00:06:37 +0100 (CET)
From:      Mikael Karpberg <karpen@ocean.campus.luth.se>
To:        dillon@apollo.backplane.com (Matthew Dillon)
Cc:        hackers@FreeBSD.ORG, freebsd-security@FreeBSD.ORG
Subject:   Re: Would this make FreeBSD more secure?
Message-ID:  <199811172306.AAA02553@ocean.campus.luth.se>
In-Reply-To: <199811172200.OAA28976@apollo.backplane.com> from Matthew Dillon at "Nov 17, 98 02:00:34 pm"

next in thread | previous in thread | raw e-mail | index | archive | help
According to Matthew Dillon:
> :Umm... I have seen no one in this discussion mention this, so I'll say it,
> :after repeating what someone DID say "Small well audited setuid programs
> :are not a problem". Now... Here's my suggestion, my_xlock.c:
> :
> :int main() {
> :  char *str;
> :  FILE *f;
> :  int done = 0;
> :  lock_screen();
> :  while (!done) {
> :    str = wait_for_passwd();
> :    f = popen("/usr/bin/check_pw", "w");
> :    fprintf(f, "%d %s\n", getuid(), str);
> :    fflush(f);
> :    if (!pclose(f)) {
> :      unlock_screen();
> :      done = 1;
> :    } else {
> :      print_errror("Wrong password");
> :    }
> :  }
> :  return 0;
> :}
> :
> :Seems simple enough to me, and could be used from scripts and everything.
> :All you need is a small util (/usr/bin/check_pw) that is setuid root.

According to William McVey:
> I believe this all started with the realization that setuid root
> shouldn't be needed to verify passwords.  A dedicated group could
> be created for this task which would be limited to only having read
> access to the shadow file.

Yes, check_pw can just as well be setgid shadow. The point is, check_pw
would be very small (well under 100 lines, I'd guess), and should then
be possible to make secure enough to run as setuid root. it also starts
and stops pretty fast, doesn't dump core, and otherwise behaves nasty
to people who want to exploit it.

According to Matthew Dillon:
>     You didn't clear the environment
>     you didn't reset the path
>     you didn't reset the resource limits
>     you didn't disable signals
>     you are using popen (even with an absolute path),

Why do I need to? I'm not setuid anything to lock the screen. That's why
I need to call check_pw, which is setuid, to check my password.
And what's wrong with popen() even if I was?

According to Matthew Dillon:
> :...
> :    *str = '\0';
> :    pw = getpwnam(buffer);
> :  }
> :  str++;
> :  setting = get_setting_and_move_str(&str);
> :  if (strcmp(pw->pw_passwd, crypt(str, setting)) == 0)
> :    return 0;
> :  return 1;
> :}
> 
>     And you haven't cleared the memory space associated with
>     either the crypted or unencrypted password info you 
>     just retrieved.

True, but it was meant as something more of a pseudo code, not as a bullet-
proof "please commit this"-code. That's just one of many things that must
be added, ofcourse.

According to Matthew Dillon:
> :I'm sure there are minor or even major mistakes in the programs above, but
> :I think everyone should get the idea, if the problems are just syntax errors,
> :and such. The check_pw program should be small enough to be quite possible to
> :do as close to 100% bug free as one can hope to get.
> 
>     Now, I know I'm being unfair.  I'm just trying to point out that
>     there are a LOT of things an suid program must do to be reasonably
>     secure, especially if it is going to go off and execute another
>     program.

The check_pw program which is setuid is not executing another program.
And as it's SOLE purpose is to check a password and return true or false,
it should be quite possible to make it as close to 100% bug free as is
possible.

According to William McVey:
> >    while (!isspace(*str))
> >      str++;
> 
> /* Zoom!!! right off the end of the string, if there were no spaces in
>  * the user input (isspace(3) doesn't return true on nulls).
>  */

Again... I didn't write that piece of code as a suggested code, but more
like a well-written pseudo-code. I think this might have been a mistake.
I should have used less correct c-code.

According to William McVey:
> >    if (!*str)
> >      exit(1);
> >    *str = '\0';
> 
> /* If I'm root, this would have just scribbled a \0 someplace in memory.
>  * If I'm setgid to group shadow, it would cause a SIGV.  Which would
>  * *you* prefer?
>  */

I think you missunderstood something seriously. The process will SEGV
no matter if it's run by root. Root is just like any user, until he
does a system call that requires authentication.

  /Mikael

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-security" in the body of the message



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199811172306.AAA02553>