From 63dc3e90e5988d510c2bbddb46408f2e9220ec16 Mon Sep 17 00:00:00 2001 From: Damien Miller Date: Wed, 7 Feb 2001 12:58:33 +1100 Subject: [PATCH] - (djm) Much KNF on PAM code - (djm) Revise auth-pam.c conversation function to be a little more readable. - (djm) Revise kbd-int PAM conversation function to fold all text messages to before first prompt. Fixes hangs if last pam_message did not require a reply. - (djm) Fix password changing when using PAM kbd-int authentication --- ChangeLog | 6 ++ auth-pam.c | 216 ++++++++++++++++++++++++++-------------------------- auth-pam.h | 1 + auth2-pam.c | 113 +++++++++++++-------------- 4 files changed, 170 insertions(+), 166 deletions(-) diff --git a/ChangeLog b/ChangeLog index 7f3d6922a..2fb64a5e0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,6 +1,12 @@ 20010107 - (bal) Save the whole path to AR in configure. Some Solaris 2.7 installs seem lose track of it while in openbsd-compat/ (two confirmed reports) + - (djm) Much KNF on PAM code + - (djm) Revise auth-pam.c conversation function to be a little more readable. + - (djm) Revise kbd-int PAM conversation function to fold all text messages + to before first prompt. Fixes hangs if last pam_message did not require + a reply. + - (djm) Fix password changing when using PAM kbd-int authentication 20010105 - (bal) Disable groupaccess by setting NGROUPS_MAX to 0 for platforms diff --git a/auth-pam.c b/auth-pam.c index ab985d15b..e6fddecc0 100644 --- a/auth-pam.c +++ b/auth-pam.c @@ -28,31 +28,29 @@ #include "ssh.h" #include "xmalloc.h" #include "log.h" +#include "auth-pam.h" #include "servconf.h" #include "canohost.h" #include "readpass.h" -RCSID("$Id: auth-pam.c,v 1.24 2001/02/05 12:42:17 stevesk Exp $"); +RCSID("$Id: auth-pam.c,v 1.25 2001/02/07 01:58:34 djm Exp $"); #define NEW_AUTHTOK_MSG \ "Warning: Your password has expired, please change it now" -/* Callbacks */ -static int pamconv(int num_msg, const struct pam_message **msg, - struct pam_response **resp, void *appdata_ptr); -void pam_cleanup_proc(void *context); -void pam_msg_cat(const char *msg); +static int do_pam_conversation(int num_msg, const struct pam_message **msg, + struct pam_response **resp, void *appdata_ptr); /* module-local variables */ static struct pam_conv conv = { - pamconv, + do_pam_conversation, NULL }; +static char *pam_msg = NULL; static pam_handle_t *pamh = NULL; static const char *pampasswd = NULL; -static char *pam_msg = NULL; -/* states for pamconv() */ +/* states for do_pam_conversation() */ enum { INITIAL_LOGIN, OTHER } pamstate = INITIAL_LOGIN; /* remember whether pam_acct_mgmt() returned PAM_NEWAUTHTOK_REQD */ static int password_change_required = 0; @@ -80,14 +78,14 @@ int do_pam_authenticate(int flags) * * INITIAL_LOGIN mode simply feeds the password from the client into * PAM in response to PAM_PROMPT_ECHO_OFF, and collects output - * messages with pam_msg_cat(). This is used during initial + * messages with into pam_msg. This is used during initial * authentication to bypass the normal PAM password prompt. * * OTHER mode handles PAM_PROMPT_ECHO_OFF with read_passphrase(prompt, 1) * and outputs messages to stderr. This mode is used if pam_chauthtok() * is called to update expired passwords. */ -static int pamconv(int num_msg, const struct pam_message **msg, +static int do_pam_conversation(int num_msg, const struct pam_message **msg, struct pam_response **resp, void *appdata_ptr) { struct pam_response *reply; @@ -100,40 +98,28 @@ static int pamconv(int num_msg, const struct pam_message **msg, return PAM_CONV_ERR; for (count = 0; count < num_msg; count++) { - switch(PAM_MSG_MEMBER(msg, count, msg_style)) { + if (pamstate == INITIAL_LOGIN) { + /* + * We can't use stdio yet, queue messages for + * printing later + */ + switch(PAM_MSG_MEMBER(msg, count, msg_style)) { case PAM_PROMPT_ECHO_ON: - if (pamstate == INITIAL_LOGIN) { + free(reply); + return PAM_CONV_ERR; + case PAM_PROMPT_ECHO_OFF: + if (pampasswd == NULL) { free(reply); return PAM_CONV_ERR; - } else { - fputs(PAM_MSG_MEMBER(msg, count, msg), stderr); - fgets(buf, sizeof(buf), stdin); - reply[count].resp = xstrdup(buf); - reply[count].resp_retcode = PAM_SUCCESS; - break; - } - case PAM_PROMPT_ECHO_OFF: - if (pamstate == INITIAL_LOGIN) { - if (pampasswd == NULL) { - free(reply); - return PAM_CONV_ERR; - } - reply[count].resp = xstrdup(pampasswd); - } else { - reply[count].resp = - xstrdup(read_passphrase(PAM_MSG_MEMBER(msg, count, msg), 1)); } + reply[count].resp = xstrdup(pampasswd); reply[count].resp_retcode = PAM_SUCCESS; break; case PAM_ERROR_MSG: case PAM_TEXT_INFO: if ((*msg)[count].msg != NULL) { - if (pamstate == INITIAL_LOGIN) - pam_msg_cat(PAM_MSG_MEMBER(msg, count, msg)); - else { - fputs(PAM_MSG_MEMBER(msg, count, msg), stderr); - fputs("\n", stderr); - } + message_cat(&pam_msg, + PAM_MSG_MEMBER(msg, count, msg)); } reply[count].resp = xstrdup(""); reply[count].resp_retcode = PAM_SUCCESS; @@ -141,6 +127,36 @@ static int pamconv(int num_msg, const struct pam_message **msg, default: free(reply); return PAM_CONV_ERR; + } + } else { + /* + * stdio is connected, so interact directly + */ + switch(PAM_MSG_MEMBER(msg, count, msg_style)) { + case PAM_PROMPT_ECHO_ON: + fputs(PAM_MSG_MEMBER(msg, count, msg), stderr); + fgets(buf, sizeof(buf), stdin); + reply[count].resp = xstrdup(buf); + reply[count].resp_retcode = PAM_SUCCESS; + break; + case PAM_PROMPT_ECHO_OFF: + reply[count].resp = xstrdup( + read_passphrase(PAM_MSG_MEMBER(msg, count, + msg), 1)); + reply[count].resp_retcode = PAM_SUCCESS; + break; + case PAM_ERROR_MSG: + case PAM_TEXT_INFO: + if ((*msg)[count].msg != NULL) + fprintf(stderr, "%s\n", + PAM_MSG_MEMBER(msg, count, msg)); + reply[count].resp = xstrdup(""); + reply[count].resp_retcode = PAM_SUCCESS; + break; + default: + free(reply); + return PAM_CONV_ERR; + } } } @@ -154,25 +170,21 @@ void pam_cleanup_proc(void *context) { int pam_retval; - if (pamh != NULL) - { + if (pamh) { pam_retval = pam_close_session(pamh, 0); - if (pam_retval != PAM_SUCCESS) { + if (pam_retval != PAM_SUCCESS) log("Cannot close PAM session[%d]: %.200s", - pam_retval, PAM_STRERROR(pamh, pam_retval)); - } + pam_retval, PAM_STRERROR(pamh, pam_retval)); pam_retval = pam_setcred(pamh, PAM_DELETE_CRED); - if (pam_retval != PAM_SUCCESS) { - debug("Cannot delete credentials[%d]: %.200s", - pam_retval, PAM_STRERROR(pamh, pam_retval)); - } + if (pam_retval != PAM_SUCCESS) + debug("Cannot delete credentials[%d]: %.200s", + pam_retval, PAM_STRERROR(pamh, pam_retval)); pam_retval = pam_end(pamh, pam_retval); - if (pam_retval != PAM_SUCCESS) { + if (pam_retval != PAM_SUCCESS) log("Cannot release PAM authentication[%d]: %.200s", - pam_retval, PAM_STRERROR(pamh, pam_retval)); - } + pam_retval, PAM_STRERROR(pamh, pam_retval)); } } @@ -197,12 +209,13 @@ int auth_pam_password(struct passwd *pw, const char *password) pamstate = INITIAL_LOGIN; pam_retval = do_pam_authenticate(0); if (pam_retval == PAM_SUCCESS) { - debug("PAM Password authentication accepted for user \"%.100s\"", - pw->pw_name); + debug("PAM Password authentication accepted for " + "user \"%.100s\"", pw->pw_name); return 1; } else { - debug("PAM Password authentication for \"%.100s\" failed[%d]: %s", - pw->pw_name, pam_retval, PAM_STRERROR(pamh, pam_retval)); + debug("PAM Password authentication for \"%.100s\" " + "failed[%d]: %s", pw->pw_name, pam_retval, + PAM_STRERROR(pamh, pam_retval)); return 0; } } @@ -213,22 +226,21 @@ int do_pam_account(char *username, char *remote_user) int pam_retval; extern ServerOptions options; + pam_set_conv(&conv); + debug("PAM setting rhost to \"%.200s\"", get_canonical_hostname(options.reverse_mapping_check)); pam_retval = pam_set_item(pamh, PAM_RHOST, get_canonical_hostname(options.reverse_mapping_check)); - if (pam_retval != PAM_SUCCESS) { - fatal("PAM set rhost failed[%d]: %.200s", - pam_retval, PAM_STRERROR(pamh, pam_retval)); - } - - if (remote_user != NULL) { + if (pam_retval != PAM_SUCCESS) + fatal("PAM set rhost failed[%d]: %.200s", pam_retval, + PAM_STRERROR(pamh, pam_retval)); + if (remote_user) { debug("PAM setting ruser to \"%.200s\"", remote_user); pam_retval = pam_set_item(pamh, PAM_RUSER, remote_user); - if (pam_retval != PAM_SUCCESS) { - fatal("PAM set ruser failed[%d]: %.200s", - pam_retval, PAM_STRERROR(pamh, pam_retval)); - } + if (pam_retval != PAM_SUCCESS) + fatal("PAM set ruser failed[%d]: %.200s", pam_retval, + PAM_STRERROR(pamh, pam_retval)); } pam_retval = pam_acct_mgmt(pamh, 0); @@ -237,13 +249,14 @@ int do_pam_account(char *username, char *remote_user) /* This is what we want */ break; case PAM_NEW_AUTHTOK_REQD: - pam_msg_cat(NEW_AUTHTOK_MSG); + message_cat(&pam_msg, NEW_AUTHTOK_MSG); /* flag that password change is necessary */ password_change_required = 1; break; default: - log("PAM rejected by account configuration[%d]: %.200s", - pam_retval, PAM_STRERROR(pamh, pam_retval)); + log("PAM rejected by account configuration[%d]: " + "%.200s", pam_retval, PAM_STRERROR(pamh, + pam_retval)); return(0); } @@ -258,17 +271,15 @@ void do_pam_session(char *username, const char *ttyname) if (ttyname != NULL) { debug("PAM setting tty to \"%.200s\"", ttyname); pam_retval = pam_set_item(pamh, PAM_TTY, ttyname); - if (pam_retval != PAM_SUCCESS) { + if (pam_retval != PAM_SUCCESS) fatal("PAM set tty failed[%d]: %.200s", - pam_retval, PAM_STRERROR(pamh, pam_retval)); - } + pam_retval, PAM_STRERROR(pamh, pam_retval)); } pam_retval = pam_open_session(pamh, 0); - if (pam_retval != PAM_SUCCESS) { + if (pam_retval != PAM_SUCCESS) fatal("PAM session setup failed[%d]: %.200s", - pam_retval, PAM_STRERROR(pamh, pam_retval)); - } + pam_retval, PAM_STRERROR(pamh, pam_retval)); } /* Set PAM credentials */ @@ -279,13 +290,12 @@ void do_pam_setcred(void) debug("PAM establishing creds"); pam_retval = pam_setcred(pamh, PAM_ESTABLISH_CRED); if (pam_retval != PAM_SUCCESS) { - if(was_authenticated) { + if (was_authenticated) fatal("PAM setcred failed[%d]: %.200s", - pam_retval, PAM_STRERROR(pamh, pam_retval)); - } else { + pam_retval, PAM_STRERROR(pamh, pam_retval)); + else debug("PAM setcred failed[%d]: %.200s", - pam_retval, PAM_STRERROR(pamh, pam_retval)); - } + pam_retval, PAM_STRERROR(pamh, pam_retval)); } } @@ -307,15 +317,13 @@ void do_pam_chauthtok(void) if (password_change_required) { pamstate = OTHER; - /* - * XXX: should we really loop forever? - */ + /* XXX: should we really loop forever? */ do { - pam_retval = pam_chauthtok(pamh, PAM_CHANGE_EXPIRED_AUTHTOK); - if (pam_retval != PAM_SUCCESS) { + pam_retval = pam_chauthtok(pamh, + PAM_CHANGE_EXPIRED_AUTHTOK); + if (pam_retval != PAM_SUCCESS) log("PAM pam_chauthtok failed[%d]: %.200s", - pam_retval, PAM_STRERROR(pamh, pam_retval)); - } + pam_retval, PAM_STRERROR(pamh, pam_retval)); } while (pam_retval != PAM_SUCCESS); } } @@ -336,11 +344,9 @@ void start_pam(const char *user) pam_retval = pam_start(SSHD_PAM_SERVICE, user, &conv, &pamh); - if (pam_retval != PAM_SUCCESS) { + if (pam_retval != PAM_SUCCESS) fatal("PAM initialisation failed[%d]: %.200s", - pam_retval, PAM_STRERROR(pamh, pam_retval)); - } - + pam_retval, PAM_STRERROR(pamh, pam_retval)); #ifdef PAM_TTY_KLUDGE /* * Some PAM modules (e.g. pam_time) require a TTY to operate, @@ -350,10 +356,9 @@ void start_pam(const char *user) * Kludge: Set a fake PAM_TTY */ pam_retval = pam_set_item(pamh, PAM_TTY, "ssh"); - if (pam_retval != PAM_SUCCESS) { + if (pam_retval != PAM_SUCCESS) fatal("PAM set tty failed[%d]: %.200s", - pam_retval, PAM_STRERROR(pamh, pam_retval)); - } + pam_retval, PAM_STRERROR(pamh, pam_retval)); #endif /* PAM_TTY_KLUDGE */ fatal_add_cleanup(&pam_cleanup_proc, NULL); @@ -377,26 +382,25 @@ void print_pam_messages(void) fputs(pam_msg, stderr); } -/* Append a message to the PAM message buffer */ -void pam_msg_cat(const char *msg) +/* Append a message to buffer */ +void message_cat(char **p, const char *a) { - char *p; - size_t new_msg_len; - size_t pam_msg_len; + char *cp; + size_t new_len; - new_msg_len = strlen(msg); + new_len = strlen(a); - if (pam_msg) { - pam_msg_len = strlen(pam_msg); - pam_msg = xrealloc(pam_msg, new_msg_len + pam_msg_len + 2); - p = pam_msg + pam_msg_len; - } else { - pam_msg = p = xmalloc(new_msg_len + 2); - } + if (*p) { + size_t len = strlen(*p); - memcpy(p, msg, new_msg_len); - p[new_msg_len] = '\n'; - p[new_msg_len + 1] = '\0'; + *p = xrealloc(*p, new_len + len + 2); + cp = *p + len; + } else + *p = cp = xmalloc(new_len + 2); + + memcpy(cp, a, new_len); + cp[new_len] = '\n'; + cp[new_len + 1] = '\0'; } #endif /* USE_PAM */ diff --git a/auth-pam.h b/auth-pam.h index 68d446592..8022493f0 100644 --- a/auth-pam.h +++ b/auth-pam.h @@ -15,5 +15,6 @@ void print_pam_messages(void); int pam_password_change_required(void); void do_pam_chauthtok(void); void pam_set_conv(struct pam_conv *); +void message_cat(char **p, const char *a); #endif /* USE_PAM */ diff --git a/auth2-pam.c b/auth2-pam.c index d7a5ff7a4..24e0e0406 100644 --- a/auth2-pam.c +++ b/auth2-pam.c @@ -1,5 +1,5 @@ #include "includes.h" -RCSID("$Id: auth2-pam.c,v 1.7 2001/01/30 23:50:49 djm Exp $"); +RCSID("$Id: auth2-pam.c,v 1.8 2001/02/07 01:58:33 djm Exp $"); #ifdef USE_PAM #include @@ -7,28 +7,28 @@ RCSID("$Id: auth2-pam.c,v 1.7 2001/01/30 23:50:49 djm Exp $"); #include "ssh.h" #include "ssh2.h" #include "auth.h" +#include "auth-pam.h" #include "packet.h" #include "xmalloc.h" #include "dispatch.h" #include "log.h" +static int do_pam_conversation_kbd_int(int num_msg, + const struct pam_message **msg, struct pam_response **resp, + void *appdata_ptr); +void input_userauth_info_response_pam(int type, int plen, void *ctxt); + struct { int finished, num_received, num_expected; int *prompts; struct pam_response *responses; } context_pam2 = {0, 0, 0, NULL}; -static int do_conversation2(int num_msg, const struct pam_message **msg, - struct pam_response **resp, void *appdata_ptr); - -static struct pam_conv -conv2 = { - do_conversation2, +static struct pam_conv conv2 = { + do_pam_conversation_kbd_int, NULL, }; -void input_userauth_info_response_pam(int type, int plen, void *ctxt); - int auth2_pam(Authctxt *authctxt) { @@ -41,7 +41,7 @@ auth2_pam(Authctxt *authctxt) pam_set_conv(&conv2); dispatch_set(SSH2_MSG_USERAUTH_INFO_RESPONSE, - &input_userauth_info_response_pam); + &input_userauth_info_response_pam); retval = (do_pam_authenticate(0) == PAM_SUCCESS); dispatch_set(SSH2_MSG_USERAUTH_INFO_RESPONSE, NULL); @@ -49,11 +49,11 @@ auth2_pam(Authctxt *authctxt) } static int -do_conversation2(int num_msg, const struct pam_message **msg, - struct pam_response **resp, void *appdata_ptr) +do_pam_conversation_kbd_int(int num_msg, const struct pam_message **msg, + struct pam_response **resp, void *appdata_ptr) { - int echo = 0, i = 0, j = 0, done = 0; - char *tmp = NULL, *text = NULL; + int i, j, done; + char *text; context_pam2.finished = 0; context_pam2.num_received = 0; @@ -62,53 +62,47 @@ do_conversation2(int num_msg, const struct pam_message **msg, context_pam2.responses = xmalloc(sizeof(struct pam_response) * num_msg); memset(context_pam2.responses, 0, sizeof(struct pam_response) * num_msg); - packet_start(SSH2_MSG_USERAUTH_INFO_REQUEST); - packet_put_cstring(""); /* Name */ - packet_put_cstring(""); /* Instructions */ - packet_put_cstring(""); /* Language */ - for (i = 0, j = 0; i < num_msg; i++) { - if((PAM_MSG_MEMBER(msg, i, msg_style) == PAM_PROMPT_ECHO_ON) || - (PAM_MSG_MEMBER(msg, i, msg_style) == PAM_PROMPT_ECHO_OFF) || - (i == num_msg - 1)) { - j++; + text = NULL; + for (i = 0, context_pam2.num_expected = 0; i < num_msg; i++) { + int style = PAM_MSG_MEMBER(msg, i, msg_style); + switch (style) { + case PAM_PROMPT_ECHO_ON: + case PAM_PROMPT_ECHO_OFF: + context_pam2.num_expected++; + break; + case PAM_TEXT_INFO: + case PAM_ERROR_MSG: + default: + /* Capture all these messages to be sent at once */ + message_cat(&text, PAM_MSG_MEMBER(msg, i, msg)); + break; } } - packet_put_int(j); /* Number of prompts. */ - context_pam2.num_expected = j; + + if (context_pam2.num_expected == 0) + return PAM_SUCCESS; + + packet_start(SSH2_MSG_USERAUTH_INFO_REQUEST); + packet_put_cstring(""); /* Name */ + packet_put_cstring(""); /* Instructions */ + packet_put_cstring(""); /* Language */ + packet_put_int(context_pam2.num_expected); + for (i = 0, j = 0; i < num_msg; i++) { - switch(PAM_MSG_MEMBER(msg, i, msg_style)) { - case PAM_PROMPT_ECHO_ON: - echo = 1; - break; - case PAM_PROMPT_ECHO_OFF: - echo = 0; - break; - default: - echo = 0; - break; - } - if(text) { - tmp = xmalloc(strlen(text) + strlen(PAM_MSG_MEMBER(msg, i, msg)) + 2); - strcpy(tmp, text); - strcat(tmp, "\n"); - strcat(tmp, PAM_MSG_MEMBER(msg, i, msg)); - xfree(text); - text = tmp; - tmp = NULL; - } else { - text = xstrdup(PAM_MSG_MEMBER(msg, i, msg)); - } - if((PAM_MSG_MEMBER(msg, i, msg_style) == PAM_PROMPT_ECHO_ON) || - (PAM_MSG_MEMBER(msg, i, msg_style) == PAM_PROMPT_ECHO_OFF) || - (i == num_msg - 1)) { - debug("sending prompt ssh-%d(pam-%d) = \"%s\"", - j, i, text); - context_pam2.prompts[j++] = i; + int style = PAM_MSG_MEMBER(msg, i, msg_style); + + /* Skip messages which don't need a reply */ + if (style != PAM_PROMPT_ECHO_ON && style != PAM_PROMPT_ECHO_OFF) + continue; + + context_pam2.prompts[j++] = i; + if (text) { + message_cat(&text, PAM_MSG_MEMBER(msg, i, msg)); packet_put_cstring(text); - packet_put_char(echo); - xfree(text); text = NULL; - } + } else + packet_put_cstring(PAM_MSG_MEMBER(msg, i, msg)); + packet_put_char(style == PAM_PROMPT_ECHO_ON); } packet_send(); packet_write_wait(); @@ -120,17 +114,15 @@ do_conversation2(int num_msg, const struct pam_message **msg, while(context_pam2.finished == 0) { done = 1; dispatch_run(DISPATCH_BLOCK, &done, appdata_ptr); - if(context_pam2.finished == 0) { + if(context_pam2.finished == 0) debug("extra packet during conversation"); - } } if(context_pam2.num_received == context_pam2.num_expected) { *resp = context_pam2.responses; return PAM_SUCCESS; - } else { + } else return PAM_CONV_ERR; - } } void @@ -151,6 +143,7 @@ input_userauth_info_response_pam(int type, int plen, void *ctxt) for (i = 0; i < nresp; i++) { int j = context_pam2.prompts[i]; + resp = packet_get_string(&rlen); context_pam2.responses[j].resp_retcode = PAM_SUCCESS; context_pam2.responses[j].resp = xstrdup(resp);