On Mon, Apr 26, 2010 at 08:13:40AM -0400, Stephen Gallagher wrote: > On 04/26/2010 07:41 AM, Sumit Bose wrote: > > On Thu, Apr 22, 2010 at 09:03:25AM -0400, Stephen Gallagher wrote: > >> On 04/22/2010 06:08 AM, Sumit Bose wrote: > >>> Hi, > >>> > >>> the two patches attached should fix #446 and #417 respectively. > >>> > >>> For #417 a different solution, where the message is generated by SSSD > >>> and send to the client, would be possilbe. But I decided against it, > >>> because with the attached patch it is possilbe to support localized > >>> messages. > >> > >> > >> Patch 0001: Ack. > >> > >> Patch 0002: Nack. You would be introducing a security hole here in the > >> form of a stat-open race condition. Please open the file first and use > >> fstat() to determine its size. It probably wouldn't be a bad idea to > >> verify that the file is owned by root and not writable by anyone but > >> root. I don't think it's important to verify whether the file is a > >> symlink or a regular file. > > > > done > > > >> > >> Even making the above check, it wouldn't be a bad idea to verify that > >> total_len == stat_buf.st_size after the read loop. > > > > done > > > >> > >> The DEBUG message in the read loop is wrong. It should be "read failed", > >> not "open failed". > > > > done > > > >> > >> Also, if you're going to allow custom messages here, we probably need to > >> make this message file be per-domain. Since it may be translatable, I > >> suggest we add a new directory structure under /etc/sssd that looks like > >> this (assuming the domain name is LDAP): > >> > >> /etc/sssd/customize > >> /etc/sssd/customize/LDAP/ > >> /etc/sssd/customize/LDAP/root_pw_reset.txt > >> /etc/sssd/customize/LDAP/root_pw_reset.zh_TW > >> ... > >> > > > > done > > > > new version attached. > > > > bye, > > Sumit > > > > Nack. > > Minor nitpick: s/then/than/ in "read fewer bytes [%d] then expected > [%d]." And also, you do not have any variables there to handle the two > %d substitutions. >
done > Please explain why you would not have a domain_name in > select_pw_reset_message(). By the time you have received a message from > PAM that the password needs to be reset, you should absolutely know > which domain you're in. If domain is NULL here, that's almost certainly > an error. I don't think it should be possible to specify > /etc/sssd/customize/root_pw_reset.* This is unneeded complexity, since > we will always have a true default message, and we should never need > this if, as mentioned above, we're properly detecting the domain. > I removed the fallback path and made a missing domain name an error. bye, Sumit > -- > Stephen Gallagher > RHCE 804006346421761 > > Delivering value year after year. > Red Hat ranks #1 in value among software vendors. > http://www.redhat.com/promo/vendor/ > _______________________________________________ > sssd-devel mailing list > sssd-devel@lists.fedorahosted.org > https://fedorahosted.org/mailman/listinfo/sssd-devel
From 1776ba3f543c8c7ce1adfc77555d5ff8c48a37e6 Mon Sep 17 00:00:00 2001 From: Sumit Bose <sb...@redhat.com> Date: Wed, 21 Apr 2010 10:46:01 +0200 Subject: [PATCH 1/2] Unset authentication tokens if password change fails --- src/sss_client/pam_sss.c | 79 ++++++++++++++++++++++++++++++---------------- 1 files changed, 52 insertions(+), 27 deletions(-) diff --git a/src/sss_client/pam_sss.c b/src/sss_client/pam_sss.c index 4208faa..77dec19 100644 --- a/src/sss_client/pam_sss.c +++ b/src/sss_client/pam_sss.c @@ -1201,37 +1201,62 @@ static int pam_sss(enum sss_cli_command task, pam_handle_t *pamh, ret = send_and_receive(pamh, &pi, task); -/* We allow sssd to send the return code PAM_NEW_AUTHTOK_REQD during - * authentication, see sss_cli.h for details */ - if (ret == PAM_NEW_AUTHTOK_REQD && task == SSS_PAM_AUTHENTICATE) { - D(("Authtoken expired, trying to change it")); - - exp_data = malloc(sizeof(int)); - if (exp_data == NULL) { - D(("malloc failed.")); - return PAM_BUF_ERR; - } - *exp_data = 1; - ret = pam_set_data(pamh, PWEXP_FLAG, exp_data, free_exp_data); - if (ret != PAM_SUCCESS) { - D(("pam_set_data failed.")); - return ret; - } - - return PAM_SUCCESS; - } + switch (task) { + case SSS_PAM_AUTHENTICATE: + /* We allow sssd to send the return code PAM_NEW_AUTHTOK_REQD during + * authentication, see sss_cli.h for details */ + if (ret == PAM_NEW_AUTHTOK_REQD) { + D(("Authtoken expired, trying to change it")); + + exp_data = malloc(sizeof(int)); + if (exp_data == NULL) { + D(("malloc failed.")); + return PAM_BUF_ERR; + } + *exp_data = 1; + ret = pam_set_data(pamh, PWEXP_FLAG, exp_data, free_exp_data); + if (ret != PAM_SUCCESS) { + D(("pam_set_data failed.")); + return ret; + } - if (ret == PAM_SUCCESS && task == SSS_PAM_ACCT_MGMT && - pam_get_data(pamh, PWEXP_FLAG, (const void **) &exp_data) == + return PAM_SUCCESS; + } + break; + case SSS_PAM_ACCT_MGMT: + if (ret == PAM_SUCCESS && + pam_get_data(pamh, PWEXP_FLAG, (const void **) &exp_data) == PAM_SUCCESS) { - ret = do_pam_conversation(pamh, PAM_TEXT_INFO, - _("Password expired. Change your password now."), NULL, NULL); - if (ret != PAM_SUCCESS) { - D(("do_pam_conversation failed.")); - } - return PAM_NEW_AUTHTOK_REQD; + ret = do_pam_conversation(pamh, PAM_TEXT_INFO, + _("Password expired. Change your password now."), + NULL, NULL); + if (ret != PAM_SUCCESS) { + D(("do_pam_conversation failed.")); + } + return PAM_NEW_AUTHTOK_REQD; + } + break; + case SSS_PAM_CHAUTHTOK: + if (ret != PAM_SUCCESS && ret != PAM_USER_UNKNOWN) { + ret = pam_set_item(pamh, PAM_AUTHTOK, NULL); + if (ret != PAM_SUCCESS) { + D(("Failed to unset PAM_AUTHTOK [%s]", + pam_strerror(pamh,ret))); + } + ret = pam_set_item(pamh, PAM_OLDAUTHTOK, NULL); + if (ret != PAM_SUCCESS) { + D(("Failed to unset PAM_OLDAUTHTOK [%s]", + pam_strerror(pamh,ret))); + } + } + break; + default: + /* nothing to do */ + break; } + + overwrite_and_free_authtoks(&pi); return ret; -- 1.6.6.1
From 1c8248e1ea3575771841a4546db4322d44e61427 Mon Sep 17 00:00:00 2001 From: Sumit Bose <sb...@redhat.com> Date: Wed, 21 Apr 2010 14:42:34 +0200 Subject: [PATCH 2/2] Display a message if a password reset by root fails --- src/man/pam_sss.8.xml | 22 ++++ src/providers/krb5/krb5_auth.c | 7 ++ src/providers/ldap/ldap_auth.c | 8 ++ src/sss_client/pam_sss.c | 206 ++++++++++++++++++++++++++++++++++++++-- 4 files changed, 235 insertions(+), 8 deletions(-) diff --git a/src/man/pam_sss.8.xml b/src/man/pam_sss.8.xml index f6ac9f4..9a68196 100644 --- a/src/man/pam_sss.8.xml +++ b/src/man/pam_sss.8.xml @@ -85,6 +85,28 @@ </para> </refsect1> + <refsect1 id='files'> + <title>FILES</title> + <para>If a password reset by root fails, because the corresponding SSSD + provider does not support password resets, an individual message can be + displayed. This message can e.g. contain instructions about how to reset + a password.</para> + + <para>The message is read from the file + <filename>pam_sss_pw_reset_message.LOC</filename> where LOC stands for a + locale string returned by <citerefentry> + <refentrytitle>setlocale</refentrytitle><manvolnum>3</manvolnum> + </citerefentry>. If there is no matching file the content of + <filename>pam_sss_pw_reset_message.txt</filename> is displayed. Root + must be the owner of the files and only root may have read and write + permissions while all other users must have only read + permisssions.</para> + + <para>These files are searched in the directory + <filename>/etc/sssd/customize/DOMAIN_NAME/</filename>. If no matching + file is present a generic message is displayed.</para> + </refsect1> + <refsect1 id='see_also'> <title>SEE ALSO</title> <para> diff --git a/src/providers/krb5/krb5_auth.c b/src/providers/krb5/krb5_auth.c index 6a57fe5..e1aaebf 100644 --- a/src/providers/krb5/krb5_auth.c +++ b/src/providers/krb5/krb5_auth.c @@ -635,7 +635,14 @@ void krb5_pam_handler(struct be_req *be_req) switch (pd->cmd) { case SSS_PAM_AUTHENTICATE: case SSS_PAM_CHAUTHTOK: + break; case SSS_PAM_CHAUTHTOK_PRELIM: + if (pd->priv == 1 && pd->authtok_size == 0) { + DEBUG(4, ("Password reset by root is not supported.\n")); + pam_status = PAM_PERM_DENIED; + dp_err = DP_ERR_OK; + goto done; + } break; case SSS_PAM_ACCT_MGMT: case SSS_PAM_SETCRED: diff --git a/src/providers/ldap/ldap_auth.c b/src/providers/ldap/ldap_auth.c index e0935da..95931ac 100644 --- a/src/providers/ldap/ldap_auth.c +++ b/src/providers/ldap/ldap_auth.c @@ -636,6 +636,14 @@ void sdap_pam_chpass_handler(struct be_req *breq) goto done; } + if (pd->priv == 1 && pd->cmd == SSS_PAM_CHAUTHTOK_PRELIM && + pd->authtok_size == 0) { + DEBUG(4, ("Password reset by root is not supported.\n")); + pd->pam_status = PAM_PERM_DENIED; + dp_err = DP_ERR_OK; + goto done; + } + DEBUG(2, ("starting password change request for user [%s].\n", pd->user)); pd->pam_status = PAM_SYSTEM_ERR; diff --git a/src/sss_client/pam_sss.c b/src/sss_client/pam_sss.c index 77dec19..3f88d68 100644 --- a/src/sss_client/pam_sss.c +++ b/src/sss_client/pam_sss.c @@ -35,6 +35,10 @@ #include <stdio.h> #include <syslog.h> #include <time.h> +#include <sys/stat.h> +#include <fcntl.h> +#include <errno.h> +#include <locale.h> #include <security/pam_modules.h> #include <security/pam_ext.h> @@ -53,6 +57,9 @@ #define PWEXP_FLAG "pam_sss:password_expired_flag" +#define PW_RESET_MSG_FILENAME_TEMPLATE SSSD_CONF_DIR"/customize/%s/pam_sss_pw_reset_message.%s" +#define PW_RESET_MSG_MAX_SIZE 4096 + struct pam_items { const char* pam_service; const char* pam_user; @@ -74,6 +81,7 @@ struct pam_items { size_t pam_newauthtok_size; pid_t cli_pid; const char *login_name; + char *domain_name; }; #define DEBUG_MGS_LEN 1024 @@ -183,7 +191,7 @@ static size_t add_string_item(enum pam_item_type type, const char *str, return rp; } -static void overwrite_and_free_authtoks(struct pam_items *pi) +static void overwrite_and_free_pam_items(struct pam_items *pi) { if (pi->pam_authtok != NULL) { _pam_overwrite_n((void *)pi->pam_authtok, pi->pam_authtok_size); @@ -196,6 +204,8 @@ static void overwrite_and_free_authtoks(struct pam_items *pi) free((void *)pi->pam_newauthtok); pi->pam_newauthtok = NULL; } + + free(pi->domain_name); } static int pack_message_v3(struct pam_items *pi, size_t *size, @@ -389,6 +399,162 @@ static int do_pam_conversation(pam_handle_t *pamh, const int msg_style, return PAM_SUCCESS; } +static errno_t display_pw_reset_message(pam_handle_t *pamh, + const char *domain_name, + const char *suffix) +{ + int ret; + struct stat stat_buf; + char *msg_buf = NULL; + int fd = -1; + size_t size; + size_t total_len; + char *filename = NULL; + + if (strchr(suffix, '/') != NULL || strchr(domain_name, '/') != NULL) { + D(("Suffix [%s] or domain name [%s] contain illegal character.", suffix, + domain_name)); + return EINVAL; + } + + size = sizeof(PW_RESET_MSG_FILENAME_TEMPLATE) + strlen(domain_name) + + strlen(suffix); + filename = malloc(size); + if (filename == NULL) { + D(("malloc failed.")); + ret = ENOMEM; + goto done; + } + ret = snprintf(filename, size, PW_RESET_MSG_FILENAME_TEMPLATE, domain_name, + suffix); + if (ret < 0 || ret >= size) { + D(("snprintf failed.")); + ret = EFAULT; + goto done; + } + + fd = open(filename, O_RDONLY); + if (fd == -1) { + ret = errno; + D(("open failed [%d][%s].\n", ret, strerror(ret))); + goto done; + } + + ret = fstat(fd, &stat_buf); + if (ret == -1) { + ret = errno; + D(("fstat failed [%d][%s].", ret, strerror(ret))); + goto done; + } + + if (!S_ISREG(stat_buf.st_mode)) { + logger(pamh, LOG_ERR, + "Password reset message file is not a regular file."); + ret = EINVAL; + goto done; + } + + if (stat_buf.st_uid != 0 || stat_buf.st_gid != 0 || + (stat_buf.st_mode & ~S_IFMT) != 0644) { + logger(pamh, LOG_ERR,"Permission error, " + "file [%s] must be owned by root with permissions 0644.", + filename); + ret = EPERM; + goto done; + } + + if (stat_buf.st_size > PW_RESET_MSG_MAX_SIZE) { + logger(pamh, LOG_ERR, "Password reset message file is too large."); + ret = EFBIG; + goto done; + } + + msg_buf = malloc(stat_buf.st_size + 1); + if (msg_buf == NULL) { + D(("malloc failed.")); + ret = ENOMEM; + goto done; + } + + + total_len = 0; + while (total_len < stat_buf.st_size) { + ret = read(fd, msg_buf + total_len, stat_buf.st_size - total_len); + if (ret == -1) { + if (errno == EINTR) continue; + ret = errno; + D(("read failed [%d][%s].", ret, strerror(ret))); + goto done; + } + total_len += ret; + } + + ret = close(fd); + fd = -1; + if (ret == -1) { + ret = errno; + D(("close failed [%d][%s].", ret, strerror(ret))); + } + + if (total_len != stat_buf.st_size) { + D(("read fewer bytes [%d] than expected [%d].", total_len, + stat_buf.st_size)); + ret = EIO; + goto done; + } + + msg_buf[stat_buf.st_size] = '\0'; + + ret = do_pam_conversation(pamh, PAM_TEXT_INFO, msg_buf, NULL, NULL); + if (ret != PAM_SUCCESS) { + D(("do_pam_conversation failed.")); + } + +done: + if (fd != -1) { + close(fd); + } + free(msg_buf); + free(filename); + + return ret; +} + +static errno_t select_pw_reset_message(pam_handle_t *pamh, struct pam_items *pi) +{ + int ret; + char *locale; + const char *domain_name; + + domain_name = pi->domain_name; + if (domain_name == NULL || *domain_name == '\0') { + D(("Domain name is unknown.")); + return EINVAL; + } + + locale = setlocale(LC_MESSAGES, NULL); + + ret = -1; + if (locale != NULL) { + ret = display_pw_reset_message(pamh, domain_name, locale); + } + + if (ret != 0) { + ret = display_pw_reset_message(pamh, domain_name, "txt"); + } + + if (ret != 0) { + ret = do_pam_conversation(pamh, PAM_TEXT_INFO, + _("Password reset by root is not supported."), + NULL, NULL); + if (ret != PAM_SUCCESS) { + D(("do_pam_conversation failed.")); + } + } + + return ret; +} + static int user_info_offline_auth(pam_handle_t *pamh, size_t buflen, uint8_t *buf) { @@ -679,7 +845,8 @@ static int eval_user_info_response(pam_handle_t *pamh, size_t buflen, return ret; } -static int eval_response(pam_handle_t *pamh, size_t buflen, uint8_t *buf) +static int eval_response(pam_handle_t *pamh, size_t buflen, uint8_t *buf, + struct pam_items *pi) { int ret; size_t p=0; @@ -727,7 +894,15 @@ static int eval_response(pam_handle_t *pamh, size_t buflen, uint8_t *buf) logger(pamh, LOG_INFO, "system info: [%s]", &buf[p]); break; case SSS_PAM_DOMAIN_NAME: + if (buf[p + (len -1)] != '\0') { + D(("domain name does not end with \\0.")); + break; + } D(("domain name: [%s]", &buf[p])); + pi->domain_name = strdup((char *) &buf[p]); + if (pi->domain_name == NULL) { + D(("strdup failed")); + } break; case SSS_ENV_ITEM: case SSS_PAM_ENV_ITEM: @@ -835,6 +1010,8 @@ static int get_pam_items(pam_handle_t *pamh, struct pam_items *pi) pi->login_name = pam_modutil_getlogin(pamh); if (pi->login_name == NULL) pi->login_name=""; + pi->domain_name = NULL; + return PAM_SUCCESS; } @@ -896,7 +1073,7 @@ static int send_and_receive(pam_handle_t *pamh, struct pam_items *pi, } pam_status = ((int32_t *)repbuf)[0]; - ret = eval_response(pamh, replen, repbuf); + ret = eval_response(pamh, replen, repbuf, pi); if (ret != PAM_SUCCESS) { D(("eval_response failed.")); pam_status = ret; @@ -1211,16 +1388,18 @@ static int pam_sss(enum sss_cli_command task, pam_handle_t *pamh, exp_data = malloc(sizeof(int)); if (exp_data == NULL) { D(("malloc failed.")); - return PAM_BUF_ERR; + ret = PAM_BUF_ERR; + break; } *exp_data = 1; ret = pam_set_data(pamh, PWEXP_FLAG, exp_data, free_exp_data); if (ret != PAM_SUCCESS) { D(("pam_set_data failed.")); - return ret; + ret = ret; + break; } - return PAM_SUCCESS; + ret = PAM_SUCCESS; } break; case SSS_PAM_ACCT_MGMT: @@ -1233,7 +1412,7 @@ static int pam_sss(enum sss_cli_command task, pam_handle_t *pamh, if (ret != PAM_SUCCESS) { D(("do_pam_conversation failed.")); } - return PAM_NEW_AUTHTOK_REQD; + ret = PAM_NEW_AUTHTOK_REQD; } break; case SSS_PAM_CHAUTHTOK: @@ -1250,6 +1429,17 @@ static int pam_sss(enum sss_cli_command task, pam_handle_t *pamh, } } break; + case SSS_PAM_CHAUTHTOK_PRELIM: + if (ret == PAM_PERM_DENIED && pi.pam_authtok_size == 0 && + getuid() == 0 && + pam_get_data(pamh, PWEXP_FLAG, (const void **) &exp_data) != + PAM_SUCCESS) { + + ret = select_pw_reset_message(pamh, &pi); + if (ret != 0) { + } + ret = PAM_PERM_DENIED; + } default: /* nothing to do */ break; @@ -1257,7 +1447,7 @@ static int pam_sss(enum sss_cli_command task, pam_handle_t *pamh, - overwrite_and_free_authtoks(&pi); + overwrite_and_free_pam_items(&pi); return ret; } -- 1.6.6.1
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel