[Patch] Fix the ";binary" suffix behavior
Niklas Abel
niklas.abel at lsexperts.de
Fri Aug 5 10:56:35 UTC 2016
Hey Jeremy,
here is the new patch version.
On 08/02/2016 11:02 PM, Jeremy Allison wrote:
> On Mon, Jun 27, 2016 at 06:59:07PM +0200, Niklas Abel wrote:
>> --
>> 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
> Sorry for the delay, here's the review !
>
> Getting there, but still needs some work I think.
>
>> From 61bc531dcdc799114dda367355038b799f9526c3 Mon Sep 17 00:00:00 2001
>> From: Niklas Abel <niklas.abel at lsexperts.de>
>> Date: Mon, 27 Jun 2016 18:26:54 +0200
>> Subject: [PATCH] Decription:
>> Fixed ";binary" suffix problem.
>> Search requests with the ";binary" suffix can now be searched with samba.
>> The suffix will now be ignored as it is meaningless to samba.
>>
>> BUG: https://lists.samba.org/archive/samba-technical/2016-May/114242.html
>> Signed-off-by: Niklas Abel <niklas.abel at lsexperts.de>
>> ---
>> source4/dsdb/samdb/ldb_modules/resolve_oids.c | 65 +++++++++++++++++++++++++++
>> source4/dsdb/tests/python/ldap_syntaxes.py | 7 +++
>> 2 files changed, 72 insertions(+)
>>
>> diff --git a/source4/dsdb/samdb/ldb_modules/resolve_oids.c b/source4/dsdb/samdb/ldb_modules/resolve_oids.c
>> index b5c5f8e..fd23866 100644
>> --- a/source4/dsdb/samdb/ldb_modules/resolve_oids.c
>> +++ b/source4/dsdb/samdb/ldb_modules/resolve_oids.c
>> @@ -440,6 +440,62 @@ static int resolve_oids_callback(struct ldb_request *req, struct ldb_reply *ares
>> return LDB_SUCCESS;
>> }
> 1). Tabs rather than spaces please ! If conditions always
> must use { } framing even if only one executed clause.
> Check out the Samba coding standards doc.
Sorry for that, fixed it.
>
>> +/**
>> + * Strips ";binary" suffix from an attribute,
>> + * if there is any because its meaningless to us.
>> +*/
> 2). This function should take the suffix to remove as a parameter.
> That way we can re-use it if we ever want to strip something else.
Changed it
>> +const static char * strip_binary_attribute (const void *mem_ctx, const char *attr)
>> +{
>> + /* suffix which has to be removed */
>> + const char *suffix = ";binary";
>> + size_t attr_length = 0;
>> + size_t suffix_length = strlen(suffix);
>> + size_t new_attr_size = 0;
>> + const char *tmp = NULL;
>> + const static char *result = NULL;
>> +
>> + if (!attr || !*attr) {
> if !attr || *attr return attr, not NULL.
> That's the default (no change) case.
Changed it
>
>> + return NULL;
>> + }
>> + attr_length = strlen(attr);
>> + if (attr_length <= suffix_length) {
> Doesn't the above case break if attr == ";binary" ?
> Shouldn't we return "" in this case ?
Changed it to "if (attr_length < suffix_length)",
it returns an empty string if attr == binary.
>
>> + return attr;
>> + }
>> + new_attr_size = (attr_length - suffix_length);
>> + tmp = attr + new_attr_size;
>> + if (0==strcasecmp(suffix, tmp)) {
> Boundary condition (mentioned above) where new_attr_size == 0.
> Does this handle it right ?
Yes it does, it returns an empty string in case of new_attr_size == 0.
>> + result = talloc_strndup(mem_ctx, attr, new_attr_size);
>> + if (result) {
>> + return result;
>> + }
>> + }
>> + return attr;
>> +}
>> +
>> +/**
>> + * Modified version of str_list_copy_const() which creates the new list without
>> + * entries with a ";binary" tail.
>> +*/
>> +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_binary_attribute(mem_ctx, list[i]);
>> + }
>> + ret[i] = NULL;
>> + return ret;
>> +}
>> +
>> static int resolve_oids_search(struct ldb_module *module, struct ldb_request *req)
>> {
>> struct ldb_context *ldb;
>> @@ -449,6 +505,7 @@ 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;
>> @@ -479,6 +536,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;
>> @@ -492,6 +553,10 @@ static int resolve_oids_search(struct ldb_module *module, struct ldb_request *re
>> needed = true;
>> break;
>> }
>> + if (needclean) {
>> + req->op.search.attrs = str_list_copy_const_clean_suffix(ldb,
>> + discard_const_p(const char *, req->op.search.attrs));
> Error condition if str_list_copy_const_clean_suffix returns NULL ?
Fixed it
It would be really cool, if it could get into samba 4.5.0. :-)
Bye,
Niklas
--
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: binary_suffix.patch
Type: text/x-patch
Size: 4432 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160805/c03089fb/binary_suffix.bin>
-------------- 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/20160805/c03089fb/smime.bin>
More information about the samba-technical
mailing list