From 74f0a451bd5e3e2afc7260ba98fc4102ec08ea1d Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: May 19 2020 09:16:16 +0000 Subject: cache_req: no refresh with CACHE_REQ_BYPASS_PROVIDER This patch fixes an unexpected behavior of the cache request code if the CACHE_REQ_BYPASS_PROVIDER option is used. Currently even if this option is used an expired entry in the cache is refreshed by calling the provider. With this patch an error is returned if the entry is expired and the provider is not called. Resolves: https://pagure.io/SSSD/sssd/issue/4098 Reviewed-by: Pavel Březina --- diff --git a/src/responder/common/cache_req/cache_req_search.c b/src/responder/common/cache_req/cache_req_search.c index 71c51a1..9acd256 100644 --- a/src/responder/common/cache_req/cache_req_search.c +++ b/src/responder/common/cache_req/cache_req_search.c @@ -309,6 +309,7 @@ cache_req_search_send(TALLOC_CTX *mem_ctx, struct tevent_req *req; bool bypass_cache = false; bool bypass_dp = false; + bool skip_refresh = false; errno_t ret; req = tevent_req_create(mem_ctx, &state, struct cache_req_search_state); @@ -340,6 +341,7 @@ cache_req_search_send(TALLOC_CTX *mem_ctx, break; case CACHE_REQ_BYPASS_PROVIDER: bypass_dp = true; + skip_refresh = true; break; default: break; @@ -370,11 +372,12 @@ cache_req_search_send(TALLOC_CTX *mem_ctx, goto done; } - /* If bypass_dp is true but we found the object in this domain, - * we will contact the data provider anyway to refresh it so - * we can return it without searching the rest of the domains. + /* For the CACHE_REQ_CACHE_FIRST case, if bypass_dp is true but we + * found the object in this domain, we will contact the data provider + * anyway to refresh it so we can return it without searching the rest + * of the domains. */ - if (status != CACHE_OBJECT_MISSING) { + if (status != CACHE_OBJECT_MISSING && !skip_refresh) { CACHE_REQ_DEBUG(SSSDBG_TRACE_FUNC, cr, "Object found, but needs to be refreshed.\n"); bypass_dp = false; diff --git a/src/tests/cmocka/test_nss_srv.c b/src/tests/cmocka/test_nss_srv.c index 1ea6ab5..2c91d0a 100644 --- a/src/tests/cmocka/test_nss_srv.c +++ b/src/tests/cmocka/test_nss_srv.c @@ -4828,7 +4828,6 @@ void test_nss_getgrgid_ex_no_members(void **state) will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); mock_account_recv_simple(); - mock_account_recv_simple(); set_cmd_cb(test_nss_getgrnam_no_members_check); ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETGRGID_EX, @@ -4843,11 +4842,6 @@ void test_nss_getgrgid_ex_no_members(void **state) /* Use flag SSS_NSS_EX_FLAG_INVALIDATE_CACHE */ mock_input_id_ex(nss_test_ctx, getgrnam_no_members.gr_gid, SSS_NSS_EX_FLAG_INVALIDATE_CACHE); - will_return(__wrap_sss_packet_get_cmd, SSS_NSS_GETGRGID_EX); - will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); - will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); - will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); - will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); set_cmd_cb(test_nss_getgrnam_no_members_check); ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETGRGID_EX, diff --git a/src/tests/cmocka/test_pam_srv.c b/src/tests/cmocka/test_pam_srv.c index 9f33990..d1b3f1e 100644 --- a/src/tests/cmocka/test_pam_srv.c +++ b/src/tests/cmocka/test_pam_srv.c @@ -505,6 +505,7 @@ void test_pam_setup(struct sss_test_conf_param dom_params[], static void pam_test_setup_common(void) { errno_t ret; + time_t now; #ifndef HAVE_NSS putenv(discard_const("SOFTHSM2_CONF=" ABS_BUILD_DIR "/src/tests/test_CA/softhsm2_one.conf")); @@ -522,15 +523,22 @@ static void pam_test_setup_common(void) pam_test_ctx->tctx->dom->name); assert_non_null(pam_test_ctx->wrong_user_fqdn); - /* integer values cannot be set by pam_params */ - pam_test_ctx->pctx->id_timeout = 5; + /* integer values cannot be set by pam_params, since we are expecting that + * the PAM id cache stay valid during a test we have to make sure the + * timeout is long enough that even a run e.g. delayed by running with + * valgrind can pass. */ + pam_test_ctx->pctx->id_timeout = 60; + now = time(NULL); /* Prime the cache with a valid user */ ret = sysdb_add_user(pam_test_ctx->tctx->dom, pam_test_ctx->pam_user_fqdn, 123, 456, "pam user", "/home/pamuser", "/bin/sh", NULL, - NULL, 300, 0); + NULL, 300, now); + assert_int_equal(ret, EOK); + ret = sysdb_set_initgr_expire_timestamp(pam_test_ctx->tctx->dom, + pam_test_ctx->pam_user_fqdn); assert_int_equal(ret, EOK); /* Add entry to the initgr cache to make sure no initgr request is sent to @@ -546,7 +554,10 @@ static void pam_test_setup_common(void) pam_test_ctx->wrong_user_fqdn, 321, 654, "wrong user", "/home/wronguser", "/bin/sh", NULL, - NULL, 300, 0); + NULL, 300, now); + assert_int_equal(ret, EOK); + ret = sysdb_set_initgr_expire_timestamp(pam_test_ctx->tctx->dom, + pam_test_ctx->wrong_user_fqdn); assert_int_equal(ret, EOK); /* Add entry to the initgr cache to make sure no initgr request is sent to @@ -563,6 +574,7 @@ static int pam_test_setup(void **state) struct sss_test_conf_param dom_params[] = { { "enumerate", "false" }, { "cache_credentials", "true" }, + { "entry_cache_timeout", "300" }, { NULL, NULL }, /* Sentinel */ }; @@ -819,22 +831,20 @@ static void mock_input_pam(TALLOC_CTX *mem_ctx, const char *pwd, const char *fa2) { - return mock_input_pam_ex(mem_ctx, name, pwd, fa2, NULL, true); + return mock_input_pam_ex(mem_ctx, name, pwd, fa2, NULL, false); } static void mock_input_pam_cert(TALLOC_CTX *mem_ctx, const char *name, const char *pin, const char *token_name, const char *module_name, const char *key_id, const char *service, - acct_cb_t acct_cb, const char *cert, - bool only_one_provider_call) + acct_cb_t acct_cb, const char *cert) { size_t buf_size; uint8_t *m_buf; uint8_t *buf; struct pam_items pi = { 0 }; int ret; - bool already_mocked = false; size_t needed_size; if (name != NULL) { @@ -888,14 +898,10 @@ static void mock_input_pam_cert(TALLOC_CTX *mem_ctx, const char *name, if (acct_cb != NULL) { mock_account_recv(0, 0, NULL, acct_cb, discard_const(cert)); - already_mocked = true; } if (name != NULL) { mock_parse_inp(name, NULL, EOK); - if (!(only_one_provider_call && already_mocked)) { - mock_account_recv_simple(); - } } } @@ -2061,7 +2067,7 @@ void test_pam_preauth_no_logon_name(void **state) int ret; mock_input_pam_cert(pam_test_ctx, NULL, NULL, NULL, NULL, NULL, NULL, NULL, - NULL, false); + NULL); will_return(__wrap_sss_packet_get_cmd, SSS_PAM_PREAUTH); will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); @@ -2126,7 +2132,6 @@ void test_pam_auth_upn_logon_name(void **state) mock_input_pam_ex(pam_test_ctx, "upn@"TEST_DOM_NAME, "12345", NULL, NULL, true); - mock_account_recv_simple(); mock_parse_inp("pamuser", NULL, EOK); @@ -2163,7 +2168,7 @@ void test_pam_preauth_cert_nocert(void **state) mock_input_pam_cert(pam_test_ctx, "pamuser", NULL, NULL, NULL, NULL, NULL, - NULL, NULL, false); + NULL, NULL); will_return(__wrap_sss_packet_get_cmd, SSS_PAM_PREAUTH); will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); @@ -2306,7 +2311,7 @@ void test_pam_preauth_cert_nomatch(void **state) set_cert_auth_param(pam_test_ctx->pctx, CA_DB); mock_input_pam_cert(pam_test_ctx, "pamuser", NULL, NULL, NULL, NULL, NULL, - test_lookup_by_cert_cb, NULL, false); + test_lookup_by_cert_cb, NULL); will_return(__wrap_sss_packet_get_cmd, SSS_PAM_PREAUTH); will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); @@ -2328,7 +2333,7 @@ void test_pam_preauth_cert_match(void **state) set_cert_auth_param(pam_test_ctx->pctx, CA_DB); mock_input_pam_cert(pam_test_ctx, "pamuser", NULL, NULL, NULL, NULL, NULL, - test_lookup_by_cert_cb, SSSD_TEST_CERT_0001, false); + test_lookup_by_cert_cb, SSSD_TEST_CERT_0001); will_return(__wrap_sss_packet_get_cmd, SSS_PAM_PREAUTH); will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); @@ -2352,7 +2357,7 @@ void test_pam_preauth_cert_match_gdm_smartcard(void **state) mock_input_pam_cert(pam_test_ctx, "pamuser", NULL, NULL, NULL, NULL, "gdm-smartcard", test_lookup_by_cert_cb, - SSSD_TEST_CERT_0001, false); + SSSD_TEST_CERT_0001); will_return(__wrap_sss_packet_get_cmd, SSS_PAM_PREAUTH); will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); @@ -2375,7 +2380,7 @@ void test_pam_preauth_cert_match_wrong_user(void **state) mock_input_pam_cert(pam_test_ctx, "pamuser", NULL, NULL, NULL, NULL, NULL, test_lookup_by_cert_wrong_user_cb, - SSSD_TEST_CERT_0001, false); + SSSD_TEST_CERT_0001); will_return(__wrap_sss_packet_get_cmd, SSS_PAM_PREAUTH); will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); @@ -2406,10 +2411,9 @@ void test_pam_preauth_cert_no_logon_name(void **state) * request will be done with the username found by the certificate * lookup. */ mock_input_pam_cert(pam_test_ctx, NULL, NULL, NULL, NULL, NULL, NULL, - test_lookup_by_cert_cb, SSSD_TEST_CERT_0001, false); + test_lookup_by_cert_cb, SSSD_TEST_CERT_0001); mock_account_recv_simple(); mock_parse_inp("pamuser", NULL, EOK); - mock_account_recv_simple(); mock_parse_inp("pamuser", NULL, EOK); will_return(__wrap_sss_packet_get_cmd, SSS_PAM_PREAUTH); @@ -2437,7 +2441,7 @@ void test_pam_preauth_cert_no_logon_name_with_hint(void **state) * during pre-auth and there is no need for an extra mocked response as in * test_pam_preauth_cert_no_logon_name. */ mock_input_pam_cert(pam_test_ctx, NULL, NULL, NULL, NULL, NULL, NULL, - test_lookup_by_cert_cb, SSSD_TEST_CERT_0001, false); + test_lookup_by_cert_cb, SSSD_TEST_CERT_0001); will_return(__wrap_sss_packet_get_cmd, SSS_PAM_PREAUTH); will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); @@ -2459,8 +2463,7 @@ void test_pam_preauth_cert_no_logon_name_double_cert(void **state) set_cert_auth_param(pam_test_ctx->pctx, CA_DB); mock_input_pam_cert(pam_test_ctx, NULL, NULL, NULL, NULL, NULL, NULL, - test_lookup_by_cert_double_cb, SSSD_TEST_CERT_0001, - false); + test_lookup_by_cert_double_cb, SSSD_TEST_CERT_0001); will_return(__wrap_sss_packet_get_cmd, SSS_PAM_PREAUTH); will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); @@ -2483,8 +2486,7 @@ void test_pam_preauth_cert_no_logon_name_double_cert_with_hint(void **state) pam_test_ctx->rctx->domains->user_name_hint = true; mock_input_pam_cert(pam_test_ctx, NULL, NULL, NULL, NULL, NULL, NULL, - test_lookup_by_cert_double_cb, SSSD_TEST_CERT_0001, - false); + test_lookup_by_cert_double_cb, SSSD_TEST_CERT_0001); will_return(__wrap_sss_packet_get_cmd, SSS_PAM_PREAUTH); will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); @@ -2506,7 +2508,7 @@ void test_pam_preauth_no_cert_no_logon_name(void **state) set_cert_auth_param(pam_test_ctx->pctx, "/no/path"); mock_input_pam_cert(pam_test_ctx, NULL, NULL, NULL, NULL, NULL, NULL, NULL, - NULL, false); + NULL); will_return(__wrap_sss_packet_get_cmd, SSS_PAM_PREAUTH); will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); @@ -2528,7 +2530,7 @@ void test_pam_preauth_cert_no_logon_name_no_match(void **state) set_cert_auth_param(pam_test_ctx->pctx, CA_DB); mock_input_pam_cert(pam_test_ctx, NULL, NULL, NULL, NULL, NULL, NULL, - test_lookup_by_cert_cb, NULL, false); + test_lookup_by_cert_cb, NULL); will_return(__wrap_sss_packet_get_cmd, SSS_PAM_PREAUTH); will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); @@ -2558,7 +2560,7 @@ void test_pam_cert_auth(void **state) mock_input_pam_cert(pam_test_ctx, "pamuser", "123456", "SSSD Test Token", TEST_MODULE_NAME, "C554C9F82C2A9D58B70921C143304153A8A42F17", NULL, - test_lookup_by_cert_cb, SSSD_TEST_CERT_0001, true); + test_lookup_by_cert_cb, SSSD_TEST_CERT_0001); will_return(__wrap_sss_packet_get_cmd, SSS_PAM_AUTHENTICATE); will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); @@ -2596,7 +2598,7 @@ void test_pam_ecc_cert_auth(void **state) "SSSD Test ECC Token", TEST_MODULE_NAME, "190E513C9A3DFAACDE5D2D0592F0FDFF559C10CB", NULL, - test_lookup_by_cert_cb, SSSD_TEST_ECC_CERT_0001, true); + test_lookup_by_cert_cb, SSSD_TEST_ECC_CERT_0001); will_return(__wrap_sss_packet_get_cmd, SSS_PAM_AUTHENTICATE); will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); @@ -2633,11 +2635,10 @@ void test_pam_cert_auth_no_logon_name(void **state) mock_input_pam_cert(pam_test_ctx, NULL, "123456", "SSSD Test Token", TEST_MODULE_NAME, "C554C9F82C2A9D58B70921C143304153A8A42F17", NULL, - test_lookup_by_cert_cb, SSSD_TEST_CERT_0001, true); + test_lookup_by_cert_cb, SSSD_TEST_CERT_0001); mock_account_recv_simple(); mock_parse_inp("pamuser", NULL, EOK); - mock_account_recv_simple(); mock_parse_inp("pamuser", NULL, EOK); will_return(__wrap_sss_packet_get_cmd, SSS_PAM_AUTHENTICATE); @@ -2670,7 +2671,7 @@ void test_pam_cert_auth_no_logon_name_no_key_id(void **state) * in the cache and no second request to the backend is needed. */ mock_input_pam_cert(pam_test_ctx, NULL, "123456", "SSSD Test Token", TEST_MODULE_NAME, NULL, NULL, - NULL, NULL, false); + NULL, NULL); will_return(__wrap_sss_packet_get_cmd, SSS_PAM_AUTHENTICATE); will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); @@ -2697,8 +2698,7 @@ void test_pam_cert_auth_double_cert(void **state) mock_input_pam_cert(pam_test_ctx, "pamuser", "123456", "SSSD Test Token", TEST_MODULE_NAME, "C554C9F82C2A9D58B70921C143304153A8A42F17", NULL, - test_lookup_by_cert_double_cb, SSSD_TEST_CERT_0001, - true); + test_lookup_by_cert_double_cb, SSSD_TEST_CERT_0001); will_return(__wrap_sss_packet_get_cmd, SSS_PAM_AUTHENTICATE); will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); @@ -2730,7 +2730,7 @@ void test_pam_cert_preauth_2certs_one_mapping(void **state) ret = test_lookup_by_cert_cb(discard_const(SSSD_TEST_CERT_0001)); assert_int_equal(ret, EOK); mock_input_pam_cert(pam_test_ctx, "pamuser", NULL, NULL, NULL, NULL, NULL, - test_lookup_by_cert_cb, NULL, false); + test_lookup_by_cert_cb, NULL); will_return(__wrap_sss_packet_get_cmd, SSS_PAM_PREAUTH); will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); @@ -2758,7 +2758,7 @@ void test_pam_cert_preauth_2certs_two_mappings(void **state) mock_input_pam_cert(pam_test_ctx, "pamuser", NULL, NULL, NULL, NULL, NULL, test_lookup_by_cert_cb_2nd_cert_same_user, - SSSD_TEST_CERT_0001, false); + SSSD_TEST_CERT_0001); will_return(__wrap_sss_packet_get_cmd, SSS_PAM_PREAUTH); will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); @@ -2787,8 +2787,7 @@ void test_pam_cert_auth_2certs_one_mapping(void **state) mock_input_pam_cert(pam_test_ctx, "pamuser", "123456", "SSSD Test Token", TEST_MODULE_NAME, "C554C9F82C2A9D58B70921C143304153A8A42F17", NULL, - test_lookup_by_cert_double_cb, SSSD_TEST_CERT_0001, - true); + test_lookup_by_cert_double_cb, SSSD_TEST_CERT_0001); will_return(__wrap_sss_packet_get_cmd, SSS_PAM_AUTHENTICATE); will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); @@ -2821,7 +2820,7 @@ void test_pam_cert_preauth_uri_token1(void **state) putenv(discard_const("SOFTHSM2_CONF=" ABS_BUILD_DIR "/src/tests/test_CA/softhsm2_2tokens.conf")); mock_input_pam_cert(pam_test_ctx, "pamuser", NULL, NULL, NULL, NULL, NULL, - test_lookup_by_cert_cb, SSSD_TEST_CERT_0001, false); + test_lookup_by_cert_cb, SSSD_TEST_CERT_0001); will_return(__wrap_sss_packet_get_cmd, SSS_PAM_PREAUTH); will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); @@ -2851,7 +2850,7 @@ void test_pam_cert_preauth_uri_token2(void **state) putenv(discard_const("SOFTHSM2_CONF=" ABS_BUILD_DIR "/src/tests/test_CA/softhsm2_2tokens.conf")); mock_input_pam_cert(pam_test_ctx, "pamuser", NULL, NULL, NULL, NULL, NULL, - test_lookup_by_cert_cb, SSSD_TEST_CERT_0002, false); + test_lookup_by_cert_cb, SSSD_TEST_CERT_0002); will_return(__wrap_sss_packet_get_cmd, SSS_PAM_PREAUTH); will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); @@ -2890,7 +2889,7 @@ void test_pam_preauth_expired_crl_file(void **state) set_cert_auth_param(pam_test_ctx->pctx, CA_DB); mock_input_pam_cert(pam_test_ctx, "pamuser", NULL, NULL, NULL, NULL, NULL, - NULL, NULL, false); + NULL, NULL); will_return(__wrap_sss_packet_get_cmd, SSS_PAM_PREAUTH); will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); @@ -2922,7 +2921,7 @@ void test_pam_preauth_expired_crl_file_soft(void **state) set_cert_auth_param(pam_test_ctx->pctx, CA_DB); mock_input_pam_cert(pam_test_ctx, "pamuser", NULL, NULL, NULL, NULL, NULL, - test_lookup_by_cert_cb, SSSD_TEST_CERT_0001, false); + test_lookup_by_cert_cb, SSSD_TEST_CERT_0001); will_return(__wrap_sss_packet_get_cmd, SSS_PAM_PREAUTH); will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); @@ -2959,7 +2958,7 @@ void test_pam_preauth_ocsp(void **state) #endif mock_input_pam_cert(pam_test_ctx, "pamuser", NULL, NULL, NULL, NULL, NULL, - NULL, NULL, false); + NULL, NULL); will_return(__wrap_sss_packet_get_cmd, SSS_PAM_PREAUTH); will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); @@ -3005,7 +3004,7 @@ void test_pam_preauth_ocsp_no_ocsp(void **state) #endif mock_input_pam_cert(pam_test_ctx, "pamuser", NULL, NULL, NULL, NULL, NULL, - test_lookup_by_cert_cb, SSSD_TEST_CERT_0005, false); + test_lookup_by_cert_cb, SSSD_TEST_CERT_0005); will_return(__wrap_sss_packet_get_cmd, SSS_PAM_PREAUTH); will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); @@ -3043,7 +3042,7 @@ void test_pam_preauth_ocsp_soft_ocsp(void **state) #endif mock_input_pam_cert(pam_test_ctx, "pamuser", NULL, NULL, NULL, NULL, NULL, - test_lookup_by_cert_cb, SSSD_TEST_CERT_0005, false); + test_lookup_by_cert_cb, SSSD_TEST_CERT_0005); will_return(__wrap_sss_packet_get_cmd, SSS_PAM_PREAUTH); will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); @@ -3284,7 +3283,7 @@ void test_not_appsvc_posix_dom(void **state) int ret; /* A different service than the app one can authenticate against a POSIX domain */ - mock_input_pam_ex(pam_test_ctx, "pamuser", NULL, NULL, "not_app_svc", true); + mock_input_pam_ex(pam_test_ctx, "pamuser", NULL, NULL, "not_app_svc", false); will_return(__wrap_sss_packet_get_cmd, SSS_PAM_AUTHENTICATE); will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL); @@ -3328,7 +3327,7 @@ void test_appsvc_app_dom(void **state) int ret; /* The domain is POSIX, the request will skip over it */ - mock_input_pam_ex(pam_test_ctx, "pamuser", NULL, NULL, "app_svc", true); + mock_input_pam_ex(pam_test_ctx, "pamuser", NULL, NULL, "app_svc", false); will_return(__wrap_sss_packet_get_cmd, SSS_PAM_AUTHENTICATE); will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL);