Remove ServerTrustManager

The implementation of ServerTrustManger contains a security
vulnerability, which could lead to unauthorized certificates being
erroneously trusted. SMACK-410
This commit is contained in:
Florian Schmaus 2014-02-10 12:07:39 +01:00
parent 5f5805cd1c
commit 93030c218c
3 changed files with 1 additions and 529 deletions

View File

@ -28,7 +28,6 @@ import javax.net.SocketFactory;
import javax.net.ssl.SSLContext;
import javax.security.auth.callback.CallbackHandler;
import java.io.File;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
@ -55,17 +54,9 @@ public class ConnectionConfiguration implements Cloneable {
private int port;
protected List<HostAddress> hostAddresses;
private String truststorePath;
private String truststoreType;
private String truststorePassword;
private String keystorePath;
private String keystoreType;
private String pkcs11Library;
private boolean verifyChainEnabled = false;
private boolean verifyRootCAEnabled = false;
private boolean selfSignedCertificateEnabled = false;
private boolean expiredCertificatesCheckEnabled = false;
private boolean notMatchingDomainCheckEnabled = false;
private SSLContext customSSLContext;
private boolean compressionEnabled = false;
@ -203,18 +194,6 @@ public class ConnectionConfiguration implements Cloneable {
this.serviceName = serviceName;
this.proxy = proxy;
// Build the default path to the cacert truststore file. By default we are
// going to use the file located in $JREHOME/lib/security/cacerts.
String javaHome = System.getProperty("java.home");
StringBuilder buffer = new StringBuilder();
buffer.append(javaHome).append(File.separator).append("lib");
buffer.append(File.separator).append("security");
buffer.append(File.separator).append("cacerts");
truststorePath = buffer.toString();
// Set the default store type
truststoreType = "jks";
// Set the default password of the cacert file that is "changeit"
truststorePassword = "changeit";
keystorePath = System.getProperty("javax.net.ssl.keyStore");
keystoreType = "jks";
pkcs11Library = "pkcs11.config";
@ -287,66 +266,6 @@ public class ConnectionConfiguration implements Cloneable {
this.securityMode = securityMode;
}
/**
* Retuns the path to the trust store file. The trust store file contains the root
* certificates of several well known CAs. By default, will attempt to use the
* the file located in $JREHOME/lib/security/cacerts.
*
* @return the path to the truststore file.
*/
public String getTruststorePath() {
return truststorePath;
}
/**
* Sets the path to the trust store file. The truststore file contains the root
* certificates of several well?known CAs. By default Smack is going to use
* the file located in $JREHOME/lib/security/cacerts.
*
* @param truststorePath the path to the truststore file.
*/
public void setTruststorePath(String truststorePath) {
this.truststorePath = truststorePath;
}
/**
* Returns the trust store type, or <tt>null</tt> if it's not set.
*
* @return the trust store type.
*/
public String getTruststoreType() {
return truststoreType;
}
/**
* Sets the trust store type.
*
* @param truststoreType the trust store type.
*/
public void setTruststoreType(String truststoreType) {
this.truststoreType = truststoreType;
}
/**
* Returns the password to use to access the trust store file. It is assumed that all
* certificates share the same password in the trust store.
*
* @return the password to use to access the truststore file.
*/
public String getTruststorePassword() {
return truststorePassword;
}
/**
* Sets the password to use to access the trust store file. It is assumed that all
* certificates share the same password in the trust store.
*
* @param truststorePassword the password to use to access the truststore file.
*/
public void setTruststorePassword(String truststorePassword) {
this.truststorePassword = truststorePassword;
}
/**
* Retuns the path to the keystore file. The key store file contains the
* certificates that may be used to authenticate the client to the server,
@ -408,110 +327,6 @@ public class ConnectionConfiguration implements Cloneable {
this.pkcs11Library = pkcs11Library;
}
/**
* Returns true if the whole chain of certificates presented by the server are going to
* be checked. By default the certificate chain is not verified.
*
* @return true if the whole chaing of certificates presented by the server are going to
* be checked.
*/
public boolean isVerifyChainEnabled() {
return verifyChainEnabled;
}
/**
* Sets if the whole chain of certificates presented by the server are going to
* be checked. By default the certificate chain is not verified.
*
* @param verifyChainEnabled if the whole chaing of certificates presented by the server
* are going to be checked.
*/
public void setVerifyChainEnabled(boolean verifyChainEnabled) {
this.verifyChainEnabled = verifyChainEnabled;
}
/**
* Returns true if root CA checking is going to be done. By default checking is disabled.
*
* @return true if root CA checking is going to be done.
*/
public boolean isVerifyRootCAEnabled() {
return verifyRootCAEnabled;
}
/**
* Sets if root CA checking is going to be done. By default checking is disabled.
*
* @param verifyRootCAEnabled if root CA checking is going to be done.
*/
public void setVerifyRootCAEnabled(boolean verifyRootCAEnabled) {
this.verifyRootCAEnabled = verifyRootCAEnabled;
}
/**
* Returns true if self-signed certificates are going to be accepted. By default
* this option is disabled.
*
* @return true if self-signed certificates are going to be accepted.
*/
public boolean isSelfSignedCertificateEnabled() {
return selfSignedCertificateEnabled;
}
/**
* Sets if self-signed certificates are going to be accepted. By default
* this option is disabled.
*
* @param selfSignedCertificateEnabled if self-signed certificates are going to be accepted.
*/
public void setSelfSignedCertificateEnabled(boolean selfSignedCertificateEnabled) {
this.selfSignedCertificateEnabled = selfSignedCertificateEnabled;
}
/**
* Returns true if certificates presented by the server are going to be checked for their
* validity. By default certificates are not verified.
*
* @return true if certificates presented by the server are going to be checked for their
* validity.
*/
public boolean isExpiredCertificatesCheckEnabled() {
return expiredCertificatesCheckEnabled;
}
/**
* Sets if certificates presented by the server are going to be checked for their
* validity. By default certificates are not verified.
*
* @param expiredCertificatesCheckEnabled if certificates presented by the server are going
* to be checked for their validity.
*/
public void setExpiredCertificatesCheckEnabled(boolean expiredCertificatesCheckEnabled) {
this.expiredCertificatesCheckEnabled = expiredCertificatesCheckEnabled;
}
/**
* Returns true if certificates presented by the server are going to be checked for their
* domain. By default certificates are not verified.
*
* @return true if certificates presented by the server are going to be checked for their
* domain.
*/
public boolean isNotMatchingDomainCheckEnabled() {
return notMatchingDomainCheckEnabled;
}
/**
* Sets if certificates presented by the server are going to be checked for their
* domain. By default certificates are not verified.
*
* @param notMatchingDomainCheckEnabled if certificates presented by the server are going
* to be checked for their domain.
*/
public void setNotMatchingDomainCheckEnabled(boolean notMatchingDomainCheckEnabled) {
this.notMatchingDomainCheckEnabled = notMatchingDomainCheckEnabled;
}
/**
* Gets the custom SSLContext for SSL sockets. This is null by default.
*

View File

@ -1,342 +0,0 @@
/**
* $RCSfile$
* $Revision$
* $Date$
*
* Copyright 2003-2005 Jive Software.
*
* All rights reserved. Licensed 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.jivesoftware.smack;
import javax.net.ssl.X509TrustManager;
import java.io.FileInputStream;
import java.io.InputStream;
import java.io.IOException;
import java.security.*;
import java.security.cert.CertificateException;
import java.security.cert.CertificateParsingException;
import java.security.cert.X509Certificate;
import java.util.*;
import java.util.logging.Logger;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
/**
* Trust manager that checks all certificates presented by the server. This class
* is used during TLS negotiation. It is possible to disable/enable some or all checkings
* by configuring the {@link ConnectionConfiguration}. The truststore file that contains
* knows and trusted CA root certificates can also be configure in {@link ConnectionConfiguration}.
*
* @author Gaston Dombiak
*/
class ServerTrustManager implements X509TrustManager {
private static Logger log = Logger.getLogger(ServerTrustManager.class.getName());
private static Pattern cnPattern = Pattern.compile("(?i)(cn=)([^,]*)");
private ConnectionConfiguration configuration;
/**
* Holds the domain of the remote server we are trying to connect
*/
private String server;
private KeyStore trustStore;
private static Map<KeyStoreOptions, KeyStore> stores = new HashMap<KeyStoreOptions, KeyStore>();
public ServerTrustManager(String server, ConnectionConfiguration configuration) {
this.configuration = configuration;
this.server = server;
InputStream in = null;
char[] password = null;
synchronized (stores) {
KeyStoreOptions options = new KeyStoreOptions(configuration.getTruststoreType(),
configuration.getTruststorePath(), configuration.getTruststorePassword());
if (stores.containsKey(options)) {
trustStore = stores.get(options);
} else {
try {
trustStore = KeyStore.getInstance(options.getType());
if (options.getPassword() != null) {
password = options.getPassword().toCharArray();
}
if (options.getPath() == null) {
trustStore.load(null, password);
}
else {
in = new FileInputStream(options.getPath());
trustStore.load(in, password);
}
} catch (Exception e) {
trustStore = null;
e.printStackTrace();
} finally {
if (in != null) {
try {
in.close();
} catch (IOException ioe) {
// Ignore.
}
}
}
stores.put(options, trustStore);
}
if (trustStore == null)
// Disable root CA checking
configuration.setVerifyRootCAEnabled(false);
}
}
public X509Certificate[] getAcceptedIssuers() {
return new X509Certificate[0];
}
public void checkClientTrusted(X509Certificate[] arg0, String arg1)
throws CertificateException {
}
public void checkServerTrusted(X509Certificate[] x509Certificates, String arg1)
throws CertificateException {
int nSize = x509Certificates.length;
List<String> peerIdentities = getPeerIdentity(x509Certificates[0]);
if (configuration.isVerifyChainEnabled()) {
// Working down the chain, for every certificate in the chain,
// verify that the subject of the certificate is the issuer of the
// next certificate in the chain.
Principal principalLast = null;
for (int i = nSize -1; i >= 0 ; i--) {
X509Certificate x509certificate = x509Certificates[i];
Principal principalIssuer = x509certificate.getIssuerDN();
Principal principalSubject = x509certificate.getSubjectDN();
if (principalLast != null) {
if (principalIssuer.equals(principalLast)) {
try {
PublicKey publickey =
x509Certificates[i + 1].getPublicKey();
x509Certificates[i].verify(publickey);
}
catch (GeneralSecurityException generalsecurityexception) {
throw new CertificateException(
"signature verification failed of " + peerIdentities);
}
}
else {
throw new CertificateException(
"subject/issuer verification failed of " + peerIdentities);
}
}
principalLast = principalSubject;
}
}
if (configuration.isVerifyRootCAEnabled()) {
// Verify that the the last certificate in the chain was issued
// by a third-party that the client trusts.
boolean trusted = false;
try {
trusted = trustStore.getCertificateAlias(x509Certificates[nSize - 1]) != null;
if (!trusted && nSize == 1 && configuration.isSelfSignedCertificateEnabled())
{
log.info("Accepting self-signed certificate of remote server: " + peerIdentities);
trusted = true;
}
}
catch (KeyStoreException e) {
e.printStackTrace();
}
if (!trusted) {
throw new CertificateException("root certificate not trusted of " + peerIdentities);
}
}
if (configuration.isNotMatchingDomainCheckEnabled()) {
// Verify that the first certificate in the chain corresponds to
// the server we desire to authenticate.
// Check if the certificate uses a wildcard indicating that subdomains are valid
if (peerIdentities.size() == 1 && peerIdentities.get(0).startsWith("*.")) {
// Remove the wildcard
String peerIdentity = peerIdentities.get(0).replace("*.", "");
// Check if the requested subdomain matches the certified domain
if (!server.endsWith(peerIdentity)) {
throw new CertificateException("target verification failed of " + peerIdentities);
}
}
else if (!peerIdentities.contains(server)) {
throw new CertificateException("target verification failed of " + peerIdentities);
}
}
if (configuration.isExpiredCertificatesCheckEnabled()) {
// For every certificate in the chain, verify that the certificate
// is valid at the current time.
Date date = new Date();
for (int i = 0; i < nSize; i++) {
try {
x509Certificates[i].checkValidity(date);
}
catch (GeneralSecurityException generalsecurityexception) {
throw new CertificateException("invalid date of " + server);
}
}
}
}
/**
* Returns the identity of the remote server as defined in the specified certificate. The
* identity is defined in the subjectDN of the certificate and it can also be defined in
* the subjectAltName extension of type "xmpp". When the extension is being used then the
* identity defined in the extension in going to be returned. Otherwise, the value stored in
* the subjectDN is returned.
*
* @param x509Certificate the certificate the holds the identity of the remote server.
* @return the identity of the remote server as defined in the specified certificate.
*/
public static List<String> getPeerIdentity(X509Certificate x509Certificate) {
// Look the identity in the subjectAltName extension if available
List<String> names = getSubjectAlternativeNames(x509Certificate);
if (names.isEmpty()) {
String name = x509Certificate.getSubjectDN().getName();
Matcher matcher = cnPattern.matcher(name);
if (matcher.find()) {
name = matcher.group(2);
}
// Create an array with the unique identity
names = new ArrayList<String>();
names.add(name);
}
return names;
}
/**
* Returns the JID representation of an XMPP entity contained as a SubjectAltName extension
* in the certificate. If none was found then return <tt>null</tt>.
*
* @param certificate the certificate presented by the remote entity.
* @return the JID representation of an XMPP entity contained as a SubjectAltName extension
* in the certificate. If none was found then return <tt>null</tt>.
*/
private static List<String> getSubjectAlternativeNames(X509Certificate certificate) {
List<String> identities = new ArrayList<String>();
try {
Collection<List<?>> altNames = certificate.getSubjectAlternativeNames();
// Check that the certificate includes the SubjectAltName extension
if (altNames == null) {
return Collections.emptyList();
}
// Use the type OtherName to search for the certified server name
/*for (List item : altNames) {
Integer type = (Integer) item.get(0);
if (type == 0) {
// Type OtherName found so return the associated value
try {
// Value is encoded using ASN.1 so decode it to get the server's identity
ASN1InputStream decoder = new ASN1InputStream((byte[]) item.toArray()[1]);
DEREncodable encoded = decoder.readObject();
encoded = ((DERSequence) encoded).getObjectAt(1);
encoded = ((DERTaggedObject) encoded).getObject();
encoded = ((DERTaggedObject) encoded).getObject();
String identity = ((DERUTF8String) encoded).getString();
// Add the decoded server name to the list of identities
identities.add(identity);
}
catch (UnsupportedEncodingException e) {
// Ignore
}
catch (IOException e) {
// Ignore
}
catch (Exception e) {
e.printStackTrace();
}
}
// Other types are not good for XMPP so ignore them
log.info("SubjectAltName of invalid type found: " + certificate);
}*/
}
catch (CertificateParsingException e) {
e.printStackTrace();
}
return identities;
}
private static class KeyStoreOptions {
private final String type;
private final String path;
private final String password;
public KeyStoreOptions(String type, String path, String password) {
super();
this.type = type;
this.path = path;
this.password = password;
}
public String getType() {
return type;
}
public String getPath() {
return path;
}
public String getPassword() {
return password;
}
@Override
public int hashCode() {
final int prime = 31;
int result = 1;
result = prime * result + ((password == null) ? 0 : password.hashCode());
result = prime * result + ((path == null) ? 0 : path.hashCode());
result = prime * result + ((type == null) ? 0 : type.hashCode());
return result;
}
@Override
public boolean equals(Object obj) {
if (this == obj)
return true;
if (obj == null)
return false;
if (getClass() != obj.getClass())
return false;
KeyStoreOptions other = (KeyStoreOptions) obj;
if (password == null) {
if (other.password != null)
return false;
} else if (!password.equals(other.password))
return false;
if (path == null) {
if (other.path != null)
return false;
} else if (!path.equals(other.path))
return false;
if (type == null) {
if (other.type != null)
return false;
} else if (!type.equals(other.type))
return false;
return true;
}
}
}

View File

@ -844,8 +844,7 @@ public class XMPPConnection extends Connection {
// Verify certificate presented by the server
if (context == null) {
context = SSLContext.getInstance("TLS");
context.init(kms, new javax.net.ssl.TrustManager[] { new ServerTrustManager(getServiceName(), config) },
new java.security.SecureRandom());
context.init(kms, null, new java.security.SecureRandom());
}
Socket plain = socket;
// Secure the plain connection