[SSSD] [PATCHES] Sysdb interface for netgroups
Sumit Bose
sbose at redhat.com
Mon Sep 20 10:46:16 UTC 2010
On Fri, Sep 17, 2010 at 11:21:40AM -0400, Stephen Gallagher wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 09/17/2010 11:16 AM, Dmitri Pal wrote:
> > Stephen Gallagher wrote:
> >> On 09/16/2010 05:05 PM, Stephen Gallagher wrote:
> >>> I've rewritten these patches. Now, instead of searching for individual
> >>> netgroup entries, the code will take advantage of the memberOf plugin to
> >>> return all netgroup triples in a single call to sysdb_getnetgr()
> >>
> >>> This approach will make life easier with LDAP as well, as we will have
> >>> built-in loop prevention.
> >>
> >>> Please review these patches while my netgroup work is ongoing.
> >>
> >>> Patch 0001: New sysdb interfaces for netgroups. Add and delete support
> >>> for netgroups entries, tuples and nested netgroups.
> >>
> >>> Patch 0002: Unit tests for the above interfaces.
> >>
> >>
> >>
> >> Yet another change to these patches.
> >>
> >> I realized that I actually need to get back the three strings from the
> >> triple instead of the internal representation. It makes the most sense
> >> for this to be done in sysdb_getnetgr(), so attached are two new patches
> >> which changes the interface (again) to return a structure of three
> >> strings instead of a single string representing the tuple.
> >>
> >>
> > Stephen, I just glanced over. Did not have a thorough review.
> > There is a bug in sysdb_netgr_split_triple
> > The user, host and domain strings need to be initialized to NULL.
> >
> > Have not looked at other places :)
> >
>
> Ah, you're correct. I accidentally initialized the wrong pointers (the
> p_* pointers). Thanks for that. New patch attached.
>
>
> This is what I get for not writing exhaustive unit tests, but I'm
> working on a deadline here :)
>
The patches apply fine. There is a whitespace issue, 0001 adds an empty
line at the end of sysdb_search.c . All tests passed, please find some
comments below.
bye,
Sumit
> - --
> Stephen Gallagher
> RHCE 804006346421761
>
> Delivering value year after year.
> Red Hat ranks #1 in value among software vendors.
> http://www.redhat.com/promo/vendor/
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkyTh4QACgkQeiVVYja6o6N1DwCeItdwGQoqPStrHs76nbzmHAIc
> IyoAn3kLMRSzopQeJevNMD4RbrJpHmha
> =IvTL
> -----END PGP SIGNATURE-----
...
> +int sysdb_set_netgroup_attr(struct sysdb_ctx *ctx,
> + struct sss_domain_info *domain,
> + const char *name,
> + struct sysdb_attrs *attrs,
> + int mod_op)
> +{
> + errno_t ret;
> + struct ldb_dn *dn;
> + TALLOC_CTX *tmp_ctx;
> +
> + tmp_ctx = talloc_new(NULL);
> + if (!tmp_ctx) {
> + return ENOMEM;
> + }
> +
> + dn = sysdb_netgroup_dn(ctx, tmp_ctx, domain->name, name);
> + if (!dn) {
ret is undefined
> + goto done;
> + }
> +
> + ret = sysdb_set_entry_attr(tmp_ctx, ctx, dn, attrs, mod_op);
> +
> +done:
> + talloc_free(tmp_ctx);
> + return ret;
> +}
>
....
> +int sysdb_delete_netgroup(struct sysdb_ctx *sysdb,
> + struct sss_domain_info *domain,
> + const char *name)
> +{
> + TALLOC_CTX *tmp_ctx;
> + struct ldb_message *msg;
> + int ret;
> +
> + if (!name) return EINVAL;
> +
> + tmp_ctx = talloc_new(NULL);
> + if (!tmp_ctx) {
> + return ENOMEM;
> + }
> +
> + ret = sysdb_search_netgroup_by_name(tmp_ctx, sysdb,
> + domain, name, NULL, &msg);
> + if (ret) {
> + DEBUG(6, ("sysdb_search_netgroup_by_name failed: %d (%s)\n",
> + ret, strerror(ret)));
> + goto fail;
> + }
> +
> + ret = sysdb_delete_entry(sysdb, msg->dn, false);
> + if (ret) {
> + goto fail;
> + }
> +
> + talloc_free(tmp_ctx);
> + return EOK;
> +
> +fail:
> + DEBUG(6, ("Error: %d (%s)\n", ret, strerror(ret)));
I think it would better match with the other functions to have a 'done'
block which is also called if ret==EOK.
> + talloc_free(tmp_ctx);
> + return ret;
> +}
> +
...
> +/* This function splits a three-tuple into three strings
> + * It assumes that any whitespace between the parentheses
> + * and commas are intentional and does not attempt to
> + * strip them out. Leading and trailing whitespace is
> + * ignored.
> + *
> + * This behavior is compatible with nss_ldap's
> + * implementation.
> + */
> +static errno_t sysdb_netgr_split_triple(TALLOC_CTX *mem_ctx,
> + const char *triple,
> + char **hostname,
> + char **username,
> + char **domainname)
> +{
> + errno_t ret;
> + TALLOC_CTX *tmp_ctx;
> + const char *p = triple;
> + const char *p_host;
> + const char *p_user;
> + const char *p_domain;
> + size_t len;
> +
> + char *host = NULL;
> + char *user = NULL;
> + char *domain = NULL;
> +
> + /* Pre-set the values to NULL here so if they are not
> + * copied, we don't return garbage below.
> + */
> + *hostname = NULL;
> + *username = NULL;
> + *domainname = NULL;
> +
> + tmp_ctx = talloc_new(NULL);
> + if (!tmp_ctx) {
> + return ENOMEM;
> + }
> +
> + /* Remove any leading whitespace */
> + while (*p && isspace(*p)) p++;
> +
> + if (*p != '(') {
> + /* Triple must start and end with parentheses */
> + ret = EINVAL;
> + goto done;
> + }
> + p++;
> + p_host = p;
> +
> + /* Find the first comma */
> + while (*p && *p != ',') p++;
> +
> + if (!*p) {
> + /* No comma was found: parse error */
> + ret = EINVAL;
> + goto done;
> + }
> +
> + len = p - p_host;
> +
> + if (len > 0) {
> + /* Copy the host string */
> + host = talloc_array(tmp_ctx, char, len+1);
> + if (!host) {
> + ret = ENOMEM;
> + goto done;
> + }
> +
> + memcpy(host, p_host, len);
> + host[len] = '\0';
talloc_strndup might be more convenient here
> + }
> + p++;
> + p_user = p;
> +
> + /* Find the second comma */
> + while (*p && *p != ',') p++;
> +
> + if (!*p) {
> + /* No comma was found: parse error */
> + ret = EINVAL;
> + goto done;
> + }
> +
> + len = p - p_user;
> +
> + if (len > 0) {
> + /* Copy the user string */
> + user = talloc_array(tmp_ctx, char, len+1);
> + if (!user) {
> + ret = ENOMEM;
> + goto done;
> + }
> +
> + memcpy(user, p_user, len);
> + user[len] = '\0';
> + }
> + p++;
> + p_domain = p;
> +
> + /* Find the closing parenthesis */
> + while (*p && *p != ')') p++;
> +
I think a missing closing parenthesis should be an error.
> + len = p - p_domain;
> +
> + if (len > 0) {
> + /* Copy the domain string */
> + domain = talloc_array(tmp_ctx, char, len+1);
> + if (!domain) {
> + ret = ENOMEM;
> + goto done;
> + }
> +
> + memcpy(domain, p_domain, len);
> + domain[len] = '\0';
> + }
> + p++;
> +
> + /* skip trailing whitespace */
> + while (*p && isspace(*p)) p++;
> +
> + if (*p) {
> + /* Extra data after the closing parenthesis
> + * is a parse error
> + */
> + ret = EINVAL;
> + goto done;
> + }
> +
> + /* Return any non-NULL values */
> + if (host) {
> + *hostname = talloc_steal(mem_ctx, host);
> + }
> +
> + if (user) {
> + *username = talloc_steal(mem_ctx, user);
> + }
> +
> + if (domain) {
> + *domainname = talloc_steal(mem_ctx, domain);
> + }
> +
> + ret = EOK;
> +
> +done:
> + talloc_free(tmp_ctx);
> + return ret;
> +}
...
> +
> +errno_t sysdb_getnetgr(TALLOC_CTX *mem_ctx,
> + struct sysdb_ctx *ctx,
> + struct sss_domain_info *domain,
> + const char *netgroup,
> + struct sysdb_netgroup_ctx ***triples)
> +{
> + TALLOC_CTX *tmp_ctx;
> + static const char *attrs[] = SYSDB_NETGR_ATTRS;
> + struct ldb_dn *base_dn;
> + struct ldb_result *result;
> + char *netgroup_dn;
> + char *triple_str;
> + struct ldb_message_element *el;
> + struct sysdb_netgroup_ctx **tmp_triples = NULL;
> + int lret;
> + int i, j;
> + size_t size = 0;
> + errno_t ret;
> +
> + if (!domain) {
> + return EINVAL;
> + }
> +
> + tmp_ctx = talloc_new(NULL);
> + if (!tmp_ctx) {
> + return ENOMEM;
> + }
> +
> + base_dn = ldb_dn_new_fmt(tmp_ctx, ctx->ldb,
> + SYSDB_TMPL_NETGROUP_BASE,
> + domain->name);
> + if (!base_dn) {
> + ret = ENOMEM;
> + goto done;
> + }
> +
> + netgroup_dn = talloc_asprintf(tmp_ctx, SYSDB_TMPL_NETGROUP,
> + netgroup, domain->name);
> + if (!netgroup_dn) {
> + ret = ENOMEM;
> + goto done;
> + }
> +
> + lret = ldb_search(ctx->ldb, tmp_ctx, &result, base_dn,
> + LDB_SCOPE_SUBTREE, attrs,
> + SYSDB_NETGR_TRIPLES_FILTER,
> + netgroup, netgroup_dn);
> + ret = sysdb_error_to_errno(lret);
> + if (ret != EOK) {
> + goto done;
> + }
> +
> + if(result->count == 0) {
> + ret = ENOENT;
> + goto done;
> + }
> +
> + for (i=0; i < result->count; i++) {
> + el = ldb_msg_find_element(result->msgs[i], SYSDB_NETGROUP_TRIPLE);
> + if (!el) {
> + /* No triples in this netgroup. It might be a nesting
> + * container only.
> + * Skip it and continue on.
> + */
> + continue;
> + }
> +
> + /* Enlarge the array by the value count
> + * Always keep one extra entry for the NULL terminator
> + */
> + tmp_triples = talloc_realloc(tmp_ctx, tmp_triples,
> + struct sysdb_netgroup_ctx *,
> + size+el->num_values+1);
> + if (!tmp_triples) {
> + ret = ENOMEM;
> + goto done;
> + }
> +
> + /* Copy in all of the triples */
> + for(j = 0; j < el->num_values; j++) {
> + triple_str = talloc_strdup(tmp_ctx,
> + (const char *)el->values[j].data);
I think it would be safer to use talloc_strndup with el->values[j].length
> + if (!triple_str) {
> + ret = ENOMEM;
> + goto done;
> + }
> +
> + tmp_triples[size] = talloc_zero(tmp_triples,
> + struct sysdb_netgroup_ctx);
> + if (!tmp_triples[size]) {
> + ret = ENOMEM;
> + goto done;
> + }
> +
> + ret = sysdb_netgr_split_triple(tmp_triples[size],
> + triple_str,
> + &tmp_triples[size]->hostname,
> + &tmp_triples[size]->username,
> + &tmp_triples[size]->domainname);
> + if (ret != EOK) {
> + goto done;
> + }
> +
> + size++;
> + }
> + }
> +
> + if (!tmp_triples) {
> + /* No entries were found
> + * Create a dummy reply
> + */
> + tmp_triples = talloc_array(tmp_ctx, struct sysdb_netgroup_ctx *, 1);
> + if (!tmp_triples) {
> + ret = ENOMEM;
> + goto done;
> + }
> + }
> + /* Add NULL terminator */
> + tmp_triples[size] = NULL;
> +
> + *triples = talloc_steal(mem_ctx, tmp_triples);
> + ret = EOK;
> +
> +done:
> + talloc_zfree(tmp_ctx);
> + return ret;
> +}
More information about the sssd-devel
mailing list