On Wed, Feb 11, 2015 at 06:07:39PM +0100, Jakub Hrozek wrote: > On Wed, Feb 11, 2015 at 05:50:50PM +0100, Sumit Bose wrote: > > On Wed, Feb 11, 2015 at 04:56:48PM +0100, Pavel Reichl wrote: > > > Hello, > > > > > > please see attached patch. I'm not sure whether using pam_strerror() is > > > the > > > right thing to do. It might be better to use our own string? > > > I'm also not sure about using _(STRING) macro on the output of > > > pam_strerror(). > > > > The _() macro will not work here. You can use it only to enclose literal > > strings. The strings will then be extracted into the *.pot file and > > translators can pick them for translation. In the pam_strerror() case > > libpam has to take care of the translations. > > > > > > > > I attached output of sequence of commands to show differences. > > > > > > 1) This is output without patch being applied. > > > $ su john > > > Password: > > > su: User account has expired > > > > > > ssh -l john `hostname` > > > Connection closed by 192.168.122.166 > > > > > > #not matching key > > > $ ssh -l john `hostname` -i /tmp/local > > > j...@dev.local.test's password: > > > Connection closed by 192.168.122.166 > > > > > > > > > 2) This is output when patch is applied. Please note the duplicity when > > > using su. > > > > The service name is available, so you can add this only if ssh is used. > > > > > $ su john > > > Password: > > > User account has expired > > > su: User account has expired > > > > > > $ ssh -l john `hostname` > > > User account has expired > > > Connection closed by 192.168.122.166 > > > > > > #not matching key > > > $ ssh -l john `hostname` -i /tmp/local > > > j...@dev.local.test's password: > > > User account has expired > > > Connection closed by 192.168.122.166 > > > > > > Thanks for comments. > > > > > From 953f1721996e6c2bf8ee53ea232de2240f168d94 Mon Sep 17 00:00:00 2001 > > > From: Pavel Reichl <prei...@redhat.com> > > > Date: Wed, 11 Feb 2015 19:38:16 -0500 > > > Subject: [PATCH] PAM: do not reject abruptly > > > > > > If account has expired use pam_conversation to pass message. > > > > > > Resolves: > > > https://fedorahosted.org/sssd/ticket/2050 > > > --- > > > src/sss_client/pam_sss.c | 7 +++++++ > > > 1 file changed, 7 insertions(+) > > > > > > diff --git a/src/sss_client/pam_sss.c b/src/sss_client/pam_sss.c > > > index > > > fdf6c9e6da75c9f7eaa7c00d9a5792fbdd97eabc..767b2a839e9f001be52c5ff4c7651b0f06ba4221 > > > 100644 > > > --- a/src/sss_client/pam_sss.c > > > +++ b/src/sss_client/pam_sss.c > > > @@ -1585,6 +1585,13 @@ static int pam_sss(enum sss_cli_command task, > > > pam_handle_t *pamh, > > > D(("do_pam_conversation failed.")); > > > } > > > pam_status = PAM_NEW_AUTHTOK_REQD; > > > + } else if (pam_status == PAM_ACCT_EXPIRED) { > > > + ret = do_pam_conversation(pamh, PAM_TEXT_INFO, > > > + _(pam_strerror(pamh, pam_status)), > > > + NULL, NULL); > > > + if (ret != PAM_SUCCESS) { > > > + D(("do_pam_conversation failed.")); > > > + } > > > > > > I would recommend to not do this in pam_sss directly but send a > > SSS_PAM_USER_INFO response back to pam_sss. This response can e.g. be > > generated in the pam responder if pam_status == PAM_ACCT_EXPIRED and the > > service is sshd. Doing it in the pam responder has the advantage that > > you do not have to duplicate code in the backends. Additionally it is > > more easy configure the behavior. E.g. you can check pam_verbosity and > > only add this message is the level is 2 (1?) or higher. > > > > bye, > > Sumit > > One thing we noticed with Pavel when looking at the issue earlier in the > office was that by default, sshd doesn't display PAM_TEXT_INFO level > messages. And there is one difference between how pam_unix handles > account expiration (they send PAM_ERROR_MSG) and how pam_sss handles it > (PAM_TEXT_INFO), I guess this is also why the original reporter filed > the ticket. > > Pavel's idea was to change the PAM_TEXT_INFO messages to PAM_ERROR_MSG on > higher pam_verbosity level..it seems like a bit of a side-effect magic to > me, but I can't think of any better solution either..
Sorry, ignore this mail :) after one more discussion with Pavel, he confirmed that even TEXT_INFO works fine with sshd after pam_verbosity is increased. So we really just need to move the code to responder.. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
