On Fri, Jun 03, 2016 at 05:56:45PM +0200, Jakub Hrozek wrote:

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?

Not that I'm aware of. So far I was not able to reproduce the error locally not with CI http://sssd-ci.duckdns.org/logs/job/44/49/summary.html . Do you maybe have your pam wrapper patches applied to check for regressions?

...

bye, Sumit

...

From 0a58ab569a7746aab54ec8e38cebce4584f0b145 Mon Sep 17 00:00:00 2001 From: Sumit Bose sbose@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.

Make sense, I fixed it in the new version.

...

•                    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..

Currently not, but I wanted to be on the safe side to avoid issues with changes in future. Additionally it might irritate the occasional reader to dereference a pointer without checking that it is not NULL. So, if you don't mind I would prefer to keep it.

bye, Sumit

...

•                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 sbose@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