376 lines
14 KiB
Diff
376 lines
14 KiB
Diff
From 6b3bab516355fdf4cc81e6da9d87ec3818ab190f Mon Sep 17 00:00:00 2001
|
|
From: Jakub Hrozek <jhrozek@redhat.com>
|
|
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 <simo@redhat.com>
|
|
Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>
|
|
---
|
|
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
|
|
</para>
|
|
</listitem>
|
|
</varlistentry>
|
|
+ <varlistentry>
|
|
+ <term>max_uid_secrets (integer)</term>
|
|
+ <listitem>
|
|
+ <para>
|
|
+ This option specifies the maximum number of secrets that
|
|
+ can be stored per-UID in the hive.
|
|
+ </para>
|
|
+ <para>
|
|
+ Default: 256 (secrets hive), 64 (kcm hive)
|
|
+ </para>
|
|
+ </listitem>
|
|
+ </varlistentry>
|
|
<varlistentry>
|
|
<term>max_payload_size (integer)</term>
|
|
<listitem>
|
|
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=<uidnumber>,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
|
|
|