[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