Re: BUG #5687: RADIUS Authentication issues

On Tue, Oct 5, 2010 at 11:01, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> On Sun, Oct 3, 2010 at 18:30, Alan T DeKok <aland(at)freeradius(dot)org> wrote:
>> Tom Lane wrote:
>>> Hm ... seems to me that is a network security problem, not our problem.
>>> Who's to say one of the spoofed packets won't pass verification?
>>
>>  The packets are signed with a shared key.  Passing verification means
>> either the attacker knows the key, or the attacker has broken MD5 in
>> ways that are currently unknown.
>>
>>> If you want to change it, I won't stand in the way, but I have real
>>> doubts about both the credibility of this threat and the usefulness
>>> of the proposed fix.
>>
>>  The credibility of the threat is high.  Anyone can trivially send a
>> packet which will cause authentication to fail.  This is a DoS attack.
>
> I don't agree about how high it is - unless I misunderstand the
> wording. You still need to have unfiltered access to the network that
> the database server is on (unlikely) and you need to guess/bruteforce
> the port (using bruteforce not really hard, but likely to be detected
> by an IDS pretty quickly)
>
> It is definitely an opportunity for a DoS attack though, so it should be fixed.
>
> I find your suggested patches kind of hard to read posted inline that
> way - any chance you can repost as attachment or publish it as a git
> repository I can fetch from?

Actually, nevermind that one. Here's a patch I worked up from your
description, and that turns out to be fairly similar to yours in what
it does I think - except I'm not rearranging the code into a separate
function. We already have a while-loop.

See attached context diff, and I've also included a diff without
whitespace changes since the majority of the diff is otherwise coming
from indenting the code one tab...

(so far untested, I seem to have deleted my test-instance of the
radius server, but I figured I should post my attempt anyway)

Also, my patch does not change from log to warning - note that warning
is actually *below* log when it comes to the logfile (see
log_min_messages comments in postgresql.conf). I keep making that
mistake myself...

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/