On 2/28/2011 5:27 PM, Mark Thomas wrote:
On 01/03/2011 00:16, Filip Hanik - Dev Lists wrote:
On 2/28/2011 4:49 PM, Mark Thomas wrote:
It isn't clear to me if you are voting -1
on the above commit, and the following commits. r1074675
Understood and agree those commits are broken. I'll get those backed out
shortly.

If you wish to do this, it should at least include:
1. input filters need to check if they retrieved the entire body
if only partial, why even attempt a reneg and make your thread hang for
soTimeout while it fails. this is another DoS scenario. the system knows
if it read the entire body or not. it's part of the protocol itself, no
need to rely on timeouts for a reneg to fail.

2. don't change the names of all the flags, since it makes the diffs so
much harder to review. just change the lines pertinent to the change.

3. implement rehandshake as simple as possible, by using the
handshake(...) and using its return code

4. SSLAuthenticator should have a flag to fail directly without trying
to reneg if the connector is misconfigured to avoid reneg for clients
vulnerable to the man in the middle reneg attack

5. SSLAuthenticator should be able to find out if the cert truly was
client-auth or if it came from another source. otherwise, putting
httpd/mod_jk in front of it, and I can bypass client-auth as the
document states is required

6. And if you want the most performant solution, instead of opening a
selector on the same thread, just call sslEngine.beginHandshake, add the
connection to the poller, and return from the call all together. this
way, the worker thread is not in use during a handshake, and it's done
in the poller just like the initial hand shake. this protects you from
slow clients using up threads. this is of course more complicated, so I
would not expect it in the first iteration.

I would say the other connectors would benefit from improvements in
1,4,5 as well.
I agree on all of those points (with a few questions - see below). My
current thinking is approaching it in this order.

Do 2 in a separate commit. The flag needs to be renamed to ease
confusion but a separate commit that does just that should be easy to
review.
Yes, that would be much better.
Address 3 for the NIO connector. That will bring it in line with BIO and
APR.

Fix 1 for all connectors.

I don't understand what you mean in point 4. Could you try and expand on
that.
Sure, a renegotiation with a non updated client, IIRC would bring
CVE-2009-3555 SSL Man-In-The-Middle attack.
Hence, some sysadmins should have the configuration option to only allow the 
initial handshake.
Add in a flag that would say disableRenegotiation="true" (or similar).
Meaning, the only time the valve would work, is if the clientAuth="true" in the 
connector.

Fix 5. I may re-word the Javadoc again. Doing the client cert validation
in httpd is valid.
But how do you know it took place in httpd? Sounds like adding httpd/mod_jk in the mixture, Tomcat makes an assumption that client-auth took place.
6 is definitely more complicated. I did try this before but gave up.
That was before I had anything working. It may well be easier to get
there from a working solution.
I can help you here. But I'd like the simple solution first.
The reason the NIO connector doesn't use individual selectors, is that on some systems with high concurrency, having too many selectors made the system puke.

Filip

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



-----
No virus found in this message.
Checked by AVG - www.avg.com
Version: 10.0.1204 / Virus Database: 1435/3473 - Release Date: 02/28/11




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to