date:20110228

Author: markt
Date: Mon Feb 28 13:35:41 2011
New Revision: 1075320

URL: http://svn.apache.org/viewvc?rev=1075320view=rev
Log:
Improve fix for https://issues.apache.org/bugzilla/show_bug.cgi?id=48863
- consistently pass absolute paths to validateFile()
- handle non-absolute catalina home/base

Modified:
tomcat/trunk/java/org/apache/catalina/startup/ClassLoaderFactory.java

Modified: tomcat/trunk/java/org/apache/catalina/startup/ClassLoaderFactory.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/startup/ClassLoaderFactory.java?rev=1075320r1=1075319r2=1075320view=diff
==
--- tomcat/trunk/java/org/apache/catalina/startup/ClassLoaderFactory.java 
(original)
+++ tomcat/trunk/java/org/apache/catalina/startup/ClassLoaderFactory.java Mon 
Feb 28 13:35:41 2011
@@ -20,6 +20,7 @@ package org.apache.catalina.startup;
 
 
 import java.io.File;
+import java.io.IOException;
 import java.net.URL;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
@@ -187,6 +188,7 @@ public final class ClassLoaderFactory {
 set.add(url);
 } else if (repository.getType() == RepositoryType.GLOB) {
 File directory=new File(repository.getLocation());
+directory = new File(directory.getCanonicalPath());
 if (!validateFile(directory, RepositoryType.GLOB)) {
 continue;
 }
@@ -232,20 +234,21 @@ public final class ClassLoaderFactory {
 }
 
 private static boolean validateFile(File file,
-RepositoryType type) {
+RepositoryType type) throws IOException {
 if (RepositoryType.DIR == type || RepositoryType.GLOB == type) {
 if (!file.exists() || !file.isDirectory() || !file.canRead()) {
-String msg = Problem with directory [ +
-file.getAbsolutePath() + ], exists: [ +
-file.exists() + ], isDirectory: [ +
-file.isDirectory() + ], canRead: [ +
-file.canRead() + ];
+String msg = Problem with directory [ + file +
+], exists: [ + file.exists() +
+], isDirectory: [ + file.isDirectory() +
+], canRead: [ + file.canRead() + ];
 
-if (!Bootstrap.getCatalinaHome().equals(
-Bootstrap.getCatalinaBase()) 
-file.getAbsolutePath().startsWith(
-Bootstrap.getCatalinaBase())) {
-
+File home = new File (Bootstrap.getCatalinaHome());
+home = home.getCanonicalFile();
+File base = new File (Bootstrap.getCatalinaBase());
+base = base.getCanonicalFile();
+
+if (!home.getPath().equals(base.getPath()) 
+file.getPath().startsWith(base.getPath())) {
 log.debug(msg);
 } else {
 log.warn(msg);
@@ -254,10 +257,9 @@ public final class ClassLoaderFactory {
 }
 } else if (RepositoryType.JAR == type) {
 if (!file.exists() || !file.canRead()) {
-log.warn(Problem with JAR file [ +
-file.getAbsolutePath() + ], exists: [ +
-file.exists() + ], canRead: [ +
-file.canRead() + ]);
+log.warn(Problem with JAR file [ + file +
+], exists: [ + file.exists() +
+], canRead: [ + file.canRead() + ]);
 return false;
 }
 }



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

svn commit: r1075326 - /tomcat/tc6.0.x/trunk/STATUS.txt

Author: markt
Date: Mon Feb 28 13:51:09 2011
New Revision: 1075326

URL: http://svn.apache.org/viewvc?rev=1075326view=rev
Log:
Update patch

Modified:
tomcat/tc6.0.x/trunk/STATUS.txt

Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL: 
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=1075326r1=1075325r2=1075326view=diff
==
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Mon Feb 28 13:51:09 2011
@@ -122,6 +122,6 @@ PATCHES PROPOSED TO BACKPORT:
 * Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=48863
   Provide an warning if there is a problem with a class path entry but use 
debug
   level logging if it is expected due to catalina home/base split
-  http://people.apache.org/~markt/patches/2011-02-21-bug48863.patch
+  http://people.apache.org/~markt/patches/2011-02-28-bug48863-tc6.patch
   +1: markt
   -1:



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

DO NOT REPLY [Bug 48863] Log directory misconfiguration for class loaders

2011-02-28 Thread bugzilla

https://issues.apache.org/bugzilla/show_bug.cgi?id=48863

--- Comment #8 from Mark Thomas ma...@apache.org 2011-02-28 08:51:34 EST ---
Thanks for the review. Patch updated in 7.0.x for 7.0.9 and proposal for 6.0.x
also updated.

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.

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

svn commit: r1075337 - in /tomcat/tc7.0.x/tags/TOMCAT_7_0_9: ./ build.properties.default modules/

Author: markt
Date: Mon Feb 28 14:13:37 2011
New Revision: 1075337

URL: http://svn.apache.org/viewvc?rev=1075337view=rev
Log:
Tag 7.0.9

Added:
tomcat/tc7.0.x/tags/TOMCAT_7_0_9/   (props changed)
  - copied from r1075336, tomcat/trunk/
Removed:
tomcat/tc7.0.x/tags/TOMCAT_7_0_9/modules/
Modified:
tomcat/tc7.0.x/tags/TOMCAT_7_0_9/build.properties.default

Propchange: tomcat/tc7.0.x/tags/TOMCAT_7_0_9/
--
--- svn:ignore (added)
+++ svn:ignore Mon Feb 28 14:13:37 2011
@@ -0,0 +1,5 @@
+.*
+output
+build.properties
+work
+logs

Propchange: tomcat/tc7.0.x/tags/TOMCAT_7_0_9/
--
svn:mergeinfo = /tomcat/tc6.0.x/trunk:742915

Modified: tomcat/tc7.0.x/tags/TOMCAT_7_0_9/build.properties.default
URL: 
http://svn.apache.org/viewvc/tomcat/tc7.0.x/tags/TOMCAT_7_0_9/build.properties.default?rev=1075337r1=1075336r2=1075337view=diff
==
--- tomcat/tc7.0.x/tags/TOMCAT_7_0_9/build.properties.default (original)
+++ tomcat/tc7.0.x/tags/TOMCAT_7_0_9/build.properties.default Mon Feb 28 
14:13:37 2011
@@ -29,7 +29,7 @@ version.major=7
 version.minor=0
 version.build=9
 version.patch=0
-version.suffix=-dev
+version.suffix=
 
 # - Build control flags -
 # Note enabling validation uses Checkstyle which is LGPL licensed



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

svn commit: r1075338 - /tomcat/trunk/build.properties.default

Author: markt
Date: Mon Feb 28 14:14:41 2011
New Revision: 1075338

URL: http://svn.apache.org/viewvc?rev=1075338view=rev
Log:
Prep for next release

Modified:
tomcat/trunk/build.properties.default

Modified: tomcat/trunk/build.properties.default
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/build.properties.default?rev=1075338r1=1075337r2=1075338view=diff
==
--- tomcat/trunk/build.properties.default (original)
+++ tomcat/trunk/build.properties.default Mon Feb 28 14:14:41 2011
@@ -27,7 +27,7 @@
 # - Version Control Flags -
 version.major=7
 version.minor=0
-version.build=9
+version.build=10
 version.patch=0
 version.suffix=-dev
 



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

[VOTE] Release Apache Tomcat 7.0.9

The proposed Apache Tomcat 7.0.9 release is now available for voting.

It can be obtained from:
http://people.apache.org/~markt/dev/tomcat-7/v7.0.9/
The svn tag is:
http://svn.apache.org/repos/asf/tomcat/tc7.0.x/tags/TOMCAT_7_0_9/

The proposed 7.0.9 release is:

[ ] Broken - do not release
[ ] Alpha  - go ahead and release as 7.0.9 Alpha
[ ] Beta   - go ahead and release as 7.0.9 Beta
[ ] Stable - go ahead and release as 7.0.9 Stable

Cheers,

Mark

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

svn commit: r1075435 - /tomcat/trunk/java/org/apache/catalina/authenticator/SSLAuthenticator.java

Author: markt
Date: Mon Feb 28 17:24:00 2011
New Revision: 1075435

URL: http://svn.apache.org/viewvc?rev=1075435view=rev
Log:
Clarify auth process for CLIENT-CERT

Modified:
tomcat/trunk/java/org/apache/catalina/authenticator/SSLAuthenticator.java

Modified: 
tomcat/trunk/java/org/apache/catalina/authenticator/SSLAuthenticator.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/authenticator/SSLAuthenticator.java?rev=1075435r1=1075434r2=1075435view=diff
==
--- tomcat/trunk/java/org/apache/catalina/authenticator/SSLAuthenticator.java 
(original)
+++ tomcat/trunk/java/org/apache/catalina/authenticator/SSLAuthenticator.java 
Mon Feb 28 17:24:00 2011
@@ -70,8 +70,8 @@ public class SSLAuthenticator
 
 /**
  * Authenticate the user by checking for the existence of a certificate
- * chain, and optionally asking a trust manager to validate that we trust
- * this user.
+ * chain, validating it against the trust manager for the connector and 
then
+ * validating the user's identity against the configured Realm.
  *
  * @param request Request we are processing
  * @param response Response we are creating



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

Re: svn commit: r1074675 - in /tomcat/trunk: java/org/apache/coyote/http11/ java/org/apache/tomcat/util/net/ webapps/docs/

On 2/27/2011 4:30 AM, Mark Thomas wrote:

On 25/02/2011 20:16, Filip Hanik - Dev Lists wrote:

The simplest solution is, would be to use an individual selector.
Register the socket and issue a select() on the thread you are running on.
If you want to use a shared selector (like NIO does for reads and
writes) it requires a bit more logic.

I have implemented the simple solution and based on a quick test with
the Eclipse debugger the handshake now blocks while waiting for client data.

A review would be good since my understanding of NIO is not as good as
yours.
My initial recommendation is to pull out this change, and as default behavior, throw an exception if the SSLAuthenticator is trying to
authenticate and the need-client-auth is not configured.

There is much complexity in implementing the renegotiation without a unit test case, as there are both application buffers and network
buffers in the NIO implementation that will need to be tested more carefully.

So for the sake of not holding up releases, implement the exception case first, where you force the user to configure client authentication,
until there is a configuration that we are more comfortable with.

best
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

Re: [VOTE] Release Apache Tomcat 7.0.9


[X] Broken - do not release

http://svn.apache.org/viewvc?rev=1075030view=rev
http://svn.apache.org/viewvc?rev=1074675view=rev

Are not completely correct implementations as they don't
- support a timeout correctly
- dont account for when a write is needed on the selector
- they potentially garble a message body larger than the limit of the input 
buffer

http://markmail.org/message/42nme6x2fwfvygqk

Filip

On 2/28/2011 9:57 AM, Mark Thomas wrote:


The proposed Apache Tomcat 7.0.9 release is now available for voting.

It can be obtained from:
http://people.apache.org/~markt/dev/tomcat-7/v7.0.9/
The svn tag is:
http://svn.apache.org/repos/asf/tomcat/tc7.0.x/tags/TOMCAT_7_0_9/

The proposed 7.0.9 release is:

[ ] Broken - do not release
[ ] Alpha  - go ahead and release as 7.0.9 Alpha
[ ] Beta   - go ahead and release as 7.0.9 Beta
[ ] Stable - go ahead and release as 7.0.9 Stable

Cheers,

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

DO NOT REPLY [Bug 49284] Implement SSL renegotiation for the NIO connector

2011-02-28 Thread bugzilla

https://issues.apache.org/bugzilla/show_bug.cgi?id=49284

Filip Hanik fha...@apache.org changed:

   What|Removed |Added

 Status|RESOLVED|REOPENED
 Resolution|FIXED   |

--- Comment #2 from Filip Hanik fha...@apache.org 2011-02-28 13:12:42 EST ---
http://markmail.org/message/42nme6x2fwfvygqk
http://markmail.org/message/7wafq6imvjocajyn

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.

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

svn commit: r1075458 - in /tomcat/trunk: java/org/apache/catalina/ant/ webapps/docs/

Author: markt
Date: Mon Feb 28 18:15:48 2011
New Revision: 1075458

URL: http://svn.apache.org/viewvc?rev=1075458view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=28852
Add URL encoding where missing to parameters in URLs presented by Ant tasks to 
the Manager application.
Based on a patch by Stephane Bailliez.

Modified:
tomcat/trunk/java/org/apache/catalina/ant/JMXGetTask.java
tomcat/trunk/java/org/apache/catalina/ant/JMXQueryTask.java
tomcat/trunk/java/org/apache/catalina/ant/JMXSetTask.java
tomcat/trunk/java/org/apache/catalina/ant/ResourcesTask.java
tomcat/trunk/java/org/apache/catalina/ant/UndeployTask.java
tomcat/trunk/webapps/docs/changelog.xml

Modified: tomcat/trunk/java/org/apache/catalina/ant/JMXGetTask.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/ant/JMXGetTask.java?rev=1075458r1=1075457r2=1075458view=diff
==
--- tomcat/trunk/java/org/apache/catalina/ant/JMXGetTask.java (original)
+++ tomcat/trunk/java/org/apache/catalina/ant/JMXGetTask.java Mon Feb 28 
18:15:48 2011
@@ -19,6 +19,9 @@
 package org.apache.catalina.ant;
 
 
+import java.io.UnsupportedEncodingException;
+import java.net.URLEncoder;
+
 import org.apache.tools.ant.BuildException;
 
 
@@ -90,8 +93,13 @@ public class JMXGetTask extends Abstract
 (Must specify 'bean' and 'attribute' attributes);
 }
 log(Getting attribute  + attribute +
- in bean  + bean ); 
-execute(/jmxproxy/?get= + bean 
-+ att= + attribute );
+ in bean  + bean );
+try {
+execute(/jmxproxy/?get= + URLEncoder.encode(bean, getCharset()) 
++ att= + URLEncoder.encode(attribute, getCharset()));
+} catch (UnsupportedEncodingException e) {
+throw new BuildException
+(Invalid 'charset' attribute:  + getCharset());
+}
 }
 }

Modified: tomcat/trunk/java/org/apache/catalina/ant/JMXQueryTask.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/ant/JMXQueryTask.java?rev=1075458r1=1075457r2=1075458view=diff
==
--- tomcat/trunk/java/org/apache/catalina/ant/JMXQueryTask.java (original)
+++ tomcat/trunk/java/org/apache/catalina/ant/JMXQueryTask.java Mon Feb 28 
18:15:48 2011
@@ -19,6 +19,9 @@
 package org.apache.catalina.ant;
 
 
+import java.io.UnsupportedEncodingException;
+import java.net.URLEncoder;
+
 import org.apache.tools.ant.BuildException;
 
 
@@ -73,7 +76,17 @@ public class JMXQueryTask extends Abstra
 @Override
 public void execute() throws BuildException {
 super.execute();
-String queryString = (query == null) ? :(?qry=+query);
+String queryString;
+if (query == null) {
+queryString = ;
+} else {
+try {
+queryString = ?qry= + URLEncoder.encode(query, getCharset());
+} catch (UnsupportedEncodingException e) {
+throw new BuildException
+(Invalid 'charset' attribute:  + getCharset());
+}
+}
 log(Query string is  + queryString); 
 execute (/jmxproxy/ + queryString);
 }

Modified: tomcat/trunk/java/org/apache/catalina/ant/JMXSetTask.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/ant/JMXSetTask.java?rev=1075458r1=1075457r2=1075458view=diff
==
--- tomcat/trunk/java/org/apache/catalina/ant/JMXSetTask.java (original)
+++ tomcat/trunk/java/org/apache/catalina/ant/JMXSetTask.java Mon Feb 28 
18:15:48 2011
@@ -19,6 +19,9 @@
 package org.apache.catalina.ant;
 
 
+import java.io.UnsupportedEncodingException;
+import java.net.URLEncoder;
+
 import org.apache.tools.ant.BuildException;
 
 
@@ -113,8 +116,13 @@ public class JMXSetTask extends Abstract
 log(Setting attribute  + attribute +
  in bean  + bean +
  to  + value); 
-execute(/jmxproxy/?set= + bean 
-+ att= + attribute 
-+ val= + value);
+try {
+execute(/jmxproxy/?set= + URLEncoder.encode(bean, getCharset()) 
++ att= + URLEncoder.encode(attribute, getCharset()) 
++ val= + URLEncoder.encode(value, getCharset()));
+} catch (UnsupportedEncodingException e) {
+throw new BuildException
+(Invalid 'charset' attribute:  + getCharset());
+}
 }
 }

Modified: tomcat/trunk/java/org/apache/catalina/ant/ResourcesTask.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/ant/ResourcesTask.java?rev=1075458r1=1075457r2=1075458view=diff
==

svn commit: r1075462 - in /tomcat: tc5.5.x/trunk/STATUS.txt tc6.0.x/trunk/STATUS.txt

Author: markt
Date: Mon Feb 28 18:21:00 2011
New Revision: 1075462

URL: http://svn.apache.org/viewvc?rev=1075462view=rev
Log:
Proposal

Modified:
tomcat/tc5.5.x/trunk/STATUS.txt
tomcat/tc6.0.x/trunk/STATUS.txt

Modified: tomcat/tc5.5.x/trunk/STATUS.txt
URL: 
http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/STATUS.txt?rev=1075462r1=1075461r2=1075462view=diff
==
--- tomcat/tc5.5.x/trunk/STATUS.txt (original)
+++ tomcat/tc5.5.x/trunk/STATUS.txt Mon Feb 28 18:21:00 2011
@@ -52,3 +52,11 @@ PATCHES PROPOSED TO BACKPORT:
   http://svn.apache.org/viewvc?rev=1072042view=rev
   +1: markt, mturk, kkolinko
   -1:
+
+* Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=28852
+  Add URL encoding where missing to parameters in URLs presented by Ant tasks 
to
+  the Manager application.
+  Based on a patch by Stephane Bailliez.
+  http://svn.apache.org/viewvc?rev=1075458view=rev
+  +1: markt
+  -1:

Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL: 
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=1075462r1=1075461r2=1075462view=diff
==
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Mon Feb 28 18:21:00 2011
@@ -125,3 +125,11 @@ PATCHES PROPOSED TO BACKPORT:
   http://people.apache.org/~markt/patches/2011-02-28-bug48863-tc6.patch
   +1: markt
   -1:
+
+* Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=28852
+  Add URL encoding where missing to parameters in URLs presented by Ant tasks 
to
+  the Manager application.
+  Based on a patch by Stephane Bailliez.
+  http://svn.apache.org/viewvc?rev=1075458view=rev
+  +1: markt
+  -1:



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

DO NOT REPLY [Bug 28852] failonerror attribute on Ant tasks

2011-02-28 Thread bugzilla

https://issues.apache.org/bugzilla/show_bug.cgi?id=28852

Mark Thomas ma...@apache.org changed:

   What|Removed |Added

  Component|Webapps:Manager |Catalina
Version|4.1.30  |5.5.33
Product|Tomcat 4|Tomcat 5

--- Comment #4 from Mark Thomas ma...@apache.org 2011-02-28 13:21:19 EST ---
failonerror support and the CR issue had already been fixed.
I have added the URL encoding where it was missing.

This has been fixed in 7.0.x for 7.0.10 onwards and has been proposed for 6.0.x
and 5.5.x.

Changing the version to 5.5.x since that is the earliest supported version
affected by this issue.

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.

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

Re: [VOTE] Release Apache Tomcat 7.0.9

On 28/02/2011 18:11, Filip Hanik - Dev Lists wrote:
 [X] Broken - do not release
 
 http://svn.apache.org/viewvc?rev=1075030view=rev
 http://svn.apache.org/viewvc?rev=1074675view=rev
 
 Are not completely correct implementations as they don't
 - support a timeout correctly
That is an issue. Should be a relatively easy fix.

 - dont account for when a write is needed on the selector
I can't figure out what circumstances the handshake process would return
OP_WRITE since it shouldn't ever be waiting for some other process to
write data. Could you explain how this might happen?

 - they potentially garble a message body larger than the limit of the
 input buffer
This should be handled by the existing code although I didn't test it.
Should be easy to write a unit test to confirm what happens in this case.

Mark

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

[RESULT] [VOTE] Release Apache Tomcat 7.0.9

On 28/02/2011 18:11, Filip Hanik - Dev Lists wrote:
 [X] Broken - do not release

Cancelled. The lack of a timeout in the NIO SSL handshake is a show stopper.

Mark

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

Re: svn commit: r1074675 - in /tomcat/trunk: java/org/apache/coyote/http11/ java/org/apache/tomcat/util/net/ webapps/docs/

Just a little bit more on this.
I'm not seeing where SSLAuthenticator.java validates that the request came in
on a SSL connection, and what if the SSL cert came from mod_jk.
I'm not sure what the requirements for CERT authentication is, but if it is that the cert MUST be validated against a trust store, then this
valve, must make sure that the validation actually has taken place.

Filip

On 2/28/2011 11:06 AM, Filip Hanik - Dev Lists wrote:

On 2/27/2011 4:30 AM, Mark Thomas wrote:

On 25/02/2011 20:16, Filip Hanik - Dev Lists wrote:

I have implemented the simple solution and based on a quick test with
the Eclipse debugger the handshake now blocks while waiting for client data.

So for the sake of not holding up releases, implement the exception case first, where you force the user to configure client
authentication, until there is a configuration that we are more comfortable with.

best
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

-
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

svn commit: r1075474 - /tomcat/trunk/java/org/apache/tomcat/util/net/SecureNioChannel.java

Author: markt
Date: Mon Feb 28 18:43:44 2011
New Revision: 1075474

URL: http://svn.apache.org/viewvc?rev=1075474view=rev
Log:
Fix an Eclipse nag

Modified:
tomcat/trunk/java/org/apache/tomcat/util/net/SecureNioChannel.java

Modified: tomcat/trunk/java/org/apache/tomcat/util/net/SecureNioChannel.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/SecureNioChannel.java?rev=1075474r1=1075473r2=1075474view=diff
==
--- tomcat/trunk/java/org/apache/tomcat/util/net/SecureNioChannel.java 
(original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/SecureNioChannel.java Mon Feb 
28 18:43:44 2011
@@ -181,6 +181,7 @@ public class SecureNioChannel extends Ni
 //fall down to NEED_UNWRAP on the same call, will result 
in a 
 //BUFFER_UNDERFLOW if it needs data
 }
+//$FALL-THROUGH$
 case NEED_UNWRAP: {
 //perform the unwrap function
 handshake = handshakeUnwrap(read);



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

Re: [VOTE] Release Apache Tomcat 7.0.9

On 2/28/2011 11:30 AM, Mark Thomas wrote:

On 28/02/2011 18:11, Filip Hanik - Dev Lists wrote:

[X] Broken - do not release

http://svn.apache.org/viewvc?rev=1075030view=rev
http://svn.apache.org/viewvc?rev=1074675view=rev

Are not completely correct implementations as they don't
- support a timeout correctly

That is an issue. Should be a relatively easy fix.

I got this in my code base, I still recommend pulling out the change until all
issues are taken care of.

- dont account for when a write is needed on the selector

I can't figure out what circumstances the handshake process would return
OP_WRITE since it shouldn't ever be waiting for some other process to
write data. Could you explain how this might happen?
When you want to initiate a renegotiating from the server, you want to write data to the client notifying it of such. This happens when you
call beginHandshake.

beginHandshake generates the bytes needed to be sent to the client, so you must
send them.
So we make an attempt to send them. If the attempt to send them fails, meaning no bytes were accepted into the buffer, the handshake(...)
calls returns OP_WRITE to let you know that you can be signaled for when this needs to take place.
This is most unlikely to happen in a scenario like this, as there TCP buffers should have plenty of space in them and we should be able to
take the bytes. However, from a state machine perspective, you still need to account for the state.

The other mistake, is that there is an assumed OP_READ, even if the handshake
completes.

The change will require a bit more careful analysis to get right. Not to
mention, this code, as you may have noticed, is fragile.

- they potentially garble a message body larger than the limit of the
input buffer

This should be handled by the existing code although I didn't test it.
Should be easy to write a unit test to confirm what happens in this case.

Try to start sending a 25MB file upload together with your headers.

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

Re: [VOTE] Release Apache Tomcat 7.0.9

On 28/02/2011 18:45, Filip Hanik - Dev Lists wrote:
 On 2/28/2011 11:30 AM, Mark Thomas wrote:
 On 28/02/2011 18:11, Filip Hanik - Dev Lists wrote:
 [X] Broken - do not release

 http://svn.apache.org/viewvc?rev=1075030view=rev
 http://svn.apache.org/viewvc?rev=1074675view=rev

 Are not completely correct implementations as they don't
 - support a timeout correctly
 That is an issue. Should be a relatively easy fix.
 I got this in my code base,
Could you commit it then?

 I still recommend pulling out the change
 until all issues are taken care of.
We can easily comment out the rehandshake() call if the issues aren't
fixed by the time we decide to do the next release.

 - dont account for when a write is needed on the selector
 I can't figure out what circumstances the handshake process would return
 OP_WRITE since it shouldn't ever be waiting for some other process to
 write data. Could you explain how this might happen?
 When you want to initiate a renegotiating from the server, you want to
 write data to the client notifying it of such. This happens when you
 call beginHandshake.
 beginHandshake generates the bytes needed to be sent to the client, so
 you must send them.
 So we make an attempt to send them. If the attempt to send them fails,
 meaning no bytes were accepted into the buffer, the handshake(...) calls
 returns OP_WRITE to let you know that you can be signaled for when this
 needs to take place.
 This is most unlikely to happen in a scenario like this, as there TCP
 buffers should have plenty of space in them and we should be able to
 take the bytes. However, from a state machine perspective, you still
 need to account for the state.
So what is the right way to respond to an OP_WRITE? Select and wait for
the signal and then call handshakeWrap()?

 The other mistake, is that there is an assumed OP_READ, even if the
 handshake completes.
I don't see what you mean here. Could you clarify please.

 The change will require a bit more careful analysis to get right. Not to
 mention, this code, as you may have noticed, is fragile.
 
 - they potentially garble a message body larger than the limit of the
 input buffer
 This should be handled by the existing code although I didn't test it.
 Should be easy to write a unit test to confirm what happens in this case.
 
 Try to start sending a 25MB file upload together with your headers.

Does it need to be that big? I was planning on writing a unit test that
POSTed 20 bytes to a resource that required CLIENT-CERT and then testing
with an input buffer of 100 bytes, 20 bytes and 5 bytes (or similar
numbers).

Mark

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

svn commit: r1075487 - in /tomcat/trunk/test/org/apache: catalina/startup/ tomcat/util/net/

Author: markt
Date: Mon Feb 28 19:03:31 2011
New Revision: 1075487

URL: http://svn.apache.org/viewvc?rev=1075487view=rev
Log:
Should have moved the keys when I moved the SSL tests

Added:
tomcat/trunk/test/org/apache/tomcat/util/net/test-cert.pem
  - copied unchanged from r1075461, 
tomcat/trunk/test/org/apache/catalina/startup/test-cert.pem
tomcat/trunk/test/org/apache/tomcat/util/net/test-key.pem
  - copied unchanged from r1075461, 
tomcat/trunk/test/org/apache/catalina/startup/test-key.pem
tomcat/trunk/test/org/apache/tomcat/util/net/test.keystore
  - copied unchanged from r1075461, 
tomcat/trunk/test/org/apache/catalina/startup/test.keystore
Removed:
tomcat/trunk/test/org/apache/catalina/startup/test-cert.pem
tomcat/trunk/test/org/apache/catalina/startup/test-key.pem
tomcat/trunk/test/org/apache/catalina/startup/test.keystore
Modified:
tomcat/trunk/test/org/apache/tomcat/util/net/TesterSupport.java

Modified: tomcat/trunk/test/org/apache/tomcat/util/net/TesterSupport.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/util/net/TesterSupport.java?rev=1075487r1=1075486r2=1075487view=diff
==
--- tomcat/trunk/test/org/apache/tomcat/util/net/TesterSupport.java (original)
+++ tomcat/trunk/test/org/apache/tomcat/util/net/TesterSupport.java Mon Feb 28 
19:03:31 2011
@@ -79,16 +79,16 @@ public final class TesterSupport {
 if (protocol.indexOf(Apr) == -1) {
 tomcat.getConnector().setProperty(sslProtocol, tls);
 File keystoreFile = new File(
-test/org/apache/catalina/startup/test.keystore);
+test/org/apache/tomcat/util/net/test.keystore);
 tomcat.getConnector().setAttribute(keystoreFile,
 keystoreFile.getAbsolutePath());
 } else {
 File keystoreFile = new File(
-test/org/apache/catalina/startup/test-cert.pem);
+test/org/apache/tomcat/util/net/test-cert.pem);
 tomcat.getConnector().setAttribute(SSLCertificateFile,
 keystoreFile.getAbsolutePath());
 keystoreFile = new File(
-test/org/apache/catalina/startup/test-key.pem);
+test/org/apache/tomcat/util/net/test-key.pem);
 tomcat.getConnector().setAttribute(SSLCertificateKeyFile,
 keystoreFile.getAbsolutePath());
 }



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

Re: [VOTE] Release Apache Tomcat 7.0.9

On 2/28/2011 12:00 PM, Mark Thomas wrote:

On 28/02/2011 18:45, Filip Hanik - Dev Lists wrote:

On 2/28/2011 11:30 AM, Mark Thomas wrote:

On 28/02/2011 18:11, Filip Hanik - Dev Lists wrote:

[X] Broken - do not release

http://svn.apache.org/viewvc?rev=1075030view=rev
http://svn.apache.org/viewvc?rev=1074675view=rev

Are not completely correct implementations as they don't
- support a timeout correctly

That is an issue. Should be a relatively easy fix.

I got this in my code base,

Could you commit it then?

I still recommend pulling out the change
until all issues are taken care of.

We can easily comment out the rehandshake() call if the issues aren't
fixed by the time we decide to do the next release.

- dont account for when a write is needed on the selector

I can't figure out what circumstances the handshake process would return
OP_WRITE since it shouldn't ever be waiting for some other process to
write data. Could you explain how this might happen?

When you want to initiate a renegotiating from the server, you want to
write data to the client notifying it of such. This happens when you
call beginHandshake.
beginHandshake generates the bytes needed to be sent to the client, so
you must send them.
So we make an attempt to send them. If the attempt to send them fails,
meaning no bytes were accepted into the buffer, the handshake(...) calls
returns OP_WRITE to let you know that you can be signaled for when this
needs to take place.
This is most unlikely to happen in a scenario like this, as there TCP
buffers should have plenty of space in them and we should be able to
take the bytes. However, from a state machine perspective, you still
need to account for the state.

So what is the right way to respond to an OP_WRITE? Select and wait for
the signal and then call handshakeWrap()?
you don't call handshakeWrap or unwrap, the handshake(boolean,boolean) does everything for you. You simply call select() with OP_WRITE, and
when it returns from that, you call handshake(...) again, and it automagically does the state machine for you

The other mistake, is that there is an assumed OP_READ, even if the
handshake completes.

I don't see what you mean here. Could you clarify please.

handshake(true, true);
297 if (handshakeStatus == HandshakeStatus.NEED_UNWRAP) {
298 // Block until there is data to read from the client
299 Selector selector = null;

again, this is ignoring the state machine. What you need to do here, is check
the results of handshake(true,true), not if NEED_UNWRAP is there.

it should be (in pseudo code):
loop:
int status = handshake(...);
switch (status): {
case 0: handshake-completed; return;
case -1: something went wrong; throw exception;
default: select.select(status);
end loop

The change will require a bit more careful analysis to get right. Not to
mention, this code, as you may have noticed, is fragile.

- they potentially garble a message body larger than the limit of the
input buffer

This should be handled by the existing code although I didn't test it.
Should be easy to write a unit test to confirm what happens in this case.

Try to start sending a 25MB file upload together with your headers.

Does it need to be that big? I was planning on writing a unit test that
POSTed 20 bytes to a resource that required CLIENT-CERT and then testing
with an input buffer of 100 bytes, 20 bytes and 5 bytes (or similar
numbers).

It would have to be bigger than maxPostSize for the data to remain in the
buffers

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

Re: svn commit: r1074675 - in /tomcat/trunk: java/org/apache/coyote/http11/ java/org/apache/tomcat/util/net/ webapps/docs/


Ok, I've done some research. My conclusion is that I'm -1 for this change.
My suggestion on the other thread, that we should simply throw an exception if the server is not configured to handle client-auth in the 
connector still stands.


Let me explain why:

1. The idea behind this code was that if the client is using SSL, the SSL request must have been authenticated against the trust store, 
meaning that the initial handshake did this.

2. If the server was not configured for client auth [1], the current 
implementation wants to do a renegotiation.
3. The current implementation tries to swallow up as much body content as 
possible, until maxPostSize has been filled up.

This breaks in the following use case:
1. Client Opens TCP Session
2. Client Completes Initial Handshake
3. Client Sends HTTP Headers and Body of Size N, where NmaxPostSize

When the server in this case has read the HTTP headers, it's already too late to renegotiate, since the body is already underway. The only 
time we could renegotiate, would be after the request had been read in its entirety. (There is a short window where this could happen, if 
the client had sent a 100-Continue header before the body was even started, but I'm excluding this use case for now.)


Since we can't read the entire body without discarding it, we can't implement 
this as the way it has been. Hence the -1.

Here is the proposed solution:
1. When SSLAuthenticator starts up, it should inspect configuration of the connector to make sure there is at least one connector with 
clientAuth=true in it

 Note: You need to decide what to do if there is only an AJP connector, do you 
trust that the webserver did client-auth on your behalf?

2. If a request comes in, and the system is not configured for client-auth, all a server can do is send a 400 Bad Request (or Not 
Authorized) back.
Note: It should also send a Connection:close so that we can close the connection without having to read the body. (See Rainer's email about 
draining implications)


3. We should not implement server initiated renegotiation at this point in time, since we can't do it in a way it would allow for the system 
to handle the above use case.


4. We should not implement renegotiation period on the NIO connector at this time, since it would still risk a vulnerability with clients 
that are not updated.


In short, the NIO code can safely be backed out. The solution is quite simple, 
and it does not involve modifying the connectors.


[1]
setNeedClientAuth(true);
http://download.oracle.com/javase/6/docs/api/javax/net/ssl/SSLEngine.html#setNeedClientAuth(boolean)


best
Filip

On 2/25/2011 1:16 PM, Filip Hanik - Dev Lists wrote:

This looks like a CPU spinning handshake to me.
The operation handshake(true, true); returns an IO interest to be registered 
with a selector.
If the client is slow here or misbehaving, you could end up in a end less loop, and hence we can have introduced a very simple DoS 
vulnerability here.


The simplest solution is, would be to use an individual selector. Register the 
socket and issue a select() on the thread you are running on.
If you want to use a shared selector (like NIO does for reads and writes) it 
requires a bit more logic.

I understand where you are going with this solution, and I can probably fix 
this today as I sit on the airplane on my way home.

best
Filip



On 02/25/2011 12:19 PM, ma...@apache.org wrote:

Author: markt
Date: Fri Feb 25 19:19:13 2011
New Revision: 1074675

URL: http://svn.apache.org/viewvc?rev=1074675view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=49284
Support SSL re-negotiation in the HTTP NIO connector
There is a fair amount of renaming in this patch. The real work is in the new 
rehandshake() method in the SecureNioChannel.

Modified:
 tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java
 tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties
 tomcat/trunk/java/org/apache/tomcat/util/net/NioChannel.java
 tomcat/trunk/java/org/apache/tomcat/util/net/SecureNioChannel.java
 tomcat/trunk/webapps/docs/changelog.xml

Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java?rev=1074675r1=1074674r2=1074675view=diff

==
--- tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java 
(original)
+++ tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java Fri Feb 
25 19:19:13 2011
@@ -23,6 +23,8 @@ import java.nio.channels.SelectionKey;
  import java.util.Locale;
  import java.util.concurrent.Executor;

+import javax.net.ssl.SSLEngine;
+
  import org.apache.coyote.ActionCode;
  import org.apache.coyote.Request;
  import org.apache.coyote.RequestInfo;
@@ -42,7 +44,9 @@ import org.apache.tomcat.util.net.NioCha
  import org.apache.tomcat.util.net.NioEndpoint;

Re: svn commit: r1074675 - in /tomcat/trunk: java/org/apache/coyote/http11/ java/org/apache/tomcat/util/net/ webapps/docs/

On 28/02/2011 22:50, Filip Hanik - Dev Lists wrote:
Ok, I've done some research. My conclusion is that I'm -1 for this change.
My suggestion on the other thread, that we should simply throw an
exception if the server is not configured to handle client-auth in the
connector still stands.

Let me explain why:

1. The idea behind this code was that if the client is using SSL, the
SSL request must have been authenticated against the trust store,
meaning that the initial handshake did this.
2. If the server was not configured for client auth [1], the current
implementation wants to do a renegotiation.
3. The current implementation tries to swallow up as much body content
as possible, until maxPostSize has been filled up.

This breaks in the following use case:
1. Client Opens TCP Session
2. Client Completes Initial Handshake
3. Client Sends HTTP Headers and Body of Size N, where NmaxPostSize

When the server in this case has read the HTTP headers, it's already too
late to renegotiate, since the body is already underway. The only time
we could renegotiate, would be after the request had been read in its
entirety. (There is a short window where this could happen, if the
client had sent a 100-Continue header before the body was even started,
but I'm excluding this use case for now.)

Since we can't read the entire body without discarding it, we can't
implement this as the way it has been. Hence the -1.

It isn't clear to me if you are voting -1 on the recent NIO change or on
how the BIO and APR connectors have implemented SSL renegotiation for
years. It is an accepted limitation of SSL renegotiation that you need
to buffer any request body and if the request body is too large
renegotiation will fail. httpd does this exactly the same way. [1]

The rare user for which the above is an issue has several workarounds
available:
1. Increase maxSavePostSize (also increases risk of DOS but may be
acceptable in some circumstances).
2. Set clientAuth=true on the connector as you suggest below.

Here is the proposed solution:
1. When SSLAuthenticator starts up, it should inspect configuration of
the connector to make sure there is at least one connector with
clientAuth=true in it
Note: You need to decide what to do if there is only an AJP connector,
do you trust that the webserver did client-auth on your behalf?

2. If a request comes in, and the system is not configured for
client-auth, all a server can do is send a 400 Bad Request (or Not
Authorized) back.
Note: It should also send a Connection:close so that we can close the
connection without having to read the body. (See Rainer's email about
draining implications)

3. We should not implement server initiated renegotiation at this point
in time, since we can't do it in a way it would allow for the system to
handle the above use case.

4. We should not implement renegotiation period on the NIO connector at
this time, since it would still risk a vulnerability with clients that
are not updated.

Such an approach would be a step backwards from what has been available
for BIO as far back as Tomcat 4 [2] and has been available for APR in
6.0.21 and 5.5.29 [3].

If the issue is that the current NIO implementation is broken, I agree
but I think it is fixable within the same limitations of the current
renegotiation support for BIO and APR. Given that the current patch is
heading in the right direction (hopefully) and that a fix shouldn't be
too hard, I'd rather leave it in and fix it rather than pull it out.
There is no huge rush at the moment for a 7.0.x release but I'd like to
get the 7.0.10 started by the end of this week. If NIO renegotiation
gets in the way then it can be reverted / commented out for now but I am
hopeful of getting this to a working state this week. With the pointers
you have provided on the current code, I think I can see how to get to a
working (in the same way as BIO APR) solution. It is actually taking
longer to get the test cases working.

If the issue is that you are -1 for the approach of buffering the
request body prior to SSL renegotiation then I think that needs a wider
discussion before any changes are made since that would involving
removing functionality from the current connectors that is currently
being used successfully by the user community.

Mark

[1] http://httpd.apache.org/docs/2.2/mod/mod_ssl.html#sslrenegbuffersize
[2]
http://svn.apache.org/viewvc/tomcat/archive/tc4.1.x/trunk/connectors/http11/src/java/org/apache/coyote/http11/Http11Processor.java?view=annotate
(lines 1118 to 1138)
[3] https://issues.apache.org/bugzilla/show_bug.cgi?id=46950

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

Re: svn commit: r1074675 - in /tomcat/trunk: java/org/apache/coyote/http11/ java/org/apache/tomcat/util/net/ webapps/docs/


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

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.

Filip



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

Re: svn commit: r1074675 - in /tomcat/trunk: java/org/apache/coyote/http11/ java/org/apache/tomcat/util/net/ webapps/docs/

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.

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.

Fix 5. I may re-word the Javadoc again. Doing the client cert validation
in httpd is valid.

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.

Mark

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

svn commit: r1075604 - in /tomcat/trunk/test/org/apache/tomcat/util/net: TestCustomSsl.java TestSsl.java TesterSupport.java ca.jks localhost-cert.pem localhost-key.pem localhost.jks test-cert.pem test

Author: markt
Date: Tue Mar  1 01:15:11 2011
New Revision: 1075604

URL: http://svn.apache.org/viewvc?rev=1075604view=rev
Log:
Start of an SSL re-negotiation test. Need to expand it to handle request bodies 
and the other issues highlighted by Filip.
Switch to using a CA since it makes the code cleaner and it is easier to get 
CLIENT-CERT working than will all self-signed certs.

Added:
tomcat/trunk/test/org/apache/tomcat/util/net/ca.jks   (with props)
tomcat/trunk/test/org/apache/tomcat/util/net/localhost-cert.pem
tomcat/trunk/test/org/apache/tomcat/util/net/localhost-key.pem
tomcat/trunk/test/org/apache/tomcat/util/net/localhost.jks   (with props)
tomcat/trunk/test/org/apache/tomcat/util/net/user1.jks   (with props)
Removed:
tomcat/trunk/test/org/apache/tomcat/util/net/test-cert.pem
tomcat/trunk/test/org/apache/tomcat/util/net/test-key.pem
tomcat/trunk/test/org/apache/tomcat/util/net/test.keystore
Modified:
tomcat/trunk/test/org/apache/tomcat/util/net/TestCustomSsl.java
tomcat/trunk/test/org/apache/tomcat/util/net/TestSsl.java
tomcat/trunk/test/org/apache/tomcat/util/net/TesterSupport.java

Modified: tomcat/trunk/test/org/apache/tomcat/util/net/TestCustomSsl.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/util/net/TestCustomSsl.java?rev=1075604r1=1075603r2=1075604view=diff
==
--- tomcat/trunk/test/org/apache/tomcat/util/net/TestCustomSsl.java (original)
+++ tomcat/trunk/test/org/apache/tomcat/util/net/TestCustomSsl.java Tue Mar  1 
01:15:11 2011
@@ -40,7 +40,7 @@ public class TestCustomSsl extends Tomca
 
 try {
 SSLContext sc = SSLContext.getInstance(SSL);
-sc.init(null, TesterSupport.TRUST_ALL_CERTS,
+sc.init(null, TesterSupport.getTrustManagers(),
 new java.security.SecureRandom());
 javax.net.ssl.HttpsURLConnection.setDefaultSSLSocketFactory(
 sc.getSocketFactory());

Modified: tomcat/trunk/test/org/apache/tomcat/util/net/TestSsl.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/util/net/TestSsl.java?rev=1075604r1=1075603r2=1075604view=diff
==
--- tomcat/trunk/test/org/apache/tomcat/util/net/TestSsl.java (original)
+++ tomcat/trunk/test/org/apache/tomcat/util/net/TestSsl.java Tue Mar  1 
01:15:11 2011
@@ -26,7 +26,17 @@ import javax.net.ssl.HandshakeCompletedL
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLSocket;
 import javax.net.ssl.SSLSocketFactory;
-
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.apache.catalina.Context;
+import org.apache.catalina.authenticator.SSLAuthenticator;
+import org.apache.catalina.deploy.LoginConfig;
+import org.apache.catalina.deploy.SecurityCollection;
+import org.apache.catalina.deploy.SecurityConstraint;
+import org.apache.catalina.startup.TestTomcat.MapRealm;
 import org.apache.catalina.startup.Tomcat;
 import org.apache.catalina.startup.TomcatBaseTest;
 import org.apache.tomcat.util.buf.ByteChunk;
@@ -40,18 +50,7 @@ import org.apache.tomcat.util.buf.ByteCh
 public class TestSsl extends TomcatBaseTest {
 
 public void testSimpleSsl() throws Exception {
-// Install the all-trusting trust manager so https:// works 
-// with unsigned certs. 
-
-try {
-SSLContext sc = SSLContext.getInstance(SSL);
-sc.init(null, TesterSupport.TRUST_ALL_CERTS,
-new java.security.SecureRandom());
-javax.net.ssl.HttpsURLConnection.setDefaultSSLSocketFactory(
-sc.getSocketFactory());
-} catch (Exception e) {
-e.printStackTrace();
-} 
+configureClientSsl();
 
 Tomcat tomcat = getTomcatInstance();
 
@@ -88,7 +87,7 @@ public class TestSsl extends TomcatBaseT
 
 tomcat.start();
 SSLContext sslCtx = SSLContext.getInstance(TLS);
-sslCtx.init(null, TesterSupport.TRUST_ALL_CERTS,
+sslCtx.init(null, TesterSupport.getTrustManagers(),
 new java.security.SecureRandom());
 SSLSocketFactory socketFactory = sslCtx.getSocketFactory();
 SSLSocket socket = (SSLSocket) socketFactory.createSocket(localhost, 
getPort());
@@ -163,7 +162,8 @@ public class TestSsl extends TomcatBaseT
 }
 
 SSLContext sslCtx = SSLContext.getInstance(TLS);
-sslCtx.init(null, TesterSupport.TRUST_ALL_CERTS, new 
java.security.SecureRandom());
+sslCtx.init(null, TesterSupport.getTrustManagers(),
+new java.security.SecureRandom());
 SSLSocketFactory socketFactory = sslCtx.getSocketFactory();
 SSLSocket socket = (SSLSocket)

svn commit: r1075606 - in /tomcat/trunk: java/org/apache/coyote/http11/ java/org/apache/tomcat/util/net/ webapps/docs/