On 09/20/2015 05:07 PM, Pavel Reichl wrote:
On 09/18/2015 05:07 PM, Michal Židek wrote:
+static errno_t check_cert(struct cli_ctx *cctx,
+ struct pam_ctx *pctx,
+ struct tevent_req *req,
What's the point of parameter *req? You are not reading it and if you
want to use it as output parameter you should use **_req, right?
+ 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;
+ }
+
+ return EOK;
+
+}
Hi,
new patches attached.
Michal
>From c4406bb01c31a797edea209b0abeeeb99dbf16f9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@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 | 63 ++++++++++++++++++++++--------------
6 files changed, 53 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'),
# [sudo]
'sudo_timed' : _('Whether to evaluate the time-based attributes in sudo rules'),
diff --git a/src/config/etc/sssd.api.conf b/src/config/etc/sssd.api.conf
index f280548..72abb8b 100644
--- a/src/config/etc/sssd.api.conf
+++ b/src/config/etc/sssd.api.conf
@@ -59,6 +59,7 @@ get_domains_timeout = int, None, false
pam_trusted_users = str, None, false
pam_public_domains = str, None, false
pam_account_expired_message = str, None, false
+p11_child_timeout = int, None, false
[sudo]
# sudo service
diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index b858347..9701f2a 100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -988,6 +988,18 @@ pam_account_expired_message = Account expired, please call help desk.
</para>
</listitem>
</varlistentry>
+ <varlistentry>
+ <term>p11_child_timeout (integer)</term>
+ <listitem>
+ <para>
+ How many seconds will pam_sss wait for
+ p11_child to finish.
+ </para>
+ <para>
+ Default: 10
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
</refsect2>
diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c
index aa5c209..27dddcf 100644
--- a/src/responder/pam/pamsrv_cmd.c
+++ b/src/responder/pam/pamsrv_cmd.c
@@ -43,11 +43,6 @@ enum pam_verbosity {
#define DEFAULT_PAM_VERBOSITY PAM_VERBOSITY_IMPORTANT
-/* TODO: Should we make this configurable? */
-#ifndef SSS_P11_CHILD_TIMEOUT
-#define SSS_P11_CHILD_TIMEOUT 10
-#endif
-
static errno_t
pam_null_last_online_auth_with_curr_token(struct sss_domain_info *domain,
const char *username);
@@ -1027,6 +1022,39 @@ static bool is_domain_public(char *name,
return false;
}
+static errno_t check_cert(TALLOC_CTX *mctx,
+ struct tevent_context *ev,
+ struct pam_ctx *pctx,
+ struct pam_auth_req *preq,
+ struct pam_data *pd)
+{
+ int p11_child_timeout;
+ const int P11_CHILD_TIMEOUT_DEFAULT = 10;
+ errno_t ret;
+ struct tevent_req *req;
+
+ 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(mctx, 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;
+ }
+
+ tevent_req_set_callback(req, pam_forwarder_cert_cb, preq);
+ return EAGAIN;
+}
+
static int pam_forwarder(struct cli_ctx *cctx, int pam_cmd)
{
struct sss_domain_info *dom;
@@ -1125,17 +1153,10 @@ static int pam_forwarder(struct cli_ctx *cctx, int pam_cmd)
}
}
- if (may_do_cert_auth(pctx, pd)) {
- req = pam_check_cert_send(cctx, cctx->ev, pctx->p11_child_debug_fd,
- pctx->nss_db, SSS_P11_CHILD_TIMEOUT, pd);
- if (req == NULL) {
- DEBUG(SSSDBG_OP_FAILURE, "pam_check_cert_send failed.\n");
- ret = ENOMEM;
- } else {
- tevent_req_set_callback(req, pam_forwarder_cert_cb, preq);
- ret = EAGAIN;
- }
+ if (may_do_cert_auth(pctx, pd)) {
+ ret = check_cert(cctx, cctx->ev, pctx, preq, pd);
+ /* Finish here */
goto done;
}
@@ -1342,16 +1363,8 @@ static void pam_forwarder_cb(struct tevent_req *req)
}
if (may_do_cert_auth(pctx, pd)) {
- req = pam_check_cert_send(cctx, cctx->ev, pctx->p11_child_debug_fd,
- pctx->nss_db, SSS_P11_CHILD_TIMEOUT, pd);
- if (req == NULL) {
- DEBUG(SSSDBG_OP_FAILURE, "pam_check_cert_send failed.\n");
- ret = ENOMEM;
- } else {
- tevent_req_set_callback(req, pam_forwarder_cert_cb, preq);
- ret = EAGAIN;
- }
-
+ ret = check_cert(cctx, cctx->ev, pctx, preq, pd);
+ /* Finish here */
goto done;
}
--
2.1.0
>From fc38d691c8470e1e0fc2aeaa9549764ebc47b486 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com>
Date: Fri, 18 Sep 2015 16:39:41 +0200
Subject: [PATCH 2/2] tests: Set p11_child_timeout to 30 in tests
Ticket:
https://fedorahosted.org/sssd/ticket/2773
Add way to set pam specific options in
pam_test_setup adn use it to set the
p11_child_timeout value to 30.
---
src/tests/cmocka/test_pam_srv.c | 35 +++++++++++++++++++++++++++++++----
1 file changed, 31 insertions(+), 4 deletions(-)
diff --git a/src/tests/cmocka/test_pam_srv.c b/src/tests/cmocka/test_pam_srv.c
index 3567b2b..6bae6e3 100644
--- a/src/tests/cmocka/test_pam_srv.c
+++ b/src/tests/cmocka/test_pam_srv.c
@@ -184,7 +184,26 @@ struct pam_ctx *mock_pctx(TALLOC_CTX *mem_ctx)
return pctx;
}
-void test_pam_setup(struct sss_test_conf_param params[],
+static int add_pam_params(struct sss_test_conf_param pam_params[],
+ struct confdb_ctx *cdb)
+{
+ const char *val[2];
+ int ret;
+
+ val[1] = NULL;
+
+ for (int i = 0; pam_params[i].key; i++) {
+ val[0] = pam_params[i].value;
+ ret = confdb_add_param(cdb, true, CONFDB_PAM_CONF_ENTRY,
+ pam_params[i].key, val);
+ assert_int_equal(ret, EOK);
+ }
+
+ return EOK;
+}
+
+void test_pam_setup(struct sss_test_conf_param dom_params[],
+ struct sss_test_conf_param pam_params[],
void **state)
{
errno_t ret;
@@ -194,7 +213,7 @@ void test_pam_setup(struct sss_test_conf_param params[],
pam_test_ctx->tctx = create_dom_test_ctx(pam_test_ctx, TESTS_PATH,
TEST_CONF_DB, TEST_DOM_NAME,
- TEST_ID_PROVIDER, params);
+ TEST_ID_PROVIDER, dom_params);
assert_non_null(pam_test_ctx->tctx);
pam_test_ctx->pam_cmds = get_pam_cmds();
@@ -217,6 +236,9 @@ void test_pam_setup(struct sss_test_conf_param params[],
pam_test_ctx->rctx->cdb = pam_test_ctx->tctx->confdb;
pam_test_ctx->pctx->rctx = pam_test_ctx->rctx;
+ ret = add_pam_params(pam_params, pam_test_ctx->rctx->cdb);
+ assert_int_equal(ret, EOK);
+
/* Create client context */
pam_test_ctx->cctx = mock_cctx(pam_test_ctx, pam_test_ctx->rctx);
assert_non_null(pam_test_ctx->cctx);
@@ -229,13 +251,18 @@ static int pam_test_setup(void **state)
{
int ret;
- struct sss_test_conf_param params[] = {
+ struct sss_test_conf_param dom_params[] = {
{ "enumerate", "false" },
{ "cache_credentials", "true" },
{ NULL, NULL }, /* Sentinel */
};
- test_pam_setup(params, state);
+ struct sss_test_conf_param pam_params[] = {
+ { "p11_child_timeout", "30"},
+ { NULL, NULL }, /* Sentinel */
+ };
+
+ test_pam_setup(dom_params, pam_params, state);
/* Prime the cache with a valid user */
ret = sysdb_add_user(pam_test_ctx->tctx->dom,
--
2.1.0
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
