Just pushed up a few patches to add LDAP accounting. Not well tested, but managed to modify attributes in my test directory. https://github.com/FreeRADIUS/freeradius-server/commit/8a62cce4a224c03b5b5fe... For the moment it doesn't support adding new LDAP objects, just modifying existing ones. If someone really really wants that I could take a look at adding it. Probably need to do something about the behaviour of post-auth now, suggestions welcome. Just drop : # # Modify user object on receiving Accounting-Request # # Useful for recording things like the last time the user logged # in, or the Acct-Session-ID for CoA/DM. # # LDAP modification items are in the format: # <ldap attr> <op> <value> # # Where: # <ldap attr>: The LDAP attribute to add modify or delete. # <op>: One of the assignment operators (:=, +=, -=). # Note: '=' is *not* supported. # <value>: The value to add modify or delete. # accounting { reference = "%{tolower:type.%{Acct-Status-Type}}" type { start { update { description := "Online at %S" } } interim-update { update { description := "Online at %S" } } stop { update { description := "Offline at %S" } } } } Into your existing config. -Arran
On 10.12.2012 01:33, Arran Cudbard-Bell wrote:
Just pushed up a few patches to add LDAP accounting.
Not well tested, but managed to modify attributes in my test directory.
I'll do some more testing. I've raised a use case for this during my sleep this night ;) I'll test it out as soon as the cfree issue is gone.
https://github.com/FreeRADIUS/freeradius-server/commit/8a62cce4a224c03b5b5fe...
For the moment it doesn't support adding new LDAP objects, just modifying existing ones. If someone really really wants that I could take a look at adding it.
you're sure? it looks like we can do += which trigger a ldapmodify with LDAP_MOD_ADD. same goes for -= Olivier -- Olivier Beytrison Network & Security Engineer, HES-SO Fribourg Mobile: +41 (0)78 619 73 53 Mail: olivier@heliosnet.org
On 10 Dec 2012, at 08:25, Olivier Beytrison <olivier@heliosnet.org> wrote:
On 10.12.2012 01:33, Arran Cudbard-Bell wrote:
Just pushed up a few patches to add LDAP accounting.
Not well tested, but managed to modify attributes in my test directory.
I'll do some more testing. I've raised a use case for this during my sleep this night ;) I'll test it out as soon as the cfree issue is gone.
Thanks
https://github.com/FreeRADIUS/freeradius-server/commit/8a62cce4a224c03b5b5fe...
For the moment it doesn't support adding new LDAP objects, just modifying existing ones. If someone really really wants that I could take a look at adding it.
you're sure? it looks like we can do += which trigger a ldapmodify with LDAP_MOD_ADD. same goes for -=
I think that's per attribute, as it was being specified at an attribute level. I.e. += added a new attribute instead of changing the value of an existing one, and -= removed an attribute with the specified value. If you want to add or delete at an object level you call ldap_add / ldap_delete. I've renamed cfree to rad_cfree to stop the conflict with std lib, should build now. -Arran
On 10.12.2012 01:33, Arran Cudbard-Bell wrote:
accounting { reference = "%{tolower:type.%{Acct-Status-Type}}"
type { start { update { description := "Online at %S" } }
interim-update { update { description := "Online at %S" } }
stop { update { description := "Offline at %S" } } } }
Would it work to update the loginTime using in the update section loginTime := "%{integer:event-time}Z"? What's the "timezone" used by the event-time? local machine time? Olivier -- Olivier Beytrison Network & Security Engineer, HES-SO Fribourg Mobile: +41 (0)78 619 73 53 Mail: olivier@heliosnet.org
On 10 Dec 2012, at 09:43, Olivier Beytrison <olivier@heliosnet.org> wrote:
On 10.12.2012 01:33, Arran Cudbard-Bell wrote:
accounting { reference = "%{tolower:type.%{Acct-Status-Type}}"
type { start { update { description := "Online at %S" } }
interim-update { update { description := "Online at %S" } }
stop { update { description := "Offline at %S" } } } }
Would it work to update the loginTime using in the update section loginTime := "%{integer:event-time}Z"? What's the "timezone" used by the event-time? local machine time?
RFC says: 5.3. Event-Timestamp Description This attribute is included in an Accounting-Request packet to record the time that this event occurred on the NAS, in seconds since January 1, 1970 00:00 UTC. But it'll probably just be the local time on the NAS. %{integer:Event-Timestamp} should get you it in original unix timestamp form yes. -Arran
Behaviour change for eDir users in 3.0, if you just want to do auth/autz you should remove the call to ldap in post-auth, and set edir_autz = "yes" in the LDAP config. This is more efficient, and frees up Post-Auth for doing accounting stuff. -Arran
On 10.12.2012 12:52, Arran Cudbard-Bell wrote:
Behaviour change for eDir users in 3.0, if you just want to do auth/autz you should remove the call to ldap in post-auth, and set edir_autz = "yes" in the LDAP config.
This is more efficient, and frees up Post-Auth for doing accounting stuff.
I'm deploying it and will report the tests results. on a side note, got two new warning for rlm_ldap.c src/modules/rlm_ldap/rlm_ldap.c: In function ‘ldap_conn_create’: src/modules/rlm_ldap/rlm_ldap.c:563:2: warning: implicit declaration of function ‘ldap_int_tls_config’ [-Wimplicit-function-declaration] src/modules/rlm_ldap/rlm_ldap.c: In function ‘user_modify’: src/modules/rlm_ldap/rlm_ldap.c:2248:22: warning: assignment discards ‘const’ qualifier from pointer target type [enabled by default] src/modules/rlm_ldap/rlm_ldap.c: In function ‘ldap_authorize’: src/modules/rlm_ldap/rlm_ldap.c:2028:5: warning: ‘user_dn’ may be used uninitialized in this function [-Wuninitialized] -- Olivier Beytrison Network & Security Engineer, HES-SO Fribourg Mobile: +41 (0)78 619 73 53 Mail: olivier@heliosnet.org
On 10 Dec 2012, at 13:30, Olivier Beytrison <olivier@heliosnet.org> wrote:
On 10.12.2012 12:52, Arran Cudbard-Bell wrote:
Behaviour change for eDir users in 3.0, if you just want to do auth/autz you should remove the call to ldap in post-auth, and set edir_autz = "yes" in the LDAP config.
This is more efficient, and frees up Post-Auth for doing accounting stuff.
I'm deploying it and will report the tests results.
on a side note, got two new warning for rlm_ldap.c
src/modules/rlm_ldap/rlm_ldap.c: In function ‘ldap_conn_create’: src/modules/rlm_ldap/rlm_ldap.c:563:2: warning: implicit declaration of function ‘ldap_int_tls_config’ [-Wimplicit-function-declaration]
Hmm the symbol is present in the library, but there's no signature definition in the headers. This is an OpenLDAP issue.
src/modules/rlm_ldap/rlm_ldap.c: In function ‘user_modify’: src/modules/rlm_ldap/rlm_ldap.c:2248:22: warning: assignment discards ‘const’ qualifier from pointer target type [enabled by default]
Stupid lack of const, grrr. OK well, fixed that.
src/modules/rlm_ldap/rlm_ldap.c: In function ‘ldap_authorize’: src/modules/rlm_ldap/rlm_ldap.c:2028:5: warning: ‘user_dn’ may be used uninitialized in this function [-Wuninitialized]
Ah quite right, well done GCC. Pushed fix. -Arran
On 10.12.2012 12:49, Arran Cudbard-Bell wrote:
Would it work to update the loginTime using in the update section loginTime := "%{integer:event-time}Z"? What's the "timezone" used by the event-time? local machine time?
RFC says:
5.3. Event-Timestamp
Description
This attribute is included in an Accounting-Request packet to record the time that this event occurred on the NAS, in seconds since January 1, 1970 00:00 UTC.
But it'll probably just be the local time on the NAS.
%{integer:Event-Timestamp} should get you it in original unix timestamp form yes.
Yep I also came across the RFC for the event-timestamp. Problem is that the format of the ldap attribute is YYYYMMDDHHMMSS + a trailing "Z". I guess we don't have time-convertion function in unlang ? :p -- Olivier Beytrison Network & Security Engineer, HES-SO Fribourg Mobile: +41 (0)78 619 73 53 Mail: olivier@heliosnet.org
On 10 Dec 2012, at 12:51, Olivier Beytrison <olivier@heliosnet.org> wrote:
On 10.12.2012 12:49, Arran Cudbard-Bell wrote:
Would it work to update the loginTime using in the update section loginTime := "%{integer:event-time}Z"? What's the "timezone" used by the event-time? local machine time?
RFC says:
5.3. Event-Timestamp
Description
This attribute is included in an Accounting-Request packet to record the time that this event occurred on the NAS, in seconds since January 1, 1970 00:00 UTC.
But it'll probably just be the local time on the NAS.
%{integer:Event-Timestamp} should get you it in original unix timestamp form yes.
Yep I also came across the RFC for the event-timestamp. Problem is that the format of the ldap attribute is YYYYMMDDHHMMSS + a trailing "Z". I guess we don't have time-convertion function in unlang ? :p
No we don't, once I have some more free time i'll take a look at doing something. Probably makes sense to do it as a module, time time_xlat { format = "<time format>" } and then "%{time_xlat:TimeAttribute}" And maybe add an Event-Timestamp in preprocess if it's an Access-Request. -Arran
On 10.12.2012 01:33, Arran Cudbard-Bell wrote:
accounting { reference = "%{tolower:type.%{Acct-Status-Type}}"
type { start { update { description := "Online at %S" } }
interim-update { update { description := "Online at %S" } }
stop { update { description := "Offline at %S" } } } }
In the default ldap files, it is stated that we can use the following section for post-auth : post-auth { update { description := "Authenticated at %S" } } but on run-time, it complains that the reference is not found. (0) - entering group post-auth {...} rlm_ldap (ldap): Reserved connection (4) (0) ldap : expand: '.' -> '.' WARNING: No such configuration item . rlm_ldap (ldap): Released connection (4) (0) [ldap] = fail post-auth{} and accounting{} don't have the exact same layout, so the code need to adapt and not fail ;) -- Olivier Beytrison Network & Security Engineer, HES-SO Fribourg Mobile: +41 (0)78 619 73 53 Mail: olivier@heliosnet.org
On 10.12.2012 14:30, Olivier Beytrison wrote:
On 10.12.2012 01:33, Arran Cudbard-Bell wrote:
accounting { reference = "%{tolower:type.%{Acct-Status-Type}}"
type { start { update { description := "Online at %S" } }
interim-update { update { description := "Online at %S" } }
stop { update { description := "Offline at %S" } } } }
In the default ldap files, it is stated that we can use the following section for post-auth : post-auth { update { description := "Authenticated at %S" } } but on run-time, it complains that the reference is not found.
(0) - entering group post-auth {...} rlm_ldap (ldap): Reserved connection (4) (0) ldap : expand: '.' -> '.' WARNING: No such configuration item . rlm_ldap (ldap): Released connection (4) (0) [ldap] = fail
post-auth{} and accounting{} don't have the exact same layout, so the code need to adapt and not fail ;)
Based on my interpretation of the code which parse the configuration for the update part, i changed the post-auth to : post-auth { reference = "post-auth" post-auth { update { description := "Authenticated at %S" } } } Now on run time it doesn't complain anymore about the reference being not present, but it segfaults :o (0) group post-auth { (0) - entering group post-auth {...} rlm_ldap (ldap): Reserved connection (4) (0) ldap : expand: 'post-auth' -> 'post-auth' (0) ldap : expand: 'Authenticated at %S' -> 'Authenticated at 2012-12-10 15:38:16' Program received signal SIGSEGV, Segmentation fault. 0x00007ffff4ee121d in user_modify (inst=0x805fb0, request=0x8af660, section=<optimized out>) at src/modules/rlm_ldap/rlm_ldap.c:2253 2253 (*mod_p)->mod_values = &passed[last_pass]; (gdb) bt #0 0x00007ffff4ee121d in user_modify (inst=0x805fb0, request=0x8af660, section=<optimized out>) at src/modules/rlm_ldap/rlm_ldap.c:2253 #1 0x0000000000423642 in call_modsingle (request=0x8af660, component=7, sp=<optimized out>) at src/main/modcall.c:305 #2 modcall (component=7, c=0x895cb0, request=<optimized out>) at src/main/modcall.c:798 #3 0x0000000000421d26 in indexed_modcall (comp=7, idx=0, request=0x8af660) at src/main/modules.c:793 #4 0x000000000040db30 in rad_postauth (request=0x8af660) at src/main/auth.c:303 #5 0x00000000004345e0 in request_finish (request=0x8af660, action=<optimized out>) at src/main/process.c:1109 #6 request_running (action=<optimized out>, request=0x8af660) at src/main/process.c:1216 #7 request_running (request=0x8af660, action=<optimized out>) at src/main/process.c:1150 #8 0x000000000043266b in request_queue_or_run (request=0x8af660, process=0x433f80 <request_running>) at src/main/process.c:822 #9 0x00000000004339df in request_insert (listener=0x8ae570, packet=0x8af530, client=0x7f4e20, fun=0x40db70 <rad_authenticate>, pnow=0x7fffffffe1f0) at src/main/process.c:1439 #10 0x0000000000433b7e in request_receive (listener=0x8ae570, packet=0x8af530, client=0x7f4e20, fun=0x40db70 <rad_authenticate>) at src/main/process.c:1333 #11 0x000000000041cfcb in auth_socket_recv (listener=0x8ae570) at src/main/listen.c:1468 #12 0x0000000000430032 in event_socket_handler (xel=<optimized out>, fd=<optimized out>, ctx=0x8ae570) at src/main/process.c:3409 #13 0x00007ffff7bc677e in fr_event_loop () from /usr/lib/freeradius/libfreeradius-radius.so #14 0x000000000040d104 in main (argc=<optimized out>, argv=<optimized out>) at src/main/radiusd.c:415 I don't know if it's due to my configuration or if it's in the code. If needed I still have gdb open and can output anything you want ;) Olivier -- Olivier Beytrison Network & Security Engineer, HES-SO Fribourg Mobile: +41 (0)78 619 73 53 Mail: olivier@heliosnet.org
On 10 Dec 2012, at 15:17, Olivier Beytrison <olivier@heliosnet.org> wrote:
On 10.12.2012 14:30, Olivier Beytrison wrote:
On 10.12.2012 01:33, Arran Cudbard-Bell wrote:
accounting { reference = "%{tolower:type.%{Acct-Status-Type}}"
type { start { update { description := "Online at %S" } }
interim-update { update { description := "Online at %S" } }
stop { update { description := "Offline at %S" } } } }
In the default ldap files, it is stated that we can use the following section for post-auth : post-auth { update { description := "Authenticated at %S" } } but on run-time, it complains that the reference is not found.
(0) - entering group post-auth {...} rlm_ldap (ldap): Reserved connection (4) (0) ldap : expand: '.' -> '.' WARNING: No such configuration item . rlm_ldap (ldap): Released connection (4) (0) [ldap] = fail
post-auth{} and accounting{} don't have the exact same layout, so the code need to adapt and not fail ;)
Based on my interpretation of the code which parse the configuration for the update part, i changed the post-auth to :
post-auth { reference = "post-auth" post-auth { update { description := "Authenticated at %S" } } }
Now on run time it doesn't complain anymore about the reference being not present, but it segfaults :o
Hmm wow, oops, this is why you shouldn't write C when tired. Should all work now. As an added bonus for your bug finding, it now supports ++, which is sort of undocumented but apparently supported according to comments in the OpenLDAP code. * Example: * LDAPMod *mods[] = { * { LDAP_MOD_ADD, "cn", { "babs jensen", "babs", 0 } }, * { LDAP_MOD_REPLACE, "sn", { "babs jensen", "babs", 0 } }, * { LDAP_MOD_DELETE, "ou", 0 }, * { LDAP_MOD_INCREMENT, "uidNumber, { "1", 0 } } * 0 * } #define LDAP_MOD_ADD (0x0000) #define LDAP_MOD_DELETE (0x0001) #define LDAP_MOD_REPLACE (0x0002) #define LDAP_MOD_INCREMENT (0x0003) /* OpenLDAP extension */ Guessed it'd be useful for maintaining a login counter or something... -Arran
On 10.12.2012 20:03, Arran Cudbard-Bell wrote:
Hmm wow, oops, this is why you shouldn't write C when tired. Should all work now.
How come, I thought we always wrote code while being tired, in the middle of the night, with a mug of coffee ;)
As an added bonus for your bug finding, it now supports ++, which is sort of undocumented but apparently supported according to comments in the OpenLDAP code.
* Example: * LDAPMod *mods[] = { * { LDAP_MOD_ADD, "cn", { "babs jensen", "babs", 0 } }, * { LDAP_MOD_REPLACE, "sn", { "babs jensen", "babs", 0 } }, * { LDAP_MOD_DELETE, "ou", 0 }, * { LDAP_MOD_INCREMENT, "uidNumber, { "1", 0 } } * 0 * }
#define LDAP_MOD_ADD (0x0000) #define LDAP_MOD_DELETE (0x0001) #define LDAP_MOD_REPLACE (0x0002) #define LDAP_MOD_INCREMENT (0x0003) /* OpenLDAP extension */
Guessed it'd be useful for maintaining a login counter or something...
Nice, christmas before time ! While doing my test I was surprised that most of my tests returned "Modification successful" while it should have thrown an error. Pull request sent to correct this ;) So, here's some results of my tests : 1. Using := set operator a. If attribute is not present in the ldap, will create it and set the value b. If attribute is present, it will replace its value. c. If multiple attributes are present, it will replace all attributes by a single attribute with the passed value (Use with caution !) 2. Using += add operator a. If attribute is not present, will create it and set the value b. If attribute is already present, will create a new one with the passed value, if multi-valuable c. If attribute is already present, and single-valued, it will fail with "Constraint violation" 3. Using -= del operator The del operator is harder to use. If you have an ldap attribute with a static value, let say description = "Logged in", putting description -= "Logged in" will remove it. If the value is dynamic, you need to xlat it first. But this will fail for multi-valued argument. Working example : roomNumber -= "%{ldap:ldap:///ou=people,o=hes-so?roomNumber?sub?uid=%{%{Stripped-User-Name}:-%{User-Name}}}" This will successfully remove an attribute that was originally added with roomNumber := "User %{%{Stripped-User-Name}:-%{User-Name}} logged in at %S" 4. Using ++ increment operator I've not been able to use the ++ operator. Novell eDirectory doesn't support RFC 4525 Ldap Modify-Increment operation. (Not listed in the root DSE in supportedExtension and LDAP_MOD_INCREMENT return protocol error). this should be tested against a OpenLDAP server. In the mean time I also noticed that using a single quote string throw a segmentation fault in the update section. Full backtrace here http://pastebin.com/E85fdKGV That's it! Olivier
- List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
-- Olivier Beytrison Network & Security Engineer, HES-SO Fribourg Mobile: +41 (0)78 619 73 53 Mail: olivier@heliosnet.org
On 11 Dec 2012, at 10:28, Olivier Beytrison <olivier@heliosnet.org> wrote:
On 10.12.2012 20:03, Arran Cudbard-Bell wrote:
Hmm wow, oops, this is why you shouldn't write C when tired. Should all work now.
How come, I thought we always wrote code while being tired, in the middle of the night, with a mug of coffee ;)
As an added bonus for your bug finding, it now supports ++, which is sort of undocumented but apparently supported according to comments in the OpenLDAP code.
* Example: * LDAPMod *mods[] = { * { LDAP_MOD_ADD, "cn", { "babs jensen", "babs", 0 } }, * { LDAP_MOD_REPLACE, "sn", { "babs jensen", "babs", 0 } }, * { LDAP_MOD_DELETE, "ou", 0 }, * { LDAP_MOD_INCREMENT, "uidNumber, { "1", 0 } } * 0 * }
#define LDAP_MOD_ADD (0x0000) #define LDAP_MOD_DELETE (0x0001) #define LDAP_MOD_REPLACE (0x0002) #define LDAP_MOD_INCREMENT (0x0003) /* OpenLDAP extension */
Guessed it'd be useful for maintaining a login counter or something...
Nice, christmas before time !
While doing my test I was surprised that most of my tests returned "Modification successful" while it should have thrown an error. Pull request sent to correct this ;)
I don't think that was quite right, it looks like the error_no can be positive or negative, pushed another fix. Thanks for reporting it though.
So, here's some results of my tests : 1. Using := set operator a. If attribute is not present in the ldap, will create it and set the value
Good.
b. If attribute is present, it will replace its value.
Good.
c. If multiple attributes are present, it will replace all attributes by a single attribute with the passed value (Use with caution !)
Huh?! What was that about Principle of Least Astonishment... wow. That's one that should go in the config with a big WARNING:/
2. Using += add operator a. If attribute is not present, will create it and set the value
Good.
b. If attribute is already present, will create a new one with the passed value, if multi-valuable
That's good behaviour, I was worried we were going to have to pre-sort the attributes and bundle them up into value arrays.
c. If attribute is already present, and single-valued, it will fail with "Constraint violation"
Good.
3. Using -= del operator The del operator is harder to use. If you have an ldap attribute with a static value, let say description = "Logged in", putting description -= "Logged in" will remove it.
Good.
If the value is dynamic, you need to xlat it first. But this will fail for multi-valued argument. Working example : roomNumber -= "%{ldap:ldap:///ou=people,o=hes-so?roomNumber?sub?uid=%{%{Stripped-User-Name}:-%{User-Name}}}" This will successfully remove an attribute that was originally added
:)
with roomNumber := "User %{%{Stripped-User-Name}:-%{User-Name}} logged in at %S"
The main thing is we have a way of doing it, even if it's a little ugly.
4. Using ++ increment operator
I've not been able to use the ++ operator. Novell eDirectory doesn't support RFC 4525 Ldap Modify-Increment operation. (Not listed in the root DSE in supportedExtension and LDAP_MOD_INCREMENT return protocol error). this should be tested against a OpenLDAP server.
Ok.
In the mean time I also noticed that using a single quote string throw a segmentation fault in the update section. Full backtrace here http://pastebin.com/E85fdKGV
Fixed. Thanks for your brilliant testing :) -Arran
with roomNumber := "User %{%{Stripped-User-Name}:-%{User-Name}} logged in at %S"
The main thing is we have a way of doing it, even if it's a little ugly.
Actually, it looks like you can do a wildcard delete if you pass a 0 length value array. Could you try update { <attribute> -= ANY } Bare ANY keyword being magic. -Arran
On 11.12.2012 13:40, Arran Cudbard-Bell wrote:
with roomNumber := "User %{%{Stripped-User-Name}:-%{User-Name}} logged in at %S"
The main thing is we have a way of doing it, even if it's a little ugly.
Actually, it looks like you can do a wildcard delete if you pass a 0 length value array.
Could you try
update { <attribute> -= ANY }
Bare ANY keyword being magic.
Works very well [1] :) Thx! and one more thing that would be nice to have. If something goes wrong with those ldap modifications, we should be able to choose if the user is rejected or not. like post-auth { update { <attr> <op> <val> } error = reject/noop } and for the := set operator on multi-valued ldap attribute, we could implement something like <attr> := <old-value>:<new-value>. But that's pushing thing too far in my opinion ... thanks for your work Arran! Olivier [1] with roomNumber -= ANY ldapsearch before request : roomNumber: Hello 2012-12-11 13:42:06 roomNumber: pouet lala roomNumber: pouet hoho roomNumber: hihohu radtest request : Sending Access-Request of id 236 from 0.0.0.0 port 52704 to 127.0.0.1 port 1812 User-Name != "olivier.beytriso" NAS-IP-Address != 160.98.240.25 NAS-Port != 0 Message-Authenticator != 0x00 MS-CHAP-Challenge != xxxxxxx MS-CHAP-Response != xxxxxxxxx rad_recv: Access-Accept packet from host 127.0.0.1 port 1812, id=236, length=84 MS-CHAP-MPPE-Keys != 0xf087f73xxxxxxx4ac880000000000000000 MS-MPPE-Encryption-Policy != Encryption-Allowed MS-MPPE-Encryption-Types != RC4-40or128-bit-Allowed ldapsearch after : dn: cn=31935762,ou=courant,ou=people,o=hes-so -- Olivier Beytrison Network & Security Engineer, HES-SO Fribourg Mobile: +41 (0)78 619 73 53 Mail: olivier@heliosnet.org
On 11 Dec 2012, at 14:15, Olivier Beytrison <olivier@heliosnet.org> wrote:
On 11.12.2012 13:40, Arran Cudbard-Bell wrote:
with roomNumber := "User %{%{Stripped-User-Name}:-%{User-Name}} logged in at %S"
The main thing is we have a way of doing it, even if it's a little ugly.
Actually, it looks like you can do a wildcard delete if you pass a 0 length value array.
Could you try
update { <attribute> -= ANY }
Bare ANY keyword being magic.
Works very well [1] :) Thx!
and one more thing that would be nice to have. If something goes wrong with those ldap modifications, we should be able to choose if the user is rejected or not. like
post-auth { update { <attr> <op> <val> } error = reject/noop }
You can do that already with rcode overrides. ldap { fail = 1 } if (fail) { ok }
and for the := set operator on multi-valued ldap attribute, we could implement something like <attr> := <old-value>:<new-value>. But that's pushing thing too far in my opinion ...
Oh is that why it replaces everything? Do you know how to represent that in the mods struct?
thanks for your work Arran! Olivier
Np. Thanks again for testing! -Arran
On 11.12.2012 15:26, Arran Cudbard-Bell wrote:
and one more thing that would be nice to have. If something goes wrong with those ldap modifications, we should be able to choose if the user is rejected or not. like
post-auth { update { <attr> <op> <val> } error = reject/noop }
You can do that already with rcode overrides.
ldap { fail = 1 } if (fail) { ok }
Fair enough :)
and for the := set operator on multi-valued ldap attribute, we could implement something like <attr> := <old-value>:<new-value>. But that's pushing thing too far in my opinion ...
Oh is that why it replaces everything?
Do you know how to represent that in the mods struct?
you pointed it out in a previous mail { LDAP_MOD_REPLACE, "sn", { "babs jensen", "babs", 0 } }, "old value", "new value", 0 same goes for LDAP_MOD_ADD, but in that case it operates the same way as REPLACE. if attribute with old value exists, replace with new value, otherwise create it. so not worth implementing it imho Olivier -- Olivier Beytrison Network & Security Engineer, HES-SO Fribourg Mobile: +41 (0)78 619 73 53 Mail: olivier@heliosnet.org
On 11 Dec 2012, at 14:31, Olivier Beytrison <olivier@heliosnet.org> wrote:
On 11.12.2012 15:26, Arran Cudbard-Bell wrote:
and one more thing that would be nice to have. If something goes wrong with those ldap modifications, we should be able to choose if the user is rejected or not. like
post-auth { update { <attr> <op> <val> } error = reject/noop }
You can do that already with rcode overrides.
ldap { fail = 1 } if (fail) { ok }
Fair enough :)
and for the := set operator on multi-valued ldap attribute, we could implement something like <attr> := <old-value>:<new-value>. But that's pushing thing too far in my opinion ...
Oh is that why it replaces everything?
Do you know how to represent that in the mods struct?
you pointed it out in a previous mail { LDAP_MOD_REPLACE, "sn", { "babs jensen", "babs", 0 } }, "old value", "new value", 0 same goes for LDAP_MOD_ADD, but in that case it operates the same way as REPLACE. if attribute with old value exists, replace with new value, otherwise create it. so not worth implementing it imho
Blerg, you'd have to escape and unescape : in xlat expansions but yes, I guess it should be possible. AFAIK the conffile API doesnt allow you to create multivalued attributes. -Arran
On 11.12.2012 15:37, Arran Cudbard-Bell wrote:
and for the := set operator on multi-valued ldap attribute, we could implement something like <attr> := <old-value>:<new-value>. But that's pushing thing too far in my opinion ...
Oh is that why it replaces everything?
Do you know how to represent that in the mods struct?
you pointed it out in a previous mail { LDAP_MOD_REPLACE, "sn", { "babs jensen", "babs", 0 } }, "old value", "new value", 0 same goes for LDAP_MOD_ADD, but in that case it operates the same way as REPLACE. if attribute with old value exists, replace with new value, otherwise create it. so not worth implementing it imho
Blerg, you'd have to escape and unescape : in xlat expansions but yes, I guess it should be possible.
AFAIK the conffile API doesnt allow you to create multivalued attributes.
that's why I think that's not something we should focus too much on. The actual way it works already allows quite some possibilities. Let's see if user request something like that. I've enough played with the code. I shall now begin deploying FR3 (3 days left until deadline :o) -- Olivier Beytrison Network & Security Engineer, HES-SO Fribourg Mobile: +41 (0)78 619 73 53 Mail: olivier@heliosnet.org
On Tue, Dec 11, 2012 at 12:40:05PM +0000, Arran Cudbard-Bell wrote:
Could you try
update { <attribute> -= ANY }
Bare ANY keyword being magic.
Aside: would it be more consistent to allow update { <attribute> !* "ANY" } where the RHS value is ignored? This is by analogy to unlang updates to AV pair lists. (See man unlang, under "Enforcement and Filtering Operators")
On 11 Dec 2012, at 15:49, Brian Candler <B.Candler@pobox.com> wrote:
On Tue, Dec 11, 2012 at 12:40:05PM +0000, Arran Cudbard-Bell wrote:
Could you try
update { <attribute> -= ANY }
Bare ANY keyword being magic.
Aside: would it be more consistent to allow
update { <attribute> !* "ANY" }
where the RHS value is ignored? This is by analogy to unlang updates to AV pair lists.
I guess so, though maybe without the double quotes. -Arran
On 11 Dec 2012, at 16:02, Arran Cudbard-Bell <a.cudbardb@freeradius.org> wrote:
On 11 Dec 2012, at 15:49, Brian Candler <B.Candler@pobox.com> wrote:
On Tue, Dec 11, 2012 at 12:40:05PM +0000, Arran Cudbard-Bell wrote:
Could you try
update { <attribute> -= ANY }
Bare ANY keyword being magic.
Aside: would it be more consistent to allow
update { <attribute> !* "ANY" }
where the RHS value is ignored? This is by analogy to unlang updates to AV pair lists.
I guess so, though maybe without the double quotes.
Done.
On 11 Dec 2012, at 15:49, Brian Candler <B.Candler@pobox.com> wrote:
On Tue, Dec 11, 2012 at 12:40:05PM +0000, Arran Cudbard-Bell wrote:
Could you try
update { <attribute> -= ANY }
Bare ANY keyword being magic.
Aside: would it be more consistent to allow
update { <attribute> !* "ANY" }
where the RHS value is ignored? This is by analogy to unlang updates to AV pair lists.
And no, I think it's good to have a consistent wildcard value. -Arran
On 11 Dec 2012, at 22:07, Arran Cudbard-Bell <a.cudbardb@freeradius.org> wrote:
On 11 Dec 2012, at 15:49, Brian Candler <B.Candler@pobox.com> wrote:
On Tue, Dec 11, 2012 at 12:40:05PM +0000, Arran Cudbard-Bell wrote:
Could you try
update { <attribute> -= ANY }
Bare ANY keyword being magic.
Aside: would it be more consistent to allow
update { <attribute> !* "ANY" }
where the RHS value is ignored? This is by analogy to unlang updates to AV pair lists.
And no, I think it's good to have a consistent wildcard value.
Actually the code is slightly simpler without it. Ok changed to ignore right operand.
On 11.12.2012 23:18, Arran Cudbard-Bell wrote:
On 11 Dec 2012, at 22:07, Arran Cudbard-Bell <a.cudbardb@freeradius.org> wrote:
On 11 Dec 2012, at 15:49, Brian Candler <B.Candler@pobox.com> wrote:
On Tue, Dec 11, 2012 at 12:40:05PM +0000, Arran Cudbard-Bell wrote:
Could you try
update { <attribute> -= ANY }
Bare ANY keyword being magic.
Aside: would it be more consistent to allow
update { <attribute> !* "ANY" }
where the RHS value is ignored? This is by analogy to unlang updates to AV pair lists.
And no, I think it's good to have a consistent wildcard value.
Actually the code is slightly simpler without it. Ok changed to ignore right operand.
Why did you remove support of -= ? It is still useful if you have fixed-multi-valued ldap attributes and you want to remove a specific one. -- Olivier Beytrison Network & Security Engineer, HES-SO Fribourg Mobile: +41 (0)78 619 73 53 Mail: olivier@heliosnet.org
On 12.12.2012 07:41, Olivier Beytrison wrote:
Why did you remove support of -= ? It is still useful if you have fixed-multi-valued ldap attributes and you want to remove a specific one. ignore this please. I'm not yet awake this morning.
-- Olivier Beytrison Network & Security Engineer, HES-SO Fribourg Mobile: +41 (0)78 619 73 53 Mail: olivier@heliosnet.org
On 10 Dec 2012, at 13:30, Olivier Beytrison <olivier@heliosnet.org> wrote:
On 10.12.2012 01:33, Arran Cudbard-Bell wrote:
accounting { reference = "%{tolower:type.%{Acct-Status-Type}}"
type { start { update { description := "Online at %S" } }
interim-update { update { description := "Online at %S" } }
stop { update { description := "Offline at %S" } } } }
In the default ldap files, it is stated that we can use the following section for post-auth : post-auth { update { description := "Authenticated at %S" } } but on run-time, it complains that the reference is not found.
(0) - entering group post-auth {...} rlm_ldap (ldap): Reserved connection (4) (0) ldap : expand: '.' -> '.' WARNING: No such configuration item . rlm_ldap (ldap): Released connection (4) (0) [ldap] = fail
post-auth{} and accounting{} don't have the exact same layout, so the code need to adapt and not fail ;)
In theory the path is correct. I'm not entirely sure why it's failing. '.' just means the current context, and then it should search in '.' for an update section. I'll test on my server and try and figure it out. -Arran
On 12/09/2012 07:33 PM, Arran Cudbard-Bell wrote:
Just pushed up a few patches to add LDAP accounting.
Just out of curiosity why are we adding support for "worst practice", shouldn't we be encouraging "best practice" via the choice of supported configurations? Maintaining accounting data in LDAP is an abuse of the LDAP design goals of "frequent lookup, infrequent modification". Databases were designed for the type of data management that radius accounting involves, directories were not. Accounting should be in a database, not a directory. Directories were designed to solve different problems. Maintaining authentication and identity information across an enterprise is exactly one of those problems LDAP was designed to handle which makes auth/authz lookups in a directory appropriate. Maintaining accounting information in a directory is not. -- John Dennis <jdennis@redhat.com> Looking to carve out IT costs? www.redhat.com/carveoutcosts/
On 10.12.2012 16:27, John Dennis wrote:
On 12/09/2012 07:33 PM, Arran Cudbard-Bell wrote:
Just pushed up a few patches to add LDAP accounting.
Just out of curiosity why are we adding support for "worst practice", shouldn't we be encouraging "best practice" via the choice of supported configurations?
Maintaining accounting data in LDAP is an abuse of the LDAP design goals of "frequent lookup, infrequent modification". Databases were designed for the type of data management that radius accounting involves, directories were not. Accounting should be in a database, not a directory. Directories were designed to solve different problems. Maintaining authentication and identity information across an enterprise is exactly one of those problems LDAP was designed to handle which makes auth/authz lookups in a directory appropriate. Maintaining accounting information in a directory is not.
I totally agree with you. But the implementation made by Arran will not allow you to perform full accounting in LDAP at the moment. But there are situation where it might be useful to update an attribute. In my case, I need to get the loginTime updated periodically, like on accounting-start and stop. I know that it will be done automatically if I bind as the radius-user, but if possible we don't want to bind, as it has other consequences. Olivier -- Olivier Beytrison Network & Security Engineer, HES-SO Fribourg Mobile: +41 (0)78 619 73 53 Mail: olivier@heliosnet.org
On 10 Dec 2012, at 15:27, John Dennis <jdennis@redhat.com> wrote:
On 12/09/2012 07:33 PM, Arran Cudbard-Bell wrote:
Just pushed up a few patches to add LDAP accounting.
Just out of curiosity why are we adding support for "worst practice", shouldn't we be encouraging "best practice" via the choice of supported configurations?
It seemed like something fun and potentially useful.
Maintaining accounting data in LDAP is an abuse of the LDAP design goals of "frequent lookup, infrequent modification". Databases were designed for the type of data management that radius accounting involves, directories were not. Accounting should be in a database, not a directory. Directories were designed to solve different problems. Maintaining authentication and identity information across an enterprise is exactly one of those problems LDAP was designed to handle which makes auth/authz lookups in a directory appropriate. Maintaining accounting information in a directory is not.
Sure, but it also contains authorization information too. The new code can be used to do things like set authentication lockouts on authentication failure, automatically rehash passwords, update current session IDs. For smaller sites the frequency of updates is very unlikely to cause issues, for larger ones, yes, they should look into a proper SQL (PostgreSQL) or noSQL database (Cassandra). I don't see why we should avoid supporting something that people have requested, which was fairly easy to implement, just because it does fit the original problem the protocol was designed to solve. If people do take down their enterprise LDAP directory then that is *their* issue. We should not avoid implementing potentially useful features because someone might misuse them. The current system also prevents people attempting full session accounting by not allowing the creation or deletion of objects (which would have been fairly trivial to add). That said, I would gladly add full object creation and deletion support if someone came up with a decent use case, i've just not seen one yet. -Arran
John Dennis wrote:
Just out of curiosity why are we adding support for "worst practice", shouldn't we be encouraging "best practice" via the choice of supported configurations?
Yes.
Maintaining accounting data in LDAP is an abuse of the LDAP design goals of "frequent lookup, infrequent modification". Databases were designed for the type of data management that radius accounting involves, directories were not. Accounting should be in a database, not a directory. Directories were designed to solve different problems. Maintaining authentication and identity information across an enterprise is exactly one of those problems LDAP was designed to handle which makes auth/authz lookups in a directory appropriate. Maintaining accounting information in a directory is not.
That's all well and good. The current configuration allows for storing "last login" time. That's well within the traditional use of LDAP: http://www.ldapguru.info/ldap/last-logon-time.html I agree doing more than that would be bad. Alan DeKok.
On Tue, Dec 11, 2012 at 4:27 AM, John Dennis <jdennis@redhat.com> wrote:
On 12/09/2012 07:33 PM, Arran Cudbard-Bell wrote:
Just pushed up a few patches to add LDAP accounting.
Just out of curiosity why are we adding support for "worst practice", shouldn't we be encouraging "best practice" via the choice of supported configurations?
Maintaining accounting data in LDAP is an abuse of the LDAP design goals of "frequent lookup, infrequent modification". Databases were designed for the type of data management that radius accounting involves, directories were not. Accounting should be in a database, not a directory. Directories were designed to solve different problems. Maintaining authentication and identity information across an enterprise is exactly one of those problems LDAP was designed to handle which makes auth/authz lookups in a directory appropriate. Maintaining accounting information in a directory is not.
In our example (which I believe kicked this all off :) We update our 7 instance Novell eDirectory LDAP Database which replicates across 3 geographically different sites when a specific Accounting Start message comes off our BNG saying that the subscribers session has started. Against the LDAP entry we write a status attribute that includes the NAS-Port-ID & Framed-IP-Address allocated to the subscriber. I currently use a small piece of perl code which is called from the accounting section to perform this add/delete of the subscriber state. Since our BNGs allocate IP addresses using an internal DHCP server. The reason behind that is once the subscriber attribute has been updated. All of our subscriber provisioning is to an internal facing instance of the database so when a subscriber profile is updated (such as to rate limit them back to "dialup esq speeds" or block / unblock port 25 from the subscriber if they want to run their own SMTP server, otherwise normally it's blocked). The change occurs on an internal instance, the background eDirectory replication replicates that to the instances sitting in the core, and we have a Novell Identity Manager driver listening to database changes, when it sees a change on the database, and the subscriber session is up it generates a CoA or DM message using the Coova JRadius client back to the BNG to either change the subscribers profile, or to kick them off their connection. It's a very simple with few moving parts solution (which is needed in carrier grade situation) and yet happily scales to 1million + subs without too much drama. We get on average 200k add/updates/delete per day give or take, and replication tends to sit around 20 seconds at the max where all instances agree they are up to date.
On 10 Dec 2012, at 20:45, Peter Lambrechtsen <peter@crypt.co.nz> wrote:
On Tue, Dec 11, 2012 at 4:27 AM, John Dennis <jdennis@redhat.com> wrote: On 12/09/2012 07:33 PM, Arran Cudbard-Bell wrote: Just pushed up a few patches to add LDAP accounting.
Just out of curiosity why are we adding support for "worst practice", shouldn't we be encouraging "best practice" via the choice of supported configurations?
Maintaining accounting data in LDAP is an abuse of the LDAP design goals of "frequent lookup, infrequent modification". Databases were designed for the type of data management that radius accounting involves, directories were not. Accounting should be in a database, not a directory. Directories were designed to solve different problems. Maintaining authentication and identity information across an enterprise is exactly one of those problems LDAP was designed to handle which makes auth/authz lookups in a directory appropriate. Maintaining accounting information in a directory is not.
In our example (which I believe kicked this all off :)
Yes.
We update our 7 instance Novell eDirectory LDAP Database which replicates across 3 geographically different sites when a specific Accounting Start message comes off our BNG
(big aggregator of subscriber tunnels/traffic in none telco speak)
saying that the subscribers session has started. Against the LDAP entry we write a status attribute that includes the NAS-Port-ID & Framed-IP-Address allocated to the subscriber. I currently use a small piece of perl code which is called from the accounting section to perform this add/delete of the subscriber state. Since our BNGs allocate IP addresses using an internal DHCP server.
Out of interest what are you using for DHCP?
The reason behind that is once the subscriber attribute has been updated. All of our subscriber provisioning is to an internal facing instance of the database so when a subscriber profile is updated (such as to rate limit them back to "dialup esq speeds" or block / unblock port 25 from the subscriber if they want to run their own SMTP server, otherwise normally it's blocked). The change occurs on an internal instance, the background eDirectory replication replicates that to the instances sitting in the core, and we have a Novell Identity Manager driver listening to database changes, when it sees a change on the database, and the subscriber session is up it generates a CoA or DM message using the Coova JRadius client back to the BNG to either change the subscribers profile, or to kick them off their connection.
It's a very simple with few moving parts solution (which is needed in carrier grade situation) and yet happily scales to 1million + subs without too much drama.
Because LDAP replication *works*.
We get on average 200k add/updates/delete per day give or take, and replication tends to sit around 20 seconds at the max where all instances agree they are up to date. - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
Arran Cudbard-Bell a.cudbardb@freeradius.org Betelwiki, Betelwiki, Betelwiki.... http://wiki.freeradius.org Will code for halva - http://www.amazon.co.uk/gp/registry/wishlist/D1E1QB70FIKC
On 12/10/2012 03:27 PM, John Dennis wrote:
On 12/09/2012 07:33 PM, Arran Cudbard-Bell wrote:
Just pushed up a few patches to add LDAP accounting.
Just out of curiosity why are we adding support for "worst practice", shouldn't we be encouraging "best practice" via the choice of supported configurations?
Maintaining accounting data in LDAP is an abuse of the LDAP design goals of "frequent lookup, infrequent modification". Databases were designed for the type of data management that radius accounting involves, directories were not. Accounting should be in a database, not a directory. Directories were designed to solve different problems.
This is a bit OT, but I'm honestly curious here... I've heard this sort of general statement about directories (specifically LDAP) being suitable for read-heavy workloads, and SQL being suitable for read/write-heavy, for well over 15 years now. No-one has ever adequately explained to me *why* sending an LDAP write op PDU is somehow different than sending an LDAP read op PDU. It may be that all or most LDAP *implementations* are relatively slow at writes, but if so, it doesn't follow that this *must* be the case. Obviously writes are slower than reads in LDAP, but that's true of almost any datastore backed by permanent storage. However, having done mass/bulk LDAP updates to OpenLDAP (and on older versions) I've found it pretty snappy. So I'd love to know why this is an "abuse" any more than using any other protocol in unanticipated-but-compliant ways. Cheers, Phil
On 11/12/2012 11:56 πμ, Phil Mayers wrote:
On 12/10/2012 03:27 PM, John Dennis wrote:
On 12/09/2012 07:33 PM, Arran Cudbard-Bell wrote:
Just pushed up a few patches to add LDAP accounting.
Just out of curiosity why are we adding support for "worst practice", shouldn't we be encouraging "best practice" via the choice of supported configurations?
Maintaining accounting data in LDAP is an abuse of the LDAP design goals of "frequent lookup, infrequent modification". Databases were designed for the type of data management that radius accounting involves, directories were not. Accounting should be in a database, not a directory. Directories were designed to solve different problems.
This is a bit OT, but I'm honestly curious here...
I've heard this sort of general statement about directories (specifically LDAP) being suitable for read-heavy workloads, and SQL being suitable for read/write-heavy, for well over 15 years now.
No-one has ever adequately explained to me *why* sending an LDAP write op PDU is somehow different than sending an LDAP read op PDU.
It may be that all or most LDAP *implementations* are relatively slow at writes, but if so, it doesn't follow that this *must* be the case.
Obviously writes are slower than reads in LDAP, but that's true of almost any datastore backed by permanent storage. However, having done mass/bulk LDAP updates to OpenLDAP (and on older versions) I've found it pretty snappy. So I'd love to know why this is an "abuse" any more than using any other protocol in unanticipated-but-compliant ways.
It's not an abuse but mainly not 'best-practice'. A well configured LDAP server with enough cache memory will be bottlenecked by memory and network speed and not by underlying I/O. A frequent writes strategy invalidates most of these performance gains since an entry write will invalidate entry and database cache entries.
Cheers, Phil - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
-- Kostas Kalevras Network Operations Center National Technical University of Athens http://kkalev.wordpress.com http://kkalev4economy.wordpress.com
On 11/12/12 11:31, Kostas Kalevras wrote:
It's not an abuse but mainly not 'best-practice'. A well configured LDAP
I hate that phrase. No offence, but it's often used as a substitute for critical thinking. *Everything* that is best practice eventually... isn't. And usually, it turns out that the "new" best practice was around for ages before anyone realised it. Case in point: RADIUS is used for things which the original designers couldn't have anticipated, and for which, arguably, it's not terribly well designed. But it *works*.
server with enough cache memory will be bottlenecked by memory and network speed and not by underlying I/O. A frequent writes strategy invalidates most of these performance gains since an entry write will invalidate entry and database cache entries.
Really? Why? LDAP doesn't provide MVCC semantics. As far as I can see, an LDAP update contains all the information required to update (not invalidate) in-RAM cache. To be clear: I don't use LDAP for data storage. I dislike it's data model, and the data typing in particular. But I don't see any reason it should be consigned to the "read-mostly" bin on a *theoretical* basis.
invalidate entry and database cache entries.
Really? Why?
LDAP doesn't provide MVCC semantics. As far as I can see, an LDAP update contains all the information required to update (not invalidate) in-RAM cache.
To be clear: I don't use LDAP for data storage. I dislike it's data model, and the data typing in particular. But I don't see any reason it should be consigned to the "read-mostly" bin on a *theoretical* basis.
Moved to a new thread. -Arran
On Tue, Dec 11, 2012 at 12:38:32PM +0000, Arran Cudbard-Bell wrote:
Moved to a new thread.
s/Moved to a new thread/Changed the subject header/ Threading is based on References: and In-Reply-To: headers, not the subject. Matthew -- Matthew Newton, Ph.D. <mcn4@le.ac.uk> Systems Architect (UNIX and Networks), Network Services, I.T. Services, University of Leicester, Leicester LE1 7RH, United Kingdom For IT help contact helpdesk extn. 2253, <ithelp@le.ac.uk>
On 11 Dec 2012, at 13:21, Matthew Newton <mcn4@LEICESTER.AC.UK> wrote:
On Tue, Dec 11, 2012 at 12:38:32PM +0000, Arran Cudbard-Bell wrote:
Moved to a new thread.
s/Moved to a new thread/Changed the subject header/
Threading is based on References: and In-Reply-To: headers, not the subject.
Matthew
Hmm the option to do that isn't obvious in Apple Mail. I guess I'd need to copy the body of the old message and insert it into a brand new message with a new subject. Thanks for pointing that out, i'll do that in future. -Arran
participants (9)
-
Alan DeKok -
Arran Cudbard-Bell -
Brian Candler -
John Dennis -
Kostas Kalevras -
Matthew Newton -
Olivier Beytrison -
Peter Lambrechtsen -
Phil Mayers