Mailing-List: contact dev-help@tomcat.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Tomcat Developers List" <dev@tomcat.apache.org>
Received-SPF: neutral (athena.apache.org: local policy)
Message-ID: <48AD23F8.2080309@apache.org>
Date: Thu, 21 Aug 2008 09:14:48 +0100
From: Mark Thomas <markt@apache.org>
User-Agent: Thunderbird 2.0.0.16 (Windows/20080708)
MIME-Version: 1.0
To: Tomcat Developers List <dev@tomcat.apache.org>
Subject: Re: svn commit: r687503 - in
 /tomcat/trunk/java/org/apache/tomcat/util/net/jsse:
 JSSESocketFactory.java res/LocalStrings.properties
References: <20080820232042.AB85223889BA@eris.apache.org>
 <427155180808201822s3385654kc30b63e631cd723e@mail.gmail.com>
In-Reply-To: <427155180808201822s3385654kc30b63e631cd723e@mail.gmail.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

Konstantin Kolinko wrote:
> 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.
Thanks for the catch. I missed that whilst I was moving stuff around.

> b)
>> +     * Checks that the cetificate is compatible with the enabled cipher suites.
> 
> s/cetificate/certificate/
Fixed.

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

It bypasses the isClosed() test too. If these tests were added, the result
would be an IOException.

As long as the cipher check happens first it will be fine. Thinking about
it logically, the cipher check has to be first as the socket can't start
accepting connections if the SSL config is invalid. Therefore, I am happy
relying on the cipher check happening before any other check.

I beginning to think that the code should catch SSLException forthe case
where the config is bad and Exception for eveything else.

That has the advantage that *if* for some reason checks were done before
the cipher check we would revert to the previous behaviour.

> Also, in this operation the server port is checked through the
> security manager for an "accept" permission. Some configurations
> might need adjusting.

And there is another reason to catch Exception.

Mark


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