New feature in v3: cast!

[Arran Cudbard-Bell]

On 12 May 2013, at 08:49, Alan DeKok <aland at deployingradius.com> wrote:

> Brian Candler wrote:
>> Hmm, I think this syntax is obscure.  Here the == operator is being changed
>> to mean "is this IP address contained within that prefix"?  They're not
>> equal to each other.  Perhaps something like =~ would be clearer?
> 
>  =~ already has a meaning as a regex operator.  So I'd like to avoid
> using that.

Yes that's extremely confusing.

> 
>> I'm also not sure that casting the LHS to <cidr> is a clear way of achieving
>> this.  The LHS is, and remains, a single IP address.  The RHS is a CIDR
>> prefix.
> 
>  The issue is that the code does type-specific comparisons.  It's hard
> to know what's right for inter-type comparisons.
> 
> 	if (127.0.0.1 < 2)
> 
>  ???
> 
>> So I'd rather see:
>> 
>> if (Framed-IP-Address =~ <cidr>192.168.0.0/24) { ... }
> 
>  That could work (maybe) with some changes.
> 
>> since the unquoted RHS literal can be unambiguously parsed as a CIDR prefix
>> anyway,
> 
>  Well... maybe.  What about:
> 
> 	if (Framed-IP-Address =~ "%{sql: ...") {

Yeah the key here is the type of comparison is done at parse time, not runtime
there'd need to be an exception for IP addresses such that the format of the
expansion was done at runtime. It's more code and more effort.

> 
>  Is the RHS a regex?  A CIDR comparison?  An IP address comparison?

Using the =~ operator is a bad idea. 

The most logical and consistent way to do this IMO is to treat the ranges
as sets of IP addresses.

== is L = R
>  is L ⊃ R
<  is L ⊂ R
>= is L ⊇ R
<= is L ⊆ R

It's a bit weird if you don't get what's going on, but arguably more
powerful if you do.

if (<cidr> Framed-IP-Address < "192.168.0.0/24") {

}

if (<cidr> "192.168.0.0/24" < "192.168.0.0/16") {

}

Where the ranges do not overlap if condition evaluates to false.

The other option:

== is L ⊂ R
>  is integer comparison of masked off bits in L and R
<  is integer comparison of masked off bits in L and R

But that's nasty...

> 
>  I'm trying to say that the code needs *unambiguous* rules.  Ad-hoc
> rules work in some cases, and don't work in others.  So they can't be used.
> 
>  The casting, however, is always unambiguous.  Cast both sides to a
> type, and do type-specific comparions.
> 
>> and a comparison of IP value with CIDR value have the obvious
>> semantics.
> 
>  "obvious"?  What happens when an IP address is smaller than the CIDR
> range?  Or greater than it?
> 
>  For CIDRs, we need at least a "contains" operator.  As in "does CIDR 1
> contain CIDR 2".

Yeah that's why the above works well. The symbols also look pretty similar :)

> 
>> Maybe the reason for casting the LHS to <cidr> so that you can go a general
>> comparison of (CIDR) to (CIDR) rather than (IP) within (CIDR).  In that
>> case, the rules for comparing two (CIDR) values need to be clear.
> 
>  See Wikipedia for "range comparison" (IIRC).  There are 20+ types of
> possible relationships for two ranges.  In contrast, integers just have
> 3 (and their negations).  ==, <, >  (and !=, >=, <=)
> 
>  It's much easier to just say define a "contains" operator for CIDRs.
> That way you can't even *express* the other types of relationships.
> Because they're not needed 99.999% of the time.  And if they are needed,
> use Perl.

*Python

> 
>> Finally, there is a problem of what to do with modules like rlm_sql and
>> rlm_files.  These are already restricted to returning LHS=attribute name,
>> RHS=string literal.  Unfortunately we can't use the =~ operator here because
>> people may already be doing things like
>> 
>>    ("Framed-IP-Address", "=~", "^192\.168\.")
>> 
>> So maybe it makes sense for these to return
>> 
>>    ("<cidr>Framed-IP-Address", "=~", "192.168.0.0/24")
>> 
>> but this still doesn't feel quite right to me.
> 
>  That will probably *not* happen.
> 
>> Aside: if this is going to be done, I have a much more pressing need for
>> being able to qualify the attribute with the list name:
>> 
>>    ("reply:Framed-IP-Address", "=*", "present")
>> 
>> At the moment I have to copy a bunch of attributes from the reply list to
>> the request list, just so that rlm_sql can match them.
> 
>  Adding a list qualifier is probably easy.  It's on the "to do" items
> for v3.

It's not *that* easy. IIRC the problem is that the SQL code creates intermediary 
linked lists of VALUE_PAIRs and as VALUE_PAIRs cannot contain destination lists 
they also get inserted into the same request list.

It becomes easier with nested attributes as you can root the VALUE_PAIRs in the
correct top level nodes, and then merge the trees.

You retain the atomic insert whilst being able to specify where the VALUE_PAIRs
should go.

Arran Cudbard-Bell <a.cudbardb at freeradius.org>
FreeRADIUS Development Team