[hunchentoot-devel] Hunchentoot effective DOS-attack

[Hans Hübner]

Peter, as I cannot reproduce the problem myself, can you please post a
stack backtrace?  Thanks!

2009/7/6 Peter Stiernström <peter.stiernstrom at blixtvik.se>:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Hans,
>
> I should have realised I was editing the lispworks implementation myself
> had I only had a cup of coffee this morning :-P
>
> Now when I tried your patch out I still get hangups but this is another
> kind of hangup:
>
> debugger invoked on a UNBOUND-VARIABLE in thread #<THREAD "Hunchentoot
> listener (*:8000)" RUNNING {AD467D1}>:
>  The variable HUNCHENTOOT:*ACCEPTOR* is unbound.
>
> I am trying this out with the hunchentoot default page by the way.
>
> /Peter
>
> Hans Hübner wrote:
>> Peter,
>>
>> sorry, I am not able to reproduce the problem myself, so I made the
>> patch blindly and made a mistake, working on the Lispworks code rather
>> than the portable code that affects you.  Please look at the first
>> hunk of this diff:
>>
>> http://bknr.net/trac/changeset/4430?format=diff&new=4430
>>
>> and let me know if that does it for you.
>>
>> Sorry,
>> Hans
>>
>> On Mon, Jul 6, 2009 at 11:09, Peter
>> Stiernström<peter.stiernstrom at blixtvik.se> wrote:
>> Hi Hans,
>>
>> Hans Hübner wrote:
>>>>> Hi Peter,
>>>>>
>>>>> 2009/7/6 Peter Stiernström <peter.stiernstrom at blixtvik.se>
>>>>>> I had a quick peek at client-as-string and it does indeed not handle the
>>>>>> sb-bsd-sockets:not-connected-error and if I put a handler-case around
>>>>>> body of client-as-string returning nil when the exception appears I can
>>>>>> not reproduce my problem anymore.
>>>>>>
>>>>>> This wouldn't provide a portable solution though :-P
>>>>> This really is a bug that requires fixing in usocket.  There are
>>>>> multiple issues:
>>>>>
>>>>> - As shown in the backtrace, the problem is that the getpeername()
>>>>> call fails for a socket that is not connected anymore.  SBCL does the
>>>>> right thing by signalling a condition.  Other implementations (I
>>>>> checked CCL) seem to behave differently, i.e. return NIL in such a
>>>>> case.
>>>>>
>>>>> - Usocket does not unify the behavior, so the implementation specific
>>>>> condition percolates to the caller, Hunchentoot in this case.
>>>>>
>>>>> - Hunchentoot is not prepared to handle conditions when creating a new
>>>>> worker thread.  The call to CLIENT-AS-STRING is made when creating the
>>>>> name for the handler process of a new incoming connection, and that is
>>>>> done in the context of the server thread.  Therefore, the signalled
>>>>> condition will stop the acceptor process.
>>>>>
>>>>> I spent some thoughts on how a "portable solution" would look like,
>>>>> but given that the Lisp implementations vary greatly in behavior,
>>>>> fixing usocket would be a pretty large task that I don't have the time
>>>>> to do right now.  Thus, I would propose this patch:
>>>>>
>>>>> http://bknr.net/trac/changeset/4428?format=diff&new=4428
>>>>>
>>>>> It handles all conditions that are signaled during worker process
>>>>> creation, under the theory that we want to prevent the acceptor
>>>>> process from crashing under all circumstances.  This is not a clean
>>>>> solution in that it may paper over bugs, but given the limited number
>>>>> of function invocations that are surrounded by a HANDLER-CASE, I'd say
>>>>> that this is a proper intermediate fix.
>>>>>
>>>>> Please give it a try and let me know if it solves the problem for you.
>> I tried your suggested patch but I can't seem to be able to get it to
>> catch the exception for me. Were you able to duplicate the error
>> yourself to verify that the suggested patch does indeed work? Since I am
>> using the 1.0.0 hunchentoot release I couldn't just apply your patch but
>> added the handler-case by hand and thus ended up with this in
>> taskmaster.lisp:
>>
>> (defmethod handle-incoming-connection ((taskmaster
>> one-thread-per-connection-taskmaster) handle)
>>  (incf *worker-counter*)
>>  ;; check if we need to perform a global GC
>>  (when (and *cleanup-interval*
>>             (zerop (mod *worker-counter* *cleanup-interval*)))
>>    (when *cleanup-function*
>>      (funcall *cleanup-function*)))
>>  (handler-case
>>      (mp:process-run-function (format nil "Hunchentoot worker \(client:
>> ~{~A:~A~})"
>>                                       (multiple-value-list
>>                                        (get-peer-address-and-port handle)))
>>                               nil #'process-connection
>>                               (taskmaster-acceptor taskmaster) handle)
>>    (error (cond)
>>      (log-message *lisp-errors-log-level*
>>                   "Error while creating worker thread for new incoming
>> connection: ~A" cond))))
>>
>> /Peter
>>
>>
>>>>> -Hans
>>>>>
>>>>> _______________________________________________
>>>>> tbnl-devel site list
>>>>> tbnl-devel at common-lisp.net
>>>>> http://common-lisp.net/mailman/listinfo/tbnl-devel
>>>
> _______________________________________________
> tbnl-devel site list
> tbnl-devel at common-lisp.net
> http://common-lisp.net/mailman/listinfo/tbnl-devel
>>>
>
>> _______________________________________________
>> tbnl-devel site list
>> tbnl-devel at common-lisp.net
>> http://common-lisp.net/mailman/listinfo/tbnl-devel
>
> - --
> Med vänlig hälsning,
>
> Peter Stiernström
> 0708-810932
> Blixtvik AB
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
>
> iEYEARECAAYFAkpR114ACgkQ0brSZD05ZzAp6ACdH3j38ycjcZqpjJiTdoZUubHn
> JQUAoJipg3faXED9AO51Y4LlKvlPQZLE
> =Vai6
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> tbnl-devel site list
> tbnl-devel at common-lisp.net
> http://common-lisp.net/mailman/listinfo/tbnl-devel
>