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 d890d26f7755b3851578688e9b481c3b52212292 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 96e23e6da06fcd6ac4469e9354ee7cebac1a4d3f 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 | 29 +++++++++++++++++++++++++++++ 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, 81 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..a9cf74d 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,25 @@ 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 +512,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 +1001,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