Patch to fix the 0x thing in *NTPassword LDAP attr... Comments?

[Arran Cudbard-Bell]

Stefan Adams wrote:
> It's been quite a challenge to maintain both radiusNTPassword and
> sambaNTPassword in a user's LDAP object, especially when
> radiusNTPassword is just sambaNTPassword, prepended with '0x'.  If
> nothing else, that's redundant.
>
> So, what about this patch?  It just takes your sambaNTPassword and
> prepends the necessary '0x' before doing anything with it.
>
> Does that make sense?  Anything wrong with doing this?  Obviouly this
> patch is teeny and not very profound...  For what reason is this
> avoided?  rlm_ldap expects the value to be in hex.  OK -- so just
> immediately assume it's in hex and prepend the '0x'.  What about
> making it a configuration parameter?  I'm sure others would like to be
> able to skip the whole redundant *NTPassword thing, too.
>
> Comments encouraged.  Thanks!!
>
> Stefan
>
> # cat /tmp/0x.patch
> diff -urN freeradius-1.1.7/src/modules/rlm_ldap/rlm_ldap.c
> freeradius-1.1.7-0x/src/modules/rlm_ldap/rlm_ldap.c
> --- freeradius-1.1.7/src/modules/rlm_ldap/rlm_ldap.c    2007-08-08
> 22:57:43.000000000 -0500
> +++ freeradius-1.1.7-0x/src/modules/rlm_ldap/rlm_ldap.c 2007-08-08
> 22:37:56.000000000 -0500
> @@ -2498,6 +2498,7 @@
>        int             vals_count;
>        int             vals_idx;
>        char           *ptr;
> +char          hex[35];
>        char        *value;
>        TLDAP_RADIUS   *element;
>        LRAD_TOKEN      token, operator;
> @@ -2517,6 +2518,7 @@
>            */
>           if ((vals = ldap_get_values(ld,entry,element->attr)) == NULL)
>            continue;
> +DEBUG("!!! %s !!!", element->attr);
>           /*
>            *   Check whether this is a one-to-one-mapped ldap
>            *   attribute or a generic attribute and set flag
> @@ -2535,6 +2537,9 @@
>
>           for (vals_idx = 0; vals_idx < vals_count; vals_idx++) {
>            value = vals[vals_idx];
> +sprintf(hex, "0x%s", value);
> +value = hex;
> +DEBUG("!!! %s !!!", value);
>            if (is_generic_attribute) {
>             /*
>              * This is a generic attribute.
> - 
> List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
>   
Yes oh dear god yes !
This is such an absolute pain, and its absolutely pointless ... we don't 
*need* to know it's hex , because hex is the only bloody format that 
anyone writes hashes out in directories these days :\

It's annoying for anyone wanting to use the same hashes for samba 
Authentication as RADIUS authentication, because samba doesn't like the 
0x prefix, and so you have to write out the same hash in two different 
attributes.

So yes, if theres not a really good reason not to apply this patch, i'm 
all for it.

Thanks,
Arran