[Patch] Fix the ";binary" suffix behavior
Niklas Abel
niklas.abel at lsexperts.de
Wed Aug 10 22:26:34 UTC 2016
On 08/10/2016 11:01 AM, Simo Sorce wrote:
> 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.
>
The patch is a result of the following conversation:
https://lists.samba.org/archive/samba/2015-November/196049.html
It will be used to share S/MIME certificates.
My test setup is a Debian based VM as a server.
The test call is:
ldapsearch -H ldap://127.0.0.1 -U Administrator at X.LOCAL -b
"OU=testou,DC=X,DC=LOCAL" -LLL "userCertificate;binary"
which should result with the certificate as same as:
ldapsearch -H ldap://127.0.0.1 -U Administrator at X.LOCAL -b
"OU=testou,DC=X,DC=LOCAL" -LLL "userCertificate"
It works fine with the written patch.
Bye,
Niklas
>> 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
>>
--
Niklas Abel (IT Security Consultant), http://www.lsexperts.de
LSE Leading Security Experts GmbH, Postfach 100121, 64201 Darmstadt
Tel: +49 6151 86086-261, Fax: -299, Mobil: +49 151 26467737
Unternehmenssitz: Weiterstadt, Amtsgericht Darmstadt: HRB8649
Geschäftsführer: Nils Manegold, Oliver Michel, Arved Graf von Stackelberg, Sven Walther
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 3958 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160811/08ff8aad/smime.bin>
More information about the samba-technical
mailing list