From f86f1c1df0fc8bfd72306d55d370e202378652b2 Mon Sep 17 00:00:00 2001 From: Nick Hudson Date: Thu, 10 Jul 2014 14:31:24 +0100 Subject: Make Stroke return peer certificate chain, rather then just EE certificate Since the initial Stroke TLS implementation was done, some changes were made in Swiften, starting with "Show Certificate dialog from certificate error window." 159e773b156f531575d0d7e241e2d20c85ee6d7cA which mean that certificate verification uses the peer's certificate chain, and not just the peer's EE certificate. This change updates Stroke so that its API now more closely matches what Swiften does. Note that any current Stroke clients that implement the "CertificateTrustChecker" interface will break, as this patch makes an incompatible change to that interface, requiring implementing classes to handle a certificate chain rather than a single certificate. Isode copyright notices are updated; Remko copyright notices are updated to reflect the current copyright notices in any equivalent Swiften source files. Test-information: Used MLC (after having patched it for CertificateTrustChecker changes) and verified that it sees the entire certificate chain coming back. Ran self-tests for Stroke and saw no junit failures Change-Id: I3d863f929bfed3324446cadf3bb4d6b9ff916660 diff --git a/src/com/isode/stroke/client/ClientSession.java b/src/com/isode/stroke/client/ClientSession.java index f6082b7..c0caeb6 100644 --- a/src/com/isode/stroke/client/ClientSession.java +++ b/src/com/isode/stroke/client/ClientSession.java @@ -1,9 +1,9 @@ /* - * Copyright (c) 2010-2012 Isode Limited, London, England. + * Copyright (c) 2010-2014 Isode Limited, London, England. * All rights reserved. */ /* - * Copyright (c) 2010-2011 Remko Tronçon. + * Copyright (c) 2010-2014 Remko Tronçon. * All rights reserved. */ package com.isode.stroke.client; @@ -48,6 +48,8 @@ import com.isode.stroke.tls.Certificate; import com.isode.stroke.tls.CertificateTrustChecker; import com.isode.stroke.tls.CertificateVerificationError; import com.isode.stroke.tls.ServerIdentityVerifier; + +import java.util.List; import java.util.UUID; public class ClientSession { @@ -513,24 +515,26 @@ public class ClientSession { if (!checkState(State.Encrypting)) { return; } - final Certificate certificate = stream.getPeerCertificate(); + final List certificateChain = stream.getPeerCertificateChain(); + final Certificate peerCertificate = + (certificateChain == null || certificateChain.isEmpty() ? null : certificateChain.get(0)); final CertificateVerificationError verificationError = stream.getPeerCertificateVerificationError(); if (verificationError != null) { - checkTrustOrFinish(certificate, verificationError); + checkTrustOrFinish(certificateChain, verificationError); } else { final ServerIdentityVerifier identityVerifier = new ServerIdentityVerifier(localJID); - if (identityVerifier.certificateVerifies(certificate)) { + if (identityVerifier.certificateVerifies(peerCertificate)) { continueAfterTLSEncrypted(); } else { - checkTrustOrFinish(certificate, new CertificateVerificationError(CertificateVerificationError.Type.InvalidServerIdentity)); + checkTrustOrFinish(certificateChain, new CertificateVerificationError(CertificateVerificationError.Type.InvalidServerIdentity)); } } } - private void checkTrustOrFinish(final Certificate certificate, final CertificateVerificationError error) { - if (certificateTrustChecker != null && certificateTrustChecker.isCertificateTrusted(certificate)) { + private void checkTrustOrFinish(final List certificateChain, final CertificateVerificationError error) { + if (certificateTrustChecker != null && certificateTrustChecker.isCertificateTrusted(certificateChain)) { continueAfterTLSEncrypted(); } else { diff --git a/src/com/isode/stroke/session/BasicSessionStream.java b/src/com/isode/stroke/session/BasicSessionStream.java index dbe13f7..f1e7bf1 100644 --- a/src/com/isode/stroke/session/BasicSessionStream.java +++ b/src/com/isode/stroke/session/BasicSessionStream.java @@ -1,17 +1,18 @@ /* - * Copyright (c) 2010 Remko Tronçon + * Copyright (c) 2010-2014 Remko Tronçon * All rights reserved. */ /* - * Copyright (c) 2010-2012, Isode Limited, London, England. + * Copyright (c) 2010-2014, Isode Limited, London, England. * All rights reserved. */ package com.isode.stroke.session; +import java.util.List; + import com.isode.stroke.base.ByteArray; import com.isode.stroke.elements.Element; import com.isode.stroke.elements.ProtocolHeader; import com.isode.stroke.elements.StreamType; -import com.isode.stroke.eventloop.EventLoop; import com.isode.stroke.network.Connection; import com.isode.stroke.network.TimerFactory; import com.isode.stroke.parser.PayloadParserFactoryCollection; @@ -23,10 +24,10 @@ import com.isode.stroke.streamstack.ConnectionLayer; import com.isode.stroke.streamstack.StreamStack; import com.isode.stroke.streamstack.TLSLayer; import com.isode.stroke.streamstack.WhitespacePingLayer; -import com.isode.stroke.tls.TLSContextFactory; import com.isode.stroke.streamstack.XMPPLayer; import com.isode.stroke.tls.Certificate; import com.isode.stroke.tls.CertificateVerificationError; +import com.isode.stroke.tls.TLSContextFactory; public class BasicSessionStream extends SessionStream { @@ -156,6 +157,11 @@ public class BasicSessionStream extends SessionStream { return tlsLayer != null; } + @Override + public List getPeerCertificateChain() { + return tlsLayer.getPeerCertificateChain(); + } + @Override public Certificate getPeerCertificate() { return tlsLayer.getPeerCertificate(); } diff --git a/src/com/isode/stroke/session/SessionStream.java b/src/com/isode/stroke/session/SessionStream.java index 5dbb0fc..2b9932b 100644 --- a/src/com/isode/stroke/session/SessionStream.java +++ b/src/com/isode/stroke/session/SessionStream.java @@ -1,13 +1,15 @@ /* - * Copyright (c) 2010 Remko Tronçon + * Copyright (c) 2010-2014 Remko Tronçon * All rights reserved. */ /* - * Copyright (c) 2010-2012, Isode Limited, London, England. + * Copyright (c) 2010-2014, Isode Limited, London, England. * All rights reserved. */ package com.isode.stroke.session; +import java.util.List; + import com.isode.stroke.base.ByteArray; import com.isode.stroke.elements.Element; import com.isode.stroke.elements.ProtocolHeader; @@ -16,7 +18,6 @@ import com.isode.stroke.signals.Signal1; import com.isode.stroke.tls.Certificate; import com.isode.stroke.tls.CertificateVerificationError; import com.isode.stroke.tls.CertificateWithKey; -import com.isode.stroke.tls.PKCS12Certificate; public abstract class SessionStream { @@ -69,6 +70,7 @@ public abstract class SessionStream { return certificate != null && !certificate.isNull(); } + public abstract List getPeerCertificateChain(); public abstract Certificate getPeerCertificate(); public abstract CertificateVerificationError getPeerCertificateVerificationError(); diff --git a/src/com/isode/stroke/streamstack/TLSLayer.java b/src/com/isode/stroke/streamstack/TLSLayer.java index 1f213fc..70bcd1a 100644 --- a/src/com/isode/stroke/streamstack/TLSLayer.java +++ b/src/com/isode/stroke/streamstack/TLSLayer.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010-2012, Isode Limited, London, England. + * Copyright (c) 2010-2014, Isode Limited, London, England. * All rights reserved. */ /* @@ -9,6 +9,8 @@ package com.isode.stroke.streamstack; +import java.util.List; + import com.isode.stroke.base.ByteArray; import com.isode.stroke.signals.Signal; import com.isode.stroke.signals.Slot1; @@ -54,6 +56,10 @@ public class TLSLayer extends StreamLayer { return context.setClientCertificate(certificate); } + public List getPeerCertificateChain() { + return context.getPeerCertificateChain(); + } + public Certificate getPeerCertificate() { return context.getPeerCertificate(); } diff --git a/src/com/isode/stroke/tls/CertificateTrustChecker.java b/src/com/isode/stroke/tls/CertificateTrustChecker.java index 2fcf3c0..7f4753b 100644 --- a/src/com/isode/stroke/tls/CertificateTrustChecker.java +++ b/src/com/isode/stroke/tls/CertificateTrustChecker.java @@ -4,11 +4,14 @@ * See Documentation/Licenses/GPLv3.txt for more information. */ /* - * Copyright (c) 2011, Isode Limited, London, England. + * Copyright (c) 2011-2014, Isode Limited, London, England. * All rights reserved. */ package com.isode.stroke.tls; +import java.util.List; + + /** * A class to implement a check for certificate trust. */ @@ -19,5 +22,6 @@ public interface CertificateTrustChecker { * trusted. This usually happens when a certificate's validation * fails, to check whether to proceed with the connection or not. */ - boolean isCertificateTrusted(Certificate certificate); + public boolean isCertificateTrusted(List chain); + } diff --git a/src/com/isode/stroke/tls/TLSContext.java b/src/com/isode/stroke/tls/TLSContext.java index ec39a3b..738c8b6 100644 --- a/src/com/isode/stroke/tls/TLSContext.java +++ b/src/com/isode/stroke/tls/TLSContext.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011-2012, Isode Limited, London, England. + * Copyright (c) 2011-2014, Isode Limited, London, England. * All rights reserved. */ /* @@ -9,6 +9,8 @@ package com.isode.stroke.tls; +import java.util.List; + import com.isode.stroke.base.ByteArray; import com.isode.stroke.signals.Signal; import com.isode.stroke.signals.Signal1; @@ -22,7 +24,16 @@ public abstract class TLSContext { public abstract void handleDataFromNetwork(ByteArray data); public abstract void handleDataFromApplication(ByteArray data); + /** + * The peer certificate, as presented by the remote entity + * @return the peer certificate, which may be null + */ public abstract Certificate getPeerCertificate(); + /** + * The peer's certificate chain, as presented by the remote entity + * @return the peer certificate chain, which may be null. + */ + public abstract List getPeerCertificateChain(); public abstract CertificateVerificationError getPeerCertificateVerificationError(); public abstract ByteArray getFinishMessage(); diff --git a/src/com/isode/stroke/tls/java/JSSEContext.java b/src/com/isode/stroke/tls/java/JSSEContext.java index 2928498..13904e8 100644 --- a/src/com/isode/stroke/tls/java/JSSEContext.java +++ b/src/com/isode/stroke/tls/java/JSSEContext.java @@ -1,4 +1,4 @@ -/* Copyright (c) 2012-2013, Isode Limited, London, England. +/* Copyright (c) 2012-2014, Isode Limited, London, England. * All rights reserved. * * Acquisition and use of this software and related materials for any @@ -26,7 +26,9 @@ import java.security.cert.CertificateException; import java.security.cert.CertificateExpiredException; import java.security.cert.CertificateNotYetValidException; import java.security.cert.X509Certificate; +import java.util.ArrayList; import java.util.HashSet; +import java.util.List; import java.util.Set; import java.util.Vector; import java.util.logging.Level; @@ -627,8 +629,10 @@ public class JSSEContext extends TLSContext { if (certs == null || certs.length == 0) { return; } - - peerCertificate = new JavaCertificate(certs[0]); + peerCertificateChain = new ArrayList(certs.length); + for (X509Certificate x509:certs) { + peerCertificateChain.add(new JavaCertificate(x509)); + } /* Swiften uses SSL_get_verify_result() for this, and the documentation * for that says it "while the verification of a certificate can fail @@ -1052,10 +1056,16 @@ public class JSSEContext extends TLSContext { } - + @Override + public List getPeerCertificateChain() { + return peerCertificateChain; + } @Override public Certificate getPeerCertificate() { - return peerCertificate; + if (peerCertificateChain == null || peerCertificateChain.isEmpty()) { + return null; + } + return (peerCertificateChain.get(0)); } @Override @@ -1161,9 +1171,9 @@ public class JSSEContext extends TLSContext { private Object recvMutex = new Object(); /** - * The server certificate as obtained from the TLS handshake + * The server certificate chain as obtained from the TLS handshake */ - private JavaCertificate peerCertificate = null; + private List peerCertificateChain = null; /** * The CertificateVerificationError derived from the peerCertificate. This -- cgit v0.10.2-6-g49f6