bug-hurd
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Concerns about login


From: Marc-Olivier Mercier
Subject: Concerns about login
Date: Sun, 24 Feb 2008 10:57:24 -0500
User-agent: Mozilla-Thunderbird 2.0.0.9 (X11/20080109)

I have installed Debian GNU/Hurd about 2 weeks ago. I have some concerns about the way login works.

-----
GNU 0.3 (stracci) (tty5)

Use `login USER' to login, or `help' for more information.
login> login bob
login: bob: Unknown user
-----

1) Shouldn't "paranoid" be the only behavior of login? Currently, the one who's trying to log in decide if he use -P or not... Well, i don't understand why this is an option. I think that we shouldn't give too much information not only about passwords but usernames too.

2) I tried to use the -P option but it just failed :
-----
GNU 0.3 (stracci) (tty5)

Use `login USER' to login, or `help' for more information.
login> login -P bob
[about 20 sec. passed then...]

GNU 0.3 (stracci) (tty5)

Use `login USER' to login, or `help' for more information.
login>
-----

So I looked in the code to see what's going on.
in utils/login.c:parse_opt :
-----
[...]
else if (paranoid)
   idvec_add (&ugids.eff_uids, -1);    // this line adds -1 as a bogus id
[...]
-----
The problem here is that uid_t is a "unsigned long"...
A couple of calls later in libshouldbeinlibc/idvec-verify.c:verify_id :
-----
[...]
if (id >= 0) // comparing uid_t to be greater or equal. So this is always true.
[...]
if (getpwuid_r (id, &_pw, id_lookup_buf, sizeof id_lookup_buf, &pw) == 0) {
      // since id is invalid, pw is NULL
   if (strcmp(pw->pw_passwd, SHADOW_PASSWORD_STRING) == 0) {
      // well... pw = NULL, so when trying to get pw->pw_passwd this line
      // cause a seg fault.
-----

There's many patches possible :
1) changing uid_t for signed long : this way every negative id are invalid. This also cut in half the number of valid ids. Also, this can be tricky to change, I still don't know much about hurd, but a change like that can cause problems where uid_t is used.
2) casting id to signed long in the line :
-    if (id >= 0)
+   if ((signed long)id >= 0)
This also cut in half the number of valid ids, but it does not interfere with anything already working. 3) check for pw != NULL before calling anything with pw (like strcmp). With this, when the user id is invalid, its fall back to asking password for root. That sucks because there's no use for paranoid in that case : you know that the user does not exist.

So what do you think guys...

Regards
--Marc O. Mercier




reply via email to

[Prev in Thread] Current Thread [Next in Thread]