eap module returning 'updated' rather than 'ok'

[Brian Candler]

On 20/10/2016 15:12, Alan DeKok wrote:
>    Those two identities don't have to be related in any way whatsoever.

Exactly. So I'm intentionally demonstrating the case where they are 
different; a potential attacker can choose whichever outer identity they 
like.

>> Note how I've chosen "steve" as the anonymous identity.
>    "steve" isn't really an "anonymous" identity.  For a discussion of anonymous identities, see my RFC:
>
> https://tools.ietf.org/html/rfc7542#section-2.4
Yes I know. An attacker can choose whichever identity they like. Even 
the stock Android client lets you enter whichever "anonymous" identity 
you like; no special tools required.

>>   What happens is that is in the second Access-Challenge response, steve's attributes are returned:
>    Which is what you told it to do.
It's what the default config tells it to do. I was surprised. It does 
not seem like desirable default behaviour: specifically, an unauthorized 
user being able to generate RADIUS reply attributes which belong to some 
other user.

>
>> It's clearly wrong to return steve's authorization attributes, since we've not authenticated at all (and certainly not as steve) - although since this only an Access-Challenge, hopefully the NAS will ignore them.  The EAP exchange does complete successfully.
>    See the comments in the raddb/sites-available/default.  Look for "Access-Challenge".  The documentation describes the problem, and shows how to fix it.
I had read that already. It says that "older configurations" had this 
problem, and it is commented out. The implication is that the current 
configuration *doesn't* have this problem.

But I see this behaviour with the current sample configuration which is 
supplied with freeradius 3.0.12.



>> My other concern is that it does an unnecessary database lookup for "steve" - actually the live config which started this investigation is an LDAP one, which is how I noticed this.
>>
>> Now, the default site has in its authorize section:
>>
>>         eap {
>>                 ok = return
>>         }
>>
>> But at this step we're getting "updated". So it looks like it would be reasonable to change this to:
>>
>>         eap {
>>                 ok = return
>>                 updated = return
>>         }
>    It depends.  Given the stable nature of 3.0, I'm inclined to leave it.

Do you mean leave the sample config as-is, or leave the behaviour of the 
rlm_eap module as-is?

Would you say rlm_eap is behaving as intended by returning 'updated' for 
this particular step of the exchange?

>> ... and this does seem to work. But I wonder why it's done this way in the default config. Is this a mistake, or this there some subtle point I am missing?
>    Other peoples systems may behave differently from yours.  So I'm inclined to leave the current configuration.
Well, obviously people's configs are different, but if they copy-pasted 
this part of eap config then they should behave the same.

This seems to be an under-documented area of freeradius. For example:
http://wiki.freeradius.org/modules/Rlm_eap
/usr/share/doc/freeradius-3.0.11/modules/rlm_eap

I couldn't find any reference to return codes, or "ok" or "updated", in 
either of these.


>
>    We will be fixing all of this in v4.  Not by accident, but by design.
>
>> Under what circumstances does rlm_eap return "updated" instead of "ok"? I want to be sure that there's no security impact by dropping out of the authorize section at this point, for example if someone uses a non-tunneled version of EAP like EAP-TLS or EAP-PWD.
>    That's what tests are for.  The server comes with configuration files for eapol_test (see src/tests/eap*.conf).
I can't see any tests which exercise the individual modules and check 
their return codes under different scenarios.

I can see high-level tests exercise the general behaviour ("does 
radclient return 0?").  I'm not sure it actually checks the returned 
RADIUS attributes at each stage of the exchange, nor scenarios which 
require a reject rather than an accept. But that's just from a cursory 
scan of the scripts.

Anyway, I'm not necessarily saying that the module behaviour is wrong. 
What I am saying is:

* the module returns different return values at different parts of the 
EAP exchange, and this appears to be undocumented
* the sample config doesn't take this into consideration, and ends up 
sometimes dropping through to later parts of the outer config part-way 
through the EAP exchange (but not every time)

In fact, it explicitly says:

         #  The EAP module returns "ok" if it is not yet ready to
         #  authenticate the user.  The configuration below checks for
         #  that code, and stops processing the "authorize" section if
         #  so.
         #
         #  Any LDAP and/or SQL servers will not be queried for the
         #  initial set of packets that go back and forth to set up
         #  TTLS or PEAP.

which means I think I was right to expect that eap { ok = return } would 
catch everything except the end of the exchange.

* therefore, the sample config could be improved to match the 
description (or the rlm_eap behaviour changed to return "ok", but that's 
a matter for you)

IMO freeradius's configurable failover mechanism is subtle at the best 
of times, so robust sample configs are especially helpful.

Regards,

Brian.