Commit b1c84166 authored by Dave Cridland's avatar Dave Cridland

OF-405 Addressing comments from Tom Evans

 - Always check validity of EE Cert
 - Change variable name (and flip sense).
parent a17da995
...@@ -196,15 +196,14 @@ public class SASLAuthentication { ...@@ -196,15 +196,14 @@ public class SASLAuthentication {
if (session instanceof IncomingServerSession) { if (session instanceof IncomingServerSession) {
// Server connections don't follow the same rules as clients // Server connections don't follow the same rules as clients
if (session.isSecure()) { if (session.isSecure()) {
boolean usingSelfSigned = true; boolean haveTrustedCertificate = false;
try { try {
X509Certificate trusted = CertificateManager.getEndEntityCertificate(((LocalSession)session).getConnection().getPeerCertificates(), SSLConfig.getKeyStore(), SSLConfig.gets2sTrustStore()); X509Certificate trusted = CertificateManager.getEndEntityCertificate(((LocalSession)session).getConnection().getPeerCertificates(), SSLConfig.getKeyStore(), SSLConfig.gets2sTrustStore());
usingSelfSigned = trusted == null; haveTrustedCertificate = trusted != null;
} catch (IOException ex) { } catch (IOException ex) {
Log.warn("Exception occurred while trying to determine whether remote certificate is trusted. Treating as untrusted.", ex); Log.warn("Exception occurred while trying to determine whether remote certificate is trusted. Treating as untrusted.", ex);
usingSelfSigned = true;
} }
if (!usingSelfSigned) { if (haveTrustedCertificate) {
// Offer SASL EXTERNAL only if TLS has already been negotiated and the peer has a trusted cert. // Offer SASL EXTERNAL only if TLS has already been negotiated and the peer has a trusted cert.
Element mechanism = mechs.addElement("mechanism"); Element mechanism = mechs.addElement("mechanism");
mechanism.setText("EXTERNAL"); mechanism.setText("EXTERNAL");
......
...@@ -44,6 +44,7 @@ import java.security.cert.CertPathValidator; ...@@ -44,6 +44,7 @@ import java.security.cert.CertPathValidator;
import java.security.cert.CertPathValidatorException; import java.security.cert.CertPathValidatorException;
import java.security.cert.CertStore; import java.security.cert.CertStore;
import java.security.cert.Certificate; import java.security.cert.Certificate;
import java.security.cert.CertificateException;
import java.security.cert.CertificateFactory; import java.security.cert.CertificateFactory;
import java.security.cert.CertificateParsingException; import java.security.cert.CertificateParsingException;
import java.security.cert.CollectionCertStoreParameters; import java.security.cert.CollectionCertStoreParameters;
...@@ -235,6 +236,12 @@ public class CertificateManager { ...@@ -235,6 +236,12 @@ public class CertificateManager {
return null; return null;
} }
X509Certificate first = (X509Certificate) chain[0]; X509Certificate first = (X509Certificate) chain[0];
try {
first.checkValidity();
} catch(CertificateException e) {
Log.warn("EE Certificate not valid: " + e.getMessage());
return null;
}
if (chain.length == 1 if (chain.length == 1
&& first.getSubjectX500Principal().equals(first.getIssuerX500Principal())) { && first.getSubjectX500Principal().equals(first.getIssuerX500Principal())) {
// Chain is single cert, and self-signed. // Chain is single cert, and self-signed.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment