[hunchentoot-devel] Hunchentoot effective DOS-attack

[Peter Stiernström]

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