You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by kw...@apache.org on 2017/08/07 11:41:35 UTC

qpid-broker-j git commit: QPID-7867: [Java Broker] Add truststore feature that insists trust anchors are within validity period.

Repository: qpid-broker-j
Updated Branches:
  refs/heads/master 65056b757 -> 555593d95


QPID-7867: [Java Broker] Add truststore feature that insists trust anchors are within validity period.


Project: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/commit/555593d9
Tree: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/tree/555593d9
Diff: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/diff/555593d9

Branch: refs/heads/master
Commit: 555593d95fdc2e6ced1377092e9e616a49a37a15
Parents: 65056b7
Author: Keith Wall <kw...@apache.org>
Authored: Wed Aug 2 13:57:19 2017 +0100
Committer: Keith Wall <kw...@apache.org>
Committed: Mon Aug 7 12:41:06 2017 +0100

----------------------------------------------------------------------
 .../apache/qpid/server/model/TrustStore.java    |   9 ++
 .../server/security/AbstractTrustStore.java     |  82 ++++++++++
 .../qpid/server/security/FileTrustStore.java    |   1 -
 .../server/security/FileTrustStoreImpl.java     |   3 +-
 .../ManagedPeerCertificateTrustStoreImpl.java   |   2 +-
 .../server/security/NonJavaTrustStoreImpl.java  |   2 +-
 .../security/SiteSpecificTrustStoreImpl.java    |   2 +-
 .../TrustAnchorValidatingTrustManager.java      | 151 +++++++++++++++++++
 .../server/security/FileTrustStoreTest.java     |  86 +++++++++--
 .../qpid/test/utils/TestSSLConstants.java       |   2 +
 .../manager/ExternalAuthenticationTest.java     |  42 +++++-
 .../ssl/java_broker_expired_truststore.jks      | Bin 0 -> 769 bytes
 .../ssl/java_client_expired_keystore.jks        | Bin 0 -> 2057 bytes
 13 files changed, 354 insertions(+), 28 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/555593d9/broker-core/src/main/java/org/apache/qpid/server/model/TrustStore.java
----------------------------------------------------------------------
diff --git a/broker-core/src/main/java/org/apache/qpid/server/model/TrustStore.java b/broker-core/src/main/java/org/apache/qpid/server/model/TrustStore.java
index 53bb1f0..c2ddeac 100644
--- a/broker-core/src/main/java/org/apache/qpid/server/model/TrustStore.java
+++ b/broker-core/src/main/java/org/apache/qpid/server/model/TrustStore.java
@@ -31,6 +31,9 @@ import org.apache.qpid.server.security.CertificateDetails;
 @ManagedObject( defaultType = "FileTrustStore" )
 public interface TrustStore<X extends TrustStore<X>> extends ConfiguredObject<X>
 {
+    String TRUST_ANCHOR_VALIDITY_ENFORCED = "trustAnchorValidityEnforced";
+    String PEERS_ONLY = "peersOnly";
+
     String CERTIFICATE_EXPIRY_WARN_PERIOD = "qpid.truststore.certificateExpiryWarnPeriod";
 
     @ManagedContextDefault(name = CERTIFICATE_EXPIRY_WARN_PERIOD)
@@ -40,6 +43,8 @@ public interface TrustStore<X extends TrustStore<X>> extends ConfiguredObject<X>
 
     @ManagedContextDefault(name = CERTIFICATE_EXPIRY_CHECK_FREQUENCY)
     int DEFAULT_CERTIFICATE_EXPIRY_CHECK_FREQUENCY = 1;
+    @ManagedContextDefault(name = "qpid.truststore.trustAnchorValidityEnforced")
+    boolean DEFAULT_TRUST_ANCHOR_VALIDITY_ENFORCED = false;
 
     @Override
     @ManagedAttribute(immutable = true)
@@ -54,6 +59,10 @@ public interface TrustStore<X extends TrustStore<X>> extends ConfiguredObject<X>
     @ManagedAttribute( defaultValue = "[]", description = "If 'exposedAsMessageSource' is true and 'includedVirtualHostNodeMessageSources' is empty, the trust store will expose its certificates only to VirtualHostNodes who are not in this list." )
     List<VirtualHostNode<?>> getExcludedVirtualHostNodeMessageSources();
 
+    @ManagedAttribute( defaultValue = "${qpid.truststore.trustAnchorValidityEnforced}",
+                       description = "If true, the trust anchor's validity dates will be enforced.")
+    boolean isTrustAnchorValidityEnforced();
+
     @DerivedAttribute(description = "List of details about the certificates like validity dates, SANs, issuer and subject names, etc.")
     List<CertificateDetails> getCertificateDetails();
 

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/555593d9/broker-core/src/main/java/org/apache/qpid/server/security/AbstractTrustStore.java
----------------------------------------------------------------------
diff --git a/broker-core/src/main/java/org/apache/qpid/server/security/AbstractTrustStore.java b/broker-core/src/main/java/org/apache/qpid/server/security/AbstractTrustStore.java
index acfdbc1..3c6dca1 100644
--- a/broker-core/src/main/java/org/apache/qpid/server/security/AbstractTrustStore.java
+++ b/broker-core/src/main/java/org/apache/qpid/server/security/AbstractTrustStore.java
@@ -19,17 +19,29 @@
  */
 package org.apache.qpid.server.security;
 
+import java.security.GeneralSecurityException;
+import java.security.InvalidKeyException;
+import java.security.PublicKey;
+import java.security.SignatureException;
+import java.security.cert.Certificate;
 import java.security.cert.CertificateExpiredException;
 import java.security.cert.CertificateNotYetValidException;
+import java.security.cert.TrustAnchor;
 import java.security.cert.X509Certificate;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Date;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
 
+import javax.net.ssl.TrustManager;
+import javax.net.ssl.X509TrustManager;
+
+import com.google.common.collect.Sets;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import org.slf4j.Logger;
@@ -67,6 +79,8 @@ public abstract class AbstractTrustStore<X extends AbstractTrustStore<X>>
     private List<VirtualHostNode<?>> _includedVirtualHostNodeMessageSources;
     @ManagedAttributeField
     private List<VirtualHostNode<?>> _excludedVirtualHostNodeMessageSources;
+    @ManagedAttributeField
+    private boolean _trustAnchorValidityEnforced;
 
     private ScheduledFuture<?> _checkExpiryTaskFuture;
 
@@ -209,6 +223,54 @@ public abstract class AbstractTrustStore<X extends AbstractTrustStore<X>>
     }
 
     @Override
+    public final TrustManager[] getTrustManagers() throws GeneralSecurityException
+    {
+        if (isTrustAnchorValidityEnforced())
+        {
+            final Set<Certificate> trustManagerCerts = Sets.newHashSet(getCertificates());
+            final Set<TrustAnchor> trustAnchors = new HashSet<>();
+            final Set<Certificate> otherCerts = new HashSet<>();
+            for (Certificate certs : trustManagerCerts)
+            {
+                if (certs instanceof X509Certificate && isSelfSigned((X509Certificate) certs))
+                {
+                    trustAnchors.add(new TrustAnchor((X509Certificate) certs, null));
+                }
+                else
+                {
+                    otherCerts.add(certs);
+                }
+            }
+
+            TrustManager[] trustManagers = getTrustManagersInternal();
+            TrustManager[] wrappedTrustManagers = new TrustManager[trustManagers.length];
+
+            for (int i = 0; i < trustManagers.length; i++)
+            {
+                final TrustManager trustManager = trustManagers[i];
+                if (trustManager instanceof X509TrustManager)
+                {
+                    wrappedTrustManagers[i] = new TrustAnchorValidatingTrustManager(getName(),
+                                                                                    (X509TrustManager) trustManager,
+                                                                                    trustAnchors,
+                                                                                    otherCerts);
+                }
+                else
+                {
+                    wrappedTrustManagers[i] = trustManager;
+                }
+            }
+            return wrappedTrustManagers;
+        }
+        else
+        {
+            return getTrustManagersInternal();
+        }
+    }
+
+    protected abstract TrustManager[] getTrustManagersInternal() throws GeneralSecurityException;
+
+    @Override
     public final int getCertificateExpiryWarnPeriod()
     {
         try
@@ -239,6 +301,12 @@ public abstract class AbstractTrustStore<X extends AbstractTrustStore<X>>
     }
 
     @Override
+    public boolean isTrustAnchorValidityEnforced()
+    {
+        return _trustAnchorValidityEnforced;
+    }
+
+    @Override
     public boolean isExposedAsMessageSource()
     {
         return _exposedAsMessageSource;
@@ -255,4 +323,18 @@ public abstract class AbstractTrustStore<X extends AbstractTrustStore<X>>
     {
         return _excludedVirtualHostNodeMessageSources;
     }
+
+    private boolean isSelfSigned(X509Certificate cert) throws GeneralSecurityException
+    {
+        try
+        {
+            PublicKey key = cert.getPublicKey();
+            cert.verify(key);
+            return true;
+        }
+        catch (SignatureException | InvalidKeyException e)
+        {
+            return false;
+        }
+    }
 }

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/555593d9/broker-core/src/main/java/org/apache/qpid/server/security/FileTrustStore.java
----------------------------------------------------------------------
diff --git a/broker-core/src/main/java/org/apache/qpid/server/security/FileTrustStore.java b/broker-core/src/main/java/org/apache/qpid/server/security/FileTrustStore.java
index 7f8b822..2efcdaa 100644
--- a/broker-core/src/main/java/org/apache/qpid/server/security/FileTrustStore.java
+++ b/broker-core/src/main/java/org/apache/qpid/server/security/FileTrustStore.java
@@ -32,7 +32,6 @@ import org.apache.qpid.server.model.TrustStore;
 public interface FileTrustStore<X extends FileTrustStore<X>> extends TrustStore<X>
 {
     String TRUST_MANAGER_FACTORY_ALGORITHM = "trustManagerFactoryAlgorithm";
-    String PEERS_ONLY = "peersOnly";
     String TRUST_STORE_TYPE = "trustStoreType";
     String PASSWORD = "password";
     String STORE_URL = "storeUrl";

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/555593d9/broker-core/src/main/java/org/apache/qpid/server/security/FileTrustStoreImpl.java
----------------------------------------------------------------------
diff --git a/broker-core/src/main/java/org/apache/qpid/server/security/FileTrustStoreImpl.java b/broker-core/src/main/java/org/apache/qpid/server/security/FileTrustStoreImpl.java
index c1cd588..122445d 100644
--- a/broker-core/src/main/java/org/apache/qpid/server/security/FileTrustStoreImpl.java
+++ b/broker-core/src/main/java/org/apache/qpid/server/security/FileTrustStoreImpl.java
@@ -198,8 +198,9 @@ public class FileTrustStoreImpl extends AbstractTrustStore<FileTrustStoreImpl> i
     {
         _password = password;
     }
+
     @Override
-    public TrustManager[] getTrustManagers() throws GeneralSecurityException
+    public TrustManager[] getTrustManagersInternal() throws GeneralSecurityException
     {
         String trustStorePassword = getPassword();
         String trustStoreType = _trustStoreType;

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/555593d9/broker-core/src/main/java/org/apache/qpid/server/security/ManagedPeerCertificateTrustStoreImpl.java
----------------------------------------------------------------------
diff --git a/broker-core/src/main/java/org/apache/qpid/server/security/ManagedPeerCertificateTrustStoreImpl.java b/broker-core/src/main/java/org/apache/qpid/server/security/ManagedPeerCertificateTrustStoreImpl.java
index 5042915..39f638d 100644
--- a/broker-core/src/main/java/org/apache/qpid/server/security/ManagedPeerCertificateTrustStoreImpl.java
+++ b/broker-core/src/main/java/org/apache/qpid/server/security/ManagedPeerCertificateTrustStoreImpl.java
@@ -74,7 +74,7 @@ public class ManagedPeerCertificateTrustStoreImpl
     }
 
     @Override
-    public TrustManager[] getTrustManagers()
+    public TrustManager[] getTrustManagersInternal()
     {
         if (_trustManagers == null || _trustManagers.length == 0)
         {

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/555593d9/broker-core/src/main/java/org/apache/qpid/server/security/NonJavaTrustStoreImpl.java
----------------------------------------------------------------------
diff --git a/broker-core/src/main/java/org/apache/qpid/server/security/NonJavaTrustStoreImpl.java b/broker-core/src/main/java/org/apache/qpid/server/security/NonJavaTrustStoreImpl.java
index d98c821..0b26944 100644
--- a/broker-core/src/main/java/org/apache/qpid/server/security/NonJavaTrustStoreImpl.java
+++ b/broker-core/src/main/java/org/apache/qpid/server/security/NonJavaTrustStoreImpl.java
@@ -99,7 +99,7 @@ public class NonJavaTrustStoreImpl
     }
 
     @Override
-    public TrustManager[] getTrustManagers() throws GeneralSecurityException
+    public TrustManager[] getTrustManagersInternal() throws GeneralSecurityException
     {
         if (_trustManagers == null || _trustManagers.length == 0)
         {

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/555593d9/broker-core/src/main/java/org/apache/qpid/server/security/SiteSpecificTrustStoreImpl.java
----------------------------------------------------------------------
diff --git a/broker-core/src/main/java/org/apache/qpid/server/security/SiteSpecificTrustStoreImpl.java b/broker-core/src/main/java/org/apache/qpid/server/security/SiteSpecificTrustStoreImpl.java
index bb93101..2e9a924 100644
--- a/broker-core/src/main/java/org/apache/qpid/server/security/SiteSpecificTrustStoreImpl.java
+++ b/broker-core/src/main/java/org/apache/qpid/server/security/SiteSpecificTrustStoreImpl.java
@@ -149,7 +149,7 @@ public class SiteSpecificTrustStoreImpl
     }
 
     @Override
-    public TrustManager[] getTrustManagers() throws GeneralSecurityException
+    public TrustManager[] getTrustManagersInternal() throws GeneralSecurityException
     {
         if (_trustManagers == null || _trustManagers.length == 0)
         {

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/555593d9/broker-core/src/main/java/org/apache/qpid/server/security/TrustAnchorValidatingTrustManager.java
----------------------------------------------------------------------
diff --git a/broker-core/src/main/java/org/apache/qpid/server/security/TrustAnchorValidatingTrustManager.java b/broker-core/src/main/java/org/apache/qpid/server/security/TrustAnchorValidatingTrustManager.java
new file mode 100644
index 0000000..42434c2
--- /dev/null
+++ b/broker-core/src/main/java/org/apache/qpid/server/security/TrustAnchorValidatingTrustManager.java
@@ -0,0 +1,151 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+package org.apache.qpid.server.security;
+
+import java.security.GeneralSecurityException;
+import java.security.cert.CertPathBuilder;
+import java.security.cert.CertStore;
+import java.security.cert.Certificate;
+import java.security.cert.CertificateException;
+import java.security.cert.CertificateExpiredException;
+import java.security.cert.CertificateNotYetValidException;
+import java.security.cert.CollectionCertStoreParameters;
+import java.security.cert.PKIXBuilderParameters;
+import java.security.cert.PKIXCertPathBuilderResult;
+import java.security.cert.TrustAnchor;
+import java.security.cert.X509CertSelector;
+import java.security.cert.X509Certificate;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Set;
+
+import javax.net.ssl.X509TrustManager;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+class TrustAnchorValidatingTrustManager implements X509TrustManager
+{
+    private static Logger LOGGER = LoggerFactory.getLogger(TrustAnchorValidatingTrustManager.class);
+
+    private String _trustStoreName;
+    private final X509TrustManager _x509TrustManager;
+    private final Set<TrustAnchor> _trustAnchors;
+    private final Set<Certificate> _otherCerts;
+
+    TrustAnchorValidatingTrustManager(final String trustStoreName, final X509TrustManager x509TrustManager,
+                                      final Set<TrustAnchor> trustAnchors,
+                                      final Set<Certificate> otherCerts)
+    {
+        _trustStoreName = trustStoreName;
+        _x509TrustManager = x509TrustManager;
+        _trustAnchors = trustAnchors;
+        _otherCerts = otherCerts;
+    }
+
+    @Override
+    public void checkClientTrusted(final X509Certificate[] x509Certificates, final String authType)
+            throws CertificateException
+    {
+        _x509TrustManager.checkClientTrusted(x509Certificates, authType);
+
+        X509Certificate peerCertificate = x509Certificates[0];
+        PKIXCertPathBuilderResult pkixCertPathBuilderResult;
+        try
+        {
+            pkixCertPathBuilderResult = getPkixCertPathBuilderResult(x509Certificates, _trustAnchors, _otherCerts);
+        }
+        catch (GeneralSecurityException e)
+        {
+            throw new CertificateException("Unexpected error whilst validating trust-anchor", e);
+        }
+
+        X509Certificate trustAnchorCert = pkixCertPathBuilderResult.getTrustAnchor().getTrustedCert();
+        try
+        {
+            trustAnchorCert.checkValidity();
+        }
+        catch (CertificateExpiredException | CertificateNotYetValidException e)
+        {
+            LOGGER.warn("Authentication failed for peer bearing certificate (subject DN '{}') "
+                        + "as the trust anchor (subject DN '{}') within truststore '{}' "
+                        + "is either expired or not yet valid. Validity range {} - {}",
+                        peerCertificate.getSubjectDN(),
+                        trustAnchorCert.getSubjectDN(),
+                        _trustStoreName,
+                        trustAnchorCert.getNotBefore(),
+                        trustAnchorCert.getNotAfter());
+            throw e;
+        }
+    }
+
+    @Override
+    public void checkServerTrusted(final X509Certificate[] x509Certificates, final String authType)
+            throws CertificateException
+    {
+        _x509TrustManager.checkServerTrusted(x509Certificates, authType);
+    }
+
+    @Override
+    public X509Certificate[] getAcceptedIssuers()
+    {
+        return _x509TrustManager.getAcceptedIssuers();
+    }
+
+    private PKIXCertPathBuilderResult getPkixCertPathBuilderResult(final X509Certificate[] x509Certificates,
+                                                                   final Set<TrustAnchor> trustAnchors,
+                                                                   final Set<Certificate> otherCerts)
+            throws GeneralSecurityException
+    {
+        Set<Certificate> intermediateCerts = new HashSet<>();
+        intermediateCerts.addAll(otherCerts);
+
+        Iterator<X509Certificate> iterator = Arrays.asList(x509Certificates).iterator();
+
+        if (!iterator.hasNext())
+        {
+            throw new IllegalArgumentException("Peer certificate not found");
+        }
+
+        final X509Certificate peerCertificate = iterator.next();
+        while (iterator.hasNext())
+        {
+            X509Certificate intermediate = iterator.next();
+            intermediateCerts.add(intermediate);
+        }
+
+        X509CertSelector selector = new X509CertSelector();
+        selector.setCertificate(peerCertificate);
+
+        PKIXBuilderParameters pkixParams = new PKIXBuilderParameters(trustAnchors, selector);
+        pkixParams.setRevocationEnabled(false);
+
+        CertStore intermediateCertStore = CertStore.getInstance("Collection",
+                                                                new CollectionCertStoreParameters(intermediateCerts));
+        pkixParams.addCertStore(intermediateCertStore);
+
+        CertPathBuilder builder = CertPathBuilder.getInstance("PKIX");
+
+        return (PKIXCertPathBuilderResult) builder.build(pkixParams);
+    }
+}

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/555593d9/broker-core/src/test/java/org/apache/qpid/server/security/FileTrustStoreTest.java
----------------------------------------------------------------------
diff --git a/broker-core/src/test/java/org/apache/qpid/server/security/FileTrustStoreTest.java b/broker-core/src/test/java/org/apache/qpid/server/security/FileTrustStoreTest.java
index ce570fc..ac8bdc9 100644
--- a/broker-core/src/test/java/org/apache/qpid/server/security/FileTrustStoreTest.java
+++ b/broker-core/src/test/java/org/apache/qpid/server/security/FileTrustStoreTest.java
@@ -23,12 +23,16 @@ package org.apache.qpid.server.security;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
+import java.security.KeyStore;
+import java.security.cert.CertificateExpiredException;
+import java.security.cert.X509Certificate;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 
 import javax.net.ssl.TrustManager;
+import javax.net.ssl.X509TrustManager;
 
 import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.configuration.updater.CurrentThreadTaskExecutor;
@@ -43,11 +47,12 @@ import org.apache.qpid.server.model.Model;
 import org.apache.qpid.server.model.Port;
 import org.apache.qpid.server.model.TrustStore;
 import org.apache.qpid.server.security.auth.manager.SimpleLDAPAuthenticationManager;
-import org.apache.qpid.test.utils.QpidTestCase;
-import org.apache.qpid.test.utils.TestSSLConstants;
 import org.apache.qpid.server.transport.network.security.ssl.QpidMultipleTrustManager;
+import org.apache.qpid.server.transport.network.security.ssl.SSLUtil;
 import org.apache.qpid.server.util.DataUrlUtils;
 import org.apache.qpid.server.util.FileUtils;
+import org.apache.qpid.test.utils.QpidTestCase;
+import org.apache.qpid.test.utils.TestSSLConstants;
 
 public class FileTrustStoreTest extends QpidTestCase
 {
@@ -77,8 +82,7 @@ public class FileTrustStoreTest extends QpidTestCase
         attributes.put(FileTrustStore.STORE_URL, TestSSLConstants.TRUSTSTORE);
         attributes.put(FileTrustStore.PASSWORD, TestSSLConstants.TRUSTSTORE_PASSWORD);
 
-        FileTrustStoreImpl fileTrustStore =
-                (FileTrustStoreImpl) _factory.create(TrustStore.class, attributes,  _broker);
+        TrustStore<?> fileTrustStore = _factory.create(TrustStore.class, attributes,  _broker);
 
         TrustManager[] trustManagers = fileTrustStore.getTrustManagers();
         assertNotNull(trustManagers);
@@ -113,8 +117,7 @@ public class FileTrustStoreTest extends QpidTestCase
         attributes.put(FileTrustStore.PASSWORD, TestSSLConstants.BROKER_PEERSTORE_PASSWORD);
         attributes.put(FileTrustStore.PEERS_ONLY, true);
 
-        FileTrustStoreImpl fileTrustStore =
-                (FileTrustStoreImpl) _factory.create(TrustStore.class, attributes,  _broker);
+        TrustStore<?> fileTrustStore = _factory.create(TrustStore.class, attributes,  _broker);
 
         TrustManager[] trustManagers = fileTrustStore.getTrustManagers();
         assertNotNull(trustManagers);
@@ -123,6 +126,62 @@ public class FileTrustStoreTest extends QpidTestCase
         assertTrue("Trust manager unexpected null", trustManagers[0] instanceof QpidMultipleTrustManager);
     }
 
+    public void testUseOfExpiredTrustAnchorAllowed() throws Exception
+    {
+        Map<String,Object> attributes = new HashMap<>();
+        attributes.put(FileTrustStore.NAME, "myFileTrustStore");
+        attributes.put(FileTrustStore.STORE_URL, TestSSLConstants.BROKER_EXPIRED_TRUSTSTORE);
+        attributes.put(FileTrustStore.PASSWORD, TestSSLConstants.BROKER_TRUSTSTORE_PASSWORD);
+
+        TrustStore trustStore = _factory.create(TrustStore.class, attributes, _broker);
+
+        TrustManager[] trustManagers = trustStore.getTrustManagers();
+        assertNotNull(trustManagers);
+        assertEquals("Unexpected number of trust managers", 1, trustManagers.length);
+        assertTrue("Unexpected trust manager type",trustManagers[0] instanceof X509TrustManager);
+        X509TrustManager trustManager = (X509TrustManager) trustManagers[0];
+
+        KeyStore clientStore = SSLUtil.getInitializedKeyStore(TestSSLConstants.EXPIRED_KEYSTORE,
+                                                              TestSSLConstants.KEYSTORE_PASSWORD,
+                                                              KeyStore.getDefaultType());
+        String alias = clientStore.aliases().nextElement();
+        X509Certificate certificate = (X509Certificate) clientStore.getCertificate(alias);
+
+        trustManager.checkClientTrusted(new X509Certificate[] {certificate}, "NULL");
+    }
+
+    public void testUseOfExpiredTrustAnchorDenied() throws Exception
+    {
+        Map<String,Object> attributes = new HashMap<>();
+        attributes.put(FileTrustStore.NAME, "myFileTrustStore");
+        attributes.put(FileTrustStore.STORE_URL, TestSSLConstants.BROKER_EXPIRED_TRUSTSTORE);
+        attributes.put(FileTrustStore.PASSWORD, TestSSLConstants.BROKER_TRUSTSTORE_PASSWORD);
+        attributes.put(FileTrustStore.TRUST_ANCHOR_VALIDITY_ENFORCED, true);
+
+        TrustStore trustStore = _factory.create(TrustStore.class, attributes, _broker);
+
+        TrustManager[] trustManagers = trustStore.getTrustManagers();
+        assertNotNull(trustManagers);
+        assertEquals("Unexpected number of trust managers", 1, trustManagers.length);
+        assertTrue("Unexpected trust manager type",trustManagers[0] instanceof X509TrustManager);
+        X509TrustManager trustManager = (X509TrustManager) trustManagers[0];
+
+        KeyStore clientStore = SSLUtil.getInitializedKeyStore(TestSSLConstants.EXPIRED_KEYSTORE,
+                                                             TestSSLConstants.KEYSTORE_PASSWORD,
+                                                             KeyStore.getDefaultType());
+        String alias = clientStore.aliases().nextElement();
+        X509Certificate certificate = (X509Certificate) clientStore.getCertificate(alias);
+
+        try
+        {
+            trustManager.checkClientTrusted(new X509Certificate[] {certificate}, "NULL");
+            fail("Exception not thrown");
+        }
+        catch (CertificateExpiredException e)
+        {
+            // PASS
+        }
+    }
 
     public void testCreateTrustStoreFromDataUrl_Success() throws Exception
     {
@@ -133,8 +192,7 @@ public class FileTrustStoreTest extends QpidTestCase
         attributes.put(FileTrustStore.STORE_URL, trustStoreAsDataUrl);
         attributes.put(FileTrustStore.PASSWORD, TestSSLConstants.TRUSTSTORE_PASSWORD);
 
-        FileTrustStoreImpl fileTrustStore =
-                (FileTrustStoreImpl) _factory.create(TrustStore.class, attributes,  _broker);
+        TrustStore<?> fileTrustStore = _factory.create(TrustStore.class, attributes,  _broker);
 
         TrustManager[] trustManagers = fileTrustStore.getTrustManagers();
         assertNotNull(trustManagers);
@@ -192,8 +250,7 @@ public class FileTrustStoreTest extends QpidTestCase
         attributes.put(FileTrustStore.STORE_URL, TestSSLConstants.TRUSTSTORE);
         attributes.put(FileTrustStore.PASSWORD, TestSSLConstants.TRUSTSTORE_PASSWORD);
 
-        FileTrustStoreImpl fileTrustStore =
-                (FileTrustStoreImpl) _factory.create(TrustStore.class, attributes,  _broker);
+        FileTrustStore<?> fileTrustStore = (FileTrustStore<?>) _factory.create(TrustStore.class, attributes,  _broker);
 
         assertEquals("Unexpected path value before change", TestSSLConstants.TRUSTSTORE, fileTrustStore.getStoreUrl());
 
@@ -231,8 +288,7 @@ public class FileTrustStoreTest extends QpidTestCase
         attributes.put(FileTrustStore.STORE_URL, TestSSLConstants.TRUSTSTORE);
         attributes.put(FileTrustStore.PASSWORD, TestSSLConstants.TRUSTSTORE_PASSWORD);
 
-        FileTrustStoreImpl fileTrustStore =
-                (FileTrustStoreImpl) _factory.create(TrustStore.class, attributes,  _broker);
+        TrustStore<?> fileTrustStore = _factory.create(TrustStore.class, attributes,  _broker);
 
         fileTrustStore.delete();
     }
@@ -244,8 +300,7 @@ public class FileTrustStoreTest extends QpidTestCase
         attributes.put(FileTrustStore.STORE_URL, TestSSLConstants.TRUSTSTORE);
         attributes.put(FileTrustStore.PASSWORD, TestSSLConstants.TRUSTSTORE_PASSWORD);
 
-        FileTrustStoreImpl fileTrustStore =
-                (FileTrustStoreImpl) _factory.create(TrustStore.class, attributes,  _broker);
+        TrustStore<?> fileTrustStore = _factory.create(TrustStore.class, attributes,  _broker);
 
         SimpleLDAPAuthenticationManager ldap = mock(SimpleLDAPAuthenticationManager.class);
         when(ldap.getTrustStore()).thenReturn(fileTrustStore);
@@ -271,8 +326,7 @@ public class FileTrustStoreTest extends QpidTestCase
         attributes.put(FileTrustStore.STORE_URL, TestSSLConstants.TRUSTSTORE);
         attributes.put(FileTrustStore.PASSWORD, TestSSLConstants.TRUSTSTORE_PASSWORD);
 
-        FileTrustStoreImpl fileTrustStore =
-                (FileTrustStoreImpl) _factory.create(TrustStore.class, attributes,  _broker);
+        TrustStore<?> fileTrustStore = _factory.create(TrustStore.class, attributes,  _broker);
 
         Port<?> port = mock(Port.class);
         when(port.getTrustStores()).thenReturn(Collections.<TrustStore>singletonList(fileTrustStore));

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/555593d9/qpid-test-utils/src/main/java/org/apache/qpid/test/utils/TestSSLConstants.java
----------------------------------------------------------------------
diff --git a/qpid-test-utils/src/main/java/org/apache/qpid/test/utils/TestSSLConstants.java b/qpid-test-utils/src/main/java/org/apache/qpid/test/utils/TestSSLConstants.java
index 840cc55..360ecc9 100644
--- a/qpid-test-utils/src/main/java/org/apache/qpid/test/utils/TestSSLConstants.java
+++ b/qpid-test-utils/src/main/java/org/apache/qpid/test/utils/TestSSLConstants.java
@@ -22,6 +22,7 @@ public interface TestSSLConstants
 {
     String KEYSTORE = "test-profiles/test_resources/ssl/java_client_keystore.jks";
     String UNTRUSTED_KEYSTORE = "test-profiles/test_resources/ssl/java_client_untrusted_keystore.jks";
+    String EXPIRED_KEYSTORE = "test-profiles/test_resources/ssl/java_client_expired_keystore.jks";
     String KEYSTORE_PASSWORD = "password";
     String TRUSTSTORE = "test-profiles/test_resources/ssl/java_client_truststore.jks";
     String TRUSTSTORE_PASSWORD = "password";
@@ -38,5 +39,6 @@ public interface TestSSLConstants
     String BROKER_PEERSTORE_PASSWORD = "password";
 
     String BROKER_TRUSTSTORE = "test-profiles/test_resources/ssl/java_broker_truststore.jks";
+    String BROKER_EXPIRED_TRUSTSTORE = "test-profiles/test_resources/ssl/java_broker_expired_truststore.jks";
     String BROKER_TRUSTSTORE_PASSWORD = "password";
 }

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/555593d9/systests/src/test/java/org/apache/qpid/server/security/auth/manager/ExternalAuthenticationTest.java
----------------------------------------------------------------------
diff --git a/systests/src/test/java/org/apache/qpid/server/security/auth/manager/ExternalAuthenticationTest.java b/systests/src/test/java/org/apache/qpid/server/security/auth/manager/ExternalAuthenticationTest.java
index 8507e21..1ad564f 100644
--- a/systests/src/test/java/org/apache/qpid/server/security/auth/manager/ExternalAuthenticationTest.java
+++ b/systests/src/test/java/org/apache/qpid/server/security/auth/manager/ExternalAuthenticationTest.java
@@ -20,13 +20,7 @@
  */
 package org.apache.qpid.server.security.auth.manager;
 
-import static org.apache.qpid.test.utils.TestSSLConstants.BROKER_PEERSTORE;
-import static org.apache.qpid.test.utils.TestSSLConstants.BROKER_PEERSTORE_PASSWORD;
-import static org.apache.qpid.test.utils.TestSSLConstants.KEYSTORE;
-import static org.apache.qpid.test.utils.TestSSLConstants.KEYSTORE_PASSWORD;
-import static org.apache.qpid.test.utils.TestSSLConstants.TRUSTSTORE;
-import static org.apache.qpid.test.utils.TestSSLConstants.TRUSTSTORE_PASSWORD;
-import static org.apache.qpid.test.utils.TestSSLConstants.UNTRUSTED_KEYSTORE;
+import static org.apache.qpid.test.utils.TestSSLConstants.*;
 
 import java.util.Arrays;
 import java.util.Collection;
@@ -164,6 +158,40 @@ public class ExternalAuthenticationTest extends QpidBrokerTestCase
         }
     }
 
+    public void testExternalAuthenticationDeniesExpiredClientCert() throws Exception
+    {
+        final String expiredTrustStore = "expiredTrustStore";
+        final List<String>
+                storeNames = Arrays.asList(TestBrokerConfiguration.ENTRY_NAME_SSL_TRUSTSTORE, expiredTrustStore);
+
+        //set the broker's SSL config, inc which SSL stores to use
+        setCommonBrokerSSLProperties(true, storeNames);
+
+        Map<String, Object> sslTrustStoreAttributes = new HashMap<>();
+        sslTrustStoreAttributes.put(TrustStore.NAME, expiredTrustStore);
+        sslTrustStoreAttributes.put(FileTrustStore.STORE_URL, BROKER_EXPIRED_TRUSTSTORE);
+        sslTrustStoreAttributes.put(FileTrustStore.PASSWORD, BROKER_TRUSTSTORE_PASSWORD);
+        sslTrustStoreAttributes.put(FileTrustStore.TRUST_ANCHOR_VALIDITY_ENFORCED, true);
+        getDefaultBrokerConfiguration().addObjectConfiguration(TrustStore.class, sslTrustStoreAttributes);
+
+        super.startDefaultBroker();
+
+        try
+        {
+            getExternalSSLConnection(false,
+                                     TRUSTSTORE,
+                                     TRUSTSTORE_PASSWORD,
+                                     EXPIRED_KEYSTORE,
+                                     KEYSTORE_PASSWORD,
+                                     null);
+            fail("Connection should not succeed");
+        }
+        catch (JMSException e)
+        {
+            // pass
+        }
+    }
+
     /**
      * Tests that when using the EXTERNAL auth provider and a 'peersOnly' truststore, clients using certs directly in
      * in the store will be able to connect and clients using certs signed by the same CA but not in the store will not.

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/555593d9/test-profiles/test_resources/ssl/java_broker_expired_truststore.jks
----------------------------------------------------------------------
diff --git a/test-profiles/test_resources/ssl/java_broker_expired_truststore.jks b/test-profiles/test_resources/ssl/java_broker_expired_truststore.jks
new file mode 100644
index 0000000..bbbd248
Binary files /dev/null and b/test-profiles/test_resources/ssl/java_broker_expired_truststore.jks differ

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/555593d9/test-profiles/test_resources/ssl/java_client_expired_keystore.jks
----------------------------------------------------------------------
diff --git a/test-profiles/test_resources/ssl/java_client_expired_keystore.jks b/test-profiles/test_resources/ssl/java_client_expired_keystore.jks
new file mode 100644
index 0000000..eb86509
Binary files /dev/null and b/test-profiles/test_resources/ssl/java_client_expired_keystore.jks differ


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org