Dne 13.2.2012 17:32, Jakub Hrozek napsal(a):
On Fri, Feb 10, 2012 at 02:32:06PM +0100, Pavel Březina wrote:
https://fedorahosted.org/sssd/ticket/1173
sysdb_sudo_build_sudouser(): Are there any cases where username would be
unset but there would be UID and groups?
Probably not.
Can we simplify the logic by
always allocating one of the three (or maybe two..uid and username..
Done.
You should use instead of sysdb_custom_subtree_dn() instead of
ldb_dn_new_fmt() to sanitize the username passed from sudo.
You probably want to use return EOK rather than goto done in
sysdb_sudo_purge_bysudouser() because you test "ret" in the "done" label
and that's uninitialized.
+ if (sudouser == NULL || sudouser[0] == NULL) {
+ goto done;
+ }
Thank you. Done.
The transaction logic should be to only commit before the "done" label
and set in_transaction to false if the commits succeeds. The current
code would try to commit the transaction if it's inside one even if
there was an error. (And remember, even _commit can fail and need a
_cancel).
Done.
This was copy and paste from somewhere. We should really do a macro for it.
Is it possible to squash the sysdb_get_sudo_filter() call that deletes
by netgroup into sysdb_sudo_build_sudouser() ? It's called
unconditionally anyway and we would save a sysdb transaction by doing
only one delete.
I assume that you meant to squash sysdb_sudo_purge_byfilter(sysdb_ctx,
filter /*ngrs*/) to sysdb_sudo_purge_bysudouser(sysdb_ctx, sudouser).
No, unless you want to have one function for two things. The
purge_bysudouser literally searches for all rules that contains at least
one value from sudouser. So far it would work for netgroups as well.
But now we want to remove the RULE that contains netgroup but remove the
VALUE that matches sudouser.
It would be theoretically possible if we would do the comparison of
values with regex (evaluate "+*") but I don't thing it is worth it.
New patch attached.
From b5a3d556d2ec0f705cb10b3c5e88cdd56883d1ed Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Tue, 7 Feb 2012 14:08:55 +0100
Subject: [PATCH] Redesign purging of the sudo cache
https://fedorahosted.org/sssd/ticket/1173
---
src/db/sysdb_sudo.c | 338 ++++++++++++++++++++++++++++++++--------
src/db/sysdb_sudo.h | 18 ++-
src/providers/ldap/sdap_sudo.c | 53 ++++--
3 files changed, 321 insertions(+), 88 deletions(-)
diff --git a/src/db/sysdb_sudo.c b/src/db/sysdb_sudo.c
index 84245d5..b154d58 100644
--- a/src/db/sysdb_sudo.c
+++ b/src/db/sysdb_sudo.c
@@ -339,33 +339,6 @@ done:
return EOK;
}
-static errno_t
-sysdb_sudo_purge_subdir(struct sysdb_ctx *sysdb,
- struct sss_domain_info *domain,
- const char *subdir)
-{
- struct ldb_dn *base_dn = NULL;
- TALLOC_CTX *tmp_ctx = NULL;
- errno_t ret;
-
- tmp_ctx = talloc_new(NULL);
- NULL_CHECK(tmp_ctx, ret, done);
-
- base_dn = sysdb_custom_subtree_dn(sysdb, tmp_ctx, domain->name, subdir);
- NULL_CHECK(base_dn, ret, done);
-
- ret = sysdb_delete_recursive(sysdb, base_dn, true);
- if (ret != EOK) {
- DEBUG(SSSDBG_OP_FAILURE, ("sysdb_delete_recursive failed.\n"));
- goto done;
- }
-
- ret = EOK;
-done:
- talloc_free(tmp_ctx);
- return EOK;
-}
-
errno_t
sysdb_save_sudorule(struct sysdb_ctx *sysdb_ctx,
const char *rule_name,
@@ -400,10 +373,147 @@ sysdb_save_sudorule(struct sysdb_ctx *sysdb_ctx,
return EOK;
}
-errno_t
-sysdb_purge_sudorule_subtree(struct sysdb_ctx *sysdb,
- struct sss_domain_info *domain,
- const char *filter)
+errno_t sysdb_sudo_set_refreshed(struct sysdb_ctx *sysdb,
+ bool refreshed)
+{
+ errno_t ret;
+ struct ldb_dn *dn;
+ TALLOC_CTX *tmp_ctx;
+
+
+ tmp_ctx = talloc_new(NULL);
+ if (!tmp_ctx) {
+ ret = ENOMEM;
+ goto done;
+ }
+
+ dn = ldb_dn_new_fmt(tmp_ctx, sysdb->ldb, SYSDB_TMPL_CUSTOM_SUBTREE,
+ SUDORULE_SUBDIR, sysdb->domain->name);
+ if (!dn) {
+ ret = ENOMEM;
+ goto done;
+ }
+
+ ret = sysdb_set_bool(sysdb, dn, SUDORULE_SUBDIR,
+ SYSDB_SUDO_AT_REFRESHED, refreshed);
+
+done:
+ talloc_free(tmp_ctx);
+ return ret;
+}
+
+errno_t sysdb_sudo_get_refreshed(struct sysdb_ctx *sysdb,
+ bool *refreshed)
+{
+ errno_t ret;
+ struct ldb_dn *dn;
+ TALLOC_CTX *tmp_ctx;
+
+
+ tmp_ctx = talloc_new(NULL);
+ if (!tmp_ctx) {
+ ret = ENOMEM;
+ goto done;
+ }
+
+ dn = ldb_dn_new_fmt(tmp_ctx, sysdb->ldb, SYSDB_TMPL_CUSTOM_SUBTREE,
+ SUDORULE_SUBDIR, sysdb->domain->name);
+ if (!dn) {
+ ret = ENOMEM;
+ goto done;
+ }
+
+ ret = sysdb_get_bool(sysdb, dn, SYSDB_SUDO_AT_REFRESHED, refreshed);
+
+done:
+ talloc_free(tmp_ctx);
+ return ret;
+}
+
+char **sysdb_sudo_build_sudouser(TALLOC_CTX *mem_ctx, const char *username,
+ uid_t uid, char **groupnames)
+{
+ char **sudouser = NULL;
+ int count = 2;
+ errno_t ret;
+ int i;
+
+ if (username == NULL || uid == 0) {
+ return NULL;
+ }
+
+ sudouser = talloc_array(NULL, char*, count);
+ NULL_CHECK(sudouser, ret, done);
+
+ sudouser[0] = talloc_strdup(sudouser, username);
+ NULL_CHECK(sudouser[0], ret, done);
+
+ sudouser[1] = talloc_asprintf(sudouser, "#%llu", (unsigned long long)uid);
+ NULL_CHECK(sudouser[1], ret, done);
+
+ if (groupnames != NULL) {
+ for (i = 0; groupnames[i] != NULL; i++) {
+ count++;
+ sudouser = talloc_realloc(NULL, sudouser, char*, count);
+ NULL_CHECK(sudouser, ret, done);
+
+ sudouser[count - 1] = talloc_asprintf(sudouser, "%s", groupnames[i]);
+ NULL_CHECK(sudouser[count - 1], ret, done);
+ }
+ }
+
+ /* NULL */
+ count++;
+ sudouser = talloc_realloc(NULL, sudouser, char*, count);
+ NULL_CHECK(sudouser, ret, done);
+ sudouser[count - 1] = NULL;
+
+ ret = EOK;
+
+done:
+ if (ret != EOK) {
+ talloc_free(sudouser);
+ return NULL;
+ }
+
+ return talloc_steal(mem_ctx, sudouser);
+}
+
+/* ==================== Purge functions ==================== */
+
+errno_t sysdb_sudo_purge_all(struct sysdb_ctx *sysdb)
+{
+ struct ldb_dn *base_dn = NULL;
+ TALLOC_CTX *tmp_ctx = NULL;
+ errno_t ret;
+
+ tmp_ctx = talloc_new(NULL);
+ NULL_CHECK(tmp_ctx, ret, done);
+
+ base_dn = sysdb_custom_subtree_dn(sysdb, tmp_ctx, sysdb->domain->name,
+ SUDORULE_SUBDIR);
+ NULL_CHECK(base_dn, ret, done);
+
+ ret = sysdb_delete_recursive(sysdb, base_dn, true);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE, ("sysdb_delete_recursive failed.\n"));
+ goto done;
+ }
+
+ ret = EOK;
+done:
+ talloc_free(tmp_ctx);
+ return EOK;
+}
+
+errno_t sysdb_sudo_purge_byname(struct sysdb_ctx *sysdb,
+ const char *name)
+{
+ return sysdb_delete_custom(sysdb, name, SUDORULE_SUBDIR);
+}
+
+errno_t sysdb_sudo_purge_byfilter(struct sysdb_ctx *sysdb,
+ const char *filter)
{
TALLOC_CTX *tmp_ctx;
size_t count;
@@ -419,7 +529,7 @@ sysdb_purge_sudorule_subtree(struct sysdb_ctx *sysdb,
/* just purge all if there's no filter */
if (!filter) {
- return sysdb_sudo_purge_subdir(sysdb, domain, SUDORULE_SUBDIR);
+ return sysdb_sudo_purge_all(sysdb);
}
tmp_ctx = talloc_new(NULL);
@@ -446,7 +556,7 @@ sysdb_purge_sudorule_subtree(struct sysdb_ctx *sysdb,
continue;
}
- ret = sysdb_delete_custom(sysdb, name, SUDORULE_SUBDIR);
+ ret = sysdb_sudo_purge_byname(sysdb, name);
if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE, ("Could not delete rule %s\n", name));
goto done;
@@ -459,59 +569,155 @@ done:
return ret;
}
-errno_t sysdb_sudo_set_refreshed(struct sysdb_ctx *sysdb,
- bool refreshed)
+errno_t sysdb_sudo_purge_bysudouser(struct sysdb_ctx *sysdb,
+ char **sudouser)
{
+ TALLOC_CTX *tmp_ctx = NULL;
+ char *filter = NULL;
+ char *value = NULL;
+ const char *rule_name = NULL;
+ struct ldb_message_element *attr = NULL;
+ struct ldb_message *msg = NULL;
+ struct ldb_message **rules = NULL;
+ size_t num_rules;
errno_t ret;
- struct ldb_dn *dn;
- TALLOC_CTX *tmp_ctx;
+ errno_t sret;
+ int lret;
+ int i, j, k;
+ bool in_transaction = false;
+ const char *attrs[] = { SYSDB_OBJECTCLASS,
+ SYSDB_NAME,
+ SYSDB_SUDO_CACHE_AT_USER,
+ NULL };
+ if (sudouser == NULL || sudouser[0] == NULL) {
+ return EOK;
+ }
tmp_ctx = talloc_new(NULL);
- if (!tmp_ctx) {
- ret = ENOMEM;
+ NULL_CHECK(tmp_ctx, ret, done);
+
+ /* create search filter */
+ filter = talloc_strdup(tmp_ctx, "(|");
+ NULL_CHECK(filter, ret, done);
+ for (i = 0; sudouser[i] != NULL; i++) {
+ filter = talloc_asprintf_append(filter, "(%s=%s)",
+ SYSDB_SUDO_CACHE_AT_USER, sudouser[i]);
+ NULL_CHECK(filter, ret, done);
+ }
+ filter = talloc_strdup_append(filter, ")");
+ NULL_CHECK(filter, ret, done);
+
+ /* search the rules */
+ ret = sysdb_search_custom(tmp_ctx, sysdb, filter, SUDORULE_SUBDIR, attrs,
+ &num_rules, &rules);
+ if (ret != EOK && ret != ENOENT) {
+ DEBUG(SSSDBG_CRIT_FAILURE, ("Error looking up SUDO rules"));
+ goto done;
+ } if (ret == ENOENT) {
+ DEBUG(SSSDBG_TRACE_FUNC, ("No rules matched\n"));
+ ret = EOK;
goto done;
}
- dn = ldb_dn_new_fmt(tmp_ctx, sysdb->ldb, SYSDB_TMPL_CUSTOM_SUBTREE,
- SUDORULE_SUBDIR, sysdb->domain->name);
- if (!dn) {
- ret = ENOMEM;
+ ret = sysdb_transaction_start(sysdb);
+ if (ret != EOK) {
goto done;
}
+ in_transaction = true;
- ret = sysdb_set_bool(sysdb, dn, SUDORULE_SUBDIR,
- SYSDB_SUDO_AT_REFRESHED, refreshed);
+ /*
+ * remove values from sudoUser and delete the rule
+ * if the attribute is empty afterwards
+ */
-done:
- talloc_free(tmp_ctx);
- return ret;
-}
+ for (i = 0; i < num_rules; i++) {
+ /* find name */
+ rule_name = ldb_msg_find_attr_as_string(rules[i], SYSDB_NAME, NULL);
+ if (rule_name == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE, ("A rule without a name?\n"));
+ /* skip this one but still delete other entries */
+ continue;
+ }
-errno_t sysdb_sudo_get_refreshed(struct sysdb_ctx *sysdb,
- bool *refreshed)
-{
- errno_t ret;
- struct ldb_dn *dn;
- TALLOC_CTX *tmp_ctx;
+ /* find sudoUser */
+ attr = ldb_msg_find_element(rules[i], SYSDB_SUDO_CACHE_AT_USER);
+ if (attr == NULL) {
+ /* this should never happen because we search by this attribute */
+ DEBUG(SSSDBG_CRIT_FAILURE, ("BUG: sudoUser attribute is missing\n"));
+ continue;
+ }
+ /* create message */
+ msg = ldb_msg_new(tmp_ctx);
+ NULL_CHECK(msg, ret, done);
- tmp_ctx = talloc_new(NULL);
- if (!tmp_ctx) {
- ret = ENOMEM;
- goto done;
- }
+ msg->dn = ldb_dn_new_fmt(msg, sysdb->ldb, SYSDB_TMPL_CUSTOM, rule_name,
+ SUDORULE_SUBDIR, sysdb->domain->name);
+ NULL_CHECK(msg->dn, ret, done);
- dn = ldb_dn_new_fmt(tmp_ctx, sysdb->ldb, SYSDB_TMPL_CUSTOM_SUBTREE,
- SUDORULE_SUBDIR, sysdb->domain->name);
- if (!dn) {
- ret = ENOMEM;
- goto done;
+ /* create empty sudoUser */
+ lret = ldb_msg_add_empty(msg, SYSDB_SUDO_CACHE_AT_USER,
+ LDB_FLAG_MOD_DELETE, NULL);
+ if (lret != LDB_SUCCESS) {
+ ret = sysdb_error_to_errno(lret);
+ goto done;
+ }
+
+ /* filter values */
+ for (j = 0; j < attr->num_values; j++) {
+ value = (char*)(attr->values[j].data);
+ for (k = 0; sudouser[k] != NULL; k++) {
+ if (strcmp(value, sudouser[k]) == 0) {
+ /* delete value from cache */
+ lret = ldb_msg_add_string(msg, SYSDB_SUDO_CACHE_AT_USER,
+ sudouser[k]);
+ if (lret != LDB_SUCCESS) {
+ ret = sysdb_error_to_errno(lret);
+ goto done;
+ }
+ break;
+ }
+ }
+ }
+
+ /* update the cache */
+ if (msg->elements[0].num_values == attr->num_values) {
+ /* sudoUser would remain empty, delete the rule */
+ ret = sysdb_sudo_purge_byname(sysdb, rule_name);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE, ("Could not delete rule %s\n",
+ rule_name));
+ goto done;
+ }
+ } else {
+ /* sudoUser will not be empty, modify the rule */
+ lret = ldb_modify(sysdb->ldb, msg);
+ if (lret != LDB_SUCCESS) {
+ DEBUG(SSSDBG_OP_FAILURE, ("Could not modify rule %s\n",
+ rule_name));
+ ret = sysdb_error_to_errno(lret);
+ goto done;
+ }
+ }
+
+ talloc_free(msg);
}
- ret = sysdb_get_bool(sysdb, dn, SYSDB_SUDO_AT_REFRESHED, refreshed);
+ ret = sysdb_transaction_commit(sysdb);
+ if (ret == EOK) {
+ in_transaction = false;
+ }
done:
+ if (in_transaction) {
+ sret = sysdb_transaction_cancel(sysdb);
+ if (sret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE, ("Could not cancel transaction\n"));
+ }
+ }
+
talloc_free(tmp_ctx);
return ret;
}
+
diff --git a/src/db/sysdb_sudo.h b/src/db/sysdb_sudo.h
index 87a3b94..fc05640 100644
--- a/src/db/sysdb_sudo.h
+++ b/src/db/sysdb_sudo.h
@@ -81,14 +81,24 @@ sysdb_save_sudorule(struct sysdb_ctx *sysdb_ctx,
const char *rule_name,
struct sysdb_attrs *attrs);
-errno_t sysdb_purge_sudorule_subtree(struct sysdb_ctx *sysdb,
- struct sss_domain_info *domain,
- const char *filter);
-
errno_t sysdb_sudo_set_refreshed(struct sysdb_ctx *sysdb,
bool refreshed);
errno_t sysdb_sudo_get_refreshed(struct sysdb_ctx *sysdb,
bool *refreshed);
+char **sysdb_sudo_build_sudouser(TALLOC_CTX *mem_ctx, const char *username,
+ uid_t uid, char **groupnames);
+
+errno_t sysdb_sudo_purge_all(struct sysdb_ctx *sysdb);
+
+errno_t sysdb_sudo_purge_byname(struct sysdb_ctx *sysdb,
+ const char *name);
+
+errno_t sysdb_sudo_purge_byfilter(struct sysdb_ctx *sysdb,
+ const char *filter);
+
+errno_t sysdb_sudo_purge_bysudouser(struct sysdb_ctx *sysdb,
+ char **sudoUser);
+
#endif /* _SYSDB_SUDO_H_ */
diff --git a/src/providers/ldap/sdap_sudo.c b/src/providers/ldap/sdap_sudo.c
index 5c7448f..5add7c8 100644
--- a/src/providers/ldap/sdap_sudo.c
+++ b/src/providers/ldap/sdap_sudo.c
@@ -653,6 +653,7 @@ int sdap_sudo_purge_sudoers(struct sysdb_ctx *sysdb_ctx,
{
TALLOC_CTX *tmp_ctx;
char *filter = NULL;
+ char **sudouser = NULL;
int ret = EOK;
tmp_ctx = talloc_new(NULL);
@@ -663,18 +664,43 @@ int sdap_sudo_purge_sudoers(struct sysdb_ctx *sysdb_ctx,
switch (sudo_req->type) {
case BE_REQ_SUDO_ALL:
- filter = NULL;
+ DEBUG(SSSDBG_TRACE_FUNC, ("Purging SUDOers cache of all rules\n"));
+ ret = sysdb_sudo_purge_all(sysdb_ctx);
break;
case BE_REQ_SUDO_DEFAULTS:
- ret = sysdb_get_sudo_filter(tmp_ctx, NULL, 0, NULL,
- SYSDB_SUDO_FILTER_INCLUDE_DFL, &filter);
+ DEBUG(SSSDBG_TRACE_FUNC, ("Purging SUDOers cache of default options\n"));
+ ret = sysdb_sudo_purge_byname(sysdb_ctx, SDAP_SUDO_DEFAULTS);
break;
case BE_REQ_SUDO_USER:
- ret = sysdb_get_sudo_filter(tmp_ctx, sudo_req->username, sudo_req->uid,
- sudo_req->groups,
- SYSDB_SUDO_FILTER_USERINFO
- | SYSDB_SUDO_FILTER_INCLUDE_ALL
- | SYSDB_SUDO_FILTER_INCLUDE_DFL, &filter);
+ DEBUG(SSSDBG_TRACE_FUNC, ("Purging SUDOers cache of user's [%s] rules\n",
+ sudo_req->username));
+
+ /* netgroups */
+ ret = sysdb_get_sudo_filter(tmp_ctx, NULL, 0, NULL,
+ SYSDB_SUDO_FILTER_NGRS, &filter);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE, ("Unable to create filter to purge "
+ "SUDOers cache [%d]: %s\n", ret, strerror(ret)));
+ goto done;
+ }
+
+ ret = sysdb_sudo_purge_byfilter(sysdb_ctx, filter);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE, ("Unable to purge SUDOers cache "
+ "(netgroups) [%d]: %s\n", ret, strerror(ret)));
+ goto done;
+ }
+
+ /* user, uid, groups */
+ sudouser = sysdb_sudo_build_sudouser(tmp_ctx, sudo_req->username,
+ sudo_req->uid, sudo_req->groups);
+ if (sudouser == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, ("Unable to create sudoUser to purge "
+ "SUDOers cache [%d]: %s\n", ret, strerror(ret)));
+ goto done;
+ }
+
+ ret = sysdb_sudo_purge_bysudouser(sysdb_ctx, sudouser);
break;
default:
DEBUG(SSSDBG_CRIT_FAILURE, ("Invalid request type %d\n", sudo_req->type));
@@ -682,16 +708,7 @@ int sdap_sudo_purge_sudoers(struct sysdb_ctx *sysdb_ctx,
}
if (ret != EOK) {
- DEBUG(SSSDBG_CRIT_FAILURE, ("Unable to create filter to purge "
- "sudoers cache [%d]: %s\n", ret, strerror(ret)));
- goto done;
- }
-
- /* Purge rules */
- DEBUG(SSSDBG_TRACE_FUNC, ("Purging sudo cache with filter [%s]\n", filter));
- ret = sysdb_purge_sudorule_subtree(sysdb_ctx, domain, filter);
- if (ret != EOK) {
- DEBUG(SSSDBG_CRIT_FAILURE, ("Unable to purge sudoers cache [%d]: %s\n",
+ DEBUG(SSSDBG_CRIT_FAILURE, ("Unable to purge SUDOers cache [%d]: %s\n",
ret, strerror(ret)));
goto done;
}
--
1.7.6.5
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
