2.2.1: if-block leads to reject

[Jakob Hirsch]

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
    }
}

debug output:

> rad_recv: Access-Request packet from host 127.1.0.5 port 33185, id=115, length=55
> 	User-Name = "test-user"
> 	User-Password = "test-password"
> 	NAS-Port = 12345
> # Executing section authorize from file /etc/ui-freeradius/sites/test.conf
> +group authorize {
> ++update control {
> ++} # update control = noop
> +} # group authorize = notfound
> Found Auth-Type = Test
> # Executing group from file /etc/ui-freeradius/sites/test.conf
> +group Test {
> ++? if (1)
> ? Evaluating (1) -> TRUE
> ++? if (1) -> TRUE
> ++if (1) {
> ++} # if (1) = reject
> +} # group Test = reject
> Failed to authenticate the user.
> Login incorrect: [test-user/test-password] (from client Arcor_test port 12345)
> Sending Access-Reject of id 115 to 127.1.0.5 port 33185
> Finished request 0.

So, for some reason, freeradius thinks that the if-block returns with a
"reject", which is at least counterintuitive (though I just noticed that
2.2.0 behaved the same, so it's ok probably).

If I activate the "noop" (corresponding instance of rlm_always, as the
above "ok") in the if-block, it works:

> +group Test {
> ++? if (1)
> ? Evaluating (1) -> TRUE
> ++? if (1) -> TRUE
> ++if (1) {
> +++[noop] = noop
> ++} # if (1) = noop
> ++[ok] = ok
> +} # group Test = ok

The update section is not suffucient, though:

> +group Test {
> ++? if (1)
> ? Evaluating (1) -> TRUE
> ++? if (1) -> TRUE
> ++if (1) {
> +++update reply {
> +++} # update reply = noop
> ++} # if (1) = reject
> +} # group Test = reject

This is the actual issue I encountered. We use several if-blocks with
update sections and this always worked (including 2.2.0).


Running 2.2.1 with our live config, I even get a return value of "noop",
which leads to a different problem:

> ++? if (control:X-Attr-Id && (control:X-Attr-Down || control:X-Attr-Up))
> ? Evaluating (control:X-Attr-Id ) -> TRUE
> ?? Evaluating (control:X-Attr-Down ) -> TRUE
> ?? Skipping (control:X-Attr-Up)
> ++? if (control:X-Attr-Id && (control:X-Attr-Down || control:X-Attr-Up)) -> TRUE
> ++if (control:X-Attr-Id && (control:X-Attr-Down || control:X-Attr-Up)) {
> +++update reply {
>         expand: %{control:X-Attr-ID} -> 1
>         expand: %{control:X-Attr-Down} -> 2
>         expand: %{control:X-Attr-Up} -> 3
> +++} # update reply = noop
> ++} # if (control:X-Attr-Id && (control:X-Attr-Down || control:X-Attr-Up)) = noop
> ++[ok] = ok
> +} # group SP-Auth = noop

The if block returns a noop, which the following "ok" cannot override,
so the whole group returns with "noop", which make freeradius reject the
request. This worked in 2.2.0 (and looked a bit different):

...
> ++? if (control:X-Attr-Id && (control:X-Attr-Down || control:X-Attr-Up)) -> TRUE
> ++- entering if (control:X-Attr-Id && (control:X-Attr-Down || control:X-Attr-Up)) {...}
> 	expand: %{control:X-Attr-Id} -> 1
> 	expand: %{control:X-Attr-Down} -> 2
> 	expand: %{control:X-Attr-Up} -> 3
> +++[reply] returns noop
> ++- if (control:SP-Drossel-DTAG-ID && (control:SP-Drossel-Down || control:SP-Drossel-Up)) returns noop
> ++[ok] returns ok
> Login OK: [xxxx-xxxx] (from client client_test port 12345)



Regards
Jakob