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(a)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.