random SSL related stuff

[Roland Weber <ht...@dubioso.net>]

Hi folks,

while hacking up a patch for HTTPCLIENT-475, I stumbled about
a few details. These are not urgent, but I'd like to write them
down now before I forget something.

1) HostnameVerifier
We have an interface with that name, and standard Java has one
too in javax.net.ssl. Both define methods called verify() that
check whether a hostname is acceptable with respect to presented
certificates and such. Although they all share the same name,
the standard Java method returns a boolean, while ours throw an
exception if the verification fails.
This is BAD. As in REALLY bad. Somebody gets used to the exception
being thrown, then someday calls the standard Java method without
noticing the difference, and a verification failure will just go
unnoticed!
Now I don't mind throwing an exception instead of returning a
boolean. It allows for an error message that tells about the
exact reason for the verification failure. But giving these methods
the same name as the original method with a completely different
usage pattern is not good. We must find a different name to point
out the different usage pattern. Maybe check(...) or assert(...)
or whatever, but not verify(...).

2) SocketFactory
We have an interface with that name, and standard Java since 1.4
has one too in javax.net. The PlainSocketFactory implementation
I modified for HTTPCLIENT-475 uses java.net directly, without the
factory. The SSLSocketFactory uses the standard SSLSocketFactory.
We could/should provide a non-SSL socket factory that uses the
standard SocketFactory interface. I'm not sure whether this should
be done directly in PlainSocketFactory, or whether we should leave
the plain one as it is and have another one.

3) SecureSocketFactory
The interface SecureSocketFactory is meant to create SSL socket
connections. These are considered to be secure, while connections
created directly from SocketFactory are considered insecure. I see
two problems with this tie-in, where the second one is the bigger:
- A SocketFactory might create secure connections by using for
  example native code. Since native code would not be able to
  layer a connection on top of a Java socket, the factory could
  not (fully) implement the SecureSocketFactory interface.
- A SecureSocketFactory might create SSL connections that should
  not be considered secure, because security depends on the cipher
  suite that is chosen.
I would prefer to untie those two aspects and introduce a method
to check/indicate whether a specific connection is secure or not.
The special thing about SecureSocketFactory is that it can layer
a connection on top of an existing one. Renaming the interface to
LayeredSocketFactory would be an option.


cheers,
  Roland


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

Re: random SSL related stuff

[Roland Weber <ht...@dubioso.net>]

Hi Julius,

> One note, though:  our HostnameVerifier interface *does* subclass
> javax.net.ssl.HostnameVerifier, so that fact probably alleviates your
> concerns somewhat as well.  People can use the boolean method provided if
> they really want to.

No, that actually gave rise to those concerns. If *different*
interfaces use a different style, there is less concern of
people mixing up the styles. Calling an exception-throwing
method and checking it like a boolean will generate a compiler
error. But calling a boolean-returning method and not checking
it because you think it is exception-throwing will not trigger
any warnings. Hence bringing those styles together in the same
interface using the same name means asking for trouble.

> I'll rename the methods as part of HTTPCLIENT-617 when I get around to that
> patch.  (Is it okay to address style concerns like this in an existing bug,
> or should a new bug be created?).

Thanks. I don't have a problem with addressing this in HC-617.
If I felt that an extra issue was needed, I would have created one :-)

cheers (and a happy new year),
  Roland


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

Re: random SSL related stuff

[Julius Davies <ju...@gmail.com>]

Hi,

I agree with Roland about #1.  A renaming of the method makes sense there.
I like the sound of "checkHostname()".

One note, though:  our HostnameVerifier interface *does* subclass
javax.net.ssl.HostnameVerifier, so that fact probably alleviates your
concerns somewhat as well.  People can use the boolean method provided if
they really want to.

I'll rename the methods as part of HTTPCLIENT-617 when I get around to that
patch.  (Is it okay to address style concerns like this in an existing bug,
or should a new bug be created?).

Thanks, Roland, for taking a look, and commenting!  I was pleased to see the
JUnit tests I created catching you (so soon after writing them!)!  :-)   I
looked at your patch for HTTPCLIENT-475 and I thought it looked really good.


yours,

Julius



1) HostnameVerifier
> We have an interface with that name, and standard Java has one
> too in javax.net.ssl. Both define methods called verify() that
> check whether a hostname is acceptable with respect to presented
> certificates and such. Although they all share the same name,
> the standard Java method returns a boolean, while ours throw an
> exception if the verification fails.
> This is BAD. As in REALLY bad. Somebody gets used to the exception
> being thrown, then someday calls the standard Java method without
> noticing the difference, and a verification failure will just go
> unnoticed!
> Now I don't mind throwing an exception instead of returning a
> boolean. It allows for an error message that tells about the
> exact reason for the verification failure. But giving these methods
> the same name as the original method with a completely different
> usage pattern is not good. We must find a different name to point
> out the different usage pattern. Maybe check(...) or assert(...)
> or whatever, but not verify(...).
>



-- 
yours,

Julius Davies
416-652-0183
http://juliusdavies.ca/