URL: https://github.com/SSSD/sssd/pull/75
Author: fidencio
Title: #75: Add configuirable max payload size limit of a secret
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/75/head:pr75
git checkout pr75
From 39f7e82bce77f81e9f8f7aa490fb2bb186d9b80c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
Date: Tue, 8 Nov 2016 16:39:48 +0100
Subject: [PATCH 1/2] SECRETS: Delete all secret stored during "max_secrets"
test
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Otherwise we will have an 507 error in case any secret is added by any
of the tests that may be implemented in the future.
Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com>
---
src/tests/intg/test_secrets.py | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/tests/intg/test_secrets.py b/src/tests/intg/test_secrets.py
index 57b8f3f..09a91e0 100644
--- a/src/tests/intg/test_secrets.py
+++ b/src/tests/intg/test_secrets.py
@@ -151,6 +151,10 @@ def test_crd_ops(setup_for_secrets, secrets_cli):
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 xrange(MAX_SECRETS):
+ cli.del_secret(str(x))
+
def test_containers(setup_for_secrets, secrets_cli):
"""
From 47497ee224f5b345487e4a3afd6ce1c84b208faf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
Date: Tue, 8 Nov 2016 16:46:21 +0100
Subject: [PATCH 2/2] SECRETS: Add configurable payload size limit of a secret
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Resolves:
https://fedorahosted.org/sssd/ticket/3169
Signed-off-by: Fabiano Fidêncio <fiden...@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 | 31 +++++++++++++++++++++++++++++++
src/responder/secrets/providers.c | 4 ++++
src/responder/secrets/secsrv.c | 13 +++++++++++++
src/responder/secrets/secsrv.h | 1 +
src/responder/secrets/secsrv_private.h | 1 +
src/tests/intg/test_secrets.py | 15 +++++++++++++++
src/util/util_errors.c | 1 +
src/util/util_errors.h | 1 +
13 files changed, 83 insertions(+)
diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index 2a1e581..12beaab 100644
--- a/src/confdb/confdb.h
+++ b/src/confdb/confdb.h
@@ -226,6 +226,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_PAYLOAD_SIZE "max_payload_size"
struct confdb_ctx;
diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in
index 381ff95..be09e8f 100644
--- a/src/config/SSSDConfig/__init__.py.in
+++ b/src/config/SSSDConfig/__init__.py.in
@@ -123,6 +123,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_payload_size': _('The maximum payload size of a secret in kilobytes'),
# secrets - proxy
'proxy_url': _('The URL Custodia server is listening on'),
'auth_type': _('The method to use when authenticating to a Custodia server'),
diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
index 882a185..ec44bff 100644
--- a/src/config/cfg_rules.ini
+++ b/src/config/cfg_rules.ini
@@ -230,6 +230,7 @@ option = client_idle_timeout
option = description
option = containers_nest_level
option = max_secrets
+option = max_payload_size
[rule/allowed_sec_users_options]
validator = ini_allowed_options
diff --git a/src/config/etc/sssd.api.conf b/src/config/etc/sssd.api.conf
index be24bce..d591228 100644
--- a/src/config/etc/sssd.api.conf
+++ b/src/config/etc/sssd.api.conf
@@ -100,6 +100,7 @@ user_attributes = str, None, false
provider = str, None, false
containers_nest_level = int, None, false
max_secrets = int, None, false
+max_payload_size = int, None, false
# Secrets service - proxy
proxy_url = str, None, false
auth_type = str, None, false
diff --git a/src/man/sssd-secrets.5.xml b/src/man/sssd-secrets.5.xml
index 7ec54c2..80e9c40 100644
--- a/src/man/sssd-secrets.5.xml
+++ b/src/man/sssd-secrets.5.xml
@@ -168,6 +168,18 @@ systemctl enable sssd-secrets.service
</para>
</listitem>
</varlistentry>
+ <varlistentry>
+ <term>max_payload_size (integer)</term>
+ <listitem>
+ <para>
+ This option specifies the maximum payload size allowed for
+ a secret payload in kilobytes.
+ </para>
+ <para>
+ Default: 16
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
<para>
The following options are only applicable for configurations that
diff --git a/src/responder/secrets/local.c b/src/responder/secrets/local.c
index f6c936f..ed70193 100644
--- a/src/responder/secrets/local.c
+++ b/src/responder/secrets/local.c
@@ -31,6 +31,7 @@ struct local_context {
struct sec_data master_key;
int containers_nest_level;
int max_secrets;
+ int max_payload_size;
};
static int local_decrypt(struct local_context *lctx, TALLOC_CTX *mem_ctx,
@@ -450,6 +451,27 @@ 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,
+ int payload_size)
+{
+ int max_payload_size;
+
+ max_payload_size = lctx->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 */
+ max_payload_size);
+
+ return ERR_SEC_PAYLOAD_SIZE_IS_TOO_LARGE;
+ }
+
+ return EOK;
+}
+
static int local_db_put_simple(TALLOC_CTX *mem_ctx,
struct local_context *lctx,
const char *req_path,
@@ -492,6 +514,14 @@ static int local_db_put_simple(TALLOC_CTX *mem_ctx,
goto done;
}
+ ret = local_check_max_payload_size(lctx, strlen(secret));
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE,
+ "local_check_max_payload_size failed [%d]: %s\n",
+ ret, sss_strerror(ret));
+ goto done;
+ }
+
ret = local_encrypt(lctx, msg, secret, enctype, &enc_secret);
if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE,
@@ -973,6 +1003,7 @@ int local_secrets_provider_handle(struct sec_ctx *sctx,
lctx->containers_nest_level = sctx->containers_nest_level;
lctx->max_secrets = sctx->max_secrets;
+ lctx->max_payload_size = sctx->max_payload_size;
lctx->master_key.data = talloc_size(lctx, MKEY_SIZE);
if (!lctx->master_key.data) return ENOMEM;
diff --git a/src/responder/secrets/providers.c b/src/responder/secrets/providers.c
index 5f4b0fc..eba555d 100644
--- a/src/responder/secrets/providers.c
+++ b/src/responder/secrets/providers.c
@@ -178,6 +178,8 @@ static struct sec_http_status_format_table {
"The request cannot be accepted." },
{ 409, "Conflict",
"The requested resource already exists." },
+ { 413, "Payload Too Large",
+ "The secret payload is too large." },
{ 500, "Internal Server Error",
"The server encountered an internal error." },
{ 504, "Gateway timeout",
@@ -352,6 +354,8 @@ enum sec_http_status_codes sec_errno_to_http_status(errno_t err)
return STATUS_406;
case EEXIST:
return STATUS_409;
+ case ERR_SEC_PAYLOAD_SIZE_IS_TOO_LARGE:
+ return STATUS_413;
case ERR_SEC_NO_PROXY:
return STATUS_504;
case ERR_SEC_INVALID_TOO_MANY_SECRETS:
diff --git a/src/responder/secrets/secsrv.c b/src/responder/secrets/secsrv.c
index 4c0824b..09b0d22 100644
--- a/src/responder/secrets/secsrv.c
+++ b/src/responder/secrets/secsrv.c
@@ -31,6 +31,7 @@
#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
static int sec_get_config(struct sec_ctx *sctx)
{
@@ -71,6 +72,18 @@ static int sec_get_config(struct sec_ctx *sctx)
goto fail;
}
+ ret = confdb_get_int(sctx->rctx->cdb,
+ sctx->rctx->confdb_service_path,
+ CONFDB_SEC_MAX_PAYLOAD_SIZE,
+ DEFAULT_SEC_MAX_PAYLOAD_SIZE,
+ &sctx->max_payload_size);
+
+ if (ret != EOK) {
+ DEBUG(SSSDBG_FATAL_FAILURE,
+ "Failed to get payload's maximum size for an entry\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 972d342..3d23c40 100644
--- a/src/responder/secrets/secsrv.h
+++ b/src/responder/secrets/secsrv.h
@@ -40,6 +40,7 @@ struct sec_ctx {
int fd_limit;
int containers_nest_level;
int max_secrets;
+ int max_payload_size;
struct provider_handle **providers;
};
diff --git a/src/responder/secrets/secsrv_private.h b/src/responder/secrets/secsrv_private.h
index 4129fe6..1c3fbd8 100644
--- a/src/responder/secrets/secsrv_private.h
+++ b/src/responder/secrets/secsrv_private.h
@@ -46,6 +46,7 @@ enum sec_http_status_codes {
STATUS_405,
STATUS_406,
STATUS_409,
+ STATUS_413,
STATUS_500,
STATUS_504,
STATUS_507,
diff --git a/src/tests/intg/test_secrets.py b/src/tests/intg/test_secrets.py
index 09a91e0..7a9de1a 100644
--- a/src/tests/intg/test_secrets.py
+++ b/src/tests/intg/test_secrets.py
@@ -87,6 +87,7 @@ def setup_for_secrets(request):
[secrets]
max_secrets = 10
+ max_payload_size = 2
""").format(**locals())
create_conf_fixture(request, conf)
@@ -155,6 +156,20 @@ def test_crd_ops(setup_for_secrets, secrets_cli):
for x in xrange(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 test_containers(setup_for_secrets, secrets_cli):
"""
diff --git a/src/util/util_errors.c b/src/util/util_errors.c
index db5130c..88ebf4e 100644
--- a/src/util/util_errors.c
+++ b/src/util/util_errors.c
@@ -101,6 +101,7 @@ struct err_string error_to_str[] = {
{ "The maximum level of nested containers has been reached" }, /* ERR_SEC_INVALID_CONTAINERS_NEST_LEVEL */
{ "No proxy server for secrets available"}, /* ERR_SEC_NO_PROXY */
{ "The maximum number of stored secrets has been reached" }, /* ERR_SEC_INVALID_TOO_MANY_SECRETS */
+ { "The secret payload size is too large" }, /* ERR_SEC_PAYLOAD_SIZE_IS_TOO_LARGE */
{ "ERR_LAST" } /* ERR_LAST */
};
diff --git a/src/util/util_errors.h b/src/util/util_errors.h
index 3690b7e..525983f 100644
--- a/src/util/util_errors.h
+++ b/src/util/util_errors.h
@@ -123,6 +123,7 @@ enum sssd_errors {
ERR_SEC_INVALID_CONTAINERS_NEST_LEVEL,
ERR_SEC_NO_PROXY,
ERR_SEC_INVALID_TOO_MANY_SECRETS,
+ ERR_SEC_PAYLOAD_SIZE_IS_TOO_LARGE,
ERR_LAST /* ALWAYS LAST */
};
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
