Initial support of Multi Valued Attributes operators
Hello, I've sent a pull request to add the initial support of MVA operators. This add 4 new operators to perform basic comparison with multivalued attributes * |== At least one attribute should equal the value * &== All attributes should equal the value * |=~ At least one attribute should match the regular expression * &=~ All attributes should match the regular expression The allowed syntax are attribute-name &==/|== "double quoted string with xlat expansion" attribute-name &==/|== 'single quoted string for a fixed value' attribute-name &=~/|=~ /regular expression/flag With the regexp, we're not matching against group. It will not populate the %{0} %{1} variables Their inverse is not available. Simply encapsulate their call inside !() For example, this will allow to simply check ldap-group with unlang without building a foreach loop or using huntfile. Arran is in the process to add this feature to rlm_ldap. With those operators, it would also make sense to retrieve all values with sql/ldap xlat. But this is to be discussed. The code has been tested here and it performs well. I tried to make it as clean as possible. What's left to do is update the unlang man page and the wiki. Well, if it is merged :) Olivier -- Olivier Beytrison Network & Security Engineer, HES-SO Fribourg Mail: olivier@heliosnet.org
On 25/03/13 16:50, Olivier Beytrison wrote:
Hello,
I've sent a pull request to add the initial support of MVA operators.
This add 4 new operators to perform basic comparison with multivalued attributes
* |== At least one attribute should equal the value * &== All attributes should equal the value * |=~ At least one attribute should match the regular expression * &=~ All attributes should match the regular expression The allowed syntax are
I'm a bit confused. Can you summarise how the *existing* operators behave, and how this is different? For example, =~ *already* matches "if one attribute" matches the regexp - we're relying on that behaviour, after my patch in 5a89c94633, and that code also populates the %{group} captures (of the first match) Likewise, I thought == matches if one of several attributes match, but that's not behaviour I'm testing. I don't understand the use-case for &==
For example, this will allow to simply check ldap-group with unlang without building a foreach loop or using huntfile. Arran is in the process to add this feature to rlm_ldap.
Again, I don't understand this; this is what "Ldap-Group == xx" already doesn't it? Have I misunderstood?
With those operators, it would also make sense to retrieve all values with sql/ldap xlat. But this is to be discussed.
Some better way to pull >1 value from SQL/LDAP would be good, and ideally a slightly cleaner syntax for doing it.
On 25.03.2013 18:12, Phil Mayers wrote:
On 25/03/13 16:50, Olivier Beytrison wrote:
Hello,
I've sent a pull request to add the initial support of MVA operators.
This add 4 new operators to perform basic comparison with multivalued attributes
* |== At least one attribute should equal the value * &== All attributes should equal the value * |=~ At least one attribute should match the regular expression * &=~ All attributes should match the regular expression The allowed syntax are
I'm a bit confused. Can you summarise how the *existing* operators behave, and how this is different?
For example, =~ *already* matches "if one attribute" matches the regexp - we're relying on that behaviour, after my patch in 5a89c94633, and that code also populates the %{group} captures (of the first match)
the %{group} capture is to match and capture part of *one* attribute. If you have multiple time the same attribute in the list, the only option was foreach (in 3.0) I'll give you a real example based on my use of those operators. My ldap populate attributes which represent the "roles" of the person within the school : control:HESSO-Role += "31935762-440774439#RORG-HEFR-EIFR-INTR-INFO#EMP#COL" (hessoRole) control:HESSO-Role += "31935762-440774439#RORG-HEFR-EIFR-INTR-INFO#STD#" (hessoRole) People can have from one to 6-7 of those attributes. With normal unlang operators it will only compare the first attribute it finds. Now I can do if(control:HESSO-Role |=~ /STD/) { student_policy }
Likewise, I thought == matches if one of several attributes match, but that's not behaviour I'm testing.
Nope, it only check the first attribute. Till now there was no looping over all the possible same attribute-name in the list.
I don't understand the use-case for &==
I don't see one. But maybe someday someone will need it ?
For example, this will allow to simply check ldap-group with unlang without building a foreach loop or using huntfile. Arran is in the process to add this feature to rlm_ldap.
Again, I don't understand this; this is what "Ldap-Group == xx" already doesn't it? Have I misunderstood?
In the huntfile yes. Not in unlang and policy, unless I've misread the code for the past two weeks ;) -- Olivier Beytrison Network & Security Engineer, HES-SO Fribourg Mail: olivier@heliosnet.org
On 25/03/13 17:23, Olivier Beytrison wrote:
People can have from one to 6-7 of those attributes. With normal unlang operators it will only compare the first attribute it finds.
In which case the case, the behaviour differs between "users" files (which compare all) and "unlang". Le sigh.
Likewise, I thought == matches if one of several attributes match, but that's not behaviour I'm testing.
Nope, it only check the first attribute. Till now there was no looping over all the possible same attribute-name in the list.
I wonder if that might not be a better solution; for the == and =~ operators, loop over all and match if any matches. This seems "least surprising" to me. You would still then want a "match all" operator, which would need to be new. But I don't feel strongly about it; I'm already abusing unlang way too much anyway, and if I get the time and energy I'm going to investigate alternative solutions (that don't involve rlm_perl).
On 25 Mar 2013, at 13:33, Phil Mayers <p.mayers@IMPERIAL.AC.UK> wrote:
On 25/03/13 17:23, Olivier Beytrison wrote:
People can have from one to 6-7 of those attributes. With normal unlang operators it will only compare the first attribute it finds.
In which case the case, the behaviour differs between "users" files (which compare all) and "unlang". Le sigh.
Likewise, I thought == matches if one of several attributes match, but that's not behaviour I'm testing.
Nope, it only check the first attribute. Till now there was no looping over all the possible same attribute-name in the list.
I wonder if that might not be a better solution; for the == and =~ operators, loop over all and match if any matches. This seems "least surprising" to me.
You would still then want a "match all" operator, which would need to be new.
Actually yes, this makes more sense. If you want to test specific instances of an attribute you can always use <attr>[<num>]. &== ∀ == ∃ We still don't have a higlander operator (∃!), but i'm not sure that'd be useful. Honestly i'm not sure if &== is at all useful and adding features for perceived completeness just means additional code to maintain. &=~ OTOH could be very useful for validation purposes.
But I don't feel strongly about it; I'm already abusing unlang way too much anyway, and if I get the time and energy I'm going to investigate alternative solutions (that don't involve rlm_perl).
Good plan. Do you have any interest in using Lua? -Arran
On 25.03.2013 22:05, Arran Cudbard-Bell wrote:
On 25 Mar 2013, at 13:33, Phil Mayers <p.mayers@IMPERIAL.AC.UK> wrote:
On 25/03/13 17:23, Olivier Beytrison wrote:
People can have from one to 6-7 of those attributes. With normal unlang operators it will only compare the first attribute it finds.
In which case the case, the behaviour differs between "users" files (which compare all) and "unlang". Le sigh.
Likewise, I thought == matches if one of several attributes match, but that's not behaviour I'm testing.
Nope, it only check the first attribute. Till now there was no looping over all the possible same attribute-name in the list.
I wonder if that might not be a better solution; for the == and =~ operators, loop over all and match if any matches. This seems "least surprising" to me.
You would still then want a "match all" operator, which would need to be new.
Actually yes, this makes more sense.
I can refactor my code so that == and =~ will act like |== and |=~. But then what about != and !~ ?
If you want to test specific instances of an attribute you can always use <attr>[<num>].
If you know how much attribute you have yes. If it's random, rather hard to use. You need to use foreach atm.
Honestly i'm not sure if &== is at all useful and adding features for perceived completeness just means additional code to maintain.
&=~ OTOH could be very useful for validation purposes. &=~ and |=~ allows me to shrink my policies from ~ 80 lines of code to
I must agree. &== is not that useful. I have a kind of use-case in my mind, but even with it, a if with two comparison would do the same. But adding is a question of 5 lines of code. But I can remove it. 15. No need to use foreach anymore.
But I don't feel strongly about it; I'm already abusing unlang way too much anyway, and if I get the time and energy I'm going to investigate alternative solutions (that don't involve rlm_perl).
Good plan. Do you have any interest in using Lua?
I'm. I like LUA. Olivier -- Olivier Beytrison Network & Security Engineer, HES-SO Fribourg Mail: olivier@heliosnet.org
On Mon, Mar 25, 2013 at 05:33:55PM +0000, Phil Mayers wrote:
I wonder if that might not be a better solution; for the == and =~ operators, loop over all and match if any matches. This seems "least surprising" to me.
I do agree with that.
You would still then want a "match all" operator, which would need to be new.
Would you? if ( ! (Tmp-String-0 != "foo") ) if ( ! (Tmp-String-0 !~ /foo/) ) Regards, Brian.
Again, I don't understand this; this is what "Ldap-Group == xx" already doesn't it? Have I misunderstood?
In the huntfile yes. Not in unlang and policy, unless I've misread the code for the past two weeks ;)
LDAP-Group and SQL-Group are different from normal attributes. They use a paircomp callback to do the comparison instead of the normal condition code. -Arran
On 25.03.2013 20:56, Arran Cudbard-Bell wrote:
Again, I don't understand this; this is what "Ldap-Group == xx" already doesn't it? Have I misunderstood?
In the huntfile yes. Not in unlang and policy, unless I've misread the code for the past two weeks ;)
LDAP-Group and SQL-Group are different from normal attributes. They use a paircomp callback to do the comparison instead of the normal condition code.
Fair enough, I didn't go check into rlm_ldap and rlm_sql for those attributes :) Olivier -- Olivier Beytrison Network & Security Engineer, HES-SO Fribourg Mail: olivier@heliosnet.org
For example, this will allow to simply check ldap-group with unlang without building a foreach loop or using huntfile. Arran is in the process to add this feature to rlm_ldap.
Again, I don't understand this; this is what "Ldap-Group == xx" already doesn't it? Have I misunderstood?
The feature i'm adding doesn't depend on these operators. The LDAP-Group paircomp function will loop over cached group attributes without the new operators. This is required to emulate behaviour where the check is being performed directly against directory. -Arran
On Mon, Mar 25, 2013 at 05:50:26PM +0100, Olivier Beytrison wrote:
I've sent a pull request to add the initial support of MVA operators.
This add 4 new operators to perform basic comparison with multivalued attributes
* |== At least one attribute should equal the value * &== All attributes should equal the value * |=~ At least one attribute should match the regular expression * &=~ All attributes should match the regular expression The allowed syntax are
attribute-name &==/|== "double quoted string with xlat expansion" attribute-name &==/|== 'single quoted string for a fixed value' attribute-name &=~/|=~ /regular expression/flag
How do you specify which list to search for the attribute - i.e. request, reply, or control? With a single-valued attribute I would do a string expansion: if ("%{control:Tmp-String-0}" =~ /foo/) { ... But that won't work here unless the &=~ operator expands the LHS multiple times (and then you get into problems if a string contains multiple expansions) I don't think freeradius will currently let you select the list directly: if (control:Tmp-String-0 =~ /foo/) { ... nope but if we could do that, the problem goes away. Taking this idea further, I would very much like to be able to return attribute names from rlm_sql which specify the list: insert into radgroupcheck (attribute, op, value) values ( "reply:Framed-IP-Address", "=~", "^192\\.168\\." ); That would let me check the reply attributes already built up, at the moment I have to copy specific attributes from the reply to the request list just so they can be checked in rlm_sql. Another idea along these lines: update { control:Tmp-String-1 += "bar" reply:Reply-Message += "Wibble" } and finally: update { /* copy _all_ instances of the attribute to another attribute */ reply:Reply-Message ++= control:Tmp-String-0 } To implement this, perhaps struct value_pair would get an extra member identifying the list that the attribute relates to (where 0/null would mean "the default list in this context") But I think you're probably not proposing such a wholesale change as this :-) Regards, Brian.
On 25/03/13 17:35, Brian Candler wrote:
With a single-valued attribute I would do a string expansion:
if ("%{control:Tmp-String-0}" =~ /foo/) { ...
But that won't work here unless the &=~ operator expands the LHS multiple times (and then you get into problems if a string contains multiple expansions)
I don't think freeradius will currently let you select the list directly:
if (control:Tmp-String-0 =~ /foo/) { ... nope
Sure it will. I do this all the time, as using a string xlat on the LHS is both messy (extraneous "%{}" - 5 chars!) and loses attribute type info (because it's string-expanded). From my standard config: update control { Tmp-String-0 = "%{new:select ...}" } if (control:Tmp-String-0 =~ /^(.+),(.+),(.+)$/) { update request { Calling-Station-Id := "%{1}" ICzone = "%{2}" ICvlan = "%{3}" } }
but if we could do that, the problem goes away. Taking this idea further, I would very much like to be able to return attribute names from rlm_sql which specify the list:
insert into radgroupcheck (attribute, op, value) values ( "reply:Framed-IP-Address", "=~", "^192\\.168\\." );
That would let me check the reply attributes already built up, at the moment I have to copy specific attributes from the reply to the request list just so they can be checked in rlm_sql.
Yeah, it would be nice to use rlm_sql in a more generic fashion, rather than the hard-coded check/reply behaviours.
Another idea along these lines:
update { control:Tmp-String-1 += "bar" reply:Reply-Message += "Wibble" }
Hasn't that already gone in?
On 25.03.2013 18:35, Brian Candler wrote:
I don't think freeradius will currently let you select the list directly:
if (control:Tmp-String-0 =~ /foo/) { ... nope
As phil said, it does. at least in HEAD
Another idea along these lines:
update { control:Tmp-String-1 += "bar" reply:Reply-Message += "Wibble" }
This is also already the case
and finally:
update { /* copy _all_ instances of the attribute to another attribute */ reply:Reply-Message ++= control:Tmp-String-0 }
This has already been implemented in rlm_cache, and Arran made it so we can reuse the logic (attrmap) in other modules. Another example of the operator in function : post-auth { if (reply:HESSO-Role-Raw =~ /STD/) { noop } if (reply:HESSO-Role-Raw |=~ /STD/) { noop } } gives (9) ? if (reply:HESSO-Role-Raw =~ /STD/) (9) ? Evaluating (reply:HESSO-Role-Raw =~ /STD/) -> FALSE (9) ? if (reply:HESSO-Role-Raw =~ /STD/) -> FALSE (9) ? if (reply:HESSO-Role-Raw |=~ /STD/) (9) ? Evaluating 31935762-440774439#RORG-HEFR-EIFR-INTR-INFO#EMP#COL |=~ STD -> FALSE (9) ? Evaluating 31935762-440774439#RORG-HEFR-EIFR-INTR-INFO#STD# |=~ STD -> TRUE (9) ? Evaluating (reply:HESSO-Role-Raw |=~ /STD/) -> TRUE (9) ? if (reply:HESSO-Role-Raw |=~ /STD/) -> TRUE -- Olivier Beytrison Network & Security Engineer, HES-SO Fribourg Mail: olivier@heliosnet.org
On 25 Mar 2013, at 14:37, Olivier Beytrison <olivier@heliosnet.org> wrote:
On 25.03.2013 18:35, Brian Candler wrote:
I don't think freeradius will currently let you select the list directly:
if (control:Tmp-String-0 =~ /foo/) { ... nope
As phil said, it does. at least in HEAD
Another idea along these lines:
update { control:Tmp-String-1 += "bar" reply:Reply-Message += "Wibble" }
This is also already the case
and finally:
update { /* copy _all_ instances of the attribute to another attribute */ reply:Reply-Message ++= control:Tmp-String-0 }
This has already been implemented in rlm_cache,
and multiple places in rlm_ldap.
and Arran made it so we can reuse the logic (attrmap) in other modules.
Another example of the operator in function : post-auth { if (reply:HESSO-Role-Raw =~ /STD/) { noop } if (reply:HESSO-Role-Raw |=~ /STD/) { noop } }
A lot of this should wait until we've finalised the syntax for nested attributes as lists may get included as part of the attribute path. e.g. control.Tmp-TLV-1.Tmp-String-1. This would allow temporary trees to be constructed and then merged in there entirity into the current request without special consideration for the different lists. This makes places where collections of attributes need to be updated atomically easier (no more tmp_check, tmp_reply etc.). It also makes dealing with multiple protocols easier, as protocol specific lists can be added without having to update the C structures. As TLV support is planned for all current modules inclding rlm_sql, rlm_ldap, rlm_files, this would cover all the scenarios described. In regards to xlat returning multiple attributes. It's probably not going to happen. xlat is just for string expansion and there are limited places where this is useful (rlm_ldap, rlm_sql and rlm_redis). What's probably a better idea is to split the connection management and the logic into two or more modules. In addition to the modules that implement the functionality we have now, it would be fairly easy to add stub modules for just doing mapping. The stub module would contain a query config item and an update section similar to the rlm_ldap and rlm_cache modules. Depending on the operators used, multiple rows would just create multiple instances of attributes. -Arran
participants (4)
-
Arran Cudbard-Bell -
Brian Candler -
Olivier Beytrison -
Phil Mayers