[Patch] Fix the ";binary" suffix behavior
Simo Sorce
simo at samba.org
Wed Aug 10 09:01:45 UTC 2016
On Tue, Aug 09, 2016 at 11:32:31AM -0700, Jeremy Allison wrote:
> On Tue, Aug 09, 2016 at 06:08:04AM +0200, Volker Lendecke wrote:
> > On Mon, Aug 08, 2016 at 05:13:59PM -0700, Jeremy Allison wrote:
> > > On Mon, Aug 08, 2016 at 06:00:00PM +0200, Niklas Abel wrote:
> > >
> > > > No problem, thanks for the review and the fast response.
> > >
> > > OK - LGTM. Can I get a second Team reviewer ?
> >
> > Attached find a set of cosmetic patches on top. Feel free to comment &
> > squash.
> >
> > On the semantics -- RFC4522 describes what this is supposed to do. Can
> > we really just ignore this?
>
> Here is the squashed patchset. Team - please don't push until
> we get clarification on if this is correct w.r.t AD and RFC4522.
RFC4522 requires that attributes that carry the ;binary tag be returned in
binary form (whatever that means for the specific syntax).
Also the comment of the commit is utterly void of useful information.
The author should explain why the commit is needed (normative rferences, test
cases, uses cases, or whatever else) and why the specific implementation path
was chosen if that may be controversial.
This commit looks incomplete in that it does not assure the values are returned
in the appropriate form, but it is possible that no transformation is needed if
we always store them unmodified. A test that insures usercertificate is stored
and returned in the correct format is needed at the very minimum.
Simo.
> Cheers,
>
> Jeremy.
> From 98dd71f121183608d240062ea5cf1539e79cf7b0 Mon Sep 17 00:00:00 2001
> From: Niklas Abel <niklas.abel at lsexperts.de>
> Date: Tue, 9 Aug 2016 11:25:14 -0700
> Subject: [PATCH] s4: ldap: Ignore searches with a suffix of ';binary'.
>
> FIXME: Does this violate RFC4522 ?
>
> https://www.ietf.org/rfc/rfc4522.txt
>
> Major contributions by: Volker Lendecke <vl at samba.org>
>
> Signed-off-by: Niklas Abel <niklas.abel at lsexperts.de>
> Reviewed-by: Jeremy Allison <jra at samba.org>
> ---
> source4/dsdb/samdb/ldb_modules/resolve_oids.c | 75 +++++++++++++++++++++++++++
> source4/dsdb/tests/python/ldap_syntaxes.py | 7 +++
> 2 files changed, 82 insertions(+)
>
> diff --git a/source4/dsdb/samdb/ldb_modules/resolve_oids.c b/source4/dsdb/samdb/ldb_modules/resolve_oids.c
> index b5c5f8e..106f637 100644
> --- a/source4/dsdb/samdb/ldb_modules/resolve_oids.c
> +++ b/source4/dsdb/samdb/ldb_modules/resolve_oids.c
> @@ -440,6 +440,66 @@ static int resolve_oids_callback(struct ldb_request *req, struct ldb_reply *ares
> return LDB_SUCCESS;
> }
>
> +/**
> + * Strips suffix from an attribute,
> + * if there is any.
> +*/
> +const static char *strip_suffix(const void *mem_ctx, const char *attr,
> + const char *suffix)
> +{
> + size_t attr_length = 0;
> + size_t suffix_length = 0;
> + size_t new_attr_size = 0;
> + const char *tmp = NULL;
> + int cmp = -1;
> +
> + if (!attr || !*attr || !suffix || !*suffix) {
> + return talloc_strdup(mem_ctx, attr);
> + }
> + attr_length = strlen(attr);
> + suffix_length = strlen(suffix);
> + if (attr_length < suffix_length) {
> + return attr;
> + }
> + new_attr_size = (attr_length - suffix_length);
> + tmp = attr + new_attr_size;
> + cmp = strcasecmp(suffix, tmp);
> + if (cmp == 0) {
> + return talloc_strndup(mem_ctx, attr, new_attr_size);
> + }
> + return talloc_strdup(mem_ctx, attr);
> +}
> +
> +/**
> + * Modified version of str_list_copy_const() which creates the new list without
> + * entries with a ";binary" tail.
> +*/
> +static const char **str_list_copy_const_clean_suffix(TALLOC_CTX *mem_ctx,
> + const char **list)
> +{
> + int i;
> + const char **ret;
> +
> + if (list == NULL) {
> + return NULL;
> + }
> +
> + ret = talloc_array(mem_ctx, const char *, str_list_length(list)+1);
> + if (ret == NULL) {
> + return NULL;
> + }
> +
> + for (i=0;list && list[i];i++) {
> + ret[i] = strip_suffix(list, list[i], ";binary");
> + if (ret[i] == NULL) {
> + TALLOC_FREE(ret);
> + return NULL;
> + }
> + }
> + ret[i] = NULL;
> + return ret;
> +}
> +
> static int resolve_oids_search(struct ldb_module *module, struct ldb_request *req)
> {
> struct ldb_context *ldb;
> @@ -449,9 +509,11 @@ static int resolve_oids_search(struct ldb_module *module, struct ldb_request *re
> struct resolve_oids_context *ac;
> int ret;
> bool needed = false;
> + bool needclean = false;
> const char * const *attrs1;
> const char **attrs2;
> unsigned int i;
> + const char ** cleaned_attrs;
>
> ldb = ldb_module_get_ctx(module);
> schema = dsdb_get_schema(ldb, NULL);
> @@ -479,6 +541,10 @@ static int resolve_oids_search(struct ldb_module *module, struct ldb_request *re
> const char *p;
> const struct dsdb_attribute *a;
>
> + p = strchr(attrs1[i], ';');
> + if (p != NULL) {
> + needclean = true;
> + }
> p = strchr(attrs1[i], '.');
> if (p == NULL) {
> continue;
> @@ -493,6 +559,15 @@ static int resolve_oids_search(struct ldb_module *module, struct ldb_request *re
> break;
> }
>
> + if (needclean) {
> + cleaned_attrs = str_list_copy_const_clean_suffix(
> + req,
> + discard_const_p(const char *, req->op.search.attrs));
> + if (cleaned_attrs) {
> + req->op.search.attrs = cleaned_attrs;
> + }
> + }
> +
> if (!needed) {
> return ldb_next_request(module, req);
> }
> diff --git a/source4/dsdb/tests/python/ldap_syntaxes.py b/source4/dsdb/tests/python/ldap_syntaxes.py
> index 56a1755..762f66e 100755
> --- a/source4/dsdb/tests/python/ldap_syntaxes.py
> +++ b/source4/dsdb/tests/python/ldap_syntaxes.py
> @@ -82,6 +82,13 @@ systemOnly: FALSE
> self.assertEquals(res[0]["lDAPDisplayName"][0], attr_ldap_display_name)
> self.assertTrue("schemaIDGUID" in res[0])
>
> + # search for created attribute with ";binary" suffix
> + res = []
> + res = self.ldb.search("cn=%s,%s" % (attr_name+";binary", self.schema_dn), scope=SCOPE_BASE, attrs=["*"])
> + self.assertEquals(len(res), 1)
> + self.assertEquals(res[0]["lDAPDisplayName"][0], attr_ldap_display_name)
> + self.assertTrue("schemaIDGUID" in res[0])
> +
> class_name = "test-Class-DN-String" + time.strftime("%s", time.gmtime())
> class_ldap_display_name = class_name.replace("-", "")
>
> --
> 2.8.0.rc3.226.g39d4020
>
More information about the samba-technical
mailing list