On Wed, Jun 01, 2016 at 06:31:29PM +0200, Sumit Bose wrote: > Hi, > > that attached two patches would allow to use the Smartcard support in > gdm with SSSD. To use it you should replace pam_pkcs11 in > /etc/pam.d/smartcard-auth in the auth section by > > auth sufficient pam_sss.so allow_missing_name > > and drop the password section completely. > > To enable the Smartcard support in gdm the easiest way is to use > dconf-editor: > > DCONF_PROFILE=gdm dconf-editor > > In the org/gnome/login-screen section you can switch the Smartcard > support on and off. Additionally you might want to tune the removal > action in org/gnome/settings-daemon/peripherals/smartcard . > > If now a Smartcard is inserted gdm should register it, call > /etc/pam.d/gdm-smartcard which calls /etc/pam.d/smartcard-auth without a > user name. With the new option from the first patch pam_sss will accept > this and send it to the pam responder. The pam responder can handle this > if Smartcard authentication is enabled, tries to read the certificate > from the Smartcard, tries to find and matching user and if successful, > returns the user name to pam_sss which puts it on the PAM stack and > continues with the authentication. > > It would be nice if someone can review the code even without testing the > functionality. In this case I will ask someone else with access to > Smartcards and reader to do some functional testing. > > I think these patches are candidates for the pam wrapper based tests > Jakub has for review on the list. I'll start reviewing those and add > tests when they are in master.
The code looks good to me with some minor nitpicks (see inline) but at least for me, the tests are failing: [ RUN ] test_pam_offline_chauthtok_prelim [ ERROR ] --- 0x2 != 0x3 [ LINE ] --- /home/remote/jhrozek/devel/sssd/src/tests/cmocka/test_pam_srv.c:641: error: Failure! [ FAILED ] test_pam_offline_chauthtok_prelim [ RUN ] test_pam_offline_chauthtok [ ERROR ] --- 0x2 != 0x3 [ LINE ] --- /home/remote/jhrozek/devel/sssd/src/tests/cmocka/test_pam_srv.c:641: error: Failure! [ FAILED ] test_pam_offline_chauthtok Do I need some other patches applied as well? > > bye, > Sumit > From 0a58ab569a7746aab54ec8e38cebce4584f0b145 Mon Sep 17 00:00:00 2001 > From: Sumit Bose <sb...@redhat.com> > Date: Mon, 14 Mar 2016 17:27:01 +0100 > Subject: [PATCH 1/2] PAM: add pam_sss option allow_missing_name > > With this option SSSD can be used with the gdm Smartcard feature. > > Resolves https://fedorahosted.org/sssd/ticket/2941 > --- > src/man/pam_sss.8.xml | 27 +++++++++++++++++++++++++++ > src/sss_client/pam_sss.c | 40 ++++++++++++++++++++++++++++++++++++---- > 2 files changed, 63 insertions(+), 4 deletions(-) > > diff --git a/src/man/pam_sss.8.xml b/src/man/pam_sss.8.xml > index > 7794d3acfdfdbde491a3e1ada44481b73588e41f..5dc08f8a5a3dee9d3bf8979594ce9c0b16bb8bbf > 100644 > --- a/src/man/pam_sss.8.xml > +++ b/src/man/pam_sss.8.xml > @@ -46,6 +46,9 @@ > <arg choice='opt'> > <replaceable>domains=X</replaceable> > </arg> > + <arg choice='opt'> > + <replaceable>allow_missing_name</replaceable> > + </arg> > </cmdsynopsis> > </refsynopsisdiv> > > @@ -157,6 +160,30 @@ > </para> > </listitem> > </varlistentry> > + <varlistentry> > + <term> > + <option>allow_missing_name</option> > + </term> > + <listitem> > + <para> > + The main purpose of this option is to let SSSD > determine > + the user name based on additional information, e.g. > the > + certificate from a Smartcard. > + </para> > + <para> I would prefer to capitalize PAM instead of pam to be consistent with the rest of the manpage. > + The current use case are login managers which can > + monitor a Smartcard reader for card events. In case a > + Smartcard is inserted the login manager will call a > pam > + stack which includes a line like > + <programlisting> > + auth sufficient pam_sss.so allow_missing_name > + </programlisting> > + In this case SSSD will try to determine the user name > + based on the content of the Smartcard, returns it to > + pam_sss which will finally put it on the pam stack. > + </para> > + </listitem> > + </varlistentry> > </variablelist> > </refsect1> > > diff --git a/src/sss_client/pam_sss.c b/src/sss_client/pam_sss.c > index > 5edbe96609b1599a0fab5d0e5b0b06cf43b9c27a..25140c6d78583885d0c1ca202b2d76055f9923f0 > 100644 > --- a/src/sss_client/pam_sss.c > +++ b/src/sss_client/pam_sss.c > @@ -53,6 +53,7 @@ > #define FLAGS_IGNORE_UNKNOWN_USER (1 << 3) > #define FLAGS_IGNORE_AUTHINFO_UNAVAIL (1 << 4) > #define FLAGS_USE_2FA (1 << 5) > +#define FLAGS_ALLOW_MISSING_NAME (1 << 6) > > #define PWEXP_FLAG "pam_sss:password_expired_flag" > #define FD_DESTRUCTOR "pam_sss:fd_destructor" > @@ -977,6 +978,27 @@ static int eval_response(pam_handle_t *pamh, size_t > buflen, uint8_t *buf, > break; > } > > + if (pi->pam_user == NULL || *(pi->pam_user) == '\0') { Can the first condition ever be true? We set pam_user to "" forcibly.. > + ret = pam_set_item(pamh, PAM_USER, pi->cert_user); > + if (ret != PAM_SUCCESS) { > + D(("Failed to set PAM_USER during " > + "Smartcard authentication [%s]", > + pam_strerror(pamh,ret))); > + break; > + } > + > + ret = pam_get_item(pamh, PAM_USER, > + (const void **) &(pi->pam_user)); > + if (ret != PAM_SUCCESS) { > + D(("Failed to get PAM_USER during " > + "Smartcard authentication [%s]", > + pam_strerror(pamh,ret))); > + break; > + } > + > + pi->pam_user_size=strlen(pi->pam_user)+1; > + } > + > offset = strlen(pi->cert_user) + 1; > if (offset >= len) { > From 1335e1fdb21e55b66d32746dabdf86d4692fc5de Mon Sep 17 00:00:00 2001 > From: Sumit Bose <sb...@redhat.com> > Date: Thu, 17 Mar 2016 17:20:52 +0100 > Subject: [PATCH 2/2] p11: add PKCS11_LOGIN_TOKEN_NAME environment variable ACK _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
