On Monday 28 May 2007 18:28, Anselmo Lacerda S. de Melo wrote:
> Hi folks!
>
> As I need PAM suport on busybox, I made a patch for busybox-1.4.2. It
> was based on patch for busybox-1.2.1 that I found searching in the
> archives of this list.
>
> All comments appreciated.
Patch against current svn is preferred.
+static struct pam_conv conv = {
+ misc_conv,
+ NULL
+};
+#endif
This struct can/should be const.
+#if ENABLE_PAM
+ pamret = PAM_SUCCESS;
+#endif
...
+#if ENABLE_PAM
+ pamret = pam_start( "login", username, &conv, &pamh );
What's the point of the first assignment?
+ else {
+ // continuing with pam authentication
+ // set TTY (so things like securetty work)
+ if((pamret = pam_set_item(pamh, PAM_TTY, short_tty)) !=
PAM_SUCCESS) {
+ bb_error_msg("Failed to pam_set_item TTY: %s",
pam_strerror(pamh, pamret));
+ goto auth_failed;
+ }
Please use tabs for indent; please do not place assignments in if()s.
+ // Everything from here to auth_ok: is skipped when running
+ // PAM. This is all PAM's responsibility anyway.
+#else
+ PWLOOKUP;
+#endif /* ENABLE_PAM */
If you will properly enclose non-PAM code in bigger #else block,
you will make life easier for people to read and for gcc to optimize
(it will notice that static check_securetty() is not needed).
+ strcpy(username, pamuser);
Unsafe versus buffer overflow!
+ifeq ($(CONFIG_PAM),y)
+ LDFLAGS += -lpam -lpam_misc
+endif
Does not compile for me, need LDLIBS instead.
See attached updated patch against current svn.
It still has some grey areas:
+ if (pamret != PAM_SUCCESS) {
+ bb_error_msg("pam_set_item(TTY) failed: %s",
pam_strerror(pamh, pamret));
+ goto auth_failed;
ok here.
+ }
+ pamret = pam_authenticate(pamh, 0);
+ if (pamret == PAM_SUCCESS) {
+ // Then check that the account is healthy.
+ pamret = pam_acct_mgmt(pamh, 0);
+ if (pamret != PAM_SUCCESS) // No, it isn't
+ bb_error_msg("user not allowed access: %s",
pam_strerror(pamh, pamret));
shouldn't you goto auth_failed here?
+ else {
+ // read user back
+ 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)
+ bb_error_msg("pam_get_item(USER)
failed: %s", pam_strerror(pamh, pamret));
and here?
+ else
+ safe_strncpy(username, pamuser,
sizeof(username));
+ }
+ }
+ if (pam_end(pamh, pamret) != PAM_SUCCESS)
+ bb_error_msg("pam_end failed");
You do it only on success.
Do you need this on pam failure too? (If not, say so in comment).
Conversely, is it needed at all?
--
vda
diff -d -urpN busybox.2/loginutils/Config.in busybox.3/loginutils/Config.in
--- busybox.2/loginutils/Config.in 2007-05-23 02:36:36.000000000 +0200
+++ busybox.3/loginutils/Config.in 2007-05-29 03:59:05.000000000 +0200
@@ -128,6 +128,16 @@ config LOGIN
Note that Busybox binary must be setuid root for this applet to
work properly.
+config PAM
+ bool "Support for PAM (Pluggable Authentication Modules)"
+ default n
+ depends on LOGIN
+ help
+ Include support for PAM in /bin/login. This will effectively disable
+ Busybox's built-in login, so pam_unix.so should be available if you wish
+ to use /etc/passwd for login.
+
+
config LOGIN_SCRIPTS
bool "Support for login scripts"
depends on LOGIN
diff -d -urpN busybox.2/loginutils/login.c busybox.3/loginutils/login.c
--- busybox.2/loginutils/login.c 2007-05-26 20:45:07.000000000 +0200
+++ busybox.3/loginutils/login.c 2007-05-29 04:50:08.000000000 +0200
@@ -12,7 +12,16 @@
#include <selinux/selinux.h> /* for is_selinux_enabled() */
#include <selinux/get_context_list.h> /* for get_default_context() */
#include <selinux/flask.h> /* for security class definitions */
-#include <errno.h>
+#endif
+
+#if ENABLE_PAM
+#include <security/pam_appl.h>
+#include <security/pam_misc.h>
+
+static const struct pam_conv conv = {
+ misc_conv,
+ NULL
+};
#endif
enum {
@@ -122,7 +131,7 @@ static void die_if_nologin_and_non_root(
puts("\r\n[Disconnect bypassed -- root login allowed.]\r");
}
-#if ENABLE_FEATURE_SECURETTY
+#if ENABLE_FEATURE_SECURETTY && !ENABLE_PAM
static int check_securetty(void)
{
FILE *fp;
@@ -210,7 +219,7 @@ int login_main(int argc, char **argv)
LOGIN_OPT_h = (1<<1),
LOGIN_OPT_p = (1<<2),
};
- char fromhost[512];
+ char *fromhost;
char username[USERNAME_SIZE];
const char *tmp;
int amroot;
@@ -222,6 +231,8 @@ int login_main(int argc, char **argv)
char full_tty[TTYNAME_SIZE];
USE_SELINUX(security_context_t user_sid = NULL;)
USE_FEATURE_UTMP(struct utmp utent;)
+ USE_PAM(pam_handle_t *pamh;)
+ USE_PAM(int pamret;)
short_tty = full_tty;
username[0] = '\0';
@@ -261,13 +272,12 @@ int login_main(int argc, char **argv)
USE_FEATURE_UTMP(
safe_strncpy(utent.ut_host, opt_host, sizeof(utent.ut_host));
)
- snprintf(fromhost, sizeof(fromhost)-1, " on '%.100s' from "
- "'%.200s'", short_tty, opt_host);
+ fromhost = xasprintf(" on '%s' from '%s'", short_tty, opt_host);
} else
- snprintf(fromhost, sizeof(fromhost)-1, " on '%.100s'", short_tty);
+ fromhost = xasprintf(" on '%s'", short_tty);
- // Was breaking "login <username>" from shell command line:
- // bb_setpgrp();
+ /* Was breaking "login <username>" from shell command line: */
+ /*bb_setpgrp();*/
openlog(applet_name, LOG_PID | LOG_CONS | LOG_NOWAIT, LOG_AUTH);
@@ -275,28 +285,68 @@ int login_main(int argc, char **argv)
if (!username[0])
get_username_or_die(username, sizeof(username));
+#if ENABLE_PAM
+ pamret = pam_start("login", username, &conv, &pamh);
+ if (pamret != PAM_SUCCESS) {
+ // pam failed, so abort the login
+ bb_error_msg("cannot init PAM: %s", pam_strerror(pamh, pamret));
+ goto auth_failed;
+ }
+ // continuing with pam authentication
+ // set TTY (so things like securetty work)
+ pamret = pam_set_item(pamh, PAM_TTY, short_tty);
+ if (pamret != PAM_SUCCESS) {
+ bb_error_msg("pam_set_item(TTY) failed: %s", pam_strerror(pamh, pamret));
+ goto auth_failed;
+ }
+ pamret = pam_authenticate(pamh, 0);
+ if (pamret == PAM_SUCCESS) {
+ // Then check that the account is healthy.
+ pamret = pam_acct_mgmt(pamh, 0);
+ if (pamret != PAM_SUCCESS) // No, it isn't
+ bb_error_msg("user not allowed access: %s", pam_strerror(pamh, pamret));
+ else {
+ // read user back
+ 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)
+ bb_error_msg("pam_get_item(USER) failed: %s", pam_strerror(pamh, pamret));
+ else
+ safe_strncpy(username, pamuser, sizeof(username));
+ }
+ }
+ // If we get here, the user was authenticated, and is
+ // granted access.
+ if (pam_end(pamh, pamret) != PAM_SUCCESS)
+ bb_error_msg("pam_end failed");
+
+ if (pamret != PAM_SUCCESS)
+ goto auth_failed;
+
+ pw = getpwnam(username);
+ if (pw)
+ break;
+ safe_strncpy(username, "UNKNOWN", sizeof(username));
+#else /* not PAM */
pw = getpwnam(username);
if (!pw) {
safe_strncpy(username, "UNKNOWN", sizeof(username));
goto auth_failed;
}
-
if (pw->pw_passwd[0] == '!' || pw->pw_passwd[0] == '*')
goto auth_failed;
-
if (opt & LOGIN_OPT_f)
break; /* -f USER: success without asking passwd */
-
if (pw->pw_uid == 0 && !check_securetty())
goto auth_failed;
-
/* Don't check the password if password entry is empty (!) */
if (!pw->pw_passwd[0])
break;
-
/* authorization takes place here */
if (correct_password(pw))
break;
+#endif /* ENABLE_PAM */
auth_failed:
opt &= ~LOGIN_OPT_f;
diff -d -urpN busybox.2/Makefile.flags busybox.3/Makefile.flags
--- busybox.2/Makefile.flags 2007-05-23 02:37:25.000000000 +0200
+++ busybox.3/Makefile.flags 2007-05-29 04:34:02.000000000 +0200
@@ -51,6 +51,10 @@ ifeq ($(CONFIG_DEBUG),y)
CFLAGS += $(call cc-option,-g)
endif
+ifeq ($(CONFIG_PAM),y)
+LDLIBS += -lpam -lpam_misc
+endif
+
ifeq ($(CONFIG_STATIC),y)
LDFLAGS += -static
endif
_______________________________________________
busybox mailing list
[email protected]
http://busybox.net/cgi-bin/mailman/listinfo/busybox
