Hi,
Several comments:
1. There are two glitches, that got carried over from the previous
version of the patch:
a)
> - private void initServerSocket(ServerSocket ssocket) {
> + private void initServerSocket(ServerSocket ssocket) throws IOException {
There is no need to declare throwing an IOException here.
b)
> + * Checks that the cetificate is compatible with the enabled cipher
> suites.
s/cetificate/certificate/
2. I do not understand how serverSocket.accept() can succeed for an
unbound socket. It bugs me.
from ServerSocket#accept() of jdk 1.5.0_12:
if (!isBound())
throw new SocketException("Socket is not bound yet");
It seems that the specific implementation, SSLServerSocketImpl,
bypasses the check (overwrites the accept() method not calling
super and not reimplementing the check), but it looks more like
a bug in this specific JDK implementation than a design decision.
Also, in this operation the server port is checked through the
security manager for an "accept" permission. Some configurations
might need adjusting.
Best regards,
Konstantin Kolinko
2008/8/21 <[EMAIL PROTECTED]>:
> Author: markt
> Date: Wed Aug 20 16:20:42 2008
> New Revision: 687503
>
> URL: http://svn.apache.org/viewvc?rev=687503&view=rev
> Log:
> Improved fix for 45528 (invalid SSL config).
> It is a variation on the previous patch that:
> - does the check earlier
> - uses an unbound socket so there is no possibility of a client connection
> - uses the String manager for the error message
> Note: I gave up on the alterntaive javax.crypto.Cipher suggestion as the
> cipher names are different and there is no easy conversion.
>
> Modified:
> tomcat/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java
>
> tomcat/trunk/java/org/apache/tomcat/util/net/jsse/res/LocalStrings.properties
>
> Modified:
> tomcat/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java
> URL:
> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java?rev=687503&r1=687502&r2=687503&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java
> (original)
> +++ tomcat/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java
> Wed Aug 20 16:20:42 2008
> @@ -26,6 +26,7 @@
> import java.net.ServerSocket;
> import java.net.Socket;
> import java.net.SocketException;
> +import java.net.SocketTimeoutException;
> import java.security.KeyStore;
> import java.security.SecureRandom;
> import java.security.cert.CRL;
> @@ -428,6 +429,9 @@
> getEnabledCiphers(requestedCiphers,
> sslProxy.getSupportedCipherSuites());
>
> + // Check the SSL config is OK
> + checkConfig();
> +
> } catch(Exception e) {
> if( e instanceof IOException )
> throw (IOException)e;
> @@ -692,7 +696,7 @@
> * Configures the given SSL server socket with the requested cipher
> suites,
> * protocol versions, and need for client authentication
> */
> - private void initServerSocket(ServerSocket ssocket) {
> + private void initServerSocket(ServerSocket ssocket) throws IOException {
>
> SSLServerSocket socket = (SSLServerSocket) ssocket;
>
> @@ -709,4 +713,33 @@
> configureClientAuth(socket);
> }
>
> + /**
> + * Checks that the cetificate is compatible with the enabled cipher
> suites.
> + * If we don't check now, the JIoEndpoint can enter a nasty logging loop.
> + * See bug 45528.
> + */
> + private void checkConfig() throws IOException {
> + // Create an unbound server socket
> + ServerSocket socket = sslProxy.createServerSocket();
> + initServerSocket(socket);
> +
> + // Set the timeout to 1ms as all we care about is if it throws an
> + // exception on accept.
> + socket.setSoTimeout(1);
> + try {
> + socket.accept();
> + // Will never get here - no client can connect to an unbound port
> + } catch (SSLException ssle) {
> + // SSL configuration is invalid. Possibly cert doesn't match
> ciphers
> + IOException ioe = new IOException(sm.getString(
> + "jsse.invalid_ssl_conf", ssle.getMessage()));
> + ioe.initCause(ssle);
> + throw ioe;
> + } catch (SocketTimeoutException ste) {
> + // Expected if all is well - do nothing
> + } finally {
> + socket.close();
> + }
> +
> + }
> }
>
> Modified:
> tomcat/trunk/java/org/apache/tomcat/util/net/jsse/res/LocalStrings.properties
> URL:
> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/jsse/res/LocalStrings.properties?rev=687503&r1=687502&r2=687503&view=diff
> ==============================================================================
> ---
> tomcat/trunk/java/org/apache/tomcat/util/net/jsse/res/LocalStrings.properties
> (original)
> +++
> tomcat/trunk/java/org/apache/tomcat/util/net/jsse/res/LocalStrings.properties
> Wed Aug 20 16:20:42 2008
> @@ -15,3 +15,4 @@
>
> jsse.alias_no_key_entry=Alias name {0} does not identify a key entry
> jsse.keystore_load_failed=Failed to load keystore type {0} with path {1} due
> to {2}
> +jsse.invalid_ssl_conf=SSL configuration is invalid due to {0}
> \ No newline at end of file
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, e-mail: [EMAIL PROTECTED]
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
