[hunchentoot-devel] Hunchentoot effective DOS-attack

[Peter Stiernström]

-----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-----