[SSSD] [PATCHES] PAM: authenticate agains cache

Jakub Hrozek jhrozek at redhat.com
Wed Jun 24 08:55:05 UTC 2015


On Tue, Jun 23, 2015 at 10:55:41AM +0200, Pavel Reichl wrote:
> On 06/23/2015 10:34 AM, Jakub Hrozek wrote:
> >On Fri, Jun 05, 2015 at 07:01:30PM +0200, Pavel Reichl wrote:
> >>On 05/20/2015 05:16 PM, Pavel Reichl wrote:
> >>>Hello,
> >>>
> >>>please see first version of these patches. I'm currently working on unit
> >>>test for the second patch which will be part of the second revision of the
> >>>patch set.
> >>>
> >>>Thanks!
> >>>
> >>>
> >>>_______________________________________________
> >>>sssd-devel mailing list
> >>>sssd-devel at lists.fedorahosted.org
> >>>https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> >>Jakub asked me off list to move resetting of
> >>SYSDB_LAST_ONLINE_AUTH_WITH_CURRENT_TOKEN attribute from ldap code to pam
> >>code. Please see updated patch set.
> >>
> >>Thanks!
> >Can you rebase the patches, please? I'll take a look at them..
> >_______________________________________________
> >sssd-devel mailing list
> >sssd-devel at lists.fedorahosted.org
> >https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> Sure. Rebased patches attached.

Hi,

We should extend the PAM responder unit test together with these
patches so that the new functionality is covered.

see some comments inline.

> From 754b855b2c2877cd2121cc0644a7450a52a5b0d4 Mon Sep 17 00:00:00 2001
> From: Pavel Reichl <preichl at redhat.com>
> Date: Mon, 18 May 2015 09:59:38 -0400
> Subject: [PATCH 1/2] sysdb: new attribute lastOnlineAuthWithCurrentToken
> 
> Introduce new user attribute lastOnlineAuthWithCurrentToken. This attribute
> behaves similarly to lastOnlineAuth but is set to NULL after password is
> changed.
> 
> This attribute is needed for use-case when cached authentication is used, to
> request online authentication after password is locally changed.
> 
> Resolves:
> https://fedorahosted.org/sssd/ticket/1807
> ---
>  src/db/sysdb.h                 | 14 +++++++
>  src/db/sysdb_ops.c             | 95 ++++++++++++++++++++++++++++++++++++++++++
>  src/responder/pam/pamsrv_cmd.c | 18 ++++++++
>  src/tests/sysdb-tests.c        | 76 +++++++++++++++++++++++++++++++++
>  4 files changed, 203 insertions(+)
> 
> diff --git a/src/db/sysdb.h b/src/db/sysdb.h
> index 4dc382f6fb3f1b6bd084312463edf1b6189ebba9..3e8ddb7b169abd84d8cbcd2f58c347622860ce98 100644
> --- a/src/db/sysdb.h
> +++ b/src/db/sysdb.h
> @@ -87,6 +87,7 @@
>  #define SYSDB_LAST_ONLINE_AUTH "lastOnlineAuth"
>  #define SYSDB_LAST_FAILED_LOGIN "lastFailedLogin"
>  #define SYSDB_FAILED_LOGIN_ATTEMPTS "failedLoginAttempts"
> +#define SYSDB_LAST_ONLINE_AUTH_WITH_CURR_TOKEN "lastOnlineAuthWithCurrentToken"
>  
>  #define SYSDB_LAST_UPDATE "lastUpdate"
>  #define SYSDB_CACHE_EXPIRE "dataExpireTimestamp"
> @@ -904,6 +905,19 @@ int sysdb_cache_password_ex(struct sss_domain_info *domain,
>                              enum sss_authtok_type authtok_type,
>                              size_t second_factor_size);
>  
> +errno_t
> +sysdb_set_last_online_auth_with_curr_token(struct sss_domain_info *domain,
> +                                           const char *username,
> +                                           uint64_t value);

Since set_last_login() sets the attribute directly, can we make this
function static?

btw I think that the diff from src/responder/pam/pamsrv_cmd.c should be
a separate patch so that we have one patch that adds the sysdb API and
one that uses the new attribute in PAM responder.

> +errno_t
> +sysdb_null_last_online_auth_with_curr_token(struct sss_domain_info *domain,
> +                                            const char *username);
> +
> +errno_t
> +sysdb_get_last_online_auth_with_curr_token(struct sss_domain_info *domain,
> +                                           const char *name,
> +                                           uint64_t *_value);
> +
>  errno_t check_failed_login_attempts(struct confdb_ctx *cdb,
>                                      struct ldb_message *ldb_msg,
>                                      uint32_t *failed_login_attempts,

[...]

> From ed449b272beafecd217cea86f514766ad35fd5b9 Mon Sep 17 00:00:00 2001
> From: Pavel Reichl <preichl at redhat.com>
> Date: Thu, 16 Apr 2015 03:41:58 -0400
> Subject: [PATCH 2/2] PAM: authenticate agains cache
> 
> Enable authenticating users from cache even when SSSD is in online mode.
> 
> Introduce new option `cached_auth_timeout`.
> 
> Resolves:
> https://fedorahosted.org/sssd/ticket/1807
> ---
>  src/confdb/confdb.c                  |  49 +++++++++++++
>  src/confdb/confdb.h                  |   2 +
>  src/config/SSSDConfig/__init__.py.in |   1 +
>  src/config/SSSDConfigTest.py         |   6 +-
>  src/config/etc/sssd.api.conf         |   1 +
>  src/db/sysdb_ops.c                   |   1 -
>  src/man/sssd.conf.5.xml              |  14 ++++
>  src/responder/pam/pamsrv.h           |   3 +
>  src/responder/pam/pamsrv_cmd.c       | 134 +++++++++++++++++++++++++++++++++--
>  9 files changed, 201 insertions(+), 10 deletions(-)

[...]

> diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in
> index f2d9bf0190745d4a5296c56212e5ff97681db59c..79a831dbec31e96f663f5472c40e6aedc7259f11 100644
> --- a/src/config/SSSDConfig/__init__.py.in
> +++ b/src/config/SSSDConfig/__init__.py.in
> @@ -149,6 +149,7 @@ option_strings = {
>      'subdomain_enumerate' : _('Control enumeration of trusted domains'),
>      'subdomain_refresh_interval' : _('How often should subdomains list be refreshed'),
>      'subdomain_inherit' : _('List of options that should be inherited into a subdomain'),
> +    'cached_auth_timeout' : _('How long can be cached credentials used for cached authentication'),

I think this should better read 'How long can cached credentials be
used..' but feel free to ping some native speaker.

>  
>      # [provider/ipa]
>      'ipa_domain' : _('IPA domain'),

[..]

> --- a/src/man/sssd.conf.5.xml
> +++ b/src/man/sssd.conf.5.xml
> @@ -988,6 +988,20 @@ pam_account_expired_message = Account expired, please call help desk.
>                          </para>
>                      </listitem>
>                  </varlistentry>
> +                <varlistentry>
> +                    <term>cached_auth_timeout (int)</term>
> +                    <listitem>
> +                        <para>
> +                            Specifies time in seconds since last successful
> +                            online authentication for which user will be
> +                            authenticated using cached credentials while
> +                            SSSD is in the online mode.
> +                        </para>
> +                        <para>
> +                            Default: 0

The manpage should specify what does the default mean.

> +                        </para>
> +                    </listitem>
> +                </varlistentry>
>  
>              </variablelist>
>          </refsect2>
> diff --git a/src/responder/pam/pamsrv.h b/src/responder/pam/pamsrv.h
> index 066f35a428a9af81d665309b4ab5a80cf69561ba..0278006467a41b28a1d47a8777074f265c5c478e 100644
> --- a/src/responder/pam/pamsrv.h
> +++ b/src/responder/pam/pamsrv.h
> @@ -60,6 +60,9 @@ struct pam_auth_req {
>      bool is_uid_trusted;
>      bool check_provider;
>      void *data;
> +    bool use_cached_auth;
> +    /* whether cached authentication was tried and failed */
> +    bool cached_auth_failed;
>  
>      struct pam_auth_dp_req *dpreq_spy;
>  };
> diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c
> index ae571fc969bfaf8c9ec90023ca1f4f38d31211e2..833fd096f64a7db1ad580f72cbd5022f77267b33 100644
> --- a/src/responder/pam/pamsrv_cmd.c
> +++ b/src/responder/pam/pamsrv_cmd.c
> @@ -565,7 +565,7 @@ static errno_t get_password_for_cache_auth(struct sss_auth_token *authtok,
>  
>  static int pam_forwarder(struct cli_ctx *cctx, int pam_cmd);
>  static void pam_handle_cached_login(struct pam_auth_req *preq, int ret,
> -                                    time_t expire_date, time_t delayed_until);
> +                                    time_t expire_date, time_t delayed_until, bool cached_auth);
>  
>  static void pam_reply(struct pam_auth_req *preq)
>  {
> @@ -603,14 +603,21 @@ static void pam_reply(struct pam_auth_req *preq)
>      DEBUG(SSSDBG_FUNC_DATA,
>            "pam_reply called with result [%d]: %s.\n",
>            pd->pam_status, pam_strerror(NULL, pd->pam_status));
> +    if (pd->pam_status == PAM_AUTHINFO_UNAVAIL || preq->use_cached_auth) {
>  
> -    if (pd->pam_status == PAM_AUTHINFO_UNAVAIL) {
>          switch(pd->cmd) {
>          case SSS_PAM_AUTHENTICATE:
>              if ((preq->domain != NULL) &&
>                  (preq->domain->cache_credentials == true) &&
>                  (pd->offline_auth == false)) {
>                  const char *password = NULL;
> +                bool use_cached_auth;
> +
> +                /* backup value of preq->use_cached_auth*/
> +                use_cached_auth = preq->use_cached_auth;
> +                /* set to false to avoid entering this branch when pam_reply()
> +                 * is recursively called from pam_handle_cached_login() */
> +                preq->use_cached_auth = false;
>  
>                  /* do auth with offline credentials */
>                  pd->offline_auth = true;
> @@ -634,7 +641,8 @@ static void pam_reply(struct pam_auth_req *preq)
>                                         pctx->rctx->cdb, false,
>                                         &exp_date, &delay_until);
>  
> -                pam_handle_cached_login(preq, ret, exp_date, delay_until);
> +                pam_handle_cached_login(preq, ret, exp_date, delay_until,
> +                                        use_cached_auth);
>                  return;
>              }
>              break;
> @@ -802,8 +810,11 @@ done:
>      sss_cmd_done(cctx, preq);
>  }
>  
> +static void pam_dom_forwarder(struct pam_auth_req *preq);
> +
>  static void pam_handle_cached_login(struct pam_auth_req *preq, int ret,
> -                                    time_t expire_date, time_t delayed_until)
> +                                    time_t expire_date, time_t delayed_until,
> +                                    bool use_cached_auth)
>  {
>      uint32_t resp_type;
>      size_t resp_len;
> @@ -852,6 +863,18 @@ static void pam_handle_cached_login(struct pam_auth_req *preq, int ret,
>                  }
>              }
>              break;
> +        case PAM_AUTH_ERR:
> +            /* Was this attempt to authenticate from cache? */
> +            if (use_cached_auth) {
> +                /* Don't try cached authentication again, try online check. */
> +                DEBUG(SSSDBG_FUNC_DATA,
> +                      "Cached authentication failed for: %s\n",
> +                      preq->pd->user);
> +                preq->cached_auth_failed = true;
> +                pam_dom_forwarder(preq);
> +                return;
> +            }
> +            break;
>          default:
>              DEBUG(SSSDBG_TRACE_LIBS,
>                    "cached login returned: %d\n", preq->pd->pam_status);
> @@ -866,7 +889,6 @@ static void pam_check_user_dp_callback(uint16_t err_maj, uint32_t err_min,
>                                         const char *err_msg, void *ptr);
>  static int pam_check_user_search(struct pam_auth_req *preq);
>  static int pam_check_user_done(struct pam_auth_req *preq, int ret);
> -static void pam_dom_forwarder(struct pam_auth_req *preq);
>  
>  /* TODO: we should probably return some sort of cookie that is set in the
>   * PAM_ENVIRONMENT, so that we can save performing some calls and cache
> @@ -1004,7 +1026,6 @@ static int pam_forwarder(struct cli_ctx *cctx, int pam_cmd)
>                cctx->client_euid);
>      }
>  
> -
>      pd->cmd = pam_cmd;
>      pd->priv = cctx->priv;
>  
> @@ -1420,9 +1441,91 @@ static void pam_check_user_dp_callback(uint16_t err_maj, uint32_t err_min,
>      }
>  }
>  
> +static errno_t pam_is_last_online_login_fresh(struct sss_domain_info *domain,
> +                                              const char* user,
> +                                              struct confdb_ctx *cdb,
> +                                              int cached_auth_timeout,
> +                                              bool *_result)

I think the code would be simpler if this function simple returned bool.
The failure is reported in the function after all. If you prefer to
logically layer retrieving the value and using it, then
maybe pam_can_user_cache_auth could be simpler and return bool straight
away?

> +{
> +    errno_t ret;
> +    bool result;
> +    uint64_t last_login;
> +
> +    ret = sysdb_get_last_online_auth_with_curr_token(domain, user,
> +                                                     &last_login);
> +    if (ret != EOK) {
> +        DEBUG(SSSDBG_MINOR_FAILURE,
> +              "sysdb_get_last_online_auth_with_curr_token failed: %s:[%d]\n",
> +              sss_strerror(ret), ret);
> +        goto done;
> +    }
> +
> +    result = time(NULL) < (last_login + cached_auth_timeout);
> +    ret = EOK;
> +
> +done:
> +    if (ret == EOK) {
> +        *_result = result;
> +    }
> +    return ret;
> +}
> +
> +static bool pam_is_cmd_cachable(int cmd)
> +{
> +    bool is_cachable;
> +
> +    switch(cmd) {
> +    case SSS_PAM_AUTHENTICATE:
> +        is_cachable = true;
> +        break;
> +    default:
> +        is_cachable = false;
> +    }
> +
> +    return is_cachable;
> +}



More information about the sssd-devel mailing list