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.. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
