Re: 2.2.1: if-block leads to reject

2 Oct 2013

      On 24/09/2013 13:10, Jakob Hirsch wrote:
...
Hi,
another issue in 2.2.1 (and the current v2.x.x git): if-blocks in an
authenticate section cause to wrong rejects.
Example with minimal config:
authorize {
     update control {
         Auth-Type := 'Test'
     }
}
authenticate {
         if (1) {
             #noop
             #update reply {
             #    Reply-Message := "bla"
             #}
         }
         ok
     }
}
I queried this a while ago, and it seems to be intentional (albeit 
sparsely documented) behaviour.
Configurable failover is described here:
http://wiki.freeradius.org/config/Fail-over

A sequence of modules like

     foo
     bar
     baz

is a group, and if one of those modules gives a failure status, the 
entire group terminates early unless you've applied a numeric value to 
the failure code, in which case it will be carried forward.

Now, an 'if' block is itself a failover block. So if there was a success 
or fail status set before the start of the block, it will cause the 
whole group within which the 'if' is enclosed to terminate at the end of 
that block - even if the 'if' block itself did nothing to set success or 
fail status. But this doesn't happen if the block is skipped because the 
condition was false.

Here's an example. I want to conditionally log CHAP failures, and I also 
want to call additional code in policy.conf (e.g. to turn some 
access-rejects into access-accept with a walled garden).

The config looks something like this:

authenticate {
         Auth-Type CHAP {
                 chap {
                         ok = return
                         reject = 1
                 }

                 if (some condition) {
                         update control {
                                 Tmp-String-0 += "CHAP 
%{%{request:CHAP-Challenge}:-%{request:Packet-Authentication-Vector}} 
%{request:CHAP-Password}"
                         }
                         # 'if' is an instance of configurable failover;
                         # we must prevent an early return
                         reject = 1
                 }

                 handleReject
         }
}

If the second "reject = 1" is omitted, then termination occurs at the 
end of the 'if' block, before handleReject is called.

Now, handleReject is defined in policy.conf. Suppose I want to update an 
attribute conditionally in there. This also has the same issue:

policy {
         handleReject {
             if (some condition) {
                 update control {
                     SomeAttr := "somevalue"
                 }
                 # Behaviour of configurable failover: a reject set
                 # earlier causes an implicit 'return' at the end of an 
'if'.
                 # We need this line to prevent it.
                 reject = 1
             }
             ... continue
         }
}

Furthermore, if handleReject decides to do nothing, but you want to 
continue after handleReject to another module in the authenticate {} 
block, it also needs to end with a 'reject = 1'

authenticate {
         Auth-Type CHAP {
...
                 handleReject
                 doSomethingElse
         }
}

policy {
     handleReject {
         ...
         reject = 1
     }
}

In this case, it has to be put at the end of the block definition in 
policy.conf, not at the point where it is invoked. What I expected (but 
is *not* the case) was:

authenticate {
         Auth-Type CHAP {
...
                 handleReject {
                     reject = 1   # CAN'T BE APPLIED HERE
                 }
                 doSomethingElse
         }
}

I wonder if this could all be made to work in a more intuitive way - for 
example, once a module has set "reject = 1" this status sticks until 
another 'real' module is called, as opposed to just update { ... } 
sections.

Anyway, this is just something I've had to grit my teeth and bear with. 
It seems really weird, but eventually makes sort of sense. I ended up 
having to write a test suite (invoking radclient with all the different 
cases) to make sure they all worked the way I wanted.

Regards,

Brian.

P.S. In some cases there may be multiple return conditions you need to tag:

         Auth-Type PAP {
                 pap {
                         ok = return
                         reject = 1      # password mismatch
                         fail = 1        # no password to match in 
control list
                         invalid = 1     # no password in request
                 }
                 ... continue

In practice, if you use the 'pap' module in the authorize section, it 
won't set Auth-Type := PAP unless there is a User-Password in the request.

Re: 2.2.1: if-block leads to reject

Brian Candler