Race condition in XMPPConnection constructor

Hi,

I’‘ve discovered yet another severe race condition in Smack. I’'ve enabled TLS connections in the library, but used an expired certificate and enabled strict checking.

Of course, this causes an exception:

javax.net.ssl.SSLHandshakeException: java.security.cert.CertificateException: target verification failed of jabber.monitzer.com, OU=Unknown, O=Andreas Monitzer, L=Vienna, ST=Vienna, C=AT

at com.sun.net.ssl.internal.ssl.Alerts.getSSLException(Alerts.java:150)

at com.sun.net.ssl.internal.ssl.SSLSocketImpl.fatal(SSLSocketImpl.java:1476)

at com.sun.net.ssl.internal.ssl.Handshaker.fatalSE(Handshaker.java:174)

at com.sun.net.ssl.internal.ssl.Handshaker.fatalSE(Handshaker.java:168)

at com.sun.net.ssl.internal.ssl.ClientHandshaker.serverCertificate(ClientHandshake r.java:847)

at com.sun.net.ssl.internal.ssl.ClientHandshaker.processMessage(ClientHandshaker.j ava:106)

at com.sun.net.ssl.internal.ssl.Handshaker.processLoop(Handshaker.java:495)

at com.sun.net.ssl.internal.ssl.Handshaker.process_record(Handshaker.java:433)

at com.sun.net.ssl.internal.ssl.SSLSocketImpl.readRecord(SSLSocketImpl.java:815)

at com.sun.net.ssl.internal.ssl.SSLSocketImpl.performInitialHandshake(SSLSocketImp l.java:1025)

at com.sun.net.ssl.internal.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:10 38)

at org.jivesoftware.smack.XMPPConnection.proceedTLSReceived(XMPPConnection.java:11 21)

at org.jivesoftware.smack.PacketReader.parsePackets(PacketReader.java:327)

at org.jivesoftware.smack.PacketReader.access$000(PacketReader.java:43)

at org.jivesoftware.smack.PacketReader$1.run(PacketReader.java:63)

Caused by: java.security.cert.CertificateException: target verification failed of jabber.monitzer.com, OU=Unknown, O=Andreas Monitzer, L=Vienna, ST=Vienna, C=AT

at org.jivesoftware.smack.ServerTrustManager.checkServerTrusted(ServerTrustManager .java:143)

at com.sun.net.ssl.internal.ssl.JsseX509TrustManager.checkServerTrusted(SSLContext Impl.java:320)

at com.sun.net.ssl.internal.ssl.ClientHandshaker.serverCertificate(ClientHandshake r.java:840)

… 10 more

/code

This would be fine, but there’‘s is no[/b] way to catch that exception in my code. The connection listener would get notice of it, but since this happpens in the constructor of XMPPConnection, there’'s no way I could add one before that.

Here’‘s my patch for it (unfortunately, this can’'t be fixed without an API change):

diff -rU5 …/smack-dev-2.2.1/source/org/jivesoftware/smack/XMPPConnection.java ./source/org/jivesoftware/smack/XMPPConnection.java

— …/smack-dev-2.2.1/source/org/jivesoftware/smack/XMPPConnection.java 2006-06-12 23:13:15.000000000 +0200

+++ ./source/org/jivesoftware/smack/XMPPConnection.java 2006-07-11 22:42:34.000000000 +0200

@@ -256,13 +256,25 @@

public XMPPConnection(ConnectionConfiguration config, SocketFactory socketFactory)

throws XMPPException {

// Set the new connection configuration

connectUsingConfiguration(config, socketFactory);

}

  • public XMPPConnection(ConnectionConfiguration config, ConnectionListener connectionListener)

  •    throws XMPPException {
    
  •    // Set the new connection configuration
    
  •    connectUsingConfiguration(config, null, connectionListener);
    
  • }

private void connectUsingConfiguration(ConnectionConfiguration config,

  •        SocketFactory socketFactory) throws XMPPException {
    
  •                                       SocketFactory socketFactory) throws XMPPException {
    
  •    connectUsingConfiguration(config,socketFactory,null);
    
  • }

  • private void connectUsingConfiguration(ConnectionConfiguration config,

  •        SocketFactory socketFactory, ConnectionListener connectionListener) throws XMPPException {
    

this.host = config.getHost();

this.port = config.getPort();

try {

if (socketFactory == null) {

this.socket = new Socket(host, port);

@@ -288,11 +300,11 @@

// Keep a copy to be sure that once the configuration has been passed to the

// constructor it cannot be modified

this.configuration = (ConnectionConfiguration) config.clone();

}

catch (CloneNotSupportedException e) {}

  •    init();
    
  •    init(connectionListener);
    

}

/**

  • Package-private default constructor. This constructor is only intended

  • for unit testing. Normal classes extending XMPPConnection should override

@@ -848,18 +860,20 @@

  • Initializes the connection by creating a packet reader and writer and opening a

  • XMPP stream to the server.

  • @throws XMPPException if establishing a connection to the server fails.

*/

  • private void init() throws XMPPException {
  • private void init(ConnectionListener connectionListener) throws XMPPException {

// Set the reader and writer instance variables

initReaderAndWriter();

try

{

packetWriter = new PacketWriter(this);

packetReader = new PacketReader(this);

  •        addConnectionListener(connectionListener);
    

// If debugging is enabled, we should start the thread that will listen for

// all packets and then log them.

if (configuration.isDebuggerEnabled()) {

packetReader.addPacketListener(debugger.getReaderListener(), null);[/code]

I’'ve filed this as SMACK-157.

Regards,

Matt