Freeradius-Devel Digest, Vol 97, Issue 15tz
hu don
hyd66 at hotmail.com
Sun May 12 18:07:04 CEST 2013
l j Ljubljana Ji
Mantel moon
On May 12, 2013, at 11:33 AM, freeradius-devel-request at lists.freeradius.org wrote:
> Send Freeradius-Devel mailing list submissions to
> freeradiudevel at lists.freeradius.org I
> To subscribe or unsubscribe via the World Wide Web, visit
> http://lists.freeradius.org/mailman/listinfo/freeradius-develn
> or, via email, send a message with subject or body 'help' to
> freeradius-devel-request at lists.freeradius.org
>
> You can reach the person managing the list at
> freeradius-devel-owner at lists.freeradius.org
>
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of Freeradius-Devel digest..."
>
>
> Today's Topics:
>
> 1. Re: New feature in v3: cast! (Brian Candler)
> 2. Re: New feature in v3: cast! (Brian Candler)
> 3. Re: New feature in v3: cast! (Alan DeKok)
> 4. Re: New feature in v3: cast! (Alan DeKok)
> 5. rad_unlockfd (Brian Candler)
> 6. Re: New feature in v3: cast! (Arran Cudbard-Bell)
>
>
> ----------------------------------------------------------------------
>
> Message: 1
> Date: Sun, 12 May 2013 11:39:30 +0100
> From: Brian Candler <B.Candler at pobox.com>
> To: FreeRadius developers mailing list
> <freeradius-devel at lists.freeradius.org>
> Subject: Re: New feature in v3: cast!
> Message-ID: <20130512103930.GA27456 at nsrc.org>
> Content-Type: text/plain; charset=us-ascii
>
> On Sat, May 11, 2013 at 10:19:02AM -0400, Alan DeKok wrote:
>> Not fishing casts, but data type casts. :) See "man unlang" for details.
>>
>> if (<ipaddr>127.0.0.1 < &Framed-IP-Address) {
>> ...
>> }
>
> 1. Why is <ipaddr> necessary before the literal? Surely an unquoted
> 127.0.0.1 can't be parsed as anything else.
>
> 2. What does the & in front of Framed-IP-Address do?
>
>
> ------------------------------
>
> Message: 2
> Date: Sun, 12 May 2013 12:18:21 +0100
> From: Brian Candler <B.Candler at pobox.com>
> To: FreeRadius developers mailing list
> <freeradius-devel at lists.freeradius.org>
> Subject: Re: New feature in v3: cast!
> Message-ID: <20130512111821.GB27456 at nsrc.org>
> Content-Type: text/plain; charset=us-ascii
>
> On Sat, May 11, 2013 at 06:48:18PM -0400, Arran Cudbard-Bell wrote:
>> One useful thing which i'm not sure Alan has implemented yet is:
>>
>> if (<cidr>&Framed-IP-Address == 192.168.0.0/24) {
>>
>> }
> ...
>> With the cast, Framed-IP-Address gets converted to a /32, and the first 24
>> bits from the values are compared.
>
> 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?
>
> 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.
>
> So I'd rather see:
>
> if (Framed-IP-Address =~ <cidr>192.168.0.0/24) { ... }
>
> or:
>
> if (Framed-IP-Address =~ 192.168.0.0/24) { ... }
>
> since the unquoted RHS literal can be unambiguously parsed as a CIDR prefix
> anyway, and a comparison of IP value with CIDR value have the obvious
> semantics.
>
> 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. The two
> prefix lengths may be different. Are you just comparing the shorter of the
> two prefixes (so that the == operator is commutative)? Or does the ==
> operator return true only if the LHS CIDR range is completely contained
> within the RHS CIDR range?
>
> Example:
>
> 192.168.0.0/24 == 192.168.0.0/23 # true or false?
> 192.168.1.0/24 == 192.168.0.0/23 # true or false?
> 192.168.0.0/23 == 192.168.1.0/24 # true or false?
>
> And what about single IP addresses in CIDR notation, where the host part is
> non-zero?
>
> 192.168.0.1/24 == 192.168.0.1/24 # true or false?
> 192.168.0.1/24 == 192.168.0.2/24 # true or false?
> 192.168.0.1/24 == 192.168.0.1/23 # true or false?
>
> Also, is there planned to be an ordering for CIDR values?
>
> 192.168.0.0/24 < 192.168.0.0/23 # true or false?
>
> 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.
>
> 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.
>
> Regards,
>
> Brian.
>
>
> ------------------------------
>
> Message: 3
> Date: Sun, 12 May 2013 08:40:14 -0400
> From: Alan DeKok <aland at deployingradius.com>
> To: FreeRadius developers mailing list
> <freeradius-devel at lists.freeradius.org>
> Subject: Re: New feature in v3: cast!
> Message-ID: <518F8DAE.40800 at deployingradius.com>
> Content-Type: text/plain; charset=ISO-8859-1
>
> Brian Candler wrote:
>> 1. Why is <ipaddr> necessary before the literal? Surely an unquoted
>> 127.0.0.1 can't be parsed as anything else.
>
> See my other example for why it's necessary.
>
> In *some* cases, you know the data type of an expression. In those
> cases, you can easily do type-specific comparisons. That's what unlang
> does today:
>
> if (Framed-IP-Address == 127.0.0.1) {
>
> The type is "ipaddr", because of Framed-IP-Address. The RHS is in
> fact parsed into a second Framed-IP-Address attribute, and the two are
> compared.
>
> However... you can't currently do a type-safe comparison like:
>
> if (127.0.0.1 < 127.0.0.2) {
>
> The interpretor does *string* comparisons. Which is wrong for IP
> addresses. Adding a cast allows you to do:
>
> if (<ipaddr>127.0.0.1 < 127.0.0.2) {
>
> Which does type-safe checks as with Framed-IP-Address, above.
>
>> 2. What does the & in front of Framed-IP-Address do?
>
> $ man unlang :)
>
> It's a reference.
>
> if (&User-Name == &Filter-Id) {
>
> Does type-safe comparisons on the *values* of the two attributes. The
> v2 unlang code would require you to do:
>
> if (User-Name == "%{Filter-Id}") {
>
> Which converts Filter-Id to a string, parses the string into a
> temporary User-Name attribute, and then compares the two User-Name
> attributes.
>
> Using a reference means you can skip 2 out of 3 of those steps.
>
> Alan DeKok.
>
>
> ------------------------------
>
> Message: 4
> Date: Sun, 12 May 2013 08:49:07 -0400
> From: Alan DeKok <aland at deployingradius.com>
> To: FreeRadius developers mailing list
> <freeradius-devel at lists.freeradius.org>
> Subject: Re: New feature in v3: cast!
> Message-ID: <518F8FC3.3020502 at deployingradius.com>
> Content-Type: text/plain; charset=ISO-8859-1
>
> 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.
>
>> 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: ...") {
>
> Is the RHS a regex? A CIDR comparison? An IP address comparison?
>
> 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".
>
>> 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.
>
>> 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.
>
> Alan DeKok.
>
>
> ------------------------------
>
> Message: 5
> Date: Sun, 12 May 2013 15:16:42 +0100
> From: Brian Candler <B.Candler at pobox.com>
> To: freeradius-devel at lists.freeradius.org
> Subject: rad_unlockfd
> Message-ID: <20130512141642.GA28314 at nsrc.org>
> Content-Type: text/plain; charset=us-ascii
>
> I noticed something odd in rad_unlockfd - both 2.x.x and master.
>
> int rad_unlockfd(int fd, int lock_len)
> {
> ...
> fl.l_type = F_WRLCK;
> fl.l_whence = SEEK_CUR;
>
> return fcntl(fd, F_UNLCK, (void *)&fl);
>
> However, according to the Linux manpage for fcntl:
>
> F_SETLK (struct flock *)
> Acquire a lock (when l_type is F_RDLCK or F_WRLCK) or release a
> lock (when l_type is F_UNLCK) on the bytes specified by the
> l_whence, l_start, and l_len fields of lock. If a conflicting
> lock is held by another process, this call returns -1 and sets
> errno to EACCES or EAGAIN.
>
> That is: to unlock, the command should be F_SETLK(W) and the l_type should
> be F_UNLCK. The OSX (BSD) manpage agrees.
>
> On Linux, the constant F_UNLCK is 2, so calling fcntl(fd, F_UNLCK...)
> is the same as calling fcntl(fd, F_SETFD...)
>
> The other oddity is using SEEK_CUR for both lock and unlock. If you have
> written to the file in the mean time, then the current file offset will have
> changed, so you may end up unlocking a different byte range to the one you
> locked. I'd say SEEK_SET with offset 0 and length 0 (which locks or unlocks
> the entire file, including past its end) is safest - at least when
> unlocking.
>
> As far as I can see, only rad_utmp calls this function, so the impact is not
> huge.
>
> Regards,
>
> Brian.
>
>
> ------------------------------
>
> Message: 6
> Date: Sun, 12 May 2013 11:31:16 -0400
> From: Arran Cudbard-Bell <a.cudbardb at freeradius.org>
> To: FreeRadius developers mailing list
> <freeradius-devel at lists.freeradius.org>
> Subject: Re: New feature in v3: cast!
> Message-ID: <C3736289-E09A-43F4-9A58-656B35B8A2B1 at freeradius.org>
> Content-Type: text/plain; charset=utf-8
>
>
> 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
>
>
>
> ------------------------------
>
> -
> List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
>
>
> End of Freeradius-Devel Digest, Vol 97, Issue 15
> ************************************************
More information about the Freeradius-Devel
mailing list