From 6b3bab516355fdf4cc81e6da9d87ec3818ab190f Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Mon, 5 Jun 2017 16:10:55 +0200 Subject: [PATCH 92/93] SECRETS: Add a new option to control per-UID limits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a new option max_uid_secrets that allows to set a limit of secrets for this particular client so that the user cannot starve other users. Resolves: https://pagure.io/SSSD/sssd/issue/3363 Reviewed-by: Simo Sorce Reviewed-by: Fabiano Fidêncio --- src/confdb/confdb.h | 1 + src/config/SSSDConfig/__init__.py.in | 1 + src/config/cfg_rules.ini | 1 + src/config/etc/sssd.api.conf | 1 + src/man/sssd-secrets.5.xml | 12 +++++ src/responder/secrets/local.c | 93 ++++++++++++++++++++++++++++++++++++ src/responder/secrets/secsrv.c | 23 ++++++++- src/responder/secrets/secsrv.h | 1 + src/tests/intg/test_secrets.py | 46 ++++++++++++++++++ 9 files changed, 178 insertions(+), 1 deletion(-) diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h index 4abc95b8183f1b430f770b55e8af0e43f65889a3..bcea99ae49a3fa5f0393ce6b2c215b5b2d4bc3fc 100644 --- a/src/confdb/confdb.h +++ b/src/confdb/confdb.h @@ -246,6 +246,7 @@ #define CONFDB_SEC_CONF_ENTRY "config/secrets" #define CONFDB_SEC_CONTAINERS_NEST_LEVEL "containers_nest_level" #define CONFDB_SEC_MAX_SECRETS "max_secrets" +#define CONFDB_SEC_MAX_UID_SECRETS "max_uid_secrets" #define CONFDB_SEC_MAX_PAYLOAD_SIZE "max_payload_size" /* KCM Service */ diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in index 8c56e4efa4ae7c648f670bb6a67290b6e835f581..227f76180686d33cf87aeed55232f33eb02f138f 100644 --- a/src/config/SSSDConfig/__init__.py.in +++ b/src/config/SSSDConfig/__init__.py.in @@ -129,6 +129,7 @@ option_strings = { 'provider': _('The provider where the secrets will be stored in'), 'containers_nest_level': _('The maximum allowed number of nested containers'), 'max_secrets': _('The maximum number of secrets that can be stored'), + 'max_uid_secrets': _('The maximum number of secrets that can be stored per UID'), 'max_payload_size': _('The maximum payload size of a secret in kilobytes'), # secrets - proxy 'proxy_url': _('The URL Custodia server is listening on'), diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini index 7590f1f5aa516c0af899371a1b7a826512469de3..3e4ce46734a6686bb6ad38f52710def4f069d296 100644 --- a/src/config/cfg_rules.ini +++ b/src/config/cfg_rules.ini @@ -265,6 +265,7 @@ section_re = ^secrets/\(secrets\|kcm\)$ # Secrets service - per-hive configuration option = containers_nest_level option = max_secrets +option = max_uid_secrets option = max_payload_size [rule/allowed_sec_users_options] diff --git a/src/config/etc/sssd.api.conf b/src/config/etc/sssd.api.conf index 9eb6aeb83bbc1989cec7465e6442a1bf7762d9d8..792c42a1f01200d49c14dcba9516af0011e6e9c8 100644 --- a/src/config/etc/sssd.api.conf +++ b/src/config/etc/sssd.api.conf @@ -106,6 +106,7 @@ user_attributes = str, None, false provider = str, None, false containers_nest_level = int, None, false max_secrets = int, None, false +max_uid_secrets = int, None, false max_payload_size = int, None, false # Secrets service - proxy proxy_url = str, None, false diff --git a/src/man/sssd-secrets.5.xml b/src/man/sssd-secrets.5.xml index ba77d623274237951de5d42bb8cff9f6d56f5fff..c74894c62ed70764ca680c3b1cfe7f903d280277 100644 --- a/src/man/sssd-secrets.5.xml +++ b/src/man/sssd-secrets.5.xml @@ -200,6 +200,18 @@ systemctl enable sssd-secrets.service + + max_uid_secrets (integer) + + + This option specifies the maximum number of secrets that + can be stored per-UID in the hive. + + + Default: 256 (secrets hive), 64 (kcm hive) + + + max_payload_size (integer) diff --git a/src/responder/secrets/local.c b/src/responder/secrets/local.c index 58e70f8b6d00976ccc86d4fbf687417dd3c3c06a..5e491ba98fdc5612db0c303258513302c1f1d9e3 100644 --- a/src/responder/secrets/local.c +++ b/src/responder/secrets/local.c @@ -412,6 +412,85 @@ static int local_db_check_containers_nest_level(struct local_db_req *lc_req, return EOK; } +static struct ldb_dn *per_uid_container(TALLOC_CTX *mem_ctx, + struct ldb_dn *req_dn) +{ + int user_comp; + int num_comp; + struct ldb_dn *uid_base_dn; + + uid_base_dn = ldb_dn_copy(mem_ctx, req_dn); + if (uid_base_dn == NULL) { + return NULL; + } + + /* Remove all the components up to the per-user base path which consists + * of three components: + * cn=,cn=users,cn=secrets + */ + user_comp = ldb_dn_get_comp_num(uid_base_dn) - 3; + + if (!ldb_dn_remove_child_components(uid_base_dn, user_comp)) { + DEBUG(SSSDBG_OP_FAILURE, "Cannot remove child components\n"); + talloc_free(uid_base_dn); + return NULL; + } + + num_comp = ldb_dn_get_comp_num(uid_base_dn); + if (num_comp != 3) { + DEBUG(SSSDBG_OP_FAILURE, "Expected 3 components got %d\n", num_comp); + talloc_free(uid_base_dn); + return NULL; + } + + return uid_base_dn; +} + +static int local_db_check_peruid_number_of_secrets(TALLOC_CTX *mem_ctx, + struct local_context *lctx, + struct local_db_req *lc_req) +{ + TALLOC_CTX *tmp_ctx; + static const char *attrs[] = { NULL }; + struct ldb_result *res = NULL; + struct ldb_dn *cli_basedn = NULL; + int ret; + + tmp_ctx = talloc_new(mem_ctx); + if (tmp_ctx == NULL) { + return ENOMEM; + } + + cli_basedn = per_uid_container(tmp_ctx, lc_req->req_dn); + if (cli_basedn == NULL) { + ret = ENOMEM; + goto done; + } + + ret = ldb_search(lctx->ldb, tmp_ctx, &res, cli_basedn, LDB_SCOPE_SUBTREE, + attrs, LOCAL_SIMPLE_FILTER); + if (ret != EOK) { + DEBUG(SSSDBG_TRACE_LIBS, + "ldb_search returned %d: %s\n", ret, ldb_strerror(ret)); + goto done; + } + + if (res->count >= lc_req->quota->max_uid_secrets) { + DEBUG(SSSDBG_OP_FAILURE, + "Cannot store any more secrets for this client (basedn %s) " + "as the maximum allowed limit (%d) has been reached\n", + ldb_dn_get_linearized(cli_basedn), + lc_req->quota->max_uid_secrets); + ret = ERR_SEC_INVALID_TOO_MANY_SECRETS; + goto done; + } + + ret = EOK; +done: + talloc_free(tmp_ctx); + return ret; +} + static int local_db_check_number_of_secrets(TALLOC_CTX *mem_ctx, struct local_context *lctx, struct local_db_req *lc_req) @@ -433,6 +512,12 @@ static int local_db_check_number_of_secrets(TALLOC_CTX *mem_ctx, ret = ldb_search(lctx->ldb, tmp_ctx, &res, dn, LDB_SCOPE_SUBTREE, attrs, LOCAL_SIMPLE_FILTER); + if (ret != EOK) { + DEBUG(SSSDBG_TRACE_LIBS, + "ldb_search returned %d: %s\n", ret, ldb_strerror(ret)); + goto done; + } + if (res->count >= lc_req->quota->max_secrets) { DEBUG(SSSDBG_OP_FAILURE, "Cannot store any more secrets as the maximum allowed limit (%d) " @@ -505,6 +590,14 @@ static int local_db_put_simple(TALLOC_CTX *mem_ctx, goto done; } + ret = local_db_check_peruid_number_of_secrets(msg, lctx, lc_req); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "local_db_check_number_of_secrets failed [%d]: %s\n", + ret, sss_strerror(ret)); + goto done; + } + ret = local_check_max_payload_size(lc_req, strlen(secret)); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, diff --git a/src/responder/secrets/secsrv.c b/src/responder/secrets/secsrv.c index 2fcdf8e6c74eaccc75f1017efdc854fa065baf74..36b257c463ccaa1f552b2b4985932dc0d3b125aa 100644 --- a/src/responder/secrets/secsrv.c +++ b/src/responder/secrets/secsrv.c @@ -31,7 +31,8 @@ #define DEFAULT_SEC_FD_LIMIT 2048 #define DEFAULT_SEC_CONTAINERS_NEST_LEVEL 4 -#define DEFAULT_SEC_MAX_SECRETS 1024 +#define DEFAULT_SEC_MAX_SECRETS 1024 +#define DEFAULT_SEC_MAX_UID_SECRETS 256 #define DEFAULT_SEC_MAX_PAYLOAD_SIZE 16 /* The number of secrets in the /kcm hive should be quite small, @@ -39,12 +40,14 @@ * hive holds the whole ccache which consists of several credentials */ #define DEFAULT_SEC_KCM_MAX_SECRETS 256 +#define DEFAULT_SEC_KCM_MAX_UID_SECRETS 64 #define DEFAULT_SEC_KCM_MAX_PAYLOAD_SIZE 65536 static int sec_get_quota(struct sec_ctx *sctx, const char *section_config_path, int default_max_containers_nest_level, int default_max_num_secrets, + int default_max_num_uid_secrets, int default_max_payload, struct sec_quota *quota) { @@ -76,6 +79,19 @@ static int sec_get_quota(struct sec_ctx *sctx, return ret; } + ret = confdb_get_int(sctx->rctx->cdb, + section_config_path, + CONFDB_SEC_MAX_UID_SECRETS, + default_max_num_uid_secrets, + "a->max_uid_secrets); + + if (ret != EOK) { + DEBUG(SSSDBG_FATAL_FAILURE, + "Failed to get maximum number of per-UID entries for %s\n", + section_config_path); + return ret; + } + ret = confdb_get_int(sctx->rctx->cdb, section_config_path, CONFDB_SEC_MAX_PAYLOAD_SIZE, @@ -97,6 +113,7 @@ static int sec_get_hive_config(struct sec_ctx *sctx, struct sec_hive_config *hive_config, int default_max_containers_nest_level, int default_max_num_secrets, + int default_max_num_uid_secrets, int default_max_payload) { int ret; @@ -119,6 +136,7 @@ static int sec_get_hive_config(struct sec_ctx *sctx, hive_config->confdb_section, default_max_containers_nest_level, default_max_num_secrets, + default_max_num_uid_secrets, default_max_payload, &hive_config->quota); if (ret != EOK) { @@ -158,6 +176,7 @@ static int sec_get_config(struct sec_ctx *sctx) sctx->rctx->confdb_service_path, DEFAULT_SEC_CONTAINERS_NEST_LEVEL, DEFAULT_SEC_MAX_SECRETS, + DEFAULT_SEC_MAX_UID_SECRETS, DEFAULT_SEC_MAX_PAYLOAD_SIZE, &sctx->sec_config.quota); if (ret != EOK) { @@ -172,6 +191,7 @@ static int sec_get_config(struct sec_ctx *sctx) &sctx->sec_config, sctx->sec_config.quota.containers_nest_level, sctx->sec_config.quota.max_secrets, + sctx->sec_config.quota.max_uid_secrets, sctx->sec_config.quota.max_payload_size); if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, @@ -184,6 +204,7 @@ static int sec_get_config(struct sec_ctx *sctx) &sctx->kcm_config, DEFAULT_SEC_CONTAINERS_NEST_LEVEL, DEFAULT_SEC_KCM_MAX_SECRETS, + DEFAULT_SEC_KCM_MAX_UID_SECRETS, DEFAULT_SEC_KCM_MAX_PAYLOAD_SIZE); if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, diff --git a/src/responder/secrets/secsrv.h b/src/responder/secrets/secsrv.h index afc092764d02671eaf2cadd6a0f2f168ba7da806..afdd731fbd44d7bb280ffc0e55db9c39a926bf22 100644 --- a/src/responder/secrets/secsrv.h +++ b/src/responder/secrets/secsrv.h @@ -32,6 +32,7 @@ struct sec_quota { int max_secrets; + int max_uid_secrets; int max_payload_size; int containers_nest_level; }; diff --git a/src/tests/intg/test_secrets.py b/src/tests/intg/test_secrets.py index bb94ffb47666f964fae2764444f7d28f3b311145..957a0a8ff9ce5e966b77ddf048eefc282b2711b6 100644 --- a/src/tests/intg/test_secrets.py +++ b/src/tests/intg/test_secrets.py @@ -499,3 +499,49 @@ def test_sec_quota(setup_for_secrets_quota, secrets_cli): # Don't allow storing more secrets after reaching the max # number of entries. run_quota_test(cli, 10, 2) + + +@pytest.fixture +def setup_for_uid_limit(request): + conf = unindent("""\ + [sssd] + domains = local + services = nss + + [domain/local] + id_provider = local + + [secrets] + + [secrets/secrets] + max_secrets = 10 + max_uid_secrets = 5 + """).format(**locals()) + + create_conf_fixture(request, conf) + create_sssd_secrets_fixture(request) + return None + + +def test_per_uid_limit(setup_for_uid_limit, secrets_cli): + """ + Test that per-UID limits are enforced even if the global limit would still + allow to store more secrets + """ + cli = secrets_cli + + # Don't allow storing more secrets after reaching the max + # number of entries. + MAX_UID_SECRETS = 5 + + sec_value = "value" + for i in range(MAX_UID_SECRETS): + cli.set_secret(str(i), sec_value) + + with pytest.raises(HTTPError) as err507: + cli.set_secret(str(MAX_UID_SECRETS), sec_value) + assert str(err507.value).startswith("507") + + # FIXME - at this point, it would be nice to test that another UID can + # still store secrets, but sadly socket_wrapper doesn't allow us to fake + # UIDs yet -- 2.14.1