[hunchentoot-devel] Hunchentoot effective DOS-attack

[Hans Hübner]

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:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> 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
>
> - --
> 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
>
> iEYEARECAAYFAkpRv0QACgkQ0brSZD05ZzDvgQCcDecaau+Hq35HtXoZkrg0EwRz
> cLAAnR6IgD6sE5PxkD6bs1Jf6wj0b+N7
> =eUDk
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> tbnl-devel site list
> tbnl-devel at common-lisp.net
> http://common-lisp.net/mailman/listinfo/tbnl-devel
>