Hi John,
On Sunday 02 September 2007 01:39, John Gumb wrote:
> Looks like there's a bug in loginutils/login.c if PAM authentication is
> enabled.
>
> Symptoms are that if a valid username is entered with an incorrect
> password then the user is allowed to log in.
>
> Problem is if pam authentication fails the code just goes on to do a
> getpwnam(username) which will succeed so long as the username is valid.
> In the authentication failure case we need to goto auth_failed.
>
> --- loginutils/login.c.orig 2007-09-02 00:50:09.000000000 +0100
> +++ loginutils/login.c 2007-09-02 00:50:58.000000000 +0100
> @@ -324,6 +324,11 @@
> }
> safe_strncpy(username, pamuser,
> sizeof(username));
> }
> + else
> + {
> + goto auth_failed;
> + }
> +
> /* If we get here, the user was authenticated, and is
> * granted access. */
> pw = getpwnam(username);
>
> seems to fix it.
PAM login support is new.
Thanks for spotting this bug. Can you test attached patch?
--
vda
diff -d -urpN busybox.3/loginutils/login.c busybox.4/loginutils/login.c
--- busybox.3/loginutils/login.c 2007-09-03 11:22:39.000000000 +0100
+++ busybox.4/loginutils/login.c 2007-09-03 12:35:10.110536000 +0100
@@ -307,18 +307,26 @@ int login_main(int argc, char **argv)
goto pam_auth_failed;
}
pamret = pam_authenticate(pamh, 0);
- if (pamret == PAM_SUCCESS) {
- char *pamuser;
- /* check that the account is healthy. */
- pamret = pam_acct_mgmt(pamh, 0);
- if (pamret != PAM_SUCCESS) {
- failed_msg = "account setup";
- goto pam_auth_failed;
- }
- /* read user back */
+ if (pamret != PAM_SUCCESS) {
+ failed_msg = "pam_authenticate";
+ goto pam_auth_failed;
+ /* TODO: or just "goto auth_failed"
+ * since user seems to enter wrong password
+ * (in this case pamret == 7)
+ */
+ }
+ /* check that the account is healthy */
+ pamret = pam_acct_mgmt(pamh, 0);
+ if (pamret != PAM_SUCCESS) {
+ failed_msg = "account setup";
+ goto pam_auth_failed;
+ }
+ /* read user back */
+ {
+ const char *pamuser;
/* gcc: "dereferencing type-punned pointer breaks aliasing rules..."
- * thus we use double cast */
- if (pam_get_item(pamh, PAM_USER, (const void **)(void*)&pamuser) != PAM_SUCCESS) {
+ * thus we cast to (void*) */
+ if (pam_get_item(pamh, PAM_USER, (void*)&pamuser) != PAM_SUCCESS) {
failed_msg = "pam_get_item(USER)";
goto pam_auth_failed;
}
@@ -331,7 +339,7 @@ int login_main(int argc, char **argv)
break;
goto auth_failed;
pam_auth_failed:
- bb_error_msg("%s failed: %s", failed_msg, pam_strerror(pamh, pamret));
+ bb_error_msg("%s failed: %s (%d)", failed_msg, pam_strerror(pamh, pamret), pamret);
safe_strncpy(username, "UNKNOWN", sizeof(username));
#else /* not PAM */
pw = getpwnam(username);
@@ -360,6 +368,7 @@ int login_main(int argc, char **argv)
auth_failed:
opt &= ~LOGIN_OPT_f;
bb_do_delay(FAIL_DELAY);
+ /* TODO: doesn't sound like correct English phrase to me */
puts("Login incorrect");
if (++count == 3) {
syslog(LOG_WARNING, "invalid password for '%s'%s",
_______________________________________________
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox
