URL: https://github.com/SSSD/sssd/pull/225 Author: jhrozek Title: #225: SECRETS: Apply separate quotas for cn=secrets and cn=kcm Action: opened
PR body: """ While testing the KCM responder some more, I realized that we always checked the (hardcoded, no less) base DN of cn=secrets when checking for quotas. These patches make the quota check separate for each of the cn=secrets/cn=kcm hives, add a test and mention in documentation that the quota from sssd-secrets applies for how many ccaches can be stored. """ To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/225/head:pr225 git checkout pr225
From 58e80dfcd0619a523b6aa09d59c15ef4ce079f93 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <[email protected]> Date: Tue, 4 Apr 2017 14:45:30 +0200 Subject: [PATCH 1/4] SECRETS: Rename local_db_req.basedn to local_db_req.req_dn This will make it possible to reuse the basedn name later for the "hive" base DN in order to differentiate quotas for different hives. There is no functional change in this patch. --- src/responder/secrets/local.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/responder/secrets/local.c b/src/responder/secrets/local.c index 02007ad..232a358 100644 --- a/src/responder/secrets/local.c +++ b/src/responder/secrets/local.c @@ -205,7 +205,7 @@ static char *local_dn_to_path(TALLOC_CTX *mem_ctx, struct local_db_req { char *path; - struct ldb_dn *basedn; + struct ldb_dn *req_dn; }; #define LOCAL_SIMPLE_FILTER "(type=simple)" @@ -230,9 +230,9 @@ static int local_db_get_simple(TALLOC_CTX *mem_ctx, DEBUG(SSSDBG_TRACE_INTERNAL, "Searching for [%s] at [%s] with scope=base\n", - LOCAL_SIMPLE_FILTER, ldb_dn_get_linearized(lc_req->basedn)); + LOCAL_SIMPLE_FILTER, ldb_dn_get_linearized(lc_req->req_dn)); - ret = ldb_search(lctx->ldb, tmp_ctx, &res, lc_req->basedn, LDB_SCOPE_BASE, + ret = ldb_search(lctx->ldb, tmp_ctx, &res, lc_req->req_dn, LDB_SCOPE_BASE, attrs, "%s", LOCAL_SIMPLE_FILTER); if (ret != EOK) { DEBUG(SSSDBG_TRACE_LIBS, @@ -296,9 +296,9 @@ static int local_db_list_keys(TALLOC_CTX *mem_ctx, DEBUG(SSSDBG_TRACE_INTERNAL, "Searching for [%s] at [%s] with scope=subtree\n", - LOCAL_SIMPLE_FILTER, ldb_dn_get_linearized(lc_req->basedn)); + LOCAL_SIMPLE_FILTER, ldb_dn_get_linearized(lc_req->req_dn)); - ret = ldb_search(lctx->ldb, tmp_ctx, &res, lc_req->basedn, LDB_SCOPE_SUBTREE, + ret = ldb_search(lctx->ldb, tmp_ctx, &res, lc_req->req_dn, LDB_SCOPE_SUBTREE, attrs, "%s", LOCAL_SIMPLE_FILTER); if (ret != EOK) { DEBUG(SSSDBG_TRACE_LIBS, @@ -320,7 +320,7 @@ static int local_db_list_keys(TALLOC_CTX *mem_ctx, } for (unsigned i = 0; i < res->count; i++) { - keys[i] = local_dn_to_path(keys, lc_req->basedn, res->msgs[i]->dn); + keys[i] = local_dn_to_path(keys, lc_req->req_dn, res->msgs[i]->dn); if (!keys[i]) { ret = ENOMEM; goto done; @@ -482,7 +482,7 @@ static int local_db_put_simple(TALLOC_CTX *mem_ctx, ret = ENOMEM; goto done; } - msg->dn = lc_req->basedn; + msg->dn = lc_req->req_dn; /* make sure containers exist */ ret = local_db_check_containers(msg, lctx, msg->dn); @@ -586,9 +586,9 @@ static int local_db_delete(TALLOC_CTX *mem_ctx, DEBUG(SSSDBG_TRACE_INTERNAL, "Searching for [%s] at [%s] with scope=base\n", - LOCAL_CONTAINER_FILTER, ldb_dn_get_linearized(lc_req->basedn)); + LOCAL_CONTAINER_FILTER, ldb_dn_get_linearized(lc_req->req_dn)); - ret = ldb_search(lctx->ldb, tmp_ctx, &res, lc_req->basedn, LDB_SCOPE_BASE, + ret = ldb_search(lctx->ldb, tmp_ctx, &res, lc_req->req_dn, LDB_SCOPE_BASE, attrs, LOCAL_CONTAINER_FILTER); if (ret != EOK) { DEBUG(SSSDBG_TRACE_LIBS, @@ -598,8 +598,8 @@ static int local_db_delete(TALLOC_CTX *mem_ctx, if (res->count == 1) { DEBUG(SSSDBG_TRACE_INTERNAL, - "Searching for children of [%s]\n", ldb_dn_get_linearized(lc_req->basedn)); - ret = ldb_search(lctx->ldb, tmp_ctx, &res, lc_req->basedn, LDB_SCOPE_ONELEVEL, + "Searching for children of [%s]\n", ldb_dn_get_linearized(lc_req->req_dn)); + ret = ldb_search(lctx->ldb, tmp_ctx, &res, lc_req->req_dn, LDB_SCOPE_ONELEVEL, attrs, NULL); if (ret != EOK) { DEBUG(SSSDBG_TRACE_LIBS, @@ -611,13 +611,13 @@ static int local_db_delete(TALLOC_CTX *mem_ctx, ret = EEXIST; DEBUG(SSSDBG_OP_FAILURE, "Failed to remove '%s': Container is not empty\n", - ldb_dn_get_linearized(lc_req->basedn)); + ldb_dn_get_linearized(lc_req->req_dn)); goto done; } } - ret = ldb_delete(lctx->ldb, lc_req->basedn); + ret = ldb_delete(lctx->ldb, lc_req->req_dn); if (ret != EOK) { DEBUG(SSSDBG_TRACE_LIBS, "ldb_delete returned %d: %s\n", ret, ldb_strerror(ret)); @@ -644,7 +644,7 @@ static int local_db_create(TALLOC_CTX *mem_ctx, ret = ENOMEM; goto done; } - msg->dn = lc_req->basedn; + msg->dn = lc_req->req_dn; /* make sure containers exist */ ret = local_db_check_containers(msg, lctx, msg->dn); @@ -759,7 +759,7 @@ static int local_secrets_map_path(TALLOC_CTX *mem_ctx, goto done; } - ret = local_db_dn(mem_ctx, ldb, basedn, lc_req->path, &lc_req->basedn); + ret = local_db_dn(mem_ctx, ldb, basedn, lc_req->path, &lc_req->req_dn); if (ret != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, "Failed to map request to local db DN\n"); From 8e7c2ff65384c70d2077800aca50abed6dc3d440 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <[email protected]> Date: Tue, 4 Apr 2017 15:33:38 +0200 Subject: [PATCH 2/4] SECRETS: make max_secrets work per base DN This would differentiate between out-of-capacity errors for secrets and for KCM as they are two independent trees in KCM. --- src/responder/secrets/local.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/responder/secrets/local.c b/src/responder/secrets/local.c index 232a358..3307d17 100644 --- a/src/responder/secrets/local.c +++ b/src/responder/secrets/local.c @@ -205,6 +205,7 @@ static char *local_dn_to_path(TALLOC_CTX *mem_ctx, struct local_db_req { char *path; + const char *basedn; struct ldb_dn *req_dn; }; @@ -409,7 +410,8 @@ static int local_db_check_containers_nest_level(struct local_context *lctx, } static int local_db_check_number_of_secrets(TALLOC_CTX *mem_ctx, - struct local_context *lctx) + struct local_context *lctx, + struct local_db_req *lc_req) { TALLOC_CTX *tmp_ctx; static const char *attrs[] = { NULL }; @@ -420,7 +422,7 @@ static int local_db_check_number_of_secrets(TALLOC_CTX *mem_ctx, tmp_ctx = talloc_new(mem_ctx); if (!tmp_ctx) return ENOMEM; - dn = ldb_dn_new(tmp_ctx, lctx->ldb, "cn=secrets"); + dn = ldb_dn_new(tmp_ctx, lctx->ldb, lc_req->basedn); if (!dn) { ret = ENOMEM; goto done; @@ -493,7 +495,7 @@ static int local_db_put_simple(TALLOC_CTX *mem_ctx, goto done; } - ret = local_db_check_number_of_secrets(msg, lctx); + ret = local_db_check_number_of_secrets(msg, lctx, lc_req); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "local_db_check_number_of_secrets failed [%d]: %s\n", @@ -703,7 +705,6 @@ static int local_secrets_map_path(TALLOC_CTX *mem_ctx, { int ret; struct local_db_req *lc_req; - const char *basedn; /* be strict for now */ if (secreq->parsed_url.fragment != NULL) { @@ -741,12 +742,12 @@ static int local_secrets_map_path(TALLOC_CTX *mem_ctx, SEC_BASEPATH, sizeof(SEC_BASEPATH) - 1) == 0) { lc_req->path = talloc_strdup(lc_req, secreq->mapped_path + (sizeof(SEC_BASEPATH) - 1)); - basedn = SECRETS_BASEDN; + lc_req->basedn = SECRETS_BASEDN; } else if (strncmp(secreq->mapped_path, SEC_KCM_BASEPATH, sizeof(SEC_KCM_BASEPATH) - 1) == 0) { lc_req->path = talloc_strdup(lc_req, secreq->mapped_path + (sizeof(SEC_KCM_BASEPATH) - 1)); - basedn = KCM_BASEDN; + lc_req->basedn = KCM_BASEDN; } else { ret = EINVAL; goto done; @@ -759,7 +760,7 @@ static int local_secrets_map_path(TALLOC_CTX *mem_ctx, goto done; } - ret = local_db_dn(mem_ctx, ldb, basedn, lc_req->path, &lc_req->req_dn); + ret = local_db_dn(mem_ctx, ldb, lc_req->basedn, lc_req->path, &lc_req->req_dn); if (ret != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, "Failed to map request to local db DN\n"); From 05cf532cb6858403ca8a7c75fc78cf0b1ed413fc Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <[email protected]> Date: Tue, 4 Apr 2017 15:34:17 +0200 Subject: [PATCH 3/4] TESTS: Test that ccaches can be stored after max_secrets is reached for regular non-ccache secrets Test that even when we store the maximum number of secrets, we can still store Kerberos credentials, but only until we reach the max_secrets limit as well. --- src/tests/intg/test_kcm.py | 51 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/src/tests/intg/test_kcm.py b/src/tests/intg/test_kcm.py index 11f80a1..c43943b 100644 --- a/src/tests/intg/test_kcm.py +++ b/src/tests/intg/test_kcm.py @@ -23,12 +23,16 @@ import socket import time import signal +from requests import HTTPError import kdc import krb5utils import config from util import unindent from test_secrets import create_sssd_secrets_fixture +from secrets import SecretsLocalClient + +MAX_SECRETS = 10 class KcmTestEnv(object): def __init__(self, k5kdc, k5util): @@ -108,7 +112,7 @@ def kcm_teardown(): return kcm_pid -def create_sssd_conf(kcm_path, ccache_storage): +def create_sssd_conf(kcm_path, ccache_storage, max_secrets=MAX_SECRETS): return unindent("""\ [sssd] domains = local @@ -120,6 +124,9 @@ def create_sssd_conf(kcm_path, ccache_storage): [kcm] socket_path = {kcm_path} ccache_storage = {ccache_storage} + + [secrets] + max_secrets = {max_secrets} """).format(**locals()) @@ -445,3 +452,45 @@ def test_kcm_sec_kdestroy_nocache(setup_for_kcm_sec, setup_secrets): testenv = setup_for_kcm_sec exercise_subsidiaries(testenv) + +def get_secrets_socket(): + return os.path.join(config.RUNSTATEDIR, "secrets.socket") + + [email protected] +def secrets_cli(request): + sock_path = get_secrets_socket() + cli = SecretsLocalClient(sock_path=sock_path) + return cli + + +def test_kcm_secrets_quota(setup_for_kcm_sec, + setup_secrets, + secrets_cli): + testenv = setup_for_kcm_sec + cli = secrets_cli + + # Make sure the secrets store is depleted first + sec_value = "value" + for x in range(MAX_SECRETS): + cli.set_secret(str(x), sec_value) + + with pytest.raises(HTTPError) as err507: + cli.set_secret(str(MAX_SECRETS), sec_value) + assert str(err507.value).startswith("507") + + # We should still be able to store KCM ccaches, but no more + # than MAX_SECRETS + for x in range(MAX_SECRETS): + princ = "%s%d" % ("kcmtest", x) + testenv.k5kdc.add_principal(princ, princ) + + for x in range(MAX_SECRETS-1): + princ = "%s%d" % ("kcmtest", x) + out, _, _ = testenv.k5util.kinit(princ, princ) + assert out == 0 + + # we stored 0 to MAX_SECRETS-1, storing another one must fail + princ = "%s%d" % ("kcmtest", MAX_SECRETS) + out, _, _ = testenv.k5util.kinit(princ, princ) + assert out != 0 From 049460e043e6e9750d56d6eba8fda629a684869e Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <[email protected]> Date: Tue, 4 Apr 2017 17:07:43 +0200 Subject: [PATCH 4/4] MAN: Document that secrets quotas apply also for storing ccaches through KCM --- src/man/sssd-kcm.8.xml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/man/sssd-kcm.8.xml b/src/man/sssd-kcm.8.xml index 5dc9383..dd5eaeb 100644 --- a/src/man/sssd-kcm.8.xml +++ b/src/man/sssd-kcm.8.xml @@ -135,6 +135,13 @@ systemctl enable sssd-secrets.service </programlisting> Your distribution should already set the dependencies between the services. </para> + <para> + Please note that the sssd-secrets storage enforces a quota + on the number of secrets stored. While the quota is applied + separately for the ccaches and the "regular" user secrets, both + are controlled by the same parameter <quote>max_secrets</quote> + in the <quote>[secrets]</quote> section of sssd.conf. + </para> </refsect1> <refsect1 id='options'>
_______________________________________________ sssd-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
