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/15 12:02:02 UTC
qpid-broker-j git commit: QPID-7869: Have the TrustStore and KeyStore
return defensive copies of KeyManager/TrustManager/Certificate arrays.
Repository: qpid-broker-j
Updated Branches:
refs/heads/master e792cccb7 -> 7d543956e
QPID-7869: Have the TrustStore and KeyStore return defensive copies of KeyManager/TrustManager/Certificate arrays.
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/7d543956
Tree: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/tree/7d543956
Diff: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/diff/7d543956
Branch: refs/heads/master
Commit: 7d543956e8628a343367eb8ecee2bcb87bd977c9
Parents: e792ccc
Author: Keith Wall <kw...@apache.org>
Authored: Tue Aug 15 11:24:43 2017 +0100
Committer: Keith Wall <kw...@apache.org>
Committed: Tue Aug 15 13:01:46 2017 +0100
----------------------------------------------------------------------
.../AutoGeneratedSelfSignedKeyStoreImpl.java | 27 ++++++++++----------
.../qpid/server/security/FileKeyStoreImpl.java | 4 ++-
.../server/security/FileTrustStoreImpl.java | 23 +++++++++++------
.../ManagedPeerCertificateTrustStoreImpl.java | 13 ++++++----
.../server/security/NonJavaKeyStoreImpl.java | 12 ++++-----
.../server/security/NonJavaTrustStoreImpl.java | 5 ++--
.../security/SiteSpecificTrustStoreImpl.java | 6 +++--
7 files changed, 53 insertions(+), 37 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/7d543956/broker-core/src/main/java/org/apache/qpid/server/security/AutoGeneratedSelfSignedKeyStoreImpl.java
----------------------------------------------------------------------
diff --git a/broker-core/src/main/java/org/apache/qpid/server/security/AutoGeneratedSelfSignedKeyStoreImpl.java b/broker-core/src/main/java/org/apache/qpid/server/security/AutoGeneratedSelfSignedKeyStoreImpl.java
index d9cd0ee..66d66e8 100644
--- a/broker-core/src/main/java/org/apache/qpid/server/security/AutoGeneratedSelfSignedKeyStoreImpl.java
+++ b/broker-core/src/main/java/org/apache/qpid/server/security/AutoGeneratedSelfSignedKeyStoreImpl.java
@@ -45,6 +45,7 @@ import java.security.cert.CertificateFactory;
import java.security.cert.X509Certificate;
import java.security.spec.InvalidKeySpecException;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Calendar;
import java.util.Collection;
import java.util.Collections;
@@ -103,21 +104,20 @@ public class AutoGeneratedSelfSignedKeyStoreImpl
private final EventLogger _eventLogger;
@ManagedAttributeField
- private String _keyAlgorithm;
+ private volatile String _keyAlgorithm;
@ManagedAttributeField
- private String _signatureAlgorithm;
+ private volatile String _signatureAlgorithm;
@ManagedAttributeField
- private int _keyLength;
+ private volatile int _keyLength;
@ManagedAttributeField
- private int _durationInMonths;
+ private volatile int _durationInMonths;
- private PrivateKey _privateKey;
- private X509Certificate _certificate;
- private KeyManager[] _keyManagers;
+ private volatile PrivateKey _privateKey;
+ private volatile X509Certificate _certificate;
+ private volatile KeyManager[] _keyManagers;
-
- private boolean _generated;
- private boolean _created;
+ private volatile boolean _generated;
+ private volatile boolean _created;
@ManagedObjectFactoryConstructor(conditionallyAvailable = true)
@@ -132,7 +132,8 @@ public class AutoGeneratedSelfSignedKeyStoreImpl
@Override
public KeyManager[] getKeyManagers() throws GeneralSecurityException
{
- return _keyManagers;
+ KeyManager[] keyManagers = _keyManagers;
+ return keyManagers == null ? new KeyManager[0] : Arrays.copyOf(keyManagers, keyManagers.length);
}
@Override
@@ -268,7 +269,7 @@ public class AutoGeneratedSelfSignedKeyStoreImpl
// verify that it is not in use
String storeName = getName();
- Collection<Port> ports = new ArrayList<Port>(_broker.getPorts());
+ Collection<Port> ports = new ArrayList<>(_broker.getPorts());
for (Port port : ports)
{
if (port.getKeyStore() == this)
@@ -543,7 +544,7 @@ public class AutoGeneratedSelfSignedKeyStoreImpl
throws CertificateEncodingException
{
_disposition = "attachment; filename=\"" + name + ".pem\"";
- StringBuffer certStringBuffer = new StringBuffer("-----BEGIN CERTIFICATE-----\n");
+ StringBuilder certStringBuffer = new StringBuilder("-----BEGIN CERTIFICATE-----\n");
String cert = DatatypeConverter.printBase64Binary(certificate.getEncoded());
int offset = 0;
while(cert.length()-offset > 64)
http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/7d543956/broker-core/src/main/java/org/apache/qpid/server/security/FileKeyStoreImpl.java
----------------------------------------------------------------------
diff --git a/broker-core/src/main/java/org/apache/qpid/server/security/FileKeyStoreImpl.java b/broker-core/src/main/java/org/apache/qpid/server/security/FileKeyStoreImpl.java
index b89eb90..4c5ed1f 100644
--- a/broker-core/src/main/java/org/apache/qpid/server/security/FileKeyStoreImpl.java
+++ b/broker-core/src/main/java/org/apache/qpid/server/security/FileKeyStoreImpl.java
@@ -29,6 +29,7 @@ import java.security.KeyStoreException;
import java.security.NoSuchAlgorithmException;
import java.security.UnrecoverableKeyException;
import java.security.cert.Certificate;
+import java.util.Arrays;
import java.util.Collections;
import java.util.Date;
import java.util.Map;
@@ -250,7 +251,8 @@ public class FileKeyStoreImpl extends AbstractKeyStore<FileKeyStoreImpl> impleme
kmf.init(ks, keyStoreCharPassword);
- return kmf.getKeyManagers();
+ KeyManager[] keyManagers = kmf.getKeyManagers();
+ return keyManagers == null ? new KeyManager[0] : Arrays.copyOf(keyManagers, keyManagers.length);
}
}
catch (IOException e)
http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/7d543956/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 ce6d037..527b668 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
@@ -31,6 +31,7 @@ import java.security.NoSuchAlgorithmException;
import java.security.UnrecoverableKeyException;
import java.security.cert.Certificate;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collection;
import java.util.Enumeration;
import java.util.Map;
@@ -60,16 +61,16 @@ public class FileTrustStoreImpl extends AbstractTrustStore<FileTrustStoreImpl> i
{
@ManagedAttributeField
- private String _trustStoreType;
+ private volatile String _trustStoreType;
@ManagedAttributeField
- private String _trustManagerFactoryAlgorithm;
+ private volatile String _trustManagerFactoryAlgorithm;
@ManagedAttributeField(afterSet = "postSetStoreUrl")
- private String _storeUrl;
- private String _path;
+ private volatile String _storeUrl;
+ private volatile String _path;
@ManagedAttributeField
- private boolean _peersOnly;
+ private volatile boolean _peersOnly;
@ManagedAttributeField
- private String _password;
+ private volatile String _password;
private volatile TrustManager[] _trustManagers;
private volatile Certificate[] _certificates;
@@ -227,13 +228,19 @@ public class FileTrustStoreImpl extends AbstractTrustStore<FileTrustStoreImpl> i
@Override
protected TrustManager[] getTrustManagersInternal() throws GeneralSecurityException
{
- return _trustManagers;
+ TrustManager[] trustManagers = _trustManagers;
+ if (trustManagers == null || trustManagers.length == 0)
+ {
+ throw new IllegalStateException("Truststore " + this + " defines no trust managers");
+ }
+ return Arrays.copyOf(trustManagers, trustManagers.length);
}
@Override
public Certificate[] getCertificates() throws GeneralSecurityException
{
- return _certificates;
+ Certificate[] certificates = _certificates;
+ return certificates == null ? new Certificate[0] : Arrays.copyOf(certificates, certificates.length);
}
private static URL getUrlFromString(String urlString) throws MalformedURLException
http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/7d543956/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 ba78084..8e02d32 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
@@ -26,6 +26,7 @@ import java.security.GeneralSecurityException;
import java.security.cert.Certificate;
import java.security.cert.X509Certificate;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
@@ -75,17 +76,19 @@ public class ManagedPeerCertificateTrustStoreImpl
@Override
protected TrustManager[] getTrustManagersInternal()
{
- if (_trustManagers == null || _trustManagers.length == 0)
+ TrustManager[] trustManagers = _trustManagers;
+ if (trustManagers == null || trustManagers.length == 0)
{
throw new IllegalStateException("Truststore " + this + " defines no trust managers");
}
- return _trustManagers;
+ return Arrays.copyOf(trustManagers, trustManagers.length);
}
@Override
public Certificate[] getCertificates()
{
- return _storedCertificates.toArray(new Certificate[_storedCertificates.size()]);
+ List<Certificate> storedCertificates = new ArrayList<>(_storedCertificates);
+ return storedCertificates.toArray(new Certificate[storedCertificates.size()]);
}
@StateTransition(currentState = {State.ACTIVE, State.ERRORED}, desiredState = State.DELETED)
@@ -121,7 +124,7 @@ public class ManagedPeerCertificateTrustStoreImpl
TrustManagerFactory tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
tmf.init(inMemoryKeyStore);
- final Collection<TrustManager> trustManagersCol = new ArrayList<TrustManager>();
+ final Collection<TrustManager> trustManagersCol = new ArrayList<>();
final QpidMultipleTrustManager mulTrustManager = new QpidMultipleTrustManager();
TrustManager[] delegateManagers = tmf.getTrustManagers();
for (TrustManager tm : delegateManagers)
@@ -182,7 +185,7 @@ public class ManagedPeerCertificateTrustStoreImpl
{
if (!certsToRemove.containsKey(cert.getIssuerName()))
{
- certsToRemove.put(cert.getIssuerName(), new HashSet<BigInteger>());
+ certsToRemove.put(cert.getIssuerName(), new HashSet<>());
}
certsToRemove.get(cert.getIssuerName()).add(new BigInteger(cert.getSerialNumber()));
}
http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/7d543956/broker-core/src/main/java/org/apache/qpid/server/security/NonJavaKeyStoreImpl.java
----------------------------------------------------------------------
diff --git a/broker-core/src/main/java/org/apache/qpid/server/security/NonJavaKeyStoreImpl.java b/broker-core/src/main/java/org/apache/qpid/server/security/NonJavaKeyStoreImpl.java
index 8ff0b67..ff84694 100644
--- a/broker-core/src/main/java/org/apache/qpid/server/security/NonJavaKeyStoreImpl.java
+++ b/broker-core/src/main/java/org/apache/qpid/server/security/NonJavaKeyStoreImpl.java
@@ -66,11 +66,11 @@ public class NonJavaKeyStoreImpl extends AbstractKeyStore<NonJavaKeyStoreImpl> i
private static final Logger LOGGER = LoggerFactory.getLogger(NonJavaKeyStoreImpl.class);
@ManagedAttributeField( afterSet = "updateKeyManagers" )
- private String _privateKeyUrl;
+ private volatile String _privateKeyUrl;
@ManagedAttributeField( afterSet = "updateKeyManagers" )
- private String _certificateUrl;
+ private volatile String _certificateUrl;
@ManagedAttributeField( afterSet = "updateKeyManagers" )
- private String _intermediateCertificateUrl;
+ private volatile String _intermediateCertificateUrl;
private volatile KeyManager[] _keyManagers = new KeyManager[0];
@@ -81,7 +81,7 @@ public class NonJavaKeyStoreImpl extends AbstractKeyStore<NonJavaKeyStoreImpl> i
Handler.register();
}
- private X509Certificate _certificate;
+ private volatile X509Certificate _certificate;
@ManagedObjectFactoryConstructor
public NonJavaKeyStoreImpl(final Map<String, Object> attributes, Broker<?> broker)
@@ -155,8 +155,8 @@ public class NonJavaKeyStoreImpl extends AbstractKeyStore<NonJavaKeyStoreImpl> i
@Override
public KeyManager[] getKeyManagers() throws GeneralSecurityException
{
-
- return _keyManagers;
+ KeyManager[] keyManagers = _keyManagers;
+ return keyManagers == null ? new KeyManager[0] : Arrays.copyOf(keyManagers, keyManagers.length);
}
@Override
http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/7d543956/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 152e4d3..287ca58 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
@@ -88,11 +88,12 @@ public class NonJavaTrustStoreImpl
@Override
protected TrustManager[] getTrustManagersInternal() throws GeneralSecurityException
{
- if (_trustManagers == null || _trustManagers.length == 0)
+ TrustManager[] trustManagers = _trustManagers;
+ if (trustManagers == null || trustManagers.length == 0)
{
throw new IllegalStateException("Truststore " + this + " defines no trust managers");
}
- return _trustManagers;
+ return Arrays.copyOf(trustManagers, trustManagers.length);
}
@Override
http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/7d543956/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 38439e5..952c74e 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
@@ -31,6 +31,7 @@ import java.security.cert.CertificateEncodingException;
import java.security.cert.CertificateException;
import java.security.cert.CertificateFactory;
import java.security.cert.X509Certificate;
+import java.util.Arrays;
import java.util.Map;
import java.util.concurrent.Callable;
import java.util.concurrent.Executors;
@@ -148,11 +149,12 @@ public class SiteSpecificTrustStoreImpl
@Override
protected TrustManager[] getTrustManagersInternal() throws GeneralSecurityException
{
- if (_trustManagers == null || _trustManagers.length == 0)
+ TrustManager[] trustManagers = _trustManagers;
+ if (trustManagers == null || trustManagers.length == 0)
{
throw new IllegalStateException("Truststore " + this + " defines no trust managers");
}
- return _trustManagers;
+ return Arrays.copyOf(trustManagers, trustManagers.length);
}
@Override
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org