ldap_pairget fix

[Nicolas Baradakis]

Markus Krause wrote:

> first a table with the ldap values (i added some more or less "sensible"
> variations just out of curiosity) for the user names, together with the reply
> by freeradius (details see below):

Many thanks for all these tests. I've a few comments about the results.

>  username   | radiusFilterId       | reply
> ------------+----------------------+--------------------------
>  testuser3  | foo bar              | Filter-Id = "foo bar"
>  testuser4  | foo_bar              | Filter-Id = "foo_bar"
>  testuser5  | "foo bar"            | Filter-Id = "foo bar"

"foo bar" is correctly quoted, therefore the quote are removed in the
RADIUS reply. -> test OK

>  testuser6  | "foo"bar             | Filter-Id = ""foo"bar"
>  testuser19 | foo"bar              | Filter-Id = "foo"bar"
>  testuser20 | foo""bar             | Filter-Id = "foo""bar"
>  testuser21 | foo`bar              | Filter-Id = "foo`bar"

The strings are incorrectly quoted, therefore they are considered as
bare words. It's the same behaviour as when reading attribute from a
sql database. -> test OK

>  testuser8  | += foo bar           | Filter-Id = "foo bar"
>  testuser9  | += 'foo bar'         | Filter-Id = "foo bar"
>  testuser10 | += 'foo ba'r         | Filter-Id = "'foo ba'r"
>  testuser17 | += "foo"bar          | Filter-Id = ""foo"bar"

The same as testuser3,5,6 with a leading operator. The operator
is correctly removed from the RADIUS reply -> test OK

>  testuser12 | "foo"bar"            | Filter-Id = "foo"
>  testuser13 | ""foo bar""          |
>  testuser14 | += ""foo bar""       |
>  testuser15 | ""foo"bar""          |
>  testuser16 | += ""foo"bar""       |
>  testuser18 | += ""foo"bar"        |

This is interesting. These strings bypass the test "start and end with
a quote" but are still not correctly quoted. The ldap module stops
parsing the string at the second unescaped quote it finds, and the
sql module does the same. (the rationale is to escape the quotes in
a quoted string)

It's arguably buggy, but it's the same behaviour as the old ldap code
(truncation) and the same behaviour as the sql module. (I did a test
with sql to be sure of this)

>  testuser7  | `Hello %{User-Name}` | (Segmentation fault)
>  testuser11 | `Hello foo bar`      | (Segmentation fault)

It's a stupid bug in my code :(

> actually i do not know if these values make sense (but i also do not
> understand what an operator could be of use in a single value ldap
> attribute (radiusFilterId) either as i said before i am not a radius
> expert, just using it ;-)

You can put the attribute Filter-Id several times in the RADIUS reply
if you want.

For example:

dn: uid=testuser22,ou=People,dc=mogli,dc=de
radiusFilterId: foo
radiusFilterId: += bar

> what to test next? ;-)

We still need to fix the back-quote expansion. Please apply the
following patch to your sources, recompile, and authenticate the
testuser7 again.

--- src/modules/rlm_ldap/rlm_ldap.c.orig	2006-01-11 18:34:22.781749360 +0100
+++ src/modules/rlm_ldap/rlm_ldap.c	2006-01-11 18:54:06.102857208 +0100
@@ -2398,7 +2398,7 @@
 
 					/* the value will be xlat'ed later */
 					case T_BACK_QUOTED_STRING:
-						value = NULL;
+						value = buf;
 						do_xlat = TRUE;
 						break;
 
@@ -2418,7 +2418,8 @@
 				 *	Create the pair.
 				 */
 				newpair = pairmake(element->radius_attr,
-						   value, operator);
+						   do_xlat ? NULL : value,
+						   operator);
 				if (newpair == NULL) {
 					radlog(L_ERR, "rlm_ldap: Failed to create the pair: %s", librad_errstr);
 					continue;

-- 
Nicolas Baradakis