[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