Re: BUG #5687: RADIUS Authentication issues

On Tue, Oct 5, 2010 at 15:18, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> 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)

ok, I've run the patch through my tests of both valid and invalid
packets, and it seems to work the correct way now. Thus, applied.

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