[SSSD] [PATCH] PAM: Make p11_child timeout configurable

Pavel Reichl preichl at redhat.com
Sun Sep 20 14:58:55 UTC 2015


Please see nitpicks inline.

On 09/18/2015 05:07 PM, Michal Židek wrote:
> 0001-PAM-Make-p11_child-timeout-configurable.patch
>
>
>  From 1b03ae46d3ae5812c0e414d3f99abadfdabf672e Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Michal=20=C5=BDidek?=<mzidek at redhat.com>
> Date: Mon, 7 Sep 2015 15:19:53 +0200
> Subject: [PATCH 1/2] PAM: Make p11_child timeout configurable
>
> Ticket:
> https://fedorahosted.org/sssd/ticket/2773
> ---
>   Makefile.am                          |  1 -
>   src/confdb/confdb.h                  |  1 +
>   src/config/SSSDConfig/__init__.py.in |  1 +
>   src/config/etc/sssd.api.conf         |  1 +
>   src/man/sssd.conf.5.xml              | 12 +++++++
>   src/responder/pam/pamsrv_cmd.c       | 65 ++++++++++++++++++++++--------------
>   6 files changed, 55 insertions(+), 26 deletions(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index 3eaf578..777a91c 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -1895,7 +1895,6 @@ pam_srv_tests_SOURCES = \
>   pam_srv_tests_CFLAGS = \
>       -U SSSD_LIBEXEC_PATH -DSSSD_LIBEXEC_PATH=\"$(abs_builddir)\" \
>       $(AM_CFLAGS) \
> -    -DSSS_P11_CHILD_TIMEOUT=30 \
>       $(NULL)
>   pam_srv_tests_LDFLAGS = \
>       -Wl,-wrap,sss_packet_get_body \
> diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
> index 427c309..96b2444 100644
> --- a/src/confdb/confdb.h
> +++ b/src/confdb/confdb.h
> @@ -118,6 +118,7 @@
>   #define CONFDB_PAM_ACCOUNT_EXPIRED_MESSAGE "pam_account_expired_message"
>   #define CONFDB_PAM_CERT_AUTH "pam_cert_auth"
>   #define CONFDB_PAM_CERT_DB_PATH "pam_cert_db_path"
> +#define CONFDB_PAM_P11_CHILD_TIMEOUT "p11_child_timeout"
>
>   /* SUDO */
>   #define CONFDB_SUDO_CONF_ENTRY "config/sudo"
> diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in
> index cb9c368..038de16 100644
> --- a/src/config/SSSDConfig/__init__.py.in
> +++ b/src/config/SSSDConfig/__init__.py.in
> @@ -89,6 +89,7 @@ option_strings = {
>       'pam_trusted_users' : _('List of trusted uids or user\'s name'),
>       'pam_public_domains' : _('List of domains accessible even for untrusted users.'),
>       'pam_account_expired_message' : _('Message printed when user account is expired.'),
> +    'p11_child_timeout' : _('How many seconds will pam_sss wait for p11_child to finish'),

Could you please add '.' at the end of the sentence?

>
> +static errno_t check_cert(struct cli_ctx *cctx,
> +                          struct pam_ctx *pctx,
> +                          struct tevent_req *req,
> +                          struct pam_auth_req *preq,
> +                          struct pam_data *pd)
> +{
> +    int p11_child_timeout;
> +    const int P11_CHILD_TIMEOUT_DEFAULT = 10;
> +    errno_t ret;
> +
> +    ret = confdb_get_int(pctx->rctx->cdb, CONFDB_PAM_CONF_ENTRY,
> +                         CONFDB_PAM_P11_CHILD_TIMEOUT,
> +                         P11_CHILD_TIMEOUT_DEFAULT,
> +                         &p11_child_timeout);
> +    if (ret != EOK) {
> +        DEBUG(SSSDBG_CRIT_FAILURE,
> +              "Failed to read p11_child_timeout from confdb: [%d]: %s\n",
> +              ret, sss_strerror(ret));
> +        return ret;
> +    }
> +
> +    req = pam_check_cert_send(cctx, cctx->ev, pctx->p11_child_debug_fd,
> +                              pctx->nss_db, p11_child_timeout, pd);
> +    if (req == NULL) {
> +        DEBUG(SSSDBG_OP_FAILURE, "pam_check_cert_send failed.\n");
> +        return ENOMEM;
> +    } else {
> +        tevent_req_set_callback(req, pam_forwarder_cert_cb, preq);
> +        return EAGAIN;

Could you move the code outside of the else branch?

> +    }
> +
> +    return EOK;

Please remove this unreachable code.



More information about the sssd-devel mailing list