URL: https://github.com/SSSD/sssd/pull/36 Author: fidencio Title: #36: Partial fix for #3169 Action: opened
PR body: """ Ticket #3169 is about having a quota for sssd secrets. This quota depends basically on the number of stored secrets and the size of (the payload of the) each stored secret and this patch implements the former for now. This patch is rebased on top of the series that fixes #3168. Relevant Patches: 1e4e914 (Fabiano Fidêncio, 17 minutes ago) SECRETS: Add a configurable limit of secrets that can be stored Related: https://fedorahosted.org/sssd/ticket/3169 """ To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/36/head:pr36 git checkout pr36
From 06a0a81193d6bbe3a0932c8b584433f3cc13fa51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Sun, 25 Sep 2016 20:49:16 +0200 Subject: [PATCH 1/7] CONFIG: Add secrets responder to the allowed sections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The regular expression used is quite specific for the two cases we support: - [secrets] - [secrets/users/$uid] It could be done a bit more generic, but the way it's right now it can easily catch errors like: [secrets/usrs/$uid] or [secrets/]. Related: https://fedorahosted.org/sssd/ticket/3207 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/config/cfg_rules.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini index 01be0c6..023ceac 100644 --- a/src/config/cfg_rules.ini +++ b/src/config/cfg_rules.ini @@ -8,6 +8,7 @@ section = autofs section = ssh section = pac section = ifp +section_re = ^secrets\(/users/\([0-9]\+\)\?\)\?$ section_re = ^domain/.*$ [rule/allowed_sssd_options] From d99acfa45e0f277e87343c9eb7ea43e3e081c0d7 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <jhro...@redhat.com> Date: Tue, 16 Aug 2016 21:15:28 +0200 Subject: [PATCH 2/7] CONFIG: List allowed secrets responder options MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Related: https://fedorahosted.org/sssd/ticket/3207 Reviewed-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/config/cfg_rules.ini | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini index 023ceac..4d9acf8 100644 --- a/src/config/cfg_rules.ini +++ b/src/config/cfg_rules.ini @@ -210,6 +210,33 @@ option = description option = allowed_uids option = user_attributes +[rule/allowed_sec_options] +validator = ini_allowed_options +section_re = ^secrets\(/users/\([0-9]\+\)\?\)\?$ + +option = timeout +option = debug +option = debug_level +option = debug_timestamps +option = debug_microseconds +option = debug_to_files +option = command +option = reconnection_retries +option = fd_limit +option = client_idle_timeout +option = description + +# Secrets service +option = provider +# Secrets service - proxy +option = proxy_url +option = auth_type +option = auth_header_name +option = auth_header_value +option = forward_headers +option = username +option = password + [rule/allowed_domain_options] validator = ini_allowed_options section_re = ^domain/.*$ From 039ccc36516925b66d6816fa400fb4b8a91d75c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Sun, 25 Sep 2016 21:52:10 +0200 Subject: [PATCH 3/7] CONFIG: Add secrets provider options MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Related: https://fedorahosted.org/sssd/ticket/3207 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/config/SSSDConfig/__init__.py.in | 11 +++++++++++ src/config/SSSDConfigTest.py | 6 ++++-- src/config/etc/sssd.api.conf | 12 ++++++++++++ 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in index e616ce3..15b9cd1 100644 --- a/src/config/SSSDConfig/__init__.py.in +++ b/src/config/SSSDConfig/__init__.py.in @@ -118,6 +118,17 @@ option_strings = { 'allowed_uids': _('List of UIDs or user names allowed to access the InfoPipe responder'), 'user_attributes': _('List of user attributes the InfoPipe is allowed to publish'), + # [secrets] + 'provider': _('The provider where the secrets will be stored in'), + # secrets - proxy + 'proxy_url': _('The URL Custodia server is listening on'), + 'auth_type': _('The method to use when authenticating to a Custodia server'), + 'auth_header_name': _('The name of the headers that will be added into a HTTP request with the value defined in auth_header_value'), + 'auth_header_value': _('The value sssd-secrets would use for auth_header_name'), + 'forward_headers': _('The list of the headers to forward to the Custodia server together with the request'), + 'username': _('The username to use when authenticating to a Custodia server using basic_auth'), + 'password': _('The password to use when authenticating to a Custodia server using basic_auth'), + # [provider] 'id_provider' : _('Identity provider'), 'auth_provider' : _('Authentication provider'), diff --git a/src/config/SSSDConfigTest.py b/src/config/SSSDConfigTest.py index 006a034..4850073 100755 --- a/src/config/SSSDConfigTest.py +++ b/src/config/SSSDConfigTest.py @@ -1352,7 +1352,8 @@ def testNewConfig(self): 'autofs', 'ssh', 'pac', - 'ifp'] + 'ifp', + 'secrets'] for section in control_list: self.assertTrue(sssdconfig.has_section(section), "Section [%s] missing" % @@ -1445,7 +1446,8 @@ def testListServices(self): 'autofs', 'ssh', 'pac', - 'ifp'] + 'ifp', + 'secrets'] service_list = sssdconfig.list_services() for service in control_list: self.assertTrue(service in service_list, diff --git a/src/config/etc/sssd.api.conf b/src/config/etc/sssd.api.conf index 9e4bf2f..f94c8d1 100644 --- a/src/config/etc/sssd.api.conf +++ b/src/config/etc/sssd.api.conf @@ -94,6 +94,18 @@ pac_lifetime = int, None, false allowed_uids = str, None, false user_attributes = str, None, false +[secrets] +# Secrets service +provider = str, None, false +# Secrets service - proxy +proxy_url = str, None, false +auth_type = str, None, false +auth_header_name = str, None, false +auth_header_value = str, None, false +forward_headers = list, None, false +username = str, None, false +password = str, None, false + [provider] #Available provider types id_provider = str, None, true From 8ab682ac3716301a3f82b70bdf19c798892a5056 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Mon, 26 Sep 2016 01:07:31 +0200 Subject: [PATCH 4/7] SECRETS: Make functions from local.c static MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There's no reason for those functions to be exposed. Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/responder/secrets/local.c | 86 +++++++++++++++++++++---------------------- 1 file changed, 43 insertions(+), 43 deletions(-) diff --git a/src/responder/secrets/local.c b/src/responder/secrets/local.c index b13e77f..0ce0526 100644 --- a/src/responder/secrets/local.c +++ b/src/responder/secrets/local.c @@ -31,9 +31,9 @@ struct local_context { struct sec_data master_key; }; -int local_decrypt(struct local_context *lctx, TALLOC_CTX *mem_ctx, - const char *secret, const char *enctype, - char **plain_secret) +static int local_decrypt(struct local_context *lctx, TALLOC_CTX *mem_ctx, + const char *secret, const char *enctype, + char **plain_secret) { char *output; @@ -66,9 +66,9 @@ int local_decrypt(struct local_context *lctx, TALLOC_CTX *mem_ctx, return EOK; } -int local_encrypt(struct local_context *lctx, TALLOC_CTX *mem_ctx, - const char *secret, const char *enctype, - char **ciphertext) +static int local_encrypt(struct local_context *lctx, TALLOC_CTX *mem_ctx, + const char *secret, const char *enctype, + char **ciphertext) { struct sec_data _secret; char *output; @@ -91,10 +91,10 @@ int local_encrypt(struct local_context *lctx, TALLOC_CTX *mem_ctx, return EOK; } -int local_db_dn(TALLOC_CTX *mem_ctx, - struct ldb_context *ldb, - const char *req_path, - struct ldb_dn **req_dn) +static int local_db_dn(TALLOC_CTX *mem_ctx, + struct ldb_context *ldb, + const char *req_path, + struct ldb_dn **req_dn) { struct ldb_dn *dn; const char *s, *e; @@ -136,9 +136,9 @@ int local_db_dn(TALLOC_CTX *mem_ctx, return ret; } -char *local_dn_to_path(TALLOC_CTX *mem_ctx, - struct ldb_dn *basedn, - struct ldb_dn *dn) +static char *local_dn_to_path(TALLOC_CTX *mem_ctx, + struct ldb_dn *basedn, + struct ldb_dn *dn) { int basecomps; int dncomps; @@ -170,10 +170,10 @@ char *local_dn_to_path(TALLOC_CTX *mem_ctx, #define LOCAL_SIMPLE_FILTER "(type=simple)" #define LOCAL_CONTAINER_FILTER "(type=container)" -int local_db_get_simple(TALLOC_CTX *mem_ctx, - struct local_context *lctx, - const char *req_path, - char **secret) +static int local_db_get_simple(TALLOC_CTX *mem_ctx, + struct local_context *lctx, + const char *req_path, + char **secret) { TALLOC_CTX *tmp_ctx; static const char *attrs[] = { "secret", "enctype", NULL }; @@ -228,11 +228,11 @@ int local_db_get_simple(TALLOC_CTX *mem_ctx, return ret; } -int local_db_list_keys(TALLOC_CTX *mem_ctx, - struct local_context *lctx, - const char *req_path, - char ***_keys, - int *num_keys) +static int local_db_list_keys(TALLOC_CTX *mem_ctx, + struct local_context *lctx, + const char *req_path, + char ***_keys, + int *num_keys) { TALLOC_CTX *tmp_ctx; static const char *attrs[] = { "secret", NULL }; @@ -282,9 +282,9 @@ int local_db_list_keys(TALLOC_CTX *mem_ctx, return ret; } -int local_db_check_containers(TALLOC_CTX *mem_ctx, - struct local_context *lctx, - struct ldb_dn *leaf_dn) +static int local_db_check_containers(TALLOC_CTX *mem_ctx, + struct local_context *lctx, + struct ldb_dn *leaf_dn) { static const char *attrs[] = { NULL}; struct ldb_result *res = NULL; @@ -316,10 +316,10 @@ int local_db_check_containers(TALLOC_CTX *mem_ctx, return EOK; } -int local_db_put_simple(TALLOC_CTX *mem_ctx, - struct local_context *lctx, - const char *req_path, - const char *secret) +static int local_db_put_simple(TALLOC_CTX *mem_ctx, + struct local_context *lctx, + const char *req_path, + const char *secret) { struct ldb_message *msg; const char *enctype = "masterkey"; @@ -368,9 +368,9 @@ int local_db_put_simple(TALLOC_CTX *mem_ctx, return ret; } -int local_db_delete(TALLOC_CTX *mem_ctx, - struct local_context *lctx, - const char *req_path) +static int local_db_delete(TALLOC_CTX *mem_ctx, + struct local_context *lctx, + const char *req_path) { TALLOC_CTX *tmp_ctx; struct ldb_dn *dn; @@ -411,9 +411,9 @@ int local_db_delete(TALLOC_CTX *mem_ctx, return ret; } -int local_db_create(TALLOC_CTX *mem_ctx, - struct local_context *lctx, - const char *req_path) +static int local_db_create(TALLOC_CTX *mem_ctx, + struct local_context *lctx, + const char *req_path) { struct ldb_message *msg; int ret; @@ -451,9 +451,9 @@ int local_db_create(TALLOC_CTX *mem_ctx, return ret; } -int local_secrets_map_path(TALLOC_CTX *mem_ctx, - struct sec_req_ctx *secreq, - char **local_db_path) +static int local_secrets_map_path(TALLOC_CTX *mem_ctx, + struct sec_req_ctx *secreq, + char **local_db_path) { int ret; @@ -501,10 +501,10 @@ struct local_secret_state { struct sec_req_ctx *secreq; }; -struct tevent_req *local_secret_req(TALLOC_CTX *mem_ctx, - struct tevent_context *ev, - void *provider_ctx, - struct sec_req_ctx *secreq) +static struct tevent_req *local_secret_req(TALLOC_CTX *mem_ctx, + struct tevent_context *ev, + void *provider_ctx, + struct sec_req_ctx *secreq) { struct tevent_req *req; struct local_secret_state *state; @@ -633,7 +633,7 @@ struct tevent_req *local_secret_req(TALLOC_CTX *mem_ctx, return tevent_req_post(req, state->ev); } -int generate_master_key(const char *filename, size_t size) +static int generate_master_key(const char *filename, size_t size) { uint8_t buf[size]; ssize_t rsize; From 0acad5c244a45ab0f8ec6c587452e80d5b1d9e71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Mon, 26 Sep 2016 01:15:56 +0200 Subject: [PATCH 5/7] SECRETS: Use a tmp_context on local_db_check_containers() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Otherwise the struct ldb_dn will be hanging on the mem_ctx till it gets freed. Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/responder/secrets/local.c | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/src/responder/secrets/local.c b/src/responder/secrets/local.c index 0ce0526..484e406 100644 --- a/src/responder/secrets/local.c +++ b/src/responder/secrets/local.c @@ -286,14 +286,21 @@ static int local_db_check_containers(TALLOC_CTX *mem_ctx, struct local_context *lctx, struct ldb_dn *leaf_dn) { + TALLOC_CTX *tmp_ctx; static const char *attrs[] = { NULL}; struct ldb_result *res = NULL; struct ldb_dn *dn; int num; int ret; - dn = ldb_dn_copy(mem_ctx, leaf_dn); - if (!dn) return ENOMEM; + tmp_ctx = talloc_new(mem_ctx); + if (!tmp_ctx) return ENOMEM; + + dn = ldb_dn_copy(tmp_ctx, leaf_dn); + if (!dn) { + ret = ENOMEM; + goto done; + } /* We need to exclude the leaf as that will be the new child entry, * We also do not care for the synthetic containers that constitute the @@ -306,14 +313,23 @@ static int local_db_check_containers(TALLOC_CTX *mem_ctx, if (!ldb_dn_remove_child_components(dn, 1)) return EFAULT; /* and check the parent container exists */ - ret = ldb_search(lctx->ldb, mem_ctx, &res, dn, LDB_SCOPE_BASE, + ret = ldb_search(lctx->ldb, tmp_ctx, &res, dn, LDB_SCOPE_BASE, attrs, LOCAL_CONTAINER_FILTER); - if (ret != LDB_SUCCESS) return ENOENT; - if (res->count != 1) return ENOENT; - talloc_free(res); + if (ret != LDB_SUCCESS) { + ret = ENOENT; + goto done; + } + if (res->count != 1) { + ret = ENOENT; + goto done; + } } - return EOK; + ret = EOK; + +done: + talloc_free(tmp_ctx); + return ret; } static int local_db_put_simple(TALLOC_CTX *mem_ctx, From 7aec450e12b9fc476132df48ebbf9ebd55750e5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Fri, 23 Sep 2016 15:23:23 +0200 Subject: [PATCH 6/7] SECRETS: Add a configurable depth limit for nested containers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves: https://fedorahosted.org/sssd/ticket/3168 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 | 26 ++++++++++++++++++++++++++ src/responder/secrets/providers.c | 1 + src/responder/secrets/secsrv.c | 13 +++++++++++++ src/responder/secrets/secsrv.h | 1 + src/tests/intg/test_secrets.py | 12 ++++++++++++ src/util/util_errors.c | 1 + src/util/util_errors.h | 1 + 12 files changed, 71 insertions(+) diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h index 36a2f21..dfbdbc7 100644 --- a/src/confdb/confdb.h +++ b/src/confdb/confdb.h @@ -223,6 +223,7 @@ /* Secrets Service */ #define CONFDB_SEC_CONF_ENTRY "config/secrets" +#define CONFDB_SEC_CONTAINERS_NEST_LEVEL "containers_nest_level" struct confdb_ctx; diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in index 15b9cd1..74c2ca5 100644 --- a/src/config/SSSDConfig/__init__.py.in +++ b/src/config/SSSDConfig/__init__.py.in @@ -120,6 +120,7 @@ option_strings = { # [secrets] 'provider': _('The provider where the secrets will be stored in'), + 'containers_nest_level': _('The maximum allowed number of nested containers'), # 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 4d9acf8..e6f23ff 100644 --- a/src/config/cfg_rules.ini +++ b/src/config/cfg_rules.ini @@ -228,6 +228,7 @@ option = description # Secrets service option = provider +option = containers_nest_level # Secrets service - proxy option = proxy_url option = auth_type diff --git a/src/config/etc/sssd.api.conf b/src/config/etc/sssd.api.conf index f94c8d1..a7757dc 100644 --- a/src/config/etc/sssd.api.conf +++ b/src/config/etc/sssd.api.conf @@ -97,6 +97,7 @@ user_attributes = str, None, false [secrets] # Secrets service provider = str, None, false +containers_nest_level = 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 71a79f5..1acaa98 100644 --- a/src/man/sssd-secrets.5.xml +++ b/src/man/sssd-secrets.5.xml @@ -144,6 +144,18 @@ systemctl enable sssd-secrets.service </para> </listitem> </varlistentry> + <varlistentry> + <term>containers_nest_level (integer)</term> + <listitem> + <para> + This option specifies the maximum allowed number of nested + containers. + </para> + <para> + Default: 4 + </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 484e406..ec84537 100644 --- a/src/responder/secrets/local.c +++ b/src/responder/secrets/local.c @@ -29,6 +29,7 @@ struct local_context { struct ldb_context *ldb; struct sec_data master_key; + int containers_nest_level; }; static int local_decrypt(struct local_context *lctx, TALLOC_CTX *mem_ctx, @@ -332,6 +333,26 @@ static int local_db_check_containers(TALLOC_CTX *mem_ctx, return ret; } +static int local_db_check_containers_nest_level(struct local_context *lctx, + struct ldb_dn *leaf_dn) +{ + int nest_level; + + /* 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) { + 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); + + return ERR_SEC_INVALID_CONTAINERS_NEST_LEVEL; + } + + return EOK; +} + static int local_db_put_simple(TALLOC_CTX *mem_ctx, struct local_context *lctx, const char *req_path, @@ -447,6 +468,9 @@ static int local_db_create(TALLOC_CTX *mem_ctx, ret = local_db_check_containers(msg, lctx, msg->dn); if (ret != EOK) goto done; + ret = local_db_check_containers_nest_level(lctx, msg->dn); + if (ret != EOK) goto done; + ret = ldb_msg_add_string(msg, "type", "container"); if (ret != EOK) goto done; @@ -708,6 +732,8 @@ int local_secrets_provider_handle(struct sec_ctx *sctx, return EIO; } + lctx->containers_nest_level = sctx->containers_nest_level; + lctx->master_key.data = talloc_size(lctx, MKEY_SIZE); if (!lctx->master_key.data) return ENOMEM; lctx->master_key.length = MKEY_SIZE; diff --git a/src/responder/secrets/providers.c b/src/responder/secrets/providers.c index 4c60198..aedb49a 100644 --- a/src/responder/secrets/providers.c +++ b/src/responder/secrets/providers.c @@ -306,6 +306,7 @@ enum sec_http_status_codes sec_errno_to_http_status(errno_t err) case EISDIR: return STATUS_405; case EMEDIUMTYPE: + case ERR_SEC_INVALID_CONTAINERS_NEST_LEVEL: return STATUS_406; case EEXIST: return STATUS_409; diff --git a/src/responder/secrets/secsrv.c b/src/responder/secrets/secsrv.c index eb194a1..4bbc1dc 100644 --- a/src/responder/secrets/secsrv.c +++ b/src/responder/secrets/secsrv.c @@ -29,6 +29,7 @@ #include "resolv/async_resolv.h" #define DEFAULT_SEC_FD_LIMIT 2048 +#define DEFAULT_SEC_CONTAINERS_NEST_LEVEL 4 static int sec_get_config(struct sec_ctx *sctx) { @@ -45,6 +46,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_CONTAINERS_NEST_LEVEL, + DEFAULT_SEC_CONTAINERS_NEST_LEVEL, + &sctx->containers_nest_level); + + if (ret != EOK) { + DEBUG(SSSDBG_FATAL_FAILURE, + "Failed to get containers' maximum depth\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 ace30f8..8ef89ab 100644 --- a/src/responder/secrets/secsrv.h +++ b/src/responder/secrets/secsrv.h @@ -38,6 +38,7 @@ struct sec_ctx { struct resolv_ctx *resctx; struct resp_ctx *rctx; int fd_limit; + int containers_nest_level; struct provider_handle **providers; }; diff --git a/src/tests/intg/test_secrets.py b/src/tests/intg/test_secrets.py index e394d12..c77c2a4 100644 --- a/src/tests/intg/test_secrets.py +++ b/src/tests/intg/test_secrets.py @@ -160,3 +160,15 @@ def test_containers(setup_for_secrets, secrets_cli): # Try removing the secret first, then the container cli.del_secret("mycontainer/foo") cli.del_secret("mycontainer/") + + # Don't allow creating a container after reaching the max nested level + DEFAULT_CONTAINERS_NEST_LEVEL = 4 + container = "mycontainer" + for x in xrange(DEFAULT_CONTAINERS_NEST_LEVEL): + container += "%s/" % str(x) + cli.create_container(container) + + container += "%s/" % str(DEFAULT_CONTAINERS_NEST_LEVEL) + with pytest.raises(HTTPError) as err406: + cli.create_container(container) + assert str(err406.value).startswith("406") diff --git a/src/util/util_errors.c b/src/util/util_errors.c index 620be3c..7d4a7f5 100644 --- a/src/util/util_errors.c +++ b/src/util/util_errors.c @@ -98,6 +98,7 @@ struct err_string error_to_str[] = { { "Dereference threshold reached" }, /* ERR_DEREF_THRESHOLD */ { "The user is not handled by SSSD" }, /* ERR_NON_SSSD_USER */ { "The internal name format cannot be parsed" }, /* ERR_WRONG_NAME_FORMAT */ + { "The maximum level of nested containers has been reached" }, /* ERR_SEC_INVALID_CONTAINERS_NEST_LEVEL */ { "ERR_LAST" } /* ERR_LAST */ }; diff --git a/src/util/util_errors.h b/src/util/util_errors.h index 1f00ba3..2cd55e1 100644 --- a/src/util/util_errors.h +++ b/src/util/util_errors.h @@ -120,6 +120,7 @@ enum sssd_errors { ERR_DEREF_THRESHOLD, ERR_NON_SSSD_USER, ERR_WRONG_NAME_FORMAT, + ERR_SEC_INVALID_CONTAINERS_NEST_LEVEL, ERR_LAST /* ALWAYS LAST */ }; From 1e4e914c9d19da4255034818a2ccad52b4adb6da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Fri, 30 Sep 2016 16:48:47 +0200 Subject: [PATCH 7/7] SECRETS: Add a configurable limit of secrets that can be stored Related: https://fedorahosted.org/sssd/ticket/3169 --- 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 | 42 ++++++++++++++++++++++++++++++++++++ src/responder/secrets/providers.c | 1 + src/responder/secrets/secsrv.c | 13 +++++++++++ src/responder/secrets/secsrv.h | 1 + src/tests/intg/test_secrets.py | 15 +++++++++++++ src/util/util_errors.c | 1 + src/util/util_errors.h | 1 + 12 files changed, 90 insertions(+) diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h index dfbdbc7..05aca14 100644 --- a/src/confdb/confdb.h +++ b/src/confdb/confdb.h @@ -224,6 +224,7 @@ /* Secrets Service */ #define CONFDB_SEC_CONF_ENTRY "config/secrets" #define CONFDB_SEC_CONTAINERS_NEST_LEVEL "containers_nest_level" +#define CONFDB_SEC_MAX_ENTRIES "max_entries" struct confdb_ctx; diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in index 74c2ca5..9d34deb 100644 --- a/src/config/SSSDConfig/__init__.py.in +++ b/src/config/SSSDConfig/__init__.py.in @@ -121,6 +121,7 @@ option_strings = { # [secrets] 'provider': _('The provider where the secrets will be stored in'), 'containers_nest_level': _('The maximum allowed number of nested containers'), + 'max_entries': _('The maximum number of secrets that can be stored'), # 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 e6f23ff..25b5444 100644 --- a/src/config/cfg_rules.ini +++ b/src/config/cfg_rules.ini @@ -229,6 +229,7 @@ option = description # Secrets service option = provider option = containers_nest_level +option = max_entries # Secrets service - proxy option = proxy_url option = auth_type diff --git a/src/config/etc/sssd.api.conf b/src/config/etc/sssd.api.conf index a7757dc..0fb1224 100644 --- a/src/config/etc/sssd.api.conf +++ b/src/config/etc/sssd.api.conf @@ -98,6 +98,7 @@ user_attributes = str, None, false # Secrets service provider = str, None, false containers_nest_level = int, None, false +max_entries = 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 1acaa98..05e3902 100644 --- a/src/man/sssd-secrets.5.xml +++ b/src/man/sssd-secrets.5.xml @@ -156,6 +156,18 @@ systemctl enable sssd-secrets.service </para> </listitem> </varlistentry> + <varlistentry> + <term>max_entries (integer)</term> + <listitem> + <para> + This option specifies the maximum number of secrets that + can be stored. + </para> + <para> + Default: 1024 + </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 ec84537..ab1fc27 100644 --- a/src/responder/secrets/local.c +++ b/src/responder/secrets/local.c @@ -30,6 +30,7 @@ struct local_context { struct ldb_context *ldb; struct sec_data master_key; int containers_nest_level; + int max_entries; }; static int local_decrypt(struct local_context *lctx, TALLOC_CTX *mem_ctx, @@ -353,6 +354,43 @@ static int local_db_check_containers_nest_level(struct local_context *lctx, return EOK; } +static int local_db_check_number_of_entries(TALLOC_CTX *mem_ctx, + struct local_context *lctx) +{ + TALLOC_CTX *tmp_ctx; + static const char *attrs[] = { NULL }; + struct ldb_result *res = NULL; + struct ldb_dn *dn; + int ret; + + tmp_ctx = talloc_new(mem_ctx); + if (!tmp_ctx) return ENOMEM; + + dn = ldb_dn_new(tmp_ctx, lctx->ldb, "cn=secrets"); + if (!dn) { + ret = ENOMEM; + goto done; + } + + ret = ldb_search(lctx->ldb, tmp_ctx, &res, dn, LDB_SCOPE_SUBTREE, + attrs, LOCAL_SIMPLE_FILTER); + + if (res->count >= lctx->max_entries) { + DEBUG(SSSDBG_OP_FAILURE, + "Cannot store any more secrets as the maximum allowed limit (%d) " + "has been reached\n", lctx->max_entries); + + ret = ERR_SEC_INVALID_TOO_MANY_SECRETS; + goto done; + } + + ret = EOK; + +done: + talloc_free(tmp_ctx); + return ret; +} + static int local_db_put_simple(TALLOC_CTX *mem_ctx, struct local_context *lctx, const char *req_path, @@ -376,6 +414,9 @@ static int local_db_put_simple(TALLOC_CTX *mem_ctx, ret = local_db_check_containers(msg, lctx, msg->dn); if (ret != EOK) goto done; + ret = local_db_check_number_of_entries(msg, lctx); + if (ret != EOK) goto done; + ret = local_encrypt(lctx, msg, secret, enctype, &enc_secret); if (ret != EOK) goto done; @@ -733,6 +774,7 @@ int local_secrets_provider_handle(struct sec_ctx *sctx, } lctx->containers_nest_level = sctx->containers_nest_level; + lctx->max_entries = sctx->max_entries; 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 aedb49a..55104fd 100644 --- a/src/responder/secrets/providers.c +++ b/src/responder/secrets/providers.c @@ -307,6 +307,7 @@ enum sec_http_status_codes sec_errno_to_http_status(errno_t err) return STATUS_405; case EMEDIUMTYPE: case ERR_SEC_INVALID_CONTAINERS_NEST_LEVEL: + case ERR_SEC_INVALID_TOO_MANY_SECRETS: return STATUS_406; case EEXIST: return STATUS_409; diff --git a/src/responder/secrets/secsrv.c b/src/responder/secrets/secsrv.c index 4bbc1dc..aefd6dc 100644 --- a/src/responder/secrets/secsrv.c +++ b/src/responder/secrets/secsrv.c @@ -30,6 +30,7 @@ #define DEFAULT_SEC_FD_LIMIT 2048 #define DEFAULT_SEC_CONTAINERS_NEST_LEVEL 4 +#define DEFAULT_SEC_MAX_ENTRIES 1024 static int sec_get_config(struct sec_ctx *sctx) { @@ -58,6 +59,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_ENTRIES, + DEFAULT_SEC_MAX_ENTRIES, + &sctx->max_entries); + + if (ret != EOK) { + DEBUG(SSSDBG_FATAL_FAILURE, + "Failed to get maximum number of entries\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 8ef89ab..d1a12df 100644 --- a/src/responder/secrets/secsrv.h +++ b/src/responder/secrets/secsrv.h @@ -39,6 +39,7 @@ struct sec_ctx { struct resp_ctx *rctx; int fd_limit; int containers_nest_level; + int max_entries; struct provider_handle **providers; }; diff --git a/src/tests/intg/test_secrets.py b/src/tests/intg/test_secrets.py index c77c2a4..a003d67 100644 --- a/src/tests/intg/test_secrets.py +++ b/src/tests/intg/test_secrets.py @@ -84,6 +84,9 @@ def setup_for_secrets(request): [domain/local] id_provider = local + + [secrets] + max_entries = 10 """).format(**locals()) create_conf_fixture(request, conf) @@ -136,6 +139,18 @@ 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_ENTRIES = 10 + + sec_value = "value" + for x in xrange(MAX_ENTRIES): + cli.set_secret(str(x), sec_value) + + with pytest.raises(HTTPError) as err406: + cli.set_secret(str(MAX_ENTRIES), sec_value) + assert str(err406.value).startswith("406") + def test_containers(setup_for_secrets, secrets_cli): """ diff --git a/src/util/util_errors.c b/src/util/util_errors.c index 7d4a7f5..249e340 100644 --- a/src/util/util_errors.c +++ b/src/util/util_errors.c @@ -99,6 +99,7 @@ struct err_string error_to_str[] = { { "The user is not handled by SSSD" }, /* ERR_NON_SSSD_USER */ { "The internal name format cannot be parsed" }, /* ERR_WRONG_NAME_FORMAT */ { "The maximum level of nested containers has been reached" }, /* ERR_SEC_INVALID_CONTAINERS_NEST_LEVEL */ + { "The maximum number of stored secrets has been reached" }, /* ERR_SEC_INVALID_TOO_MANY_SECRETS */ { "ERR_LAST" } /* ERR_LAST */ }; diff --git a/src/util/util_errors.h b/src/util/util_errors.h index 2cd55e1..f974133 100644 --- a/src/util/util_errors.h +++ b/src/util/util_errors.h @@ -121,6 +121,7 @@ enum sssd_errors { ERR_NON_SSSD_USER, ERR_WRONG_NAME_FORMAT, ERR_SEC_INVALID_CONTAINERS_NEST_LEVEL, + ERR_SEC_INVALID_TOO_MANY_SECRETS, 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