[PATCH] Fix initialization of multiple variables
by Ondrej Kos
Hi,
While trying to figure culprit of #1833 and trying all possible
optimization levels, I noticed several uninitialized variable warnings.
Attached patch fixes those.
Ondra
--
Ondrej Kos
Associate Software Engineer
Identity Management
Red Hat Czech
phone: +420-532-294-558
cell: +420-736-417-909
ext: 82-62558
loc: 1013 Brno 1 office
irc: okos @ #sssd #brno
11 years, 1 month
[PATCH] Removing unused header file providers.h
by Lukas Slebodnik
Hi,
I found two header files, which are not included in any other file
src/providers/providers.h and src/sss_client/protos.h
First one is removed in attached patch and second one was commented
with "#if 0" more then four years ago. But I am not sure it should be
removed (is it documentation?), therefore is not removed in attached patch.
LS
11 years, 1 month
[PATCH] BUILD: Fix up whitespace in Makefile.am
by Stephen Gallagher
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
No functional changes here, I just noticed some unexpected tabs were
showing up.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.13 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iEUEARECAAYFAlE93tsACgkQeiVVYja6o6Pd7gCfTradWR3HlBoKq/FptKAIr4lZ
LdUAl16aTf1nwHzaR02U5j7Gn4NNo9w=
=lgQF
-----END PGP SIGNATURE-----
11 years, 1 month
[PATCH] BUILD: Fix cmocka
by Stephen Gallagher
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
We were not properly detecting that cmocka was unavailable. It was
expecting an empty value and getting "no" instead. This patch
corrects the expectation, so we will now skip building and running
cmocka tests on platforms that do not have it available.
Also, the common_cmocka.h header was missing from the distribution
tarball, causing 'make distcheck' to fail.
Note: the nss cmocka test is consistently segfaulting for me during
'make distcheck' on F18 x86_64. I haven't had time to look into why,
yet. It works fine in normal 'make check', so I suspect there's likely
something going on with permissions (since 'make check' sets $libdir
as read-only). But that's just a guess.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.13 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iEYEARECAAYFAlE+CIYACgkQeiVVYja6o6O66ACgkhgxp6LcyPGrAPcn/mmc6ZlE
APgAoJqIq8MJAggW0K8mYpuFTNsiL7Hn
=CEvx
-----END PGP SIGNATURE-----
11 years, 1 month
[PATCH] RFC: Unit test for the NSS responder based on the cmocka library
by Jakub Hrozek
Hi,
Short version - attached is a patch proposal for a unit test based on
cmocka. The patches are an example of a complex, yet isolated unit test
and I'd like to get opinions on whether this would be a good way to go.
Long version:
Our check based test suite is OK for developing simple tests for APIs
like sysdb but not really great for testing of more complex interfaces.
In particular, it's hard to simulate certain pieces of functionality
that require interaction with network or the rest of the system such as
LDAP searches or logins, especially in environments like buildsystems.
I think we might want to look at Cmocka to provide this missing functionality
and test complex interfaces in isolation. Cmocka[1] is a fork of Google's
cmockery which is not maintained upstream anymore. Cmocka is maintained
by Andreas Schneider.
I've been playing with cmocka lately and wrote a couple of patches that
add a unit test for the NSS responder, in particular the getpwnam()
function. It was quite easy to simulate the Data Provider and the client
using two concepts:
1) cmocka's mocking feature
Functions that provide external interface, like the dummy DP functions
in my patches, can access a stack of values. The unit test can push some
expected return values onto the stack before launching the test and the
dummy functions would then pop the values and use them as appropriate.
It's quite easy to simulate a Data Provider saving an entry to cache
with call like this (pseudocode):
int unit_test()
{
will_return(sss_dp_get_account_recv, "testuser");
will_return(sss_dp_get_account_recv, 10);
will_return(sss_dp_get_account_recv, 11);
}
int sss_dp_get_account_recv(TALLOC_CTX *mem_ctx, tevent_req *req)
{
char *username = mock();
uid_t uid = mock();
gid_t gid = mock();
sysdb_store_user(username, uid, gid);
}
2) the -wrap feature of ld
It is possible to selectively override functions from modules linked against
to intercept calls using the -wrap feature of ld. In my unit tests I used
this feature to check results of responder search with defined callbacks
instead of returning the results to the nss client. Here is a simple example
of intercepting a call to negative cache to make sure the negative cache
is hit when it's supposed to:
int __wrap_sss_ncache_check_user(struct sss_nc_ctx *ctx, int ttl,
struct sss_domain_info *dom,
const char *name)
{
int ret;
ret = __real_sss_ncache_check_user(ctx, ttl, dom, name);
if (ret == EEXIST) {
nss_test_ctx->ncache_hit = true;
}
return ret;
}
There's a couple of boilerplate functions but once these are in place,
they can be reused to cover different pieces of functionality..the
attached patches test the following requests:
* cached user including assertion that DP was not contacted
* nonexistant user including assertion that on a subsequent lookup, the
request is returned from negative cache
* search for a user that has not been cache prior to the search
* updating an expired user including assertions that the user had been
updated from DP and the updated entry is returned.
The downsides of using cmocka as far as I can see are:
* writing the mocking code can be tedious. On the other hand, the NSS
responder I picked was probably the most complex code I could test
as it communicates with both DP and the client library. Even more
isolated testing (the AD sites feature comes to mind) would be much
easier I think.
* cmocka is not present in RHEL
I'll be glad to hear comments about this unit test proposal. Patches
are attached.
[1] http://cmocka.cryptomilk.org/
11 years, 1 month
[PATCH] Add support for krb5 1.11's responder callback.
by Nathaniel McCallum
---
src/external/krb5.m4 | 1 +
src/providers/krb5/krb5_auth.c | 2 +-
src/providers/krb5/krb5_auth.h | 1 +
src/providers/krb5/krb5_child.c | 201 ++++++++++++++++++++++++++++++++
src/providers/krb5/krb5_child_handler.c | 7 ++
src/sss_client/sss_cli.h | 3 +
6 files changed, 214 insertions(+), 1 deletion(-)
diff --git a/src/external/krb5.m4 b/src/external/krb5.m4
index f1679a1..56c6484 100644
--- a/src/external/krb5.m4
+++ b/src/external/krb5.m4
@@ -50,6 +50,7 @@ AC_CHECK_FUNCS([krb5_get_init_creds_opt_alloc krb5_get_error_message \
krb5_get_init_creds_opt_set_fast_ccache_name \
krb5_get_init_creds_opt_set_fast_flags \
krb5_get_init_creds_opt_set_canonicalize \
+ krb5_get_init_creds_opt_set_responder \
krb5_unparse_name_flags \
krb5_get_init_creds_opt_set_change_password_prompt \
krb5_free_keytab_entry_contents \
diff --git a/src/providers/krb5/krb5_auth.c b/src/providers/krb5/krb5_auth.c
index e41e1a1..d3e11a2 100644
--- a/src/providers/krb5/krb5_auth.c
+++ b/src/providers/krb5/krb5_auth.c
@@ -1107,7 +1107,7 @@ static void krb5_auth_done(struct tevent_req *subreq)
goto done;
}
- if (state->be_ctx->domain->cache_credentials == TRUE) {
+ if (state->be_ctx->domain->cache_credentials == TRUE && !res->otp) {
krb5_auth_store_creds(state->sysdb, state->domain, pd);
}
diff --git a/src/providers/krb5/krb5_auth.h b/src/providers/krb5/krb5_auth.h
index 078a31d..cf290ca 100644
--- a/src/providers/krb5/krb5_auth.h
+++ b/src/providers/krb5/krb5_auth.h
@@ -81,6 +81,7 @@ struct krb5_child_response {
struct tgt_times tgtt;
char *ccname;
char *correct_upn;
+ bool otp;
};
errno_t
diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
index 9cc0f55..a55647f 100644
--- a/src/providers/krb5/krb5_child.c
+++ b/src/providers/krb5/krb5_child.c
@@ -146,6 +146,198 @@ static void sss_krb5_expire_callback_func(krb5_context context, void *data,
return;
}
+#ifdef HAVE_KRB5_GET_INIT_CREDS_OPT_SET_RESPONDER
+/* TODO: features that don't work:
+ * 1. tokeninfo selection
+ * 2. challenge
+ * 3. discreet token/pin prompting
+ * 4. interactive otp format correction
+ * 5. nextOTP
+ */
+typedef int (*checker)(int c);
+
+static inline checker pick_checker(int format)
+{
+ switch (format) {
+ case KRB5_RESPONDER_OTP_FORMAT_DECIMAL:
+ return isdigit;
+ case KRB5_RESPONDER_OTP_FORMAT_HEXADECIMAL:
+ return isxdigit;
+ case KRB5_RESPONDER_OTP_FORMAT_ALPHANUMERIC:
+ return isalnum;
+ }
+
+ return NULL;
+}
+
+static krb5_error_code tokeninfo_matches(const struct pam_data *pd,
+ const krb5_responder_otp_tokeninfo *ti,
+ char **out_token, char **out_pin)
+{
+ char *token = NULL, *pin = NULL;
+ checker check = NULL;
+ int i;
+
+ if (ti->flags & KRB5_RESPONDER_OTP_FLAGS_NEXTOTP) {
+ return ENOTSUP;
+ }
+
+ if (ti->challenge != NULL) {
+ return ENOTSUP;
+ }
+
+ /* This is a non-sensical value. */
+ if (ti->length == 0) {
+ return EPROTO;
+ }
+
+ if (ti->flags & KRB5_RESPONDER_OTP_FLAGS_COLLECT_TOKEN) {
+ /* ASSUMPTION: authtok has one of the following formats:
+ * 1. TokenValue
+ * 2. PIN+TokenValue
+ */
+ token = talloc_strndup(NULL, (char*) pd->authtok.data,
+ pd->authtok.length);
+ if (token == NULL) {
+ return ENOMEM;
+ }
+
+ if (ti->flags & KRB5_RESPONDER_OTP_FLAGS_COLLECT_PIN) {
+ /* If the server desires a separate pin, we will split it.
+ * ASSUMPTION: Format of authtok is PIN+TokenValue. */
+ if (ti->flags & KRB5_RESPONDER_OTP_FLAGS_SEPARATE_PIN) {
+ if (ti->length < 1) {
+ talloc_free(token);
+ return ENOTSUP;
+ }
+
+ if (ti->length >= pd->authtok.length) {
+ talloc_free(token);
+ return EMSGSIZE;
+ }
+
+ /* Copy the PIN from the front of the value. */
+ pin = talloc_strndup(NULL, (char*) pd->authtok.data,
+ pd->authtok.length - ti->length);
+ if (pin == NULL) {
+ talloc_free(token);
+ return ENOMEM;
+ }
+
+ /* Remove the PIN from the front of the token value. */
+ memmove(token, token + pd->authtok.length - ti->length,
+ ti->length + 1);
+
+ check = pick_checker(ti->format);
+ } else {
+ if (ti->length > 0 && ti->length > pd->authtok.length) {
+ talloc_free(token);
+ return EMSGSIZE;
+ }
+ }
+ } else {
+ if (ti->length > 0 && ti->length != pd->authtok.length) {
+ talloc_free(token);
+ return EMSGSIZE;
+ }
+
+ check = pick_checker(ti->format);
+ }
+ } else {
+ pin = talloc_strndup(NULL, (char*) pd->authtok.data,
+ pd->authtok.length);
+ if (pin == NULL) {
+ return ENOMEM;
+ }
+ }
+
+ /* If check is set, we need to verify the contents of the token. */
+ for (i = 0; check != NULL && token[i] != '\0'; i++) {
+ if (!check(token[i])) {
+ talloc_free(token);
+ talloc_free(pin);
+ return EBADMSG;
+ }
+ }
+
+ *out_token = token;
+ *out_pin = pin;
+ return 0;
+}
+
+static krb5_error_code answer_otp(krb5_context ctx,
+ struct krb5_req *kr,
+ krb5_responder_context rctx)
+{
+ krb5_responder_otp_challenge *chl;
+ char *token = NULL, *pin = NULL;
+ krb5_error_code ret;
+ size_t i;
+
+ ret = krb5_responder_otp_get_challenge(ctx, rctx, &chl);
+ if (ret != EOK || chl == NULL) {
+ /* Either an error, or nothing to do. */
+ return ret;
+ }
+
+ if (chl->tokeninfo == NULL || chl->tokeninfo[0] == NULL) {
+ /* No tokeninfos? Absurd! */
+ ret = EINVAL;
+ goto done;
+ }
+
+ /* Validate our assumptions about the contents of authtok. */
+ if (kr->pd->authtok.type != SSS_AUTHTOK_TYPE_PASSWORD) {
+ ret = EOK;
+ goto done;
+ }
+
+ /* Find the first supported tokeninfo which matches our authtoken. */
+ for (i = 0; chl->tokeninfo[i] != NULL; i++) {
+ ret = tokeninfo_matches(kr->pd, chl->tokeninfo[i], &token, &pin);
+ if (ret == EOK)
+ break;
+ else if (ret == ENOMEM)
+ goto done;
+ }
+ if (chl->tokeninfo[i] == NULL) {
+ DEBUG(1, ("No tokeninfos found which match our credentials.\n"));
+ ret = EOK;
+ goto done;
+ }
+
+ if (chl->tokeninfo[i]->flags & KRB5_RESPONDER_OTP_FLAGS_COLLECT_TOKEN) {
+ /* Don't let SSSD cache the OTP authtok since it is single-use. */
+ ret = pam_add_response(kr->pd, SSS_OTP, 0, NULL);
+ if (ret != EOK) {
+ DEBUG(1, ("pam_add_response failed.\n"));
+ goto done;
+ }
+ }
+
+ /* Respond with the appropriate answer. */
+ ret = krb5_responder_otp_set_answer(ctx, rctx, i, token, pin);
+done:
+ talloc_free(token);
+ talloc_free(pin);
+ krb5_responder_otp_challenge_free(ctx, rctx, chl);
+ return ret;
+}
+
+static krb5_error_code sss_krb5_responder(krb5_context ctx,
+ void *data,
+ krb5_responder_context rctx)
+{
+ struct krb5_req *kr = talloc_get_type(data, struct krb5_req);
+
+ if (kr == NULL) {
+ return EINVAL;
+ }
+
+ return answer_otp(ctx, kr, rctx);
+}
+#endif
+
static krb5_error_code sss_krb5_prompter(krb5_context context, void *data,
const char *name, const char *banner,
int num_prompts, krb5_prompt prompts[])
@@ -1746,6 +1938,15 @@ static int k5c_setup(struct krb5_req *kr, uint32_t offline)
return kerr;
}
+#ifdef HAVE_KRB5_GET_INIT_CREDS_OPT_SET_RESPONDER
+ kerr = krb5_get_init_creds_opt_set_responder(kr->ctx, kr->options,
+ sss_krb5_responder, kr);
+ if (kerr != 0) {
+ KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr);
+ return kerr;
+ }
+#endif
+
#ifdef HAVE_KRB5_GET_INIT_CREDS_OPT_SET_CHANGE_PASSWORD_PROMPT
/* A prompter is used to catch messages about when a password will
* expired. The library shall not use the prompter to ask for a new password
diff --git a/src/providers/krb5/krb5_child_handler.c b/src/providers/krb5/krb5_child_handler.c
index 5adbcf7..99922b4 100644
--- a/src/providers/krb5/krb5_child_handler.c
+++ b/src/providers/krb5/krb5_child_handler.c
@@ -482,6 +482,7 @@ parse_krb5_child_response(TALLOC_CTX *mem_ctx, uint8_t *buf, ssize_t len,
struct krb5_child_response *res;
const char *upn = NULL;
size_t upn_len;
+ bool otp = false;
if ((size_t) len < sizeof(int32_t)) {
DEBUG(SSSDBG_CRIT_FAILURE, ("message too short.\n"));
@@ -563,6 +564,11 @@ parse_krb5_child_response(TALLOC_CTX *mem_ctx, uint8_t *buf, ssize_t len,
}
}
+ if (msg_type == SSS_OTP) {
+ otp = true;
+ skip = true;
+ }
+
if (!skip) {
ret = pam_add_response(pd, msg_type, msg_len, &buf[p]);
if (ret != EOK) {
@@ -583,6 +589,7 @@ parse_krb5_child_response(TALLOC_CTX *mem_ctx, uint8_t *buf, ssize_t len,
res = talloc_zero(mem_ctx, struct krb5_child_response);
if (!res) return ENOMEM;
+ res->otp = otp;
res->msg_status = msg_status;
memcpy(&res->tgtt, &tgtt, sizeof(tgtt));
diff --git a/src/sss_client/sss_cli.h b/src/sss_client/sss_cli.h
index 372bcee..b0d7b82 100644
--- a/src/sss_client/sss_cli.h
+++ b/src/sss_client/sss_cli.h
@@ -368,6 +368,9 @@ enum response_type {
* the user.This should only be used in the case where
* it is not possile to use SSS_PAM_USER_INFO.
* @param A zero terminated string. */
+ SSS_OTP, /**< Indicates that the autotok was a OTP, so don't
+ * cache it. There is no message.
+ * @param None. */
};
/**
--
1.8.1.4
11 years, 1 month