Race condition in XMPPConnection constructor

Smack Smack Dev
#1

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.

#2

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]

#3

I’'ve filed this as SMACK-157.

Regards,

Matt