URL: https://github.com/SSSD/sssd/pull/225
Author: jhrozek
Title: #225: SECRETS: Apply separate quotas for cn=secrets and cn=kcm
Action: synchronized
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 45dc03220c333d06c0cf147167e47a56db217059 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Tue, 30 May 2017 12:19:53 +0200
Subject: [PATCH 1/9] SECRETS: Remove unused declarations
---
src/responder/secrets/secsrv.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/responder/secrets/secsrv.h b/src/responder/secrets/secsrv.h
index 3d23c405b..0575cbaba 100644
--- a/src/responder/secrets/secsrv.h
+++ b/src/responder/secrets/secsrv.h
@@ -32,8 +32,6 @@
#define SEC_NET_TIMEOUT 5
-struct resctx;
-
struct sec_ctx {
struct resolv_ctx *resctx;
struct resp_ctx *rctx;
From a9f4aa3e544001cd2f963957cfdc2213834e4d23 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Mon, 5 Jun 2017 15:19:13 +0200
Subject: [PATCH 2/9] SECRETS: Do not link with c-ares
Since we started using libcurl for the proxy provider, there is no point
in initializing or linking against c-ares.
If we want to explicitly use a resolver in the future, we should use
libcurl callbacks.
---
Makefile.am | 1 -
src/responder/secrets/proxy.c | 2 --
src/responder/secrets/secsrv.c | 6 ------
src/responder/secrets/secsrv.h | 3 ---
4 files changed, 12 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index e7d69d2f0..9e3b492e8 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1494,7 +1494,6 @@ sssd_secrets_SOURCES = \
src/util/sss_iobuf.c \
src/util/tev_curl.c \
$(SSSD_RESPONDER_OBJ) \
- $(SSSD_RESOLV_OBJ) \
$(NULL)
sssd_secrets_LDADD = \
$(HTTP_PARSER_LIBS) \
diff --git a/src/responder/secrets/proxy.c b/src/responder/secrets/proxy.c
index a4e97f83e..a910b3853 100644
--- a/src/responder/secrets/proxy.c
+++ b/src/responder/secrets/proxy.c
@@ -29,7 +29,6 @@
#define SEC_PROXY_TIMEOUT 5
struct proxy_context {
- struct resolv_ctx *resctx;
struct confdb_ctx *cdb;
struct tcurl_ctx *tcurl;
};
@@ -585,7 +584,6 @@ int proxy_secrets_provider_handle(struct sec_ctx *sctx,
pctx = talloc(handle, struct proxy_context);
if (!pctx) return ENOMEM;
- pctx->resctx = sctx->resctx;
pctx->cdb = sctx->rctx->cdb;
pctx->tcurl = tcurl_init(pctx, sctx->rctx->ev);
if (pctx->tcurl == NULL) {
diff --git a/src/responder/secrets/secsrv.c b/src/responder/secrets/secsrv.c
index b0467e90e..ae2a658ae 100644
--- a/src/responder/secrets/secsrv.c
+++ b/src/responder/secrets/secsrv.c
@@ -162,12 +162,6 @@ static int sec_process_init(TALLOC_CTX *mem_ctx,
goto fail;
}
- ret = resolv_init(sctx, ev, SEC_NET_TIMEOUT, &sctx->resctx);
- if (ret != EOK) {
- /* not fatal for now */
- DEBUG(SSSDBG_CRIT_FAILURE, "Failed to initialize resolver library\n");
- }
-
/* Set up file descriptor limits */
responder_set_fd_limit(sctx->fd_limit);
diff --git a/src/responder/secrets/secsrv.h b/src/responder/secrets/secsrv.h
index 0575cbaba..1aad272da 100644
--- a/src/responder/secrets/secsrv.h
+++ b/src/responder/secrets/secsrv.h
@@ -30,10 +30,7 @@
#include <tevent.h>
#include <ldb.h>
-#define SEC_NET_TIMEOUT 5
-
struct sec_ctx {
- struct resolv_ctx *resctx;
struct resp_ctx *rctx;
int fd_limit;
int containers_nest_level;
From 90b768e06daa7c719ba6b94a58ecadead00cacea Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Tue, 30 May 2017 12:31:57 +0200
Subject: [PATCH 3/9] SECRETS: Store quotas in a per-hive configuration
structure
Adds two new structures to hold the quotas and associate a quota with a hive.
This is just an internal change for now, but will allow us to read quota
configuration from per-hive sections later.
---
src/responder/secrets/local.c | 21 +++++++++------------
src/responder/secrets/secsrv.c | 6 +++---
src/responder/secrets/secsrv.h | 17 ++++++++++++++---
3 files changed, 26 insertions(+), 18 deletions(-)
diff --git a/src/responder/secrets/local.c b/src/responder/secrets/local.c
index 66401ef50..0b879939f 100644
--- a/src/responder/secrets/local.c
+++ b/src/responder/secrets/local.c
@@ -34,9 +34,8 @@
struct local_context {
struct ldb_context *ldb;
struct sec_data master_key;
- int containers_nest_level;
- int max_secrets;
- int max_payload_size;
+
+ struct sec_quota *quota_secrets;
};
static int local_decrypt(struct local_context *lctx, TALLOC_CTX *mem_ctx,
@@ -398,11 +397,11 @@ static int local_db_check_containers_nest_level(struct local_context *lctx,
/* We need do not care for the synthetic containers that constitute the
* base path (cn=<uidnumber>,cn=user,cn=secrets). */
nest_level = ldb_dn_get_comp_num(leaf_dn) - 3;
- if (nest_level > lctx->containers_nest_level) {
+ if (nest_level > lctx->quota_secrets->containers_nest_level) {
DEBUG(SSSDBG_OP_FAILURE,
"Cannot create a nested container of depth %d as the maximum"
"allowed number of nested containers is %d.\n",
- nest_level, lctx->containers_nest_level);
+ nest_level, lctx->quota_secrets->containers_nest_level);
return ERR_SEC_INVALID_CONTAINERS_NEST_LEVEL;
}
@@ -430,10 +429,10 @@ 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 (res->count >= lctx->max_secrets) {
+ if (res->count >= lctx->quota_secrets->max_secrets) {
DEBUG(SSSDBG_OP_FAILURE,
"Cannot store any more secrets as the maximum allowed limit (%d) "
- "has been reached\n", lctx->max_secrets);
+ "has been reached\n", lctx->quota_secrets->max_secrets);
ret = ERR_SEC_INVALID_TOO_MANY_SECRETS;
goto done;
@@ -451,14 +450,14 @@ static int local_check_max_payload_size(struct local_context *lctx,
{
int max_payload_size;
- max_payload_size = lctx->max_payload_size * 1024; /* kb */
+ max_payload_size = lctx->quota_secrets->max_payload_size * 1024; /* kb */
if (payload_size > max_payload_size) {
DEBUG(SSSDBG_OP_FAILURE,
"Secrets' payload size [%d kb (%d)] exceeds the maximum allowed "
"payload size [%d kb (%d)]\n",
payload_size * 1024, /* kb */
payload_size,
- lctx->max_payload_size, /* kb */
+ lctx->quota_secrets->max_payload_size, /* kb */
max_payload_size);
return ERR_SEC_PAYLOAD_SIZE_IS_TOO_LARGE;
@@ -1019,9 +1018,7 @@ int local_secrets_provider_handle(struct sec_ctx *sctx,
return EIO;
}
- lctx->containers_nest_level = sctx->containers_nest_level;
- lctx->max_secrets = sctx->max_secrets;
- lctx->max_payload_size = sctx->max_payload_size;
+ lctx->quota_secrets = &sctx->sec_config.quota;
lctx->master_key.data = talloc_size(lctx, MKEY_SIZE);
if (!lctx->master_key.data) return ENOMEM;
diff --git a/src/responder/secrets/secsrv.c b/src/responder/secrets/secsrv.c
index ae2a658ae..e3a8c1476 100644
--- a/src/responder/secrets/secsrv.c
+++ b/src/responder/secrets/secsrv.c
@@ -52,7 +52,7 @@ static int sec_get_config(struct sec_ctx *sctx)
sctx->rctx->confdb_service_path,
CONFDB_SEC_CONTAINERS_NEST_LEVEL,
DEFAULT_SEC_CONTAINERS_NEST_LEVEL,
- &sctx->containers_nest_level);
+ &sctx->sec_config.quota.containers_nest_level);
if (ret != EOK) {
DEBUG(SSSDBG_FATAL_FAILURE,
@@ -64,7 +64,7 @@ static int sec_get_config(struct sec_ctx *sctx)
sctx->rctx->confdb_service_path,
CONFDB_SEC_MAX_SECRETS,
DEFAULT_SEC_MAX_SECRETS,
- &sctx->max_secrets);
+ &sctx->sec_config.quota.max_secrets);
if (ret != EOK) {
DEBUG(SSSDBG_FATAL_FAILURE,
@@ -76,7 +76,7 @@ static int sec_get_config(struct sec_ctx *sctx)
sctx->rctx->confdb_service_path,
CONFDB_SEC_MAX_PAYLOAD_SIZE,
DEFAULT_SEC_MAX_PAYLOAD_SIZE,
- &sctx->max_payload_size);
+ &sctx->sec_config.quota.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 1aad272da..629b027f6 100644
--- a/src/responder/secrets/secsrv.h
+++ b/src/responder/secrets/secsrv.h
@@ -30,12 +30,23 @@
#include <tevent.h>
#include <ldb.h>
+struct sec_quota {
+ int max_secrets;
+ int max_payload_size;
+ int containers_nest_level;
+};
+
+struct sec_hive_config {
+ const char *confdb_section;
+
+ struct sec_quota quota;
+};
+
struct sec_ctx {
struct resp_ctx *rctx;
int fd_limit;
- int containers_nest_level;
- int max_secrets;
- int max_payload_size;
+
+ struct sec_hive_config sec_config;
struct provider_handle **providers;
};
From cf3f7e5c6a254b9a2ab3bbd5dd68dcfb52e94f4f Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Tue, 30 May 2017 12:51:19 +0200
Subject: [PATCH 4/9] SECRETS: Read the quotas for cn=secrets from
[secrets/secrets] configuration subsection
This patch makes obsoletes the old way of configuring quotas for the
secrets responder. Instead, adds a new way of configuring each hive
separately in a configuration subsection, e.g.
[secrets/secrets]
max_secrets = 123
The old way is still supported as a backwards-compatible method.
---
src/config/cfg_rules.ini | 10 ++++
src/man/sssd-secrets.5.xml | 45 +++++++++++++-
src/responder/secrets/secsrv.c | 133 ++++++++++++++++++++++++++++++++++-------
src/tests/intg/test_secrets.py | 124 ++++++++++++++++++++++++++++----------
4 files changed, 258 insertions(+), 54 deletions(-)
diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
index 4537d0fe8..7db1d9711 100644
--- a/src/config/cfg_rules.ini
+++ b/src/config/cfg_rules.ini
@@ -12,6 +12,7 @@ section = secrets
section = kcm
section = session_recording
section_re = ^secrets/users/[0-9]\+$
+section_re = ^secrets/secrets$
section_re = ^domain/[^/\@]\+$
section_re = ^domain/[^/\@]\+/[^/\@]\+$
section_re = ^application/[^/\@]\+$
@@ -255,6 +256,15 @@ option = max_secrets
option = max_payload_size
option = responder_idle_timeout
+[rule/allowed_sec_hive_options]
+validator = ini_allowed_options
+section_re = ^secrets/secrets$
+
+# Secrets service - per-hive configuration
+option = containers_nest_level
+option = max_secrets
+option = max_payload_size
+
[rule/allowed_sec_users_options]
validator = ini_allowed_options
section_re = ^secrets/users/[0-9]\+$
diff --git a/src/man/sssd-secrets.5.xml b/src/man/sssd-secrets.5.xml
index 44a86c3fb..d50cb13d8 100644
--- a/src/man/sssd-secrets.5.xml
+++ b/src/man/sssd-secrets.5.xml
@@ -57,6 +57,32 @@
collide between users. Secrets can be stored inside
<quote>containers</quote> which can be nested.
</para>
+ <para>
+ Since the secrets responder can be used both externally to store
+ general secrets, as described in the rest of this man page, but
+ also internally by other SSSD components to store their secret
+ material, some configuration options, like quotas can be configured
+ per <quote>hive</quote> in a configuration subsection named after
+ the hive. The currently supported hives are:
+ <variablelist>
+ <varlistentry>
+ <term>secrets</term>
+ <listitem><para>secrets for general usage</para></listitem>
+ </varlistentry>
+ <varlistentry>
+ <term>kcm</term>
+ <listitem>
+ <para>used by the
+ <citerefentry>
+ <refentrytitle>sssd-kcm</refentrytitle>
+ <manvolnum>8</manvolnum>
+ </citerefentry>
+ service.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </para>
</refsect1>
<refsect1 id='usage'>
@@ -144,6 +170,12 @@ systemctl enable sssd-secrets.service
</para>
</listitem>
</varlistentry>
+ </variablelist>
+ <para>
+ The following options affect only the secrets <quote>hive</quote>
+ and therefore should be set in a per-hive subsection.
+ </para>
+ <variablelist>
<varlistentry>
<term>containers_nest_level (integer)</term>
<listitem>
@@ -161,7 +193,7 @@ systemctl enable sssd-secrets.service
<listitem>
<para>
This option specifies the maximum number of secrets that
- can be stored.
+ can be stored in the hive.
</para>
<para>
Default: 1024
@@ -182,6 +214,17 @@ systemctl enable sssd-secrets.service
</varlistentry>
</variablelist>
<para>
+ For example, to adjust quotas differently for both the <quote>secrets</quote>
+ and the <quote>kcm</quote> hives, configure the following:
+ <programlisting>
+[secrets/secrets]
+max_payload_size = 128
+
+[secrets/kcm]
+max_payload_size = 256
+ </programlisting>
+ </para>
+ <para>
The following options are only applicable for configurations that
use the <quote>proxy</quote> provider.
</para>
diff --git a/src/responder/secrets/secsrv.c b/src/responder/secrets/secsrv.c
index e3a8c1476..db12cbbc3 100644
--- a/src/responder/secrets/secsrv.c
+++ b/src/responder/secrets/secsrv.c
@@ -33,54 +33,141 @@
#define DEFAULT_SEC_MAX_SECRETS 1024
#define DEFAULT_SEC_MAX_PAYLOAD_SIZE 16
-static int sec_get_config(struct sec_ctx *sctx)
+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_payload,
+ struct sec_quota *quota)
{
int ret;
ret = confdb_get_int(sctx->rctx->cdb,
- sctx->rctx->confdb_service_path,
- CONFDB_SERVICE_FD_LIMIT,
- DEFAULT_SEC_FD_LIMIT,
- &sctx->fd_limit);
+ section_config_path,
+ CONFDB_SEC_CONTAINERS_NEST_LEVEL,
+ default_max_containers_nest_level,
+ "a->containers_nest_level);
+
if (ret != EOK) {
DEBUG(SSSDBG_FATAL_FAILURE,
- "Failed to get file descriptors limit\n");
- goto fail;
+ "Failed to get container nesting level for %s\n",
+ section_config_path);
+ return ret;
}
ret = confdb_get_int(sctx->rctx->cdb,
- sctx->rctx->confdb_service_path,
- CONFDB_SEC_CONTAINERS_NEST_LEVEL,
- DEFAULT_SEC_CONTAINERS_NEST_LEVEL,
- &sctx->sec_config.quota.containers_nest_level);
+ section_config_path,
+ CONFDB_SEC_MAX_SECRETS,
+ default_max_num_secrets,
+ "a->max_secrets);
if (ret != EOK) {
DEBUG(SSSDBG_FATAL_FAILURE,
- "Failed to get containers' maximum depth\n");
- goto fail;
+ "Failed to get maximum number of entries for %s\n",
+ section_config_path);
+ return ret;
}
ret = confdb_get_int(sctx->rctx->cdb,
- sctx->rctx->confdb_service_path,
- CONFDB_SEC_MAX_SECRETS,
- DEFAULT_SEC_MAX_SECRETS,
- &sctx->sec_config.quota.max_secrets);
+ section_config_path,
+ CONFDB_SEC_MAX_PAYLOAD_SIZE,
+ default_max_payload,
+ "a->max_payload_size);
if (ret != EOK) {
DEBUG(SSSDBG_FATAL_FAILURE,
- "Failed to get maximum number of entries\n");
- goto fail;
+ "Failed to get payload's maximum size for an entry in %s\n",
+ section_config_path);
+ return ret;
+ }
+
+ return EOK;
+}
+
+static int sec_get_hive_config(struct sec_ctx *sctx,
+ const char *hive_name,
+ struct sec_hive_config *hive_config,
+ int default_max_containers_nest_level,
+ int default_max_num_secrets,
+ int default_max_payload)
+{
+ int ret;
+ TALLOC_CTX *tmp_ctx;
+
+ tmp_ctx = talloc_new(sctx);
+ if (tmp_ctx == NULL) {
+ return ENOMEM;
+ }
+
+ hive_config->confdb_section = talloc_asprintf(sctx,
+ "config/secrets/%s",
+ hive_name);
+ if (hive_config->confdb_section == NULL) {
+ ret = ENOMEM;
+ goto done;
}
+ ret = sec_get_quota(sctx,
+ hive_config->confdb_section,
+ default_max_containers_nest_level,
+ default_max_num_secrets,
+ default_max_payload,
+ &hive_config->quota);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE,
+ "Cannot read quota settings for %s [%d]: %s\n",
+ hive_name, ret, sss_strerror(ret));
+ goto done;
+ }
+
+ ret = EOK;
+
+done:
+ talloc_free(tmp_ctx);
+ return ret;
+}
+
+static int sec_get_config(struct sec_ctx *sctx)
+{
+ int ret;
+
ret = confdb_get_int(sctx->rctx->cdb,
sctx->rctx->confdb_service_path,
- CONFDB_SEC_MAX_PAYLOAD_SIZE,
- DEFAULT_SEC_MAX_PAYLOAD_SIZE,
- &sctx->sec_config.quota.max_payload_size);
+ CONFDB_SERVICE_FD_LIMIT,
+ DEFAULT_SEC_FD_LIMIT,
+ &sctx->fd_limit);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_FATAL_FAILURE,
+ "Failed to get file descriptors limit\n");
+ goto fail;
+ }
+
+ /* Read the global quota first -- this should be removed in a future release */
+ /* Note that this sets the defaults for the sec_config quota to be used
+ * in sec_get_hive_config()
+ */
+ ret = sec_get_quota(sctx,
+ sctx->rctx->confdb_service_path,
+ DEFAULT_SEC_CONTAINERS_NEST_LEVEL,
+ DEFAULT_SEC_MAX_SECRETS,
+ DEFAULT_SEC_MAX_PAYLOAD_SIZE,
+ &sctx->sec_config.quota);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_FATAL_FAILURE,
+ "Failed to get legacy global quotas\n");
+ goto fail;
+ }
+ /* Read the per-hive configuration */
+ ret = sec_get_hive_config(sctx,
+ "secrets",
+ &sctx->sec_config,
+ sctx->sec_config.quota.containers_nest_level,
+ sctx->sec_config.quota.max_secrets,
+ sctx->sec_config.quota.max_payload_size);
if (ret != EOK) {
DEBUG(SSSDBG_FATAL_FAILURE,
- "Failed to get payload's maximum size for an entry\n");
+ "Failed to get configuration of the secrets hive\n");
goto fail;
}
diff --git a/src/tests/intg/test_secrets.py b/src/tests/intg/test_secrets.py
index a66431ff4..0f997cb25 100644
--- a/src/tests/intg/test_secrets.py
+++ b/src/tests/intg/test_secrets.py
@@ -169,36 +169,6 @@ def test_crd_ops(setup_for_secrets, secrets_cli):
cli.del_secret("foo")
assert str(err404.value).startswith("404")
- # Don't allow storing more secrets after reaching the max
- # number of entries.
- MAX_SECRETS = 10
-
- 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")
-
- # Delete all stored secrets used for max secrets tests
- for x in range(MAX_SECRETS):
- cli.del_secret(str(x))
-
- # Don't allow storing a secrets which has a payload larger
- # than max_payload_size
- KILOBYTE = 1024
- MAX_PAYLOAD_SIZE = 2 * KILOBYTE
-
- sec_value = "x" * MAX_PAYLOAD_SIZE
-
- cli.set_secret("foo", sec_value)
-
- sec_value += "x"
- with pytest.raises(HTTPError) as err413:
- cli.set_secret("bar", sec_value)
- assert str(err413.value).startswith("413")
-
def run_curlwrap_tool(args, exp_http_code):
cmd = subprocess.Popen(args,
@@ -434,3 +404,97 @@ def test_idle_timeout(setup_for_cli_timeout_test):
nfds_post = get_num_fds(secpid)
assert nfds_pre == nfds_post
+
+def run_quota_test(cli, max_secrets, max_payload_size):
+ 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")
+
+ # Delete all stored secrets used for max secrets tests
+ for x in range(max_secrets):
+ cli.del_secret(str(x))
+
+ # Don't allow storing a secrets which has a payload larger
+ # than max_payload_size
+ KILOBYTE = 1024
+ kb_payload_size = max_payload_size * KILOBYTE
+
+ sec_value = "x" * kb_payload_size
+
+ cli.set_secret("foo", sec_value)
+
+ sec_value += "x"
+ with pytest.raises(HTTPError) as err413:
+ cli.set_secret("bar", sec_value)
+ assert str(err413.value).startswith("413")
+
+
+@pytest.fixture
+def setup_for_global_quota(request):
+ conf = unindent("""\
+ [sssd]
+ domains = local
+ services = nss
+
+ [domain/local]
+ id_provider = local
+
+ [secrets]
+ max_secrets = 10
+ max_payload_size = 2
+ """).format(**locals())
+
+ create_conf_fixture(request, conf)
+ create_sssd_secrets_fixture(request)
+ return None
+
+
+def test_global_quota(setup_for_global_quota, secrets_cli):
+ """
+ Test that the deprecated configuration of quotas in the global
+ secrets section is still supported
+ """
+ cli = 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_secrets_quota(request):
+ conf = unindent("""\
+ [sssd]
+ domains = local
+ services = nss
+
+ [domain/local]
+ id_provider = local
+
+ [secrets]
+ max_secrets = 5
+ max_payload_size = 1
+
+ [secrets/secrets]
+ max_secrets = 10
+ max_payload_size = 2
+ """).format(**locals())
+
+ create_conf_fixture(request, conf)
+ create_sssd_secrets_fixture(request)
+ return None
+
+
+def test_sec_quota(setup_for_secrets_quota, secrets_cli):
+ """
+ Test that the new secrets/secrets section takes precedence.
+ """
+ cli = secrets_cli
+
+ # Don't allow storing more secrets after reaching the max
+ # number of entries.
+ run_quota_test(cli, 10, 2)
From a3a0c6e8487247b0a5b61d27b1a3e3f432946139 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Tue, 4 Apr 2017 14:45:30 +0200
Subject: [PATCH 5/9] 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 0b879939f..c833f1d27 100644
--- a/src/responder/secrets/local.c
+++ b/src/responder/secrets/local.c
@@ -206,7 +206,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)"
@@ -231,9 +231,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,
@@ -297,9 +297,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,
@@ -321,7 +321,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;
@@ -483,7 +483,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);
@@ -587,9 +587,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,
@@ -599,8 +599,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,
@@ -612,13 +612,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));
@@ -645,7 +645,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);
@@ -760,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->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 bb5d620fc5893fc4a4da2bfe2f59bdb8a47fcc0d Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Tue, 4 Apr 2017 15:33:38 +0200
Subject: [PATCH 6/9] SECRETS: Use separate quotas for /kcm and /secrets hives
This would differentiate between out-of-capacity errors for secrets and
for KCM as they are two independent trees as far as sssd-secrets is
concerned.
The quotas for /kcm are also different in their defaults. For the /secrets
hive, we presume a large amount of small secrets. For the /kcm hive, we
presume a small amount of large secrets, because the secret is a ccache
which contains multiple credentials.
The operations are also passed in a struct quota from the local request
context instead of local_context. The quota is assigned to the request
context when the hive is selected.
---
src/config/cfg_rules.ini | 3 ++-
src/man/sssd-secrets.5.xml | 4 ++--
src/responder/secrets/local.c | 46 ++++++++++++++++++++++++------------------
src/responder/secrets/secsrv.c | 20 ++++++++++++++++++
src/responder/secrets/secsrv.h | 1 +
5 files changed, 51 insertions(+), 23 deletions(-)
diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
index 7db1d9711..feb1d912d 100644
--- a/src/config/cfg_rules.ini
+++ b/src/config/cfg_rules.ini
@@ -13,6 +13,7 @@ section = kcm
section = session_recording
section_re = ^secrets/users/[0-9]\+$
section_re = ^secrets/secrets$
+section_re = ^secrets/kcm$
section_re = ^domain/[^/\@]\+$
section_re = ^domain/[^/\@]\+/[^/\@]\+$
section_re = ^application/[^/\@]\+$
@@ -258,7 +259,7 @@ option = responder_idle_timeout
[rule/allowed_sec_hive_options]
validator = ini_allowed_options
-section_re = ^secrets/secrets$
+section_re = ^secrets/\(secrets\|kcm\)$
# Secrets service - per-hive configuration
option = containers_nest_level
diff --git a/src/man/sssd-secrets.5.xml b/src/man/sssd-secrets.5.xml
index d50cb13d8..ba77d6232 100644
--- a/src/man/sssd-secrets.5.xml
+++ b/src/man/sssd-secrets.5.xml
@@ -196,7 +196,7 @@ systemctl enable sssd-secrets.service
can be stored in the hive.
</para>
<para>
- Default: 1024
+ Default: 1024 (secrets hive), 256 (kcm hive)
</para>
</listitem>
</varlistentry>
@@ -208,7 +208,7 @@ systemctl enable sssd-secrets.service
a secret payload in kilobytes.
</para>
<para>
- Default: 16
+ Default: 16 (secrets hive), 65536 (64 MiB) (kcm hive)
</para>
</listitem>
</varlistentry>
diff --git a/src/responder/secrets/local.c b/src/responder/secrets/local.c
index c833f1d27..58e70f8b6 100644
--- a/src/responder/secrets/local.c
+++ b/src/responder/secrets/local.c
@@ -36,6 +36,7 @@ struct local_context {
struct sec_data master_key;
struct sec_quota *quota_secrets;
+ struct sec_quota *quota_kcm;
};
static int local_decrypt(struct local_context *lctx, TALLOC_CTX *mem_ctx,
@@ -206,7 +207,9 @@ static char *local_dn_to_path(TALLOC_CTX *mem_ctx,
struct local_db_req {
char *path;
+ const char *basedn;
struct ldb_dn *req_dn;
+ struct sec_quota *quota;
};
#define LOCAL_SIMPLE_FILTER "(type=simple)"
@@ -389,7 +392,7 @@ static int local_db_check_containers(TALLOC_CTX *mem_ctx,
return ret;
}
-static int local_db_check_containers_nest_level(struct local_context *lctx,
+static int local_db_check_containers_nest_level(struct local_db_req *lc_req,
struct ldb_dn *leaf_dn)
{
int nest_level;
@@ -397,11 +400,11 @@ static int local_db_check_containers_nest_level(struct local_context *lctx,
/* We need do not care for the synthetic containers that constitute the
* base path (cn=<uidnumber>,cn=user,cn=secrets). */
nest_level = ldb_dn_get_comp_num(leaf_dn) - 3;
- if (nest_level > lctx->quota_secrets->containers_nest_level) {
+ if (nest_level > lc_req->quota->containers_nest_level) {
DEBUG(SSSDBG_OP_FAILURE,
"Cannot create a nested container of depth %d as the maximum"
"allowed number of nested containers is %d.\n",
- nest_level, lctx->quota_secrets->containers_nest_level);
+ nest_level, lc_req->quota->containers_nest_level);
return ERR_SEC_INVALID_CONTAINERS_NEST_LEVEL;
}
@@ -410,7 +413,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 };
@@ -421,7 +425,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;
@@ -429,11 +433,10 @@ 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 (res->count >= lctx->quota_secrets->max_secrets) {
+ if (res->count >= lc_req->quota->max_secrets) {
DEBUG(SSSDBG_OP_FAILURE,
"Cannot store any more secrets as the maximum allowed limit (%d) "
- "has been reached\n", lctx->quota_secrets->max_secrets);
-
+ "has been reached\n", lc_req->quota->max_secrets);
ret = ERR_SEC_INVALID_TOO_MANY_SECRETS;
goto done;
}
@@ -445,19 +448,19 @@ static int local_db_check_number_of_secrets(TALLOC_CTX *mem_ctx,
return ret;
}
-static int local_check_max_payload_size(struct local_context *lctx,
+static int local_check_max_payload_size(struct local_db_req *lc_req,
int payload_size)
{
int max_payload_size;
- max_payload_size = lctx->quota_secrets->max_payload_size * 1024; /* kb */
+ max_payload_size = lc_req->quota->max_payload_size * 1024; /* kb */
if (payload_size > max_payload_size) {
DEBUG(SSSDBG_OP_FAILURE,
"Secrets' payload size [%d kb (%d)] exceeds the maximum allowed "
"payload size [%d kb (%d)]\n",
payload_size * 1024, /* kb */
payload_size,
- lctx->quota_secrets->max_payload_size, /* kb */
+ lc_req->quota->max_payload_size, /* kb */
max_payload_size);
return ERR_SEC_PAYLOAD_SIZE_IS_TOO_LARGE;
@@ -494,7 +497,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",
@@ -502,7 +505,7 @@ static int local_db_put_simple(TALLOC_CTX *mem_ctx,
goto done;
}
- ret = local_check_max_payload_size(lctx, strlen(secret));
+ ret = local_check_max_payload_size(lc_req, strlen(secret));
if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE,
"local_check_max_payload_size failed [%d]: %s\n",
@@ -656,7 +659,7 @@ static int local_db_create(TALLOC_CTX *mem_ctx,
goto done;
}
- ret = local_db_check_containers_nest_level(lctx, msg->dn);
+ ret = local_db_check_containers_nest_level(lc_req, msg->dn);
if (ret != EOK) goto done;
ret = ldb_msg_add_string(msg, "type", "container");
@@ -698,13 +701,13 @@ static int local_db_create(TALLOC_CTX *mem_ctx,
}
static int local_secrets_map_path(TALLOC_CTX *mem_ctx,
- struct ldb_context *ldb,
+ struct local_context *lctx,
struct sec_req_ctx *secreq,
struct local_db_req **_lc_req)
{
int ret;
struct local_db_req *lc_req;
- const char *basedn;
+ struct ldb_context *ldb = lctx->ldb;
/* be strict for now */
if (secreq->parsed_url.fragment != NULL) {
@@ -742,12 +745,14 @@ 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;
+ lc_req->quota = lctx->quota_secrets;
} 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;
+ lc_req->quota = lctx->quota_kcm;
} else {
ret = EINVAL;
goto done;
@@ -760,7 +765,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");
@@ -829,7 +834,7 @@ static struct tevent_req *local_secret_req(TALLOC_CTX *mem_ctx,
}
DEBUG(SSSDBG_TRACE_LIBS, "Content-Type: %s\n", content_type);
- ret = local_secrets_map_path(state, lctx->ldb, secreq, &lc_req);
+ ret = local_secrets_map_path(state, lctx, secreq, &lc_req);
if (ret) {
DEBUG(SSSDBG_OP_FAILURE, "Cannot map request path to local path\n");
goto done;
@@ -1019,6 +1024,7 @@ int local_secrets_provider_handle(struct sec_ctx *sctx,
}
lctx->quota_secrets = &sctx->sec_config.quota;
+ lctx->quota_kcm = &sctx->kcm_config.quota;
lctx->master_key.data = talloc_size(lctx, MKEY_SIZE);
if (!lctx->master_key.data) return ENOMEM;
diff --git a/src/responder/secrets/secsrv.c b/src/responder/secrets/secsrv.c
index db12cbbc3..2fcdf8e6c 100644
--- a/src/responder/secrets/secsrv.c
+++ b/src/responder/secrets/secsrv.c
@@ -30,9 +30,17 @@
#define DEFAULT_SEC_FD_LIMIT 2048
#define DEFAULT_SEC_CONTAINERS_NEST_LEVEL 4
+
#define DEFAULT_SEC_MAX_SECRETS 1024
#define DEFAULT_SEC_MAX_PAYLOAD_SIZE 16
+/* The number of secrets in the /kcm hive should be quite small,
+ * but the secret size must be large because one secret in the /kcm
+ * hive holds the whole ccache which consists of several credentials
+ */
+#define DEFAULT_SEC_KCM_MAX_SECRETS 256
+#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,
@@ -171,6 +179,18 @@ static int sec_get_config(struct sec_ctx *sctx)
goto fail;
}
+ ret = sec_get_hive_config(sctx,
+ "kcm",
+ &sctx->kcm_config,
+ DEFAULT_SEC_CONTAINERS_NEST_LEVEL,
+ DEFAULT_SEC_KCM_MAX_SECRETS,
+ DEFAULT_SEC_KCM_MAX_PAYLOAD_SIZE);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_FATAL_FAILURE,
+ "Failed to get configuration of the secrets hive\n");
+ goto fail;
+ }
+
ret = confdb_get_int(sctx->rctx->cdb, sctx->rctx->confdb_service_path,
CONFDB_RESPONDER_CLI_IDLE_TIMEOUT,
CONFDB_RESPONDER_CLI_IDLE_DEFAULT_TIMEOUT,
diff --git a/src/responder/secrets/secsrv.h b/src/responder/secrets/secsrv.h
index 629b027f6..afc092764 100644
--- a/src/responder/secrets/secsrv.h
+++ b/src/responder/secrets/secsrv.h
@@ -47,6 +47,7 @@ struct sec_ctx {
int fd_limit;
struct sec_hive_config sec_config;
+ struct sec_hive_config kcm_config;
struct provider_handle **providers;
};
From 2ac069a7f623a44e6d4b8d3eb9b1a986cee0d7c9 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Tue, 4 Apr 2017 15:34:17 +0200
Subject: [PATCH 7/9] 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 | 52 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 51 insertions(+), 1 deletion(-)
diff --git a/src/tests/intg/test_kcm.py b/src/tests/intg/test_kcm.py
index ae49eca80..fd2b37e1a 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):
@@ -109,7 +113,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
@@ -121,6 +125,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())
@@ -464,3 +471,46 @@ def test_kcm_sec_parallel_klist(setup_for_kcm_sec,
for p in processes:
rc = p.wait()
assert rc == 0
+
+
+def get_secrets_socket():
+ return os.path.join(config.RUNSTATEDIR, "secrets.socket")
+
+
+@pytest.fixture
+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 667a8e85f1ffe97dff814787a2f32dbcdd8e6a45 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Mon, 5 Jun 2017 16:10:55 +0200
Subject: [PATCH 8/9] SECRETS: Add a new option to control per-UID limits
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
---
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 | 45 +++++++++++++++++
9 files changed, 177 insertions(+), 1 deletion(-)
diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index 377335837..d4e179d90 100644
--- a/src/confdb/confdb.h
+++ b/src/confdb/confdb.h
@@ -242,6 +242,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 303bc62f4..f5daeb0a9 100644
--- a/src/config/SSSDConfig/__init__.py.in
+++ b/src/config/SSSDConfig/__init__.py.in
@@ -128,6 +128,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 feb1d912d..8410f0489 100644
--- a/src/config/cfg_rules.ini
+++ b/src/config/cfg_rules.ini
@@ -264,6 +264,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 ef910f0df..1a4b9a8f1 100644
--- a/src/config/etc/sssd.api.conf
+++ b/src/config/etc/sssd.api.conf
@@ -105,6 +105,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 ba77d6232..c74894c62 100644
--- a/src/man/sssd-secrets.5.xml
+++ b/src/man/sssd-secrets.5.xml
@@ -201,6 +201,18 @@ systemctl enable sssd-secrets.service
</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>
<para>
diff --git a/src/responder/secrets/local.c b/src/responder/secrets/local.c
index 58e70f8b6..5e491ba98 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 2fcdf8e6c..36b257c46 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)
{
@@ -78,6 +81,19 @@ static int sec_get_quota(struct sec_ctx *sctx,
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,
default_max_payload,
"a->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 afc092764..afdd731fb 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 0f997cb25..22b85a5d1 100644
--- a/src/tests/intg/test_secrets.py
+++ b/src/tests/intg/test_secrets.py
@@ -498,3 +498,48 @@ 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 x in range(MAX_UID_SECRETS):
+ cli.set_secret(str(x), 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
From 1de2ccc7a1c3dff5957d27ee3f6a328f1dff5e1e Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Wed, 7 Jun 2017 17:20:43 +0200
Subject: [PATCH 9/9] SECRETS: Support 0 as unlimited for the quotas
Add a special value for all the quota-like settings that means 'no
limit'.
Because the responder also had a global limit on the size of the
accepted body (64kB), this patch also removes the hardcoded limit and
insteads keep track of the biggest quota value on startup.
---
src/man/sssd-secrets.5.xml | 3 +-
src/responder/secrets/local.c | 16 ++++++++++
src/responder/secrets/secsrv.c | 15 ++++++++++
src/responder/secrets/secsrv.h | 1 +
src/responder/secrets/secsrv_cmd.c | 6 +++-
src/responder/secrets/secsrv_private.h | 2 +-
src/tests/intg/test_secrets.py | 55 ++++++++++++++++++++++++++++++++++
7 files changed, 95 insertions(+), 3 deletions(-)
diff --git a/src/man/sssd-secrets.5.xml b/src/man/sssd-secrets.5.xml
index c74894c62..d43dcf21c 100644
--- a/src/man/sssd-secrets.5.xml
+++ b/src/man/sssd-secrets.5.xml
@@ -173,7 +173,8 @@ systemctl enable sssd-secrets.service
</variablelist>
<para>
The following options affect only the secrets <quote>hive</quote>
- and therefore should be set in a per-hive subsection.
+ and therefore should be set in a per-hive subsection. Setting the
+ option to 0 means "unlimited".
</para>
<variablelist>
<varlistentry>
diff --git a/src/responder/secrets/local.c b/src/responder/secrets/local.c
index 5e491ba98..5a8b67731 100644
--- a/src/responder/secrets/local.c
+++ b/src/responder/secrets/local.c
@@ -397,6 +397,10 @@ static int local_db_check_containers_nest_level(struct local_db_req *lc_req,
{
int nest_level;
+ if (lc_req->quota->containers_nest_level == 0) {
+ return EOK;
+ }
+
/* We need do not care for the synthetic containers that constitute the
* base path (cn=<uidnumber>,cn=user,cn=secrets). */
nest_level = ldb_dn_get_comp_num(leaf_dn) - 3;
@@ -456,6 +460,10 @@ static int local_db_check_peruid_number_of_secrets(TALLOC_CTX *mem_ctx,
struct ldb_dn *cli_basedn = NULL;
int ret;
+ if (lc_req->quota->max_uid_secrets == 0) {
+ return EOK;
+ }
+
tmp_ctx = talloc_new(mem_ctx);
if (tmp_ctx == NULL) {
return ENOMEM;
@@ -501,6 +509,10 @@ static int local_db_check_number_of_secrets(TALLOC_CTX *mem_ctx,
struct ldb_dn *dn;
int ret;
+ if (lc_req->quota->max_secrets == 0) {
+ return EOK;
+ }
+
tmp_ctx = talloc_new(mem_ctx);
if (!tmp_ctx) return ENOMEM;
@@ -538,6 +550,10 @@ static int local_check_max_payload_size(struct local_db_req *lc_req,
{
int max_payload_size;
+ if (lc_req->quota->max_payload_size == 0) {
+ return EOK;
+ }
+
max_payload_size = lc_req->quota->max_payload_size * 1024; /* kb */
if (payload_size > max_payload_size) {
DEBUG(SSSDBG_OP_FAILURE,
diff --git a/src/responder/secrets/secsrv.c b/src/responder/secrets/secsrv.c
index 36b257c46..2b661b165 100644
--- a/src/responder/secrets/secsrv.c
+++ b/src/responder/secrets/secsrv.c
@@ -146,6 +146,16 @@ static int sec_get_hive_config(struct sec_ctx *sctx,
goto done;
}
+ if (hive_config->quota.max_payload_size == 0
+ || (sctx->max_payload_size != 0
+ && hive_config->quota.max_payload_size > sctx->max_payload_size)) {
+ /* If the quota is unlimited or it's larger than what
+ * we already have, save the total limit so we know how much to
+ * accept from clients
+ */
+ sctx->max_payload_size = hive_config->quota.max_payload_size;
+ }
+
ret = EOK;
done:
@@ -168,6 +178,11 @@ static int sec_get_config(struct sec_ctx *sctx)
goto fail;
}
+ /* Set the global max_payload to ridiculously small value so that either 0 (unlimited)
+ * or any sensible value overwrite it
+ */
+ sctx->max_payload_size = 1;
+
/* Read the global quota first -- this should be removed in a future release */
/* Note that this sets the defaults for the sec_config quota to be used
* in sec_get_hive_config()
diff --git a/src/responder/secrets/secsrv.h b/src/responder/secrets/secsrv.h
index afdd731fb..302311640 100644
--- a/src/responder/secrets/secsrv.h
+++ b/src/responder/secrets/secsrv.h
@@ -49,6 +49,7 @@ struct sec_ctx {
struct sec_hive_config sec_config;
struct sec_hive_config kcm_config;
+ int max_payload_size;
struct provider_handle **providers;
};
diff --git a/src/responder/secrets/secsrv_cmd.c b/src/responder/secrets/secsrv_cmd.c
index b88680c3d..fa5970504 100644
--- a/src/responder/secrets/secsrv_cmd.c
+++ b/src/responder/secrets/secsrv_cmd.c
@@ -178,7 +178,8 @@ static void sec_append_string(TALLOC_CTX *memctx, char **dest,
static bool sec_too_much_data(struct sec_req_ctx *req, size_t length)
{
req->total_size += length;
- if (req->total_size > SEC_REQUEST_MAX_SIZE) {
+ if (req->max_payload_size > 0
+ && req->total_size > req->max_payload_size) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Request too big, aborting client!\n");
return true;
@@ -513,6 +514,8 @@ static void sec_recv(struct cli_ctx *cctx)
{
struct sec_proto_ctx *prctx;
struct sec_req_ctx *req;
+ struct sec_ctx *sec_ctx = talloc_get_type(cctx->rctx->pvt_ctx,
+ struct sec_ctx);
char buffer[SEC_PACKET_MAX_RECV_SIZE];
struct sec_data data = { buffer,
SEC_PACKET_MAX_RECV_SIZE };
@@ -531,6 +534,7 @@ static void sec_recv(struct cli_ctx *cctx)
return;
}
req->cctx = cctx;
+ req->max_payload_size = sec_ctx->max_payload_size;
cctx->state_ctx = req;
http_parser_init(&prctx->parser, HTTP_REQUEST);
prctx->parser.data = req;
diff --git a/src/responder/secrets/secsrv_private.h b/src/responder/secrets/secsrv_private.h
index 2e68628f6..c4a0c5745 100644
--- a/src/responder/secrets/secsrv_private.h
+++ b/src/responder/secrets/secsrv_private.h
@@ -75,6 +75,7 @@ struct sec_req_ctx {
bool complete;
size_t total_size;
+ size_t max_payload_size;
char *request_url;
char *mapped_path;
@@ -151,7 +152,6 @@ bool sec_req_has_header(struct sec_req_ctx *req,
const char *name, const char *value);
/* secsrv_cmd.c */
-#define SEC_REQUEST_MAX_SIZE 65536
#define SEC_PACKET_MAX_RECV_SIZE 8192
int sec_send_data(int fd, struct sec_data *data);
diff --git a/src/tests/intg/test_secrets.py b/src/tests/intg/test_secrets.py
index 22b85a5d1..a205d2f7b 100644
--- a/src/tests/intg/test_secrets.py
+++ b/src/tests/intg/test_secrets.py
@@ -543,3 +543,58 @@ def test_per_uid_limit(setup_for_uid_limit, secrets_cli):
# 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
+
+
+@pytest.fixture
+def setup_for_unlimited_quotas(request):
+ conf = unindent("""\
+ [sssd]
+ domains = local
+ services = nss
+
+ [domain/local]
+ id_provider = local
+
+ [secrets]
+ debug_level = 10
+
+ [secrets/secrets]
+ max_secrets = 0
+ max_uid_secrets = 0
+ max_payload_size = 0
+ containers_nest_level = 0
+ """).format(**locals())
+
+ create_conf_fixture(request, conf)
+ create_sssd_secrets_fixture(request)
+ return None
+
+
+def test_unlimited_quotas(setup_for_unlimited_quotas, secrets_cli):
+ """
+ Test that setting quotas to zero disabled any checks and lets
+ store whatever.
+ """
+ cli = secrets_cli
+
+ # test much larger amount of secrets that we allow by default
+ sec_value = "value"
+ for x in range(2048):
+ cli.set_secret(str(x), sec_value)
+
+ # test a much larger secret size than the default one
+ KILOBYTE = 1024
+ payload_size = 32 * KILOBYTE
+
+ sec_value = "x" * payload_size
+ cli.set_secret("foo", sec_value)
+
+ fooval = cli.get_secret("foo")
+ assert fooval == sec_value
+
+ # test a deep secret nesting structure
+ DEFAULT_CONTAINERS_NEST_LEVEL = 128
+ container = "mycontainer"
+ for x in range(DEFAULT_CONTAINERS_NEST_LEVEL):
+ container += "%s/" % str(x)
+ cli.create_container(container)
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
