Denys Vlasenko schrieb:
> On Saturday 31 January 2009 18:37, walter harms wrote:
>> Hi Denis,
>> i have broken down my patch to login.c into several pieces (4).
>> Shall i send the diff or the .c ?
>
> Please send diffs
attached all patches should apply on top of each other. The patches are based
on the login.c i send to the list end of last year.
v0-v1: basicly moves SELINUX and LOGIN_SCRIPTS out of the way
v1-v2: rework the get ttynam() part, move from array to pointer
v2-v3: move pswd check for PAM and !PAM into check_pswd()
- fix getpwnam_r buffer size
Other utilities that check against the system password may use this code also.
It is the first time i venture in this area so i not know.
v3-v4: remove char *tmp
I do not claim that everything is perfect so be careful while testing.
>
>> can i ping about ionice again again ? It is simply and 2.6 only.
>
> Quality of code is still low.
>
> For example, analyze what happens with variable "pid".
> It will end up uninitialized if -p is not given,
> and you will set ioprio to some random process.
>
> Did you test "ioprio -c 1 -n 1 sleep 999"? Did you notice
> that prio is not set correctly because of the above?
>
> I cleaned the applet up, tested and applied it to svn.
> I am asking you to test your code better.
i am bad finding my own bugs and i do not introduce them intentionally.
Unfortunately i had no testers what means you get everything that is normaly
already filtered.
re,
wh
> --
> vda
>
>
>
--- login.c.org 2008-12-28 23:24:53.000000000 +0100
+++ login.v1.c 2009-01-30 21:49:14.000000000 +0100
@@ -3,6 +3,7 @@
* Licensed under GPLv2 or later, see file LICENSE in this tarball for details.
*/
+
#include "libbb.h"
#include <syslog.h>
#include <utmp.h>
@@ -165,6 +166,58 @@
static ALWAYS_INLINE int check_securetty(void) { return 1; }
#endif
+
+#if ENABLE_SELINUX
+static void initselinux(char *username, char *full_tty,
+ security_context_t * user_sid)
+{
+ security_context_t old_tty_sid, new_tty_sid;
+
+ if (get_default_context(username, NULL, user_sid)) {
+ bb_error_msg_and_die("cannot get SID for %s", username);
+ }
+ if (getfilecon(full_tty, &old_tty_sid) < 0) {
+ bb_perror_msg_and_die("getfilecon(%s) failed", full_tty);
+ }
+ if (security_compute_relabel(user_sid, old_tty_sid,
+ SECCLASS_CHR_FILE, &new_tty_sid) != 0) {
+ bb_perror_msg_and_die("security_change_sid(%s) failed", full_tty);
+ }
+ if (setfilecon(full_tty, new_tty_sid) != 0) {
+ bb_perror_msg_and_die("chsid(%s, %s) failed", full_tty, new_tty_sid);
+ }
+
+}
+
+#endif
+
+#if ENABLE_LOGIN_SCRIPTS
+static ALWAYS_INLINE
+void run_login_script(struct passwd *pw, char *full_tty)
+{
+ char *t_argv[2];
+
+ t_argv[0] = getenv("LOGIN_PRE_SUID_SCRIPT");
+ if (t_argv[0]) {
+ t_argv[1] = NULL;
+ xsetenv("LOGIN_TTY", full_tty);
+ xsetenv("LOGIN_USER", pw->pw_name);
+ xsetenv("LOGIN_UID", utoa(pw->pw_uid));
+ xsetenv("LOGIN_GID", utoa(pw->pw_gid));
+ xsetenv("LOGIN_SHELL", pw->pw_shell);
+ spawn_and_wait(t_argv); /* NOMMU-friendly */
+ unsetenv("LOGIN_TTY");
+ unsetenv("LOGIN_USER");
+ unsetenv("LOGIN_UID");
+ unsetenv("LOGIN_GID");
+ unsetenv("LOGIN_SHELL");
+ }
+
+}
+#else
+static void run_login_script(struct passwd *pw, char *full_tty) {}
+#endif
+
static void get_username_or_die(char *buf, int size_buf)
{
int c, cntdown;
@@ -285,12 +338,11 @@
read_or_build_utent(&utent, run_by_root);
if (opt & LOGIN_OPT_h) {
- USE_FEATURE_UTMP(
- safe_strncpy(utent.ut_host, opt_host, sizeof(utent.ut_host));
- )
+ USE_FEATURE_UTMP(safe_strncpy(utent.ut_host, opt_host, sizeof(utent.ut_host));)
fromhost = xasprintf(" on '%s' from '%s'", short_tty, opt_host);
- } else
+ } else {
fromhost = xasprintf(" on '%s'", short_tty);
+ }
/* Was breaking "login <username>" from shell command line: */
/*bb_setpgrp();*/
@@ -410,54 +462,16 @@
write_utent(&utent, username);
-#if ENABLE_SELINUX
- if (is_selinux_enabled()) {
- security_context_t old_tty_sid, new_tty_sid;
+ USE_SELINUX(initselinux(username, full_tty, &user_sid));
- if (get_default_context(username, NULL, &user_sid)) {
- bb_error_msg_and_die("cannot get SID for %s",
- username);
- }
- if (getfilecon(full_tty, &old_tty_sid) < 0) {
- bb_perror_msg_and_die("getfilecon(%s) failed",
- full_tty);
- }
- if (security_compute_relabel(user_sid, old_tty_sid,
- SECCLASS_CHR_FILE, &new_tty_sid) != 0) {
- bb_perror_msg_and_die("security_change_sid(%s) failed",
- full_tty);
- }
- if (setfilecon(full_tty, new_tty_sid) != 0) {
- bb_perror_msg_and_die("chsid(%s, %s) failed",
- full_tty, new_tty_sid);
- }
- }
-#endif
/* Try these, but don't complain if they fail.
* _f_chown is safe wrt race t=ttyname(0);...;chown(t); */
fchown(0, pw->pw_uid, pw->pw_gid);
fchmod(0, 0600);
/* We trust environment only if we run by root */
- if (ENABLE_LOGIN_SCRIPTS && run_by_root) {
- char *t_argv[2];
-
- t_argv[0] = getenv("LOGIN_PRE_SUID_SCRIPT");
- if (t_argv[0]) {
- t_argv[1] = NULL;
- xsetenv("LOGIN_TTY", full_tty);
- xsetenv("LOGIN_USER", pw->pw_name);
- xsetenv("LOGIN_UID", utoa(pw->pw_uid));
- xsetenv("LOGIN_GID", utoa(pw->pw_gid));
- xsetenv("LOGIN_SHELL", pw->pw_shell);
- spawn_and_wait(t_argv); /* NOMMU-friendly */
- unsetenv("LOGIN_TTY" );
- unsetenv("LOGIN_USER" );
- unsetenv("LOGIN_UID" );
- unsetenv("LOGIN_GID" );
- unsetenv("LOGIN_SHELL");
- }
- }
+ if (ENABLE_LOGIN_SCRIPTS && run_by_root)
+ run_login_script(pw, full_tty);
change_identity(pw);
tmp = pw->pw_shell;
@@ -470,11 +484,11 @@
if (pw->pw_uid == 0)
syslog(LOG_INFO, "root login%s", fromhost);
-#if ENABLE_SELINUX
+
/* well, a simple setexeccon() here would do the job as well,
* but let's play the game for now */
- set_current_security_context(user_sid);
-#endif
+ USE_SELINUX(set_current_security_context(user_sid);)
+
// util-linux login also does:
// /* start new session */
--- login.v1.c 2009-01-30 21:49:14.000000000 +0100
+++ login.v2.c 2009-01-30 22:37:09.000000000 +0100
@@ -287,7 +287,7 @@
struct passwd *pw;
char *opt_host = opt_host; /* for compiler */
char *opt_user = opt_user; /* for compiler */
- char full_tty[TTYNAME_SIZE];
+ char *full_tty;
USE_SELINUX(security_context_t user_sid = NULL;)
USE_FEATURE_UTMP(struct utmp utent;)
#if ENABLE_PAM
@@ -299,7 +299,6 @@
char pwdbuf[256];
#endif
- short_tty = full_tty;
username[0] = '\0';
signal(SIGALRM, alarm_handler);
alarm(TIMEOUT);
@@ -325,14 +324,15 @@
safe_strncpy(username, argv[0], sizeof(username));
/* Let's find out and memorize our tty */
- if (!isatty(0) || !isatty(1) || !isatty(2))
+ if (!isatty(STDIN_FILENO) || !isatty(STDOUT_FILENO) || !isatty(STDERR_FILENO))
return EXIT_FAILURE; /* Must be a terminal */
- safe_strncpy(full_tty, "UNKNOWN", sizeof(full_tty));
- tmp = ttyname(0);
- if (tmp) {
- safe_strncpy(full_tty, tmp, sizeof(full_tty));
- if (strncmp(full_tty, "/dev/", 5) == 0)
- short_tty = full_tty + 5;
+
+ full_tty = ttyname(STDIN_FILENO);
+ if (!full_tty || strncmp(full_tty, "/dev/", 5)) {
+ full_tty = (char *) "UNKNOWN";
+ short_tty = full_tty;
+ } else {
+ short_tty = full_tty + 5;
}
read_or_build_utent(&utent, run_by_root);
--- login.v2.c 2009-01-30 22:37:09.000000000 +0100
+++ login.v3.c 2009-02-01 20:45:36.000000000 +0100
@@ -28,13 +28,22 @@
};
#endif
+
enum {
TIMEOUT = 60,
EMPTY_USERNAME_COUNT = 10,
USERNAME_SIZE = 32,
- TTYNAME_SIZE = 32,
+// TTYNAME_SIZE = 32,
};
+enum {
+ LOGIN_OPT_f = (1 << 0),
+ LOGIN_OPT_h = (1 << 1),
+ LOGIN_OPT_p = (1 << 2),
+};
+
+
+
static char* short_tty;
#if ENABLE_FEATURE_UTMP
@@ -270,14 +279,182 @@
_exit(EXIT_SUCCESS);
}
+
+#if ENABLE_PAM
+/*
+ask sysconf what size we need
+then execute function
+*/
+
+
+/*
+ return struct passwd or NULL on error
+ you need to provide *only* a pointer
+ that need to be freed on success.
+
+ all memory will be freed on error
+
+*/
+
+
+static
+struct passwd *bb_getpwnam_r(char *username)
+{
+ struct passwd *pwdstruct,*ptr;
+ struct passwd *pwd1;
+ size_t len = 1024;
+ long int initlen = sysconf(_SC_GETPW_R_SIZE_MAX);
+
+ if (initlen > 0)
+ len = (size_t) initlen;
+
+ /*
+ pwdstruct is a list of pointers to
+ data stored in pwdbuf
+ */
+
+ pwdstruct= xmalloc(len+sizeof(*pwdstruct));
+ ptr=pwdstruct+len;
+ /*
+ pwd1 will become NULL on not found
+ getpwnam_r will become 0 on eror
+ */
+
+ if ( 0 == getpwnam_r(username, pwdstruct, (char *)ptr, len, &pwd1 ) && pwd1 != NULL )
+ return pwdstruct;
+ else {
+ free(pwdstruct);
+ return NULL;
+ }
+
+}
+
+/*
+ THIS is the only important part check the pswd
+ calling MUST be the same as with !ENABLE_PAM
+*/
+
+static int check_pswd(char *username, int userlen, struct passwd **pw)
+{
+ int pamret;
+ pam_handle_t *pamh;
+ const char *pamuser;
+ const char *failed_msg;
+
+ pamret = pam_start("login", username, &conv, &pamh);
+ if (pamret != PAM_SUCCESS) {
+ failed_msg = "start";
+ goto pam_auth_failed;
+ }
+ /* set TTY (so things like securetty work) */
+ pamret = pam_set_item(pamh, PAM_TTY, short_tty);
+ if (pamret != PAM_SUCCESS) {
+ failed_msg = "set_item(TTY)";
+ goto pam_auth_failed;
+ }
+ pamret = pam_authenticate(pamh, 0);
+ if (pamret != PAM_SUCCESS) {
+ failed_msg = "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 = "acct_mgmt";
+ goto pam_auth_failed;
+ }
+ /* read user back */
+ pamuser = NULL;
+ /* gcc: "dereferencing type-punned pointer breaks aliasing rules..."
+ * thus we cast to (void*) */
+ if (pam_get_item(pamh, PAM_USER, (void *) &pamuser) != PAM_SUCCESS) {
+ failed_msg = "get_item(USER)";
+ goto pam_auth_failed;
+ }
+/*
+ this is a special case
+*/
+ if (!pamuser || !pamuser[0])
+ goto auth_failed;
+
+ safe_strncpy(username, pamuser, userlen);
+ /* Don't use "pw = getpwnam(username);",
+ * PAM is said to be capable of destroying static storage
+ * used by getpwnam(). We are using safe(r) function */
+
+ *pw = bb_getpwnam_r(username);
+
+ if (!*pw)
+ goto auth_failed;
+ pamret = pam_open_session(pamh, 0);
+ if (pamret != PAM_SUCCESS) {
+ failed_msg = "open_session";
+ goto pam_auth_failed;
+ }
+ pamret = pam_setcred(pamh, PAM_ESTABLISH_CRED);
+ if (pamret != PAM_SUCCESS) {
+ failed_msg = "setcred";
+ goto pam_auth_failed;
+ }
+ return 0; /* success, continue login process */
+
+ pam_auth_failed:
+ bb_error_msg("pam_%s call failed: %s (%d)", failed_msg,
+ pam_strerror(pamh, pamret), pamret);
+ auth_failed:
+ safe_strncpy(username, "UNKNOWN", userlen);
+ return -1;
+
+}
+#else
+/*
+ FIXME: replace goto with return ?
+*/
+
+static int check_pswd(char *username, int userlen, struct passwd **pw2)
+{
+ struct passwd *pw;
+
+ pw = getpwnam(username); /* replace with _r ?? */
+ if (!pw) {
+ safe_strncpy(username, "UNKNOWN", userlen);
+ goto fake_it;
+ }
+
+ if (pw->pw_passwd[0] == '!' || pw->pw_passwd[0] == '*')
+ goto auth_failed;
+
+ if (option_mask32 & LOGIN_OPT_f)
+ goto auth_success; /* -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])
+ goto auth_success;
+ fake_it:
+ /* authorization takes place here */
+ if (correct_password(pw))
+ return 0;
+ auth_failed:
+ return -1;
+ auth_success:
+ *pw2 = pw;
+ return 0;
+
+}
+#endif
+
+
+
+
int login_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
int login_main(int argc UNUSED_PARAM, char **argv)
{
- enum {
- LOGIN_OPT_f = (1<<0),
- LOGIN_OPT_h = (1<<1),
- LOGIN_OPT_p = (1<<2),
- };
char *fromhost;
char username[USERNAME_SIZE];
const char *tmp;
@@ -290,14 +467,6 @@
char *full_tty;
USE_SELINUX(security_context_t user_sid = NULL;)
USE_FEATURE_UTMP(struct utmp utent;)
-#if ENABLE_PAM
- int pamret;
- pam_handle_t *pamh;
- const char *pamuser;
- const char *failed_msg;
- struct passwd pwdstruct;
- char pwdbuf[256];
-#endif
username[0] = '\0';
signal(SIGALRM, alarm_handler);
@@ -356,102 +525,23 @@
if (!username[0])
get_username_or_die(username, sizeof(username));
-#if ENABLE_PAM
- pamret = pam_start("login", username, &conv, &pamh);
- if (pamret != PAM_SUCCESS) {
- failed_msg = "start";
- goto pam_auth_failed;
- }
- /* set TTY (so things like securetty work) */
- pamret = pam_set_item(pamh, PAM_TTY, short_tty);
- if (pamret != PAM_SUCCESS) {
- failed_msg = "set_item(TTY)";
- goto pam_auth_failed;
- }
- pamret = pam_authenticate(pamh, 0);
- if (pamret != PAM_SUCCESS) {
- failed_msg = "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 = "acct_mgmt";
- goto pam_auth_failed;
- }
- /* read user back */
- pamuser = NULL;
- /* gcc: "dereferencing type-punned pointer breaks aliasing rules..."
- * thus we cast to (void*) */
- if (pam_get_item(pamh, PAM_USER, (void*)&pamuser) != PAM_SUCCESS) {
- failed_msg = "get_item(USER)";
- goto pam_auth_failed;
- }
- if (!pamuser || !pamuser[0])
- goto auth_failed;
- safe_strncpy(username, pamuser, sizeof(username));
- /* Don't use "pw = getpwnam(username);",
- * PAM is said to be capable of destroying static storage
- * used by getpwnam(). We are using safe(r) function */
- pw = NULL;
- getpwnam_r(username, &pwdstruct, pwdbuf, sizeof(pwdbuf), &pw);
- if (!pw)
- goto auth_failed;
- pamret = pam_open_session(pamh, 0);
- if (pamret != PAM_SUCCESS) {
- failed_msg = "open_session";
- goto pam_auth_failed;
- }
- pamret = pam_setcred(pamh, PAM_ESTABLISH_CRED);
- if (pamret != PAM_SUCCESS) {
- failed_msg = "setcred";
- goto pam_auth_failed;
- }
- break; /* success, continue login process */
-
- pam_auth_failed:
- bb_error_msg("pam_%s call failed: %s (%d)", failed_msg,
- pam_strerror(pamh, pamret), pamret);
- safe_strncpy(username, "UNKNOWN", sizeof(username));
-#else /* not PAM */
- pw = getpwnam(username);
- if (!pw) {
- strcpy(username, "UNKNOWN");
- goto fake_it;
- }
-
- 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])
+ if (check_pswd(username, sizeof(username), &pw) == 0)
break;
- fake_it:
- /* authorization takes place here */
- if (correct_password(pw))
- break;
-#endif /* ENABLE_PAM */
- auth_failed:
+
opt &= ~LOGIN_OPT_f;
bb_do_delay(FAIL_DELAY);
/* TODO: doesn't sound like correct English phrase to me */
puts("Login incorrect");
+/*
+ FIXME: PAM may have it own ideas how often we can try
+*/
if (++count == 3) {
syslog(LOG_WARNING, "invalid password for '%s'%s",
- username, fromhost);
+ username, fromhost);
return EXIT_FAILURE;
}
username[0] = '\0';
+
} /* while (1) */
alarm(0);
--- login.v3.c 2009-02-01 20:45:36.000000000 +0100
+++ login.v4.c 2009-01-30 23:40:45.000000000 +0100
@@ -283,7 +283,12 @@
#if ENABLE_PAM
/*
ask sysconf what size we need
-then execute function
+alloc buffer
+execute function
+FIXME:
+if we want to free the buffer and the pointer we need to return both
+either as globals (arg) or a special struct (eeeck)
+
*/
@@ -296,11 +301,12 @@
*/
+static char *ptr;
static
struct passwd *bb_getpwnam_r(char *username)
{
- struct passwd *pwdstruct,*ptr;
+ struct passwd *pwdstruct;
struct passwd *pwd1;
size_t len = 1024;
long int initlen = sysconf(_SC_GETPW_R_SIZE_MAX);
@@ -313,17 +319,20 @@
data stored in pwdbuf
*/
- pwdstruct= xmalloc(len+sizeof(*pwdstruct));
- ptr=pwdstruct+len;
+ ptr = malloc(len);
+ pwdstruct= malloc(sizeof(*pwdstruct));
+
/*
pwd1 will become NULL on not found
getpwnam_r will become 0 on eror
*/
- if ( 0 == getpwnam_r(username, pwdstruct, (char *)ptr, len, &pwd1 ) && pwd1 != NULL )
+ if ( 0 == getpwnam_r(username, pwdstruct, ptr, len, &pwd1 ) && pwd1 != NULL )
return pwdstruct;
else {
free(pwdstruct);
+ free(ptr);
+ ptr=NULL;
return NULL;
}
@@ -338,6 +347,7 @@
{
int pamret;
pam_handle_t *pamh;
+ char *buffer;
const char *pamuser;
const char *failed_msg;
@@ -386,7 +396,7 @@
* PAM is said to be capable of destroying static storage
* used by getpwnam(). We are using safe(r) function */
- *pw = bb_getpwnam_r(username);
+ *pw = bb_getpwnam_r(username,&buffer);
if (!*pw)
goto auth_failed;
@@ -456,8 +466,8 @@
int login_main(int argc UNUSED_PARAM, char **argv)
{
char *fromhost;
- char username[USERNAME_SIZE];
- const char *tmp;
+ char username[USERNAME_SIZE]; /* better: _SC_LOGIN_NAME_MAX */
+ // const char *tmp;
int run_by_root;
unsigned opt;
int count = 0;
@@ -465,8 +475,8 @@
char *opt_host = opt_host; /* for compiler */
char *opt_user = opt_user; /* for compiler */
char *full_tty;
- USE_SELINUX(security_context_t user_sid = NULL;)
- USE_FEATURE_UTMP(struct utmp utent;)
+ USE_SELINUX(security_context_t user_sid = NULL;)
+ USE_FEATURE_UTMP(struct utmp utent;)
username[0] = '\0';
signal(SIGALRM, alarm_handler);
@@ -564,11 +574,14 @@
run_login_script(pw, full_tty);
change_identity(pw);
- tmp = pw->pw_shell;
- if (!tmp || !*tmp)
- tmp = DEFAULT_SHELL;
+
+ /*
+ make sure we have a propper shell
+ */
+ if ( !pw->pw_shell || !*pw->pw_shell )
+ pw->pw_shell = (char *)DEFAULT_SHELL;
/* setup_environment params: shell, clear_env, change_env, pw */
- setup_environment(tmp, !(opt & LOGIN_OPT_p), 1, pw);
+ setup_environment(pw->pw_shell, !(opt & LOGIN_OPT_p), 1, pw);
motd();
@@ -577,7 +590,7 @@
/* well, a simple setexeccon() here would do the job as well,
* but let's play the game for now */
- USE_SELINUX(set_current_security_context(user_sid);)
+ USE_SELINUX(set_current_security_context(user_sid););
// util-linux login also does:
@@ -601,8 +614,14 @@
* should we leave SIGINT etc enabled or disabled? */
signal(SIGINT, SIG_DFL);
+ /*
+ note: in PAM case we still have a large buffer allocated
+ we should make a copy of pw->pw_shell and free the buffer
+ */
+
+
/* Exec login shell with no additional parameters */
- run_shell(tmp, 1, NULL, NULL);
+ run_shell( pw->pw_shell, 1, NULL, NULL);
/* return EXIT_FAILURE; - not reached */
}
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
