From 9095e29a2b3b063b4ab5f6344f2b1598222ae457 Mon Sep 17 00:00:00 2001 From: Tim Vernum Date: Mon, 4 Nov 2019 16:04:13 -0500 Subject: [PATCH 1/6] Improved diagnostics for TLS trust failures - Improves HTTP client hostname verification failure messages - Adds "DiagnosticTrustManager" which logs certificate information when trust cannot be established (hostname failure, CA path failure, etc) These diagnostic messages are designed so that many common TLS problems can be diagnosed based solely (or primarily) on the elasticsearch logs. --- .../common/ssl/DiagnosticTrustManager.java | 164 +++++++ .../elasticsearch/common/ssl/PemUtils.java | 10 +- .../common/ssl/SslDiagnostics.java | 373 ++++++++++++++++ .../org/elasticsearch/common/ssl/SslUtil.java | 73 +++ .../common/ssl/SslDiagnosticsTests.java | 416 ++++++++++++++++++ .../src/test/resources/certs/README.txt | 6 + .../src/test/resources/certs/ca1-b/ca.crt | 19 + .../src/test/resources/certs/ca1-b/ca.key | 27 ++ x-pack/plugin/core/build.gradle | 1 + .../xpack/core/ssl/CertParsingUtils.java | 28 +- .../xpack/core/ssl/SSLService.java | 66 ++- .../test/http/MockWebServer.java | 17 +- .../oidc/OpenIdConnectAuthenticator.java | 6 +- .../xpack/security/authc/saml/SamlRealm.java | 5 +- ...orMessageCertificateVerificationTests.java | 207 +++++++++ ...sts.java => SSLErrorMessageFileTests.java} | 9 +- .../README.txt | 34 +- .../ca1.crt | 0 .../ca1.jks | Bin .../ca1.p12 | Bin .../cert1a.crt | 0 .../cert1a.jks | Bin .../cert1a.key | 0 .../cert1a.p12 | Bin .../SSLErrorMessageTests/not-this-host.crt | 21 + .../SSLErrorMessageTests/not-this-host.key | 27 ++ .../xpack/watcher/common/http/HttpClient.java | 5 +- 27 files changed, 1471 insertions(+), 43 deletions(-) create mode 100644 libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/DiagnosticTrustManager.java create mode 100644 libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/SslDiagnostics.java create mode 100644 libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/SslUtil.java create mode 100644 libs/ssl-config/src/test/java/org/elasticsearch/common/ssl/SslDiagnosticsTests.java create mode 100644 libs/ssl-config/src/test/resources/certs/ca1-b/ca.crt create mode 100644 libs/ssl-config/src/test/resources/certs/ca1-b/ca.key create mode 100644 x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLErrorMessageCertificateVerificationTests.java rename x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/{SSLErrorMessageTests.java => SSLErrorMessageFileTests.java} (98%) rename x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/{SSLServiceErrorMessageTests => SSLErrorMessageTests}/README.txt (79%) rename x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/{SSLServiceErrorMessageTests => SSLErrorMessageTests}/ca1.crt (100%) rename x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/{SSLServiceErrorMessageTests => SSLErrorMessageTests}/ca1.jks (100%) rename x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/{SSLServiceErrorMessageTests => SSLErrorMessageTests}/ca1.p12 (100%) rename x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/{SSLServiceErrorMessageTests => SSLErrorMessageTests}/cert1a.crt (100%) rename x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/{SSLServiceErrorMessageTests => SSLErrorMessageTests}/cert1a.jks (100%) rename x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/{SSLServiceErrorMessageTests => SSLErrorMessageTests}/cert1a.key (100%) rename x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/{SSLServiceErrorMessageTests => SSLErrorMessageTests}/cert1a.p12 (100%) create mode 100644 x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLErrorMessageTests/not-this-host.crt create mode 100644 x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLErrorMessageTests/not-this-host.key diff --git a/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/DiagnosticTrustManager.java b/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/DiagnosticTrustManager.java new file mode 100644 index 0000000000000..b38b5e4e284c6 --- /dev/null +++ b/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/DiagnosticTrustManager.java @@ -0,0 +1,164 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch 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.elasticsearch.common.ssl; + +import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLSession; +import javax.net.ssl.SSLSocket; +import javax.net.ssl.X509ExtendedTrustManager; +import java.net.Socket; +import java.security.GeneralSecurityException; +import java.security.cert.CertificateException; +import java.security.cert.X509Certificate; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.function.Supplier; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static org.elasticsearch.common.ssl.SslDiagnostics.getTrustDiagnosticFailure; + +public final class DiagnosticTrustManager extends X509ExtendedTrustManager { + + + /** + * This interface exists because the ssl-config library does not depend on log4j, however the whole purpose of this class is to log + * diagnostic messages, so it must be provided with a function by which it can do that. + */ + @FunctionalInterface + public interface DiagnosticLogger { + void warning(String message, GeneralSecurityException cause); + } + + + private final X509ExtendedTrustManager delegate; + private final Supplier contextName; + private final DiagnosticLogger logger; + private final Map> issuers; + + /** + * @param contextName The descriptive name of the context that this trust manager is operating in (e.g "xpack.security.http.ssl") + * @param logger For uses that depend on log4j, it is recommended that this parameter be equivalent to + * {@code LogManager.getLogger(DiagnosticTrustManager.class)::warn} + */ + public DiagnosticTrustManager(X509ExtendedTrustManager delegate, Supplier contextName, DiagnosticLogger logger) { + this.delegate = delegate; + this.contextName = contextName; + this.logger = logger; + this.issuers = Stream.of(delegate.getAcceptedIssuers()) + .collect(Collectors.toMap(cert -> cert.getSubjectX500Principal().getName(), List::of, + (List a, List b) -> { + final ArrayList list = new ArrayList<>(a.size() + b.size()); + list.addAll(a); + list.addAll(b); + return list; + })); + } + + @Override + public void checkClientTrusted(X509Certificate[] chain, String authType, Socket socket) throws CertificateException { + try { + delegate.checkClientTrusted(chain, authType, socket); + } catch (CertificateException e) { + diagnose(e, chain, SslDiagnostics.PeerType.CLIENT, session(socket)); + throw e; + } + } + + @Override + public void checkServerTrusted(X509Certificate[] chain, String authType, Socket socket) throws CertificateException { + try { + delegate.checkServerTrusted(chain, authType, socket); + } catch (CertificateException e) { + diagnose(e, chain, SslDiagnostics.PeerType.SERVER, session(socket)); + throw e; + } + } + + @Override + public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngine engine) throws CertificateException { + try { + delegate.checkClientTrusted(chain, authType, engine); + } catch (CertificateException e) { + diagnose(e, chain, SslDiagnostics.PeerType.CLIENT, session(engine)); + throw e; + } + } + + @Override + public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngine engine) throws CertificateException { + try { + delegate.checkServerTrusted(chain, authType, engine); + } catch (CertificateException e) { + diagnose(e, chain, SslDiagnostics.PeerType.SERVER, session(engine)); + throw e; + } + } + + @Override + public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException { + try { + delegate.checkClientTrusted(chain, authType); + } catch (CertificateException e) { + diagnose(e, chain, SslDiagnostics.PeerType.CLIENT, null); + throw e; + } + } + + @Override + public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException { + try { + delegate.checkServerTrusted(chain, authType); + } catch (CertificateException e) { + diagnose(e, chain, SslDiagnostics.PeerType.SERVER, null); + throw e; + } + } + + @Override + public X509Certificate[] getAcceptedIssuers() { + return delegate.getAcceptedIssuers(); + } + + private void diagnose(CertificateException cause, X509Certificate[] chain, SslDiagnostics.PeerType peerType, SSLSession session) { + final String diagnostic = getTrustDiagnosticFailure(chain, peerType, session, this.contextName.get(), this.issuers); + logger.warning(diagnostic, cause); + } + + private SSLSession session(Socket socket) { + if (socket instanceof SSLSocket) { + final SSLSocket ssl = (SSLSocket) socket; + final SSLSession handshakeSession = ssl.getHandshakeSession(); + if (handshakeSession == null) { + return ssl.getSession(); + } else { + return handshakeSession; + } + } else { + return null; + } + } + + private SSLSession session(SSLEngine engine) { + return engine.getHandshakeSession(); + } + +} diff --git a/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/PemUtils.java b/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/PemUtils.java index 79943870c52f6..45476f125d75b 100644 --- a/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/PemUtils.java +++ b/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/PemUtils.java @@ -41,7 +41,6 @@ import java.security.KeyFactory; import java.security.KeyPairGenerator; import java.security.MessageDigest; -import java.security.NoSuchAlgorithmException; import java.security.PrivateKey; import java.security.cert.Certificate; import java.security.cert.CertificateException; @@ -452,7 +451,7 @@ private static Cipher getCipherFromParameters(String dekHeaderValue, char[] pass */ private static byte[] generateOpenSslKey(char[] password, byte[] salt, int keyLength) { byte[] passwordBytes = CharArrays.toUtf8Bytes(password); - MessageDigest md5 = messageDigest("md5"); + MessageDigest md5 = SslUtil.messageDigest("md5"); byte[] key = new byte[keyLength]; int copied = 0; int remaining; @@ -603,11 +602,4 @@ static List readCertificates(Collection certPaths) throws Cer return certificates; } - private static MessageDigest messageDigest(String digestAlgorithm) { - try { - return MessageDigest.getInstance(digestAlgorithm); - } catch (NoSuchAlgorithmException e) { - throw new SslConfigException("unexpected exception creating MessageDigest instance for [" + digestAlgorithm + "]", e); - } - } } diff --git a/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/SslDiagnostics.java b/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/SslDiagnostics.java new file mode 100644 index 0000000000000..5100bf9614a5b --- /dev/null +++ b/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/SslDiagnostics.java @@ -0,0 +1,373 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch 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.elasticsearch.common.ssl; + +import org.elasticsearch.common.Nullable; + +import javax.net.ssl.SSLSession; +import java.security.cert.CertificateEncodingException; +import java.security.cert.CertificateParsingException; +import java.security.cert.X509Certificate; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.stream.Collectors; + +public class SslDiagnostics { + + public static List describeValidHostnames(X509Certificate certificate) { + try { + final Collection> names = certificate.getSubjectAlternativeNames(); + if (names == null || names.isEmpty()) { + return Collections.emptyList(); + } + final List description = new ArrayList<>(names.size()); + for (List pair : names) { + if (pair == null || pair.size() != 2) { + continue; + } + if ((pair.get(0) instanceof Integer) == false || (pair.get(1) instanceof String) == false) { + continue; + } + final int type = ((Integer) pair.get(0)).intValue(); + final String name = (String) pair.get(1); + switch (type) { + case 2: + description.add("DNS:" + name); + break; + case 7: + description.add("IP:" + name); + break; + // Other type, we don't care. + default: + break; + } + } + return description; + } catch (CertificateParsingException e) { + return Collections.emptyList(); + } + } + + public enum PeerType { + CLIENT, SERVER + } + + private static class IssuerTrust { + private final List issuerCerts; + private final boolean verified; + + private IssuerTrust(List issuerCerts, boolean verified) { + this.issuerCerts = issuerCerts; + this.verified = verified; + } + + private static IssuerTrust noMatchingCertificate() { + return new IssuerTrust(null, false); + } + + private static IssuerTrust verifiedCertificates(List issuerCert) { + return new IssuerTrust(issuerCert, true); + } + + private static IssuerTrust unverifiedCertificates(List issuerCert) { + return new IssuerTrust(issuerCert, false); + } + + boolean isVerified() { + return issuerCerts != null && verified; + } + + boolean foundCertificateForDn() { + return issuerCerts != null; + } + } + + private static class CertificateTrust { + private final List certificates; + private final boolean match; + private final boolean identicalCertificate; + + private CertificateTrust(List certificates, boolean match, boolean identicalCertificate) { + this.certificates = certificates; + this.match = match; + this.identicalCertificate = identicalCertificate; + } + + private static CertificateTrust noMatchingIssuer() { + return new CertificateTrust(null, false, false); + } + + private static CertificateTrust sameCertificate(X509Certificate issuerCert) { + return new CertificateTrust(List.of(issuerCert), true, true); + } + + private static CertificateTrust samePublicKey(List issuerCerts) { + return new CertificateTrust(issuerCerts, true, false); + } + + private static CertificateTrust nonMatchingCertificates(List certificates) { + return new CertificateTrust(certificates, false, false); + } + + boolean hasCertificates() { + return certificates != null && certificates.isEmpty() == false; + } + + boolean isTrusted() { + return hasCertificates() && match; + } + + boolean isSameCertificate() { + return isTrusted() && identicalCertificate; + } + } + + /** + * @param contextName The descriptive name of this SSL context (e.g. "xpack.security.transport.ssl") + * @param trustedIssuers A Map of DN to Certificate, for the issuers that were trusted in the context in which this failure occurred + * (see {@link javax.net.ssl.X509TrustManager#getAcceptedIssuers()}) + */ + public static String getTrustDiagnosticFailure(X509Certificate[] chain, PeerType peerType, SSLSession session, + String contextName, @Nullable Map> trustedIssuers) { + final String peerAddress = session == null ? "" : session.getPeerHost(); + + final StringBuilder message = new StringBuilder("failed to establish trust with [") + .append(peerType.name()) + .append("] at [") + .append(peerAddress) + .append("]; "); + + if (chain == null || chain.length == 0) { + message.append("the ").append(peerType.name().toLowerCase(Locale.ROOT)).append(" did not provide a certificate"); + return message.toString(); + } + + final X509Certificate peerCert = chain[0]; + + message.append("the ") + .append(peerType.name().toLowerCase(Locale.ROOT)) + .append(" provided a certificate with subject name [") + .append(peerCert.getSubjectX500Principal().getName()) + .append("] and ") + .append(fingerprintDescription(peerCert)); + + if (peerType == PeerType.SERVER) { + try { + final Collection> alternativeNames = peerCert.getSubjectAlternativeNames(); + if (alternativeNames == null || alternativeNames.isEmpty()) { + message.append("; the certificate does not have any subject alternative names"); + } else { + final List hostnames = describeValidHostnames(peerCert); + if (hostnames.isEmpty()) { + message.append("; the certificate does not have any DNS/IP subject alternative names"); + } else { + message.append("; the certificate has subject alternative names [") + .append(hostnames.stream().collect(Collectors.joining(","))) + .append("]"); + } + } + } catch (CertificateParsingException e) { + message.append("; the certificate's subject alternative names cannot be parsed"); + } + } + + if (isSelfIssued(peerCert)) { + message.append("; the certificate is ") + .append(describeSelfIssuedCertificate(peerCert, contextName, trustedIssuers)); + } else { + final String issuerName = peerCert.getIssuerX500Principal().getName(); + message.append("; the certificate is issued by [").append(issuerName).append("]"); + if (chain.length == 1) { + message.append(" but the ") + .append(peerType.name().toLowerCase(Locale.ROOT)) + .append(" did not provide a copy of the issuing certificate") + .append(describeIssuerTrust(contextName, trustedIssuers, peerCert, issuerName)); + } + } + + if (chain.length > 1) { + message.append("; the certificate is "); + // skip index-0, that's the peer cert. + for (int i = 1; i < chain.length; i++) { + message.append("signed by (subject [") + .append(chain[i].getSubjectX500Principal().getName()) + .append("] ") + .append(fingerprintDescription(chain[i])); + + if (trustedIssuers != null) { + if (isTrusted(trustedIssuers, chain[i]).isTrusted()) { + message.append(" {trusted issuer}"); + } + } + message.append(") "); + } + final X509Certificate root = chain[chain.length - 1]; + if (isSelfIssued(root)) { + message.append("which is ").append(describeSelfIssuedCertificate(root, contextName, trustedIssuers)); + } else { + final String rootIssuer = root.getIssuerX500Principal().getName(); + message.append("which is issued by [") + .append(rootIssuer) + .append("] (but that issuer certificate was not provided in the chain)") + .append(describeIssuerTrust(contextName, trustedIssuers, root, rootIssuer)); + + } + } + return message.toString(); + } + + private static CharSequence describeIssuerTrust(String contextName, @Nullable Map> trustedIssuers, + X509Certificate certificate, String issuerName) { + if (trustedIssuers == null) { + return ""; + } + StringBuilder message = new StringBuilder(); + final IssuerTrust trust = checkIssuerTrust(trustedIssuers, certificate); + if (trust.isVerified()) { + message.append("; the issuing certificate is trusted in this ssl context ([") + .append(contextName) + .append("]) with ") + .append(fingerprintDescription(trust.issuerCerts)); + } else if (trust.foundCertificateForDn()) { + message.append("; this ssl context ([") + .append(contextName) + .append("]) trusts [") + .append(trust.issuerCerts.size()) + .append("] ").append(trust.issuerCerts.size() == 1 ? "certificate" : "certificates") + .append(" with subject name [") + .append(issuerName) + .append("] and ") + .append(fingerprintDescription(trust.issuerCerts)) + .append(" but the signatures do not match"); + } else { + message.append("; this ssl context ([") + .append(contextName) + .append("]) is not configured to trust that issuer"); + } + return message; + } + + private static CharSequence describeSelfIssuedCertificate(X509Certificate certificate, String contextName, + @Nullable Map> trustedIssuers) { + final StringBuilder message = new StringBuilder(); + final CertificateTrust trust = isTrusted(trustedIssuers, certificate); + message.append("self-issued [").append(certificate.getIssuerX500Principal().getName()).append("] and is ") + .append(trust.isTrusted() ? "trusted" : "not trusted") + .append(" in this ssl context ([") + .append(contextName) + .append("])"); + if (trust.isTrusted()) { + if (trust.isSameCertificate() == false) { + if (trust.certificates.size() == 1) { + message.append(" using a certificate with ") + .append(fingerprintDescription(trust.certificates.get(0))) + .append(" but the same public key"); + } else { + message.append(" using [") + .append(trust.certificates.size()) + .append("] certificates with ") + .append(fingerprintDescription(trust.certificates)) + .append(" but the same public key"); + } + } + } else { + if (trust.hasCertificates()) { + if (trust.certificates.size() == 1) { + final X509Certificate match = trust.certificates.get(0); + message.append("; this ssl context does trust a certificate with subject [") + .append(match.getSubjectX500Principal().getName()) + .append("] but it has ") + .append(fingerprintDescription(match)); + } else { + message.append("; this ssl context does trust [") + .append(trust.certificates.size()) + .append("] certificates with subject [") + .append(certificate.getSubjectX500Principal().getName()) + .append("] but those certificates have ") + .append(fingerprintDescription(trust.certificates)); + } + } + } + return message; + } + + private static CertificateTrust isTrusted(Map> trustedIssuers, X509Certificate certificate) { + final List trustedCerts = trustedIssuers.get(certificate.getSubjectX500Principal().getName()); + if (trustedCerts == null || trustedCerts.isEmpty()) { + return CertificateTrust.noMatchingIssuer(); + } + final int index = trustedCerts.indexOf(certificate); + if (index != -1) { + return CertificateTrust.sameCertificate(trustedCerts.get(index)); + } + final List sameKey = trustedCerts.stream() + .filter(c -> c.getPublicKey().equals(certificate.getPublicKey())) + .collect(Collectors.toList()); + if (sameKey.isEmpty() == false) { + return CertificateTrust.samePublicKey(sameKey); + } else { + return CertificateTrust.nonMatchingCertificates(trustedCerts); + } + } + + public static IssuerTrust checkIssuerTrust(Map> trustedIssuers, X509Certificate peerCert) { + final List knownIssuers = trustedIssuers.get(peerCert.getIssuerX500Principal().getName()); + if (knownIssuers == null || knownIssuers.isEmpty()) { + return IssuerTrust.noMatchingCertificate(); + } + final List matchIssuers = knownIssuers.stream().filter(i -> checkIssuer(peerCert, i)).collect(Collectors.toList()); + if (matchIssuers.isEmpty() == false) { + return IssuerTrust.verifiedCertificates(matchIssuers); + } else { + return IssuerTrust.unverifiedCertificates(knownIssuers); + } + } + + private static String fingerprintDescription(List certificates) { + return certificates.stream().map(SslDiagnostics::fingerprintDescription).collect(Collectors.joining(", ")); + } + + private static String fingerprintDescription(X509Certificate certificate) { + try { + final String fingerprint = SslUtil.calculateFingerprint(certificate); + return "fingerprint [" + fingerprint + "]"; + } catch (CertificateEncodingException e) { + return "invalid encoding [" + e.toString() + "]"; + } + } + + private static boolean checkIssuer(X509Certificate certificate, X509Certificate possibleIssuer) { + try { + certificate.verify(possibleIssuer.getPublicKey()); + return true; + } catch (Exception e) { + return false; + } + } + + private static boolean isSelfIssued(X509Certificate certificate) { + return certificate.getIssuerX500Principal().equals(certificate.getSubjectX500Principal()); + } + +} diff --git a/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/SslUtil.java b/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/SslUtil.java new file mode 100644 index 0000000000000..024c409330f28 --- /dev/null +++ b/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/SslUtil.java @@ -0,0 +1,73 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch 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.elasticsearch.common.ssl; + +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.security.cert.CertificateEncodingException; +import java.security.cert.X509Certificate; +import java.util.Objects; + +public class SslUtil { + + public static String calculateFingerprint(X509Certificate certificate) throws CertificateEncodingException { + final MessageDigest sha1 = messageDigest("SHA-1"); + sha1.update(certificate.getEncoded()); + return toHexString(sha1.digest()); + } + + static MessageDigest messageDigest(String digestAlgorithm) { + try { + return MessageDigest.getInstance(digestAlgorithm); + } catch (NoSuchAlgorithmException e) { + throw new SslConfigException("unexpected exception creating MessageDigest instance for [" + digestAlgorithm + "]", e); + } + } + + private static final char[] HEX_DIGITS = "0123456789abcdef".toCharArray(); + + /** + * Format a byte array as a hex string. + * + * @param bytes the input to be represented as hex. + * @return a hex representation of the input as a String. + */ + static String toHexString(byte[] bytes) { + return new String(toHexCharArray(bytes)); + } + + /** + * Encodes the byte array into a newly created hex char array, without allocating any other temporary variables. + * + * @param bytes the input to be encoded as hex. + * @return the hex encoding of the input as a char array. + */ + static char[] toHexCharArray(byte[] bytes) { + Objects.requireNonNull(bytes); + final char[] result = new char[2 * bytes.length]; + for (int i = 0; i < bytes.length; i++) { + byte b = bytes[i]; + result[2 * i] = HEX_DIGITS[b >> 4 & 0xf]; + result[2 * i + 1] = HEX_DIGITS[b & 0xf]; + } + return result; + } + +} diff --git a/libs/ssl-config/src/test/java/org/elasticsearch/common/ssl/SslDiagnosticsTests.java b/libs/ssl-config/src/test/java/org/elasticsearch/common/ssl/SslDiagnosticsTests.java new file mode 100644 index 0000000000000..26a6cad02fa2a --- /dev/null +++ b/libs/ssl-config/src/test/java/org/elasticsearch/common/ssl/SslDiagnosticsTests.java @@ -0,0 +1,416 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch 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.elasticsearch.common.ssl; + +import org.elasticsearch.common.Nullable; +import org.elasticsearch.test.ESTestCase; +import org.hamcrest.Matchers; +import org.mockito.Mockito; + +import javax.net.ssl.SSLSession; +import javax.security.auth.x500.X500Principal; +import java.io.IOException; +import java.nio.file.Path; +import java.security.PublicKey; +import java.security.cert.Certificate; +import java.security.cert.CertificateEncodingException; +import java.security.cert.CertificateException; +import java.security.cert.CertificateParsingException; +import java.security.cert.X509Certificate; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +public class SslDiagnosticsTests extends ESTestCase { + + // Some constants for use in mock certificates + private static final byte[] MOCK_ENCODING_1 = { 0x61, 0x62, 0x63, 0x64, 0x65, 0x66 }; + private static final String MOCK_FINGERPRINT_1 = "1f8ac10f23c5b5bc1167bda84b833e5c057a77d2"; + private static final byte[] MOCK_ENCODING_2 = { 0x62, 0x63, 0x64, 0x65, 0x66, 0x67 }; + private static final String MOCK_FINGERPRINT_2 = "836d472783f4a210cfa3ab5621f757d1a2964aca"; + private static final byte[] MOCK_ENCODING_3 = { 0x63, 0x64, 0x65, 0x66, 0x67, 0x68 }; + private static final String MOCK_FINGERPRINT_3 = "da8e062d74919f549a9764c24ab0fcde3af3719f"; + private static final byte[] MOCK_ENCODING_4 = { 0x64, 0x65, 0x66, 0x67, 0x68, 0x69 }; + private static final String MOCK_FINGERPRINT_4 = "5d96965bfae50bf2be0d6259eb87a6cc9f5d0b26"; + + public void testDiagnosticMessageWhenServerProvidesAFullCertChainThatIsTrusted() throws Exception { + X509Certificate[] chain = loadCertChain("cert1/cert1.crt", "ca1/ca.crt"); + final SSLSession session = session("192.168.1.1"); + final Map> trustIssuers = trust("ca1/ca.crt", "ca2/ca.crt", "ca3/ca.crt"); + final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, + "xpack.http.ssl", trustIssuers); + assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.1.1];" + + " the server provided a certificate with subject name [CN=cert1] and fingerprint [3bebe388a66362784afd6c51a9000961a4e10050];" + + " the certificate has subject alternative names [DNS:localhost,IP:127.0.0.1];" + + " the certificate is issued by [CN=Test CA 1];" + + " the certificate is signed by" + + " (subject [CN=Test CA 1] fingerprint [2b7b0416391bdf86502505c23149022d2213dadc] {trusted issuer})" + + " which is self-issued [CN=Test CA 1] and is trusted in this ssl context ([xpack.http.ssl])")); + } + + public void testDiagnosticMessageWhenServerProvidesAFullCertChainThatIsntTrusted() throws Exception { + X509Certificate[] chain = loadCertChain("cert1/cert1.crt", "ca1/ca.crt"); + final SSLSession session = session("192.168.1.1"); + final Map> trustIssuers = trust("ca2/ca.crt", "ca3/ca.crt"); + final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, + "xpack.http.ssl", trustIssuers); + assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.1.1];" + + " the server provided a certificate with subject name [CN=cert1] and fingerprint [3bebe388a66362784afd6c51a9000961a4e10050];" + + " the certificate has subject alternative names [DNS:localhost,IP:127.0.0.1];" + + " the certificate is issued by [CN=Test CA 1];" + + " the certificate is signed by (subject [CN=Test CA 1] fingerprint [2b7b0416391bdf86502505c23149022d2213dadc])" + + " which is self-issued [CN=Test CA 1] and is not trusted in this ssl context ([xpack.http.ssl])")); + } + + public void testDiagnosticMessageWhenServerFullCertChainIsntTrustedButMimicIssuerExists() throws Exception { + X509Certificate[] chain = loadCertChain("cert1/cert1.crt", "ca1/ca.crt"); + final SSLSession session = session("192.168.1.1"); + final Map> trustIssuers = trust("ca1-b/ca.crt", "ca2/ca.crt", "ca3/ca.crt"); + final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, + "xpack.http.ssl", trustIssuers); + assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.1.1];" + + " the server provided a certificate with subject name [CN=cert1] and fingerprint [3bebe388a66362784afd6c51a9000961a4e10050];" + + " the certificate has subject alternative names [DNS:localhost,IP:127.0.0.1];" + + " the certificate is issued by [CN=Test CA 1];" + + " the certificate is signed by (subject [CN=Test CA 1] fingerprint [2b7b0416391bdf86502505c23149022d2213dadc])" + + " which is self-issued [CN=Test CA 1] and is not trusted in this ssl context ([xpack.http.ssl]);" + + " this ssl context does trust a certificate with subject [CN=Test CA 1]" + + " but it has fingerprint [b095bf2526be20783e1f26dfd69c7aae910e3663]")); + } + + public void testDiagnosticMessageWhenServerProvidesEndCertificateOnlyAndTheCertAuthIsTrusted() throws Exception { + X509Certificate[] chain = loadCertChain("cert1/cert1.crt"); + final SSLSession session = session("192.168.1.1"); + final Map> trustIssuers = trust("ca1/ca.crt", "ca2/ca.crt", "ca3/ca.crt"); + final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, + "xpack.http.ssl", trustIssuers); + assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.1.1];" + + " the server provided a certificate with subject name [CN=cert1] and fingerprint [3bebe388a66362784afd6c51a9000961a4e10050];" + + " the certificate has subject alternative names [DNS:localhost,IP:127.0.0.1];" + + " the certificate is issued by [CN=Test CA 1] but the server did not provide a copy of the issuing certificate;" + + " the issuing certificate is trusted in this ssl context ([xpack.http.ssl])" + + " with fingerprint [2b7b0416391bdf86502505c23149022d2213dadc]")); + } + + public void testDiagnosticMessageWhenServerProvidesEndCertificateOnlyButTheCertAuthIsNotTrusted() throws Exception { + X509Certificate[] chain = loadCertChain("cert1/cert1.crt"); + final SSLSession session = session("192.168.1.1"); + final Map> trustIssuers = trust("ca2/ca.crt", "ca3/ca.crt"); + final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, + "xpack.http.ssl", trustIssuers); + assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.1.1];" + + " the server provided a certificate with subject name [CN=cert1] and fingerprint [3bebe388a66362784afd6c51a9000961a4e10050];" + + " the certificate has subject alternative names [DNS:localhost,IP:127.0.0.1];" + + " the certificate is issued by [CN=Test CA 1] but the server did not provide a copy of the issuing certificate;" + + " this ssl context ([xpack.http.ssl]) is not configured to trust that issuer")); + } + + public void testDiagnosticMessageWhenServerProvidesEndCertificateOnlyWithMimicIssuer() throws Exception { + X509Certificate[] chain = loadCertChain("cert1/cert1.crt"); + final SSLSession session = session("192.168.1.1"); + final Map> trustIssuers = trust("ca1-b/ca.crt", "ca2/ca.crt", "ca3/ca.crt"); + final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, + "xpack.http.ssl", trustIssuers); + assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.1.1];" + + " the server provided a certificate with subject name [CN=cert1] and fingerprint [3bebe388a66362784afd6c51a9000961a4e10050];" + + " the certificate has subject alternative names [DNS:localhost,IP:127.0.0.1];" + + " the certificate is issued by [CN=Test CA 1] but the server did not provide a copy of the issuing certificate;" + + " this ssl context ([xpack.http.ssl]) trusts [1] certificate with subject name [CN=Test CA 1]" + + " and fingerprint [b095bf2526be20783e1f26dfd69c7aae910e3663] but the signatures do not match")); + } + + public void testDiagnosticMessageWhenServerProvidesEndCertificateWithMultipleMimicIssuers() throws Exception { + X509Certificate[] chain = loadCertChain("cert1/cert1.crt"); + final SSLSession session = session("192.168.1.9"); + final X509Certificate ca1b = loadCertificate("ca1-b/ca.crt"); + final Map> trustIssuers = trust(ca1b, cloneCertificateAsMock(ca1b)); + final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, + "xpack.http.ssl", trustIssuers); + assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.1.9];" + + " the server provided a certificate with subject name [CN=cert1] and fingerprint [3bebe388a66362784afd6c51a9000961a4e10050];" + + " the certificate has subject alternative names [DNS:localhost,IP:127.0.0.1];" + + " the certificate is issued by [CN=Test CA 1] but the server did not provide a copy of the issuing certificate;" + + " this ssl context ([xpack.http.ssl]) trusts [2] certificates with subject name [CN=Test CA 1]" + + " and fingerprint [b095bf2526be20783e1f26dfd69c7aae910e3663], fingerprint [" + MOCK_FINGERPRINT_1 + "]" + + " but the signatures do not match")); + } + + public void testDiagnosticMessageWhenServerProvidePartialChainFromTrustedCA() throws Exception { + final X509Certificate rootCA = mockCertificateWithIssuer("CN=root-ca,DC=example,DC=com", MOCK_ENCODING_1, + Collections.emptyList(), null); + final X509Certificate issuingCA = mockCertificateWithIssuer("CN=issuing-ca,DC=example,DC=com", MOCK_ENCODING_2, + Collections.emptyList(), rootCA); + final X509Certificate localCA = mockCertificateWithIssuer("CN=ca,OU=windows,DC=example,DC=com", MOCK_ENCODING_3, + Collections.emptyList(), issuingCA); + final X509Certificate endCert = mockCertificateWithIssuer("CN=elastic1,OU=windows,DC=example,DC=com", MOCK_ENCODING_4, + Collections.emptyList(), localCA); + + final X509Certificate[] chain = { endCert, localCA, issuingCA }; + + final SSLSession session = session("192.168.1.5"); + final Map> trustIssuers = trust(issuingCA, rootCA); + final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, + "xpack.security.authc.realms.ldap.ldap1.ssl", trustIssuers); + assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.1.5];" + + " the server provided a certificate with subject name [CN=elastic1,OU=windows,DC=example,DC=com]" + + " and fingerprint [" + MOCK_FINGERPRINT_4 + "];" + + " the certificate does not have any subject alternative names;" + + " the certificate is issued by [CN=ca,OU=windows,DC=example,DC=com];" + + " the certificate is" + + " signed by (subject [CN=ca,OU=windows,DC=example,DC=com] fingerprint [" + MOCK_FINGERPRINT_3 + "])" + + " signed by (subject [CN=issuing-ca,DC=example,DC=com] fingerprint [" + MOCK_FINGERPRINT_2 + "] {trusted issuer})" + + " which is issued by [CN=root-ca,DC=example,DC=com] (but that issuer certificate was not provided in the chain);" + + " the issuing certificate is trusted in this ssl context ([xpack.security.authc.realms.ldap.ldap1.ssl])" + + " with fingerprint [" + MOCK_FINGERPRINT_1 + "]")); + } + + public void testDiagnosticMessageWhenServerProvidePartialChainFromUntrustedCA() throws Exception { + final X509Certificate rootCA = mockCertificateWithIssuer("CN=root-ca,DC=example,DC=com", MOCK_ENCODING_1, + Collections.emptyList(), null); + final X509Certificate issuingCA = mockCertificateWithIssuer("CN=issuing-ca,DC=example,DC=com", MOCK_ENCODING_2, + Collections.emptyList(), rootCA); + final X509Certificate localCA = mockCertificateWithIssuer("CN=ca,OU=windows,DC=example,DC=com", MOCK_ENCODING_3, + Collections.emptyList(), issuingCA); + final X509Certificate endCert = mockCertificateWithIssuer("CN=elastic1,OU=windows,DC=example,DC=com", MOCK_ENCODING_4, + Collections.emptyList(), localCA); + + final X509Certificate[] chain = { endCert, localCA, issuingCA }; + + final SSLSession session = session("192.168.1.6"); + final Map> trustIssuers = trust(Collections.emptyList()); + final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, + "xpack.security.authc.realms.ldap.ldap1.ssl", trustIssuers); + assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.1.6];" + + " the server provided a certificate with subject name [CN=elastic1,OU=windows,DC=example,DC=com]" + + " and fingerprint [" + MOCK_FINGERPRINT_4 + "];" + + " the certificate does not have any subject alternative names;" + + " the certificate is issued by [CN=ca,OU=windows,DC=example,DC=com];" + + " the certificate is" + + " signed by (subject [CN=ca,OU=windows,DC=example,DC=com] fingerprint [" + MOCK_FINGERPRINT_3 + "])" + + " signed by (subject [CN=issuing-ca,DC=example,DC=com] fingerprint [" + MOCK_FINGERPRINT_2 + "])" + + " which is issued by [CN=root-ca,DC=example,DC=com] (but that issuer certificate was not provided in the chain);" + + " this ssl context ([xpack.security.authc.realms.ldap.ldap1.ssl]) is not configured to trust that issuer")); + } + + public void testDiagnosticMessageWhenServerProvidesASelfSignedCertThatIsDirectlyTrusted() throws Exception { + X509Certificate[] chain = loadCertChain("ca1/ca.crt"); + final SSLSession session = session("192.168.1.1"); + final Map> trustIssuers = trust("ca1/ca.crt", "ca2/ca.crt"); + final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, + "xpack.http.ssl", trustIssuers); + assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.1.1];" + + " the server provided a certificate with subject name [CN=Test CA 1]" + + " and fingerprint [2b7b0416391bdf86502505c23149022d2213dadc];" + + " the certificate does not have any subject alternative names;" + + " the certificate is self-issued [CN=Test CA 1] and is trusted in this ssl context ([xpack.http.ssl])")); + } + + public void testDiagnosticMessageWhenServerProvidesASelfSignedCertThatIsNotTrusted() throws Exception { + X509Certificate[] chain = loadCertChain("ca1/ca.crt"); + final SSLSession session = session("192.168.10.10"); + final Map> trustIssuers = Collections.emptyMap(); + final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, + "xpack.http.ssl", trustIssuers); + assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.10.10];" + + " the server provided a certificate with subject name [CN=Test CA 1]" + + " and fingerprint [2b7b0416391bdf86502505c23149022d2213dadc];" + + " the certificate does not have any subject alternative names;" + + " the certificate is self-issued [CN=Test CA 1] and is not trusted in this ssl context ([xpack.http.ssl])")); + } + + public void testDiagnosticMessageWhenServerProvidesASelfSignedCertWithMimicName() throws Exception { + X509Certificate[] chain = loadCertChain("ca1/ca.crt"); + final SSLSession session = session("192.168.1.1"); + final Map> trustIssuers = trust("ca1-b/ca.crt", "ca2/ca.crt"); + final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, + "xpack.http.ssl", trustIssuers); + assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.1.1];" + + " the server provided a certificate with subject name [CN=Test CA 1]" + + " and fingerprint [2b7b0416391bdf86502505c23149022d2213dadc];" + + " the certificate does not have any subject alternative names;" + + " the certificate is self-issued [CN=Test CA 1] and is not trusted in this ssl context ([xpack.http.ssl]);" + + " this ssl context does trust a certificate with subject [CN=Test CA 1]" + + " but it has fingerprint [b095bf2526be20783e1f26dfd69c7aae910e3663]")); + } + + public void testDiagnosticMessageWithEmptyChain() throws Exception { + X509Certificate[] chain = new X509Certificate[0]; + final SSLSession session = session("192.168.1.2"); + final Map> trustIssuers = Collections.emptyMap(); + final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, + "xpack.http.ssl", trustIssuers); + assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.1.2];" + + " the server did not provide a certificate")); + } + + public void testDiagnosticMessageWhenServerProvidesAnEmailSubjAltName() throws Exception { + final String subjectName = "CN=foo,DC=example,DC=com"; + final X509Certificate certificate = mockCertificateWithIssuer(subjectName, + MOCK_ENCODING_1, Collections.singletonList(List.of(1, "foo@example.com")), null); + X509Certificate[] chain = new X509Certificate[] { certificate }; + + final SSLSession session = session("192.168.1.3"); + final Map> trustIssuers = trust(certificate); + final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, + "xpack.monitoring.exporters.elastic-cloud.ssl", trustIssuers); + assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.1.3];" + + " the server provided a certificate with subject name [CN=foo,DC=example,DC=com]" + + " and fingerprint [" + MOCK_FINGERPRINT_1 + "];" + + " the certificate does not have any DNS/IP subject alternative names;" + + " the certificate is self-issued [CN=foo,DC=example,DC=com]" + + " and is trusted in this ssl context ([xpack.monitoring.exporters.elastic-cloud.ssl])")); + } + + public void testDiagnosticMessageWhenACertificateHasAnInvalidEncoding() throws Exception { + final String subjectName = "CN=foo,DC=example,DC=com"; + final X509Certificate certificate = mockCertificateWithIssuer(subjectName, new byte[0], Collections.emptyList(), null); + Mockito.when(certificate.getEncoded()).thenThrow(new CertificateEncodingException("MOCK INVALID ENCODING")); + X509Certificate[] chain = new X509Certificate[] { certificate }; + + final SSLSession session = session("192.168.1.6"); + final Map> trustIssuers = trust(Collections.emptyList()); + final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, + "xpack.security.transport.ssl", trustIssuers); + assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.1.6];" + + " the server provided a certificate with subject name [CN=foo,DC=example,DC=com]" + + " and invalid encoding [java.security.cert.CertificateEncodingException: MOCK INVALID ENCODING];" + + " the certificate does not have any subject alternative names;" + + " the certificate is self-issued [CN=foo,DC=example,DC=com]" + + " and is not trusted in this ssl context ([xpack.security.transport.ssl])")); + } + + public void testDiagnosticMessageForClientCertificate() throws Exception { + X509Certificate[] chain = loadCertChain("cert1/cert1.crt"); + + final SSLSession session = session("192.168.1.7"); + final Map> trustIssuers = trust("ca1/ca.crt"); + final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.CLIENT, session, + "xpack.security.transport.ssl", trustIssuers); + assertThat(message, Matchers.equalTo("failed to establish trust with [CLIENT] at [192.168.1.7];" + + " the client provided a certificate with subject name [CN=cert1]" + + " and fingerprint [3bebe388a66362784afd6c51a9000961a4e10050];" + + " the certificate is issued by [CN=Test CA 1] but the client did not provide a copy of the issuing certificate;" + + " the issuing certificate is trusted in this ssl context ([xpack.security.transport.ssl])" + + " with fingerprint [2b7b0416391bdf86502505c23149022d2213dadc]")); + } + + public void testDiagnosticMessageWhenCaHasNewIssuingCertificate() throws Exception { + // From time to time, CAs issue updated certificates based on the same underlying key-pair. + // For example, they might move to new signature algorithms (dropping SHA-1), or the certificate might be + // expiring and need to be reissued with a new expiry date. + // In this test, we assume that the server provides a certificate that is signed by the new CA cert, and we trust the old CA cert + // Our diagnostic message should make clear that we trust the CA, but using a different cert fingerprint. + // Note: This would normally succeed, so we wouldn't have an exception to diagnose, but it's possible that the hostname is wrong. + final X509Certificate newCaCert = loadCertificate("ca1/ca.crt"); + final X509Certificate oldCaCert = cloneCertificateAsMock(newCaCert); + X509Certificate[] chain = loadCertChain("cert1/cert1.crt", "ca1/ca.crt"); // uses "new" CA + + final SSLSession session = session("192.168.1.4"); + final Map> trustIssuers = trust(oldCaCert); + final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, + "xpack.security.authc.realms.saml.saml1.ssl", trustIssuers); + assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.1.4];" + + " the server provided a certificate with subject name [CN=cert1] and fingerprint [3bebe388a66362784afd6c51a9000961a4e10050];" + + " the certificate has subject alternative names [DNS:localhost,IP:127.0.0.1];" + + " the certificate is issued by [CN=Test CA 1];" + + " the certificate is signed by (subject [CN=Test CA 1]" + + " fingerprint [2b7b0416391bdf86502505c23149022d2213dadc] {trusted issuer})" + + " which is self-issued [CN=Test CA 1] and is trusted in this ssl context ([xpack.security.authc.realms.saml.saml1.ssl])" + + " using a certificate with fingerprint [1f8ac10f23c5b5bc1167bda84b833e5c057a77d2]" + + " but the same public key")); + } + + public X509Certificate cloneCertificateAsMock(X509Certificate clone) throws CertificateParsingException, CertificateEncodingException { + final X509Certificate cert = Mockito.mock(X509Certificate.class); + final X500Principal principal = clone.getSubjectX500Principal(); + Mockito.when(cert.getSubjectX500Principal()).thenReturn(principal); + Mockito.when(cert.getSubjectAlternativeNames()).thenReturn(clone.getSubjectAlternativeNames()); + Mockito.when(cert.getIssuerX500Principal()).thenReturn(clone.getIssuerX500Principal()); + Mockito.when(cert.getPublicKey()).thenReturn(clone.getPublicKey()); + Mockito.when(cert.getEncoded()).thenReturn(new byte[] { 0x61, 0x62, 0x63, 0x64, 0x65, 0x66 }); + return cert; + } + + public X509Certificate mockCertificateWithIssuer(String principal, byte[] encoding, List> subjAltNames, + @Nullable X509Certificate issuer) throws CertificateException { + + final X509Certificate cert = Mockito.mock(X509Certificate.class); + final X500Principal x500Principal = new X500Principal(principal); + final PublicKey key = Mockito.mock(PublicKey.class); + + Mockito.when(cert.getSubjectX500Principal()).thenReturn(x500Principal); + Mockito.when(cert.getSubjectAlternativeNames()).thenReturn(subjAltNames); + final X500Principal issuerPrincipal = issuer == null ? x500Principal : issuer.getSubjectX500Principal(); + Mockito.when(cert.getIssuerX500Principal()).thenReturn(issuerPrincipal); + Mockito.when(cert.getPublicKey()).thenReturn(key); + Mockito.when(cert.getEncoded()).thenReturn(encoding); + + return cert; + } + + private X509Certificate[] loadCertChain(String... names) throws CertificateException, IOException { + final List paths = Stream.of(names).map(p -> "/certs/" + p).map(this::getDataPath).collect(Collectors.toList()); + return PemUtils.readCertificates(paths).stream().map(X509Certificate.class::cast).toArray(X509Certificate[]::new); + } + + private X509Certificate loadCertificate(String name) throws CertificateException, IOException { + final Path path = getDataPath("/certs/" + name); + final List certificates = PemUtils.readCertificates(Collections.singleton(path)); + if (certificates.size() == 1) { + return (X509Certificate) certificates.get(0); + } else { + throw new IllegalStateException("Expected 1 certificate in [" + path.toAbsolutePath() + + "] but found [" + certificates.size() + "] - " + certificates); + } + } + + private Map> trust(String... certNames) throws CertificateException, IOException { + final List paths = Stream.of(certNames).map(p -> "/certs/" + p).map(this::getDataPath).collect(Collectors.toList()); + return trust(PemUtils.readCertificates(paths)); + } + + private Map> trust(X509Certificate... caCerts) { + return trust(Arrays.asList(caCerts)); + } + + private Map> trust(Collection caCerts) { + return caCerts.stream() + .map(X509Certificate.class::cast) + .collect(Collectors.toMap(x -> x.getSubjectX500Principal().getName(), List::of, + (List a, List b) -> { + List merge = new ArrayList<>(); + merge.addAll(a); + merge.addAll(b); + return merge; + })); + } + + private SSLSession session(String peerHost) { + final SSLSession mock = Mockito.mock(SSLSession.class); + Mockito.when(mock.getPeerHost()).thenReturn(peerHost); + return mock; + } + +} diff --git a/libs/ssl-config/src/test/resources/certs/README.txt b/libs/ssl-config/src/test/resources/certs/README.txt index a04a31011b4dd..ac4b5603c069e 100644 --- a/libs/ssl-config/src/test/resources/certs/README.txt +++ b/libs/ssl-config/src/test/resources/certs/README.txt @@ -73,3 +73,9 @@ do keytool -keypasswd -keystore cert-all/certs.jks -alias $Cert -keypass p12-pass -new key-pass -storepass jks-pass done +# 11. Create a mimic of the first CA ("ca1b") for testing certificates with the same name but different keys + +elasticsearch-certutil ca --pem --out ${PWD}/ca1-b.zip --days 9999 --ca-dn "CN=Test CA 1" +unzip ca1-b.zip +mv ca ca1-b + diff --git a/libs/ssl-config/src/test/resources/certs/ca1-b/ca.crt b/libs/ssl-config/src/test/resources/certs/ca1-b/ca.crt new file mode 100644 index 0000000000000..22c33082c31ef --- /dev/null +++ b/libs/ssl-config/src/test/resources/certs/ca1-b/ca.crt @@ -0,0 +1,19 @@ +-----BEGIN CERTIFICATE----- +MIIDCTCCAfGgAwIBAgIULMvXGEkDgj4OwuFSxeyBtGjDoYAwDQYJKoZIhvcNAQEL +BQAwFDESMBAGA1UEAxMJVGVzdCBDQSAxMB4XDTE5MTAyNTAzMjkwM1oXDTQ3MDMx +MTAzMjkwM1owFDESMBAGA1UEAxMJVGVzdCBDQSAxMIIBIjANBgkqhkiG9w0BAQEF +AAOCAQ8AMIIBCgKCAQEAwpVeirFFTVpx5xWvwRZFNFatw0weSsW9HgpARM7Af3qx +4DUgu+SAKjdxM0zD08BmbCCe8Soa2L+RNA0xx2jv5DUkJ4rjOyToYSMR2P+ei88O +D/Ho/rYSayMZKmoGSmfTE3Uvzh8nlPXL9eu1KCbpSAU/V4/pzR0htjTL7THgdfmN +A+PezH0XsINH+otRt9CdeRoJpK1UuvnJup08iwItC4aUZsBKoJA/iMl5lWnfgaSO +mUA64gRNpooSGxrm8q2W/VRnCj/1RfKUKAKs/Rum/JPtrO1INa+gDgVIiLWDRn7M +a0FlycGE2ckBFt6BY8wsf07feEXLb3Lzjc5aZHIY3wIDAQABo1MwUTAdBgNVHQ4E +FgQUGwhGu5NJudMWdljMDyiuI7/N0IQwHwYDVR0jBBgwFoAUGwhGu5NJudMWdljM +DyiuI7/N0IQwDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAQEAiSQ2 +BiBid6edkc69SlwxUv0jm6FXCXYwf2eGsJ/+LMYZFsfl0gFjbSXEQA1GP9x0p9vm +2sDseTp/7FitamnaBX8H47BaQEfhvz3mWdZVClKO3fDEeLM8D03Pg9KOqIsDftc/ +EoVS4uGOFrwFh8HubbaQCk3l55DJh+RTUliWpSSijRC9W97W0p7isqFQIgarkQ5E +S+BkB1TTvadLBGcim1Ulf/ExCuWbiNDfdXr0GO+xT6eoZHar6pGm5GESbh/vMTCi +ii49DomVeG8AMpFHFMgi8xuAFvr/LwhNkQHbcA5c+wY89grlpON1v6UPlFOY6zAd ++XR/yt7O9YRxPCSIAA== +-----END CERTIFICATE----- diff --git a/libs/ssl-config/src/test/resources/certs/ca1-b/ca.key b/libs/ssl-config/src/test/resources/certs/ca1-b/ca.key new file mode 100644 index 0000000000000..9409623bc4d2e --- /dev/null +++ b/libs/ssl-config/src/test/resources/certs/ca1-b/ca.key @@ -0,0 +1,27 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEowIBAAKCAQEAwpVeirFFTVpx5xWvwRZFNFatw0weSsW9HgpARM7Af3qx4DUg +u+SAKjdxM0zD08BmbCCe8Soa2L+RNA0xx2jv5DUkJ4rjOyToYSMR2P+ei88OD/Ho +/rYSayMZKmoGSmfTE3Uvzh8nlPXL9eu1KCbpSAU/V4/pzR0htjTL7THgdfmNA+Pe +zH0XsINH+otRt9CdeRoJpK1UuvnJup08iwItC4aUZsBKoJA/iMl5lWnfgaSOmUA6 +4gRNpooSGxrm8q2W/VRnCj/1RfKUKAKs/Rum/JPtrO1INa+gDgVIiLWDRn7Ma0Fl +ycGE2ckBFt6BY8wsf07feEXLb3Lzjc5aZHIY3wIDAQABAoIBAAlk5LeVb18Yqr8Z +UO7lgFnShXkUR7IccdDtdcTcpnaBGe1VI0tu8LEZFCIB5sJmW4uE16eU/M7SENSO +foS+EDbYSfKOxgGo+n8WDlqHnMPrLyad6z3A1IrPmvttOviEorAhzDkLUAHlC//H +bWuO1VxgxHoZvJPmgCYnzmCZM9j+KarCFV2umkXWbc5oOv5cYgsyuHgAyQaUmVbb +z7PV+YDyctgStIkMgDLfwJEIuKWtfYqFVfFmEhkbVCmsr8jYIohCzbzGF1r3ohO2 +I4mQNZy9R9DsQvcsFGg+q+Q3fpdcwQLYdbkwyh1f7B0K9VtpdvEZ2Jcxocrx5nCv +EzVQWIECgYEA84XCkbDYbrt+Pyst/gxmgx1c7+qX2+V/fFCKBg+nQsbKuf24pDs3 +3PIdDCVj/3+NZn5ELVgjRo8jyZz+C2B2tpVpDJbii2QmKTDdAeoHB8VBKKA4yVER +Nx0twNWfMlBal/7kSqJzdKldKXZnSLjMlqhcXwKD0P+VOKrwOyWx8e8CgYEAzI2v +XpqDVCCnh+7qpQR3VunBVbQoKPImfGLcokhoZk9q9bc5w936wH66Y6osk29O4thn +ya0nn0Tq341BT6JnJkfFU11fBXe54sP+kYfveE1rpSePnBEI6/02/j36lF8N1+T2 +d0H2vIPBfJsJ9c5e3nOGl62RWjAiOtbFNd5XeBECgYEAw26+dlOZbkx0BdfuYiqr +fl/rPPNlCemFRUwRaMnZLsMA4QDY1JxduahoXV4IgXxpCy8cIdPRA4hObTfbvV+e +BeukUaEpkDpAUeBQDYA2Qiswnpzu1cmEalm1ZUNLLoLEIaVCqw7yX1aoWGUYgIkm +T2YwM2N+TBVBOSgeASnAQ+sCgYBTS5LRBDxcQpvV161HGyV5h+CJhL4hxlFzr1JG +5xNULhzRCBaGstrMDg7aTM6wDtBhwVuqHU+YJJk8BSpGQkycovcwdkIeWN2iAMul ++WPDakteSljJZYprhoYhS53BU1+wvXS7pWnS5BgjJIMOzEWHciWpmC0rO5SEvzY5 +NFwL8QKBgEqZpZzrcho0mZ/GYqpXEbD9U8YjZ9eGg1dy6bEp7urm4cDI7I7TrTev +tvKdIfDR2u/PrvE/BqIoDA37x+Dz+RbkEt3deT0VgW5xcxj+jg2y86ELvNcXj6XP +cF8PnqZr3Cm2huY9Eq83qAKLFDgiZrPXeG94qY7Bnl0n3mYp2684 +-----END RSA PRIVATE KEY----- diff --git a/x-pack/plugin/core/build.gradle b/x-pack/plugin/core/build.gradle index 5773dbebc941d..70a69a7b84de4 100644 --- a/x-pack/plugin/core/build.gradle +++ b/x-pack/plugin/core/build.gradle @@ -24,6 +24,7 @@ dependencyLicenses { dependencies { compileOnly project(":server") + compile project(":libs:elasticsearch-ssl-config") compile "org.apache.httpcomponents:httpclient:${versions.httpclient}" compile "org.apache.httpcomponents:httpcore:${versions.httpcore}" compile "org.apache.httpcomponents:httpcore-nio:${versions.httpcore}" diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/CertParsingUtils.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/CertParsingUtils.java index 39111ceb610b4..fac69fd972790 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/CertParsingUtils.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/CertParsingUtils.java @@ -17,7 +17,6 @@ import javax.net.ssl.TrustManagerFactory; import javax.net.ssl.X509ExtendedKeyManager; import javax.net.ssl.X509ExtendedTrustManager; - import java.io.IOException; import java.io.InputStream; import java.nio.file.Files; @@ -59,7 +58,7 @@ static List resolvePaths(List certPaths, Environment environment) } public static KeyStore readKeyStore(Path path, String type, char[] password) - throws IOException, KeyStoreException, CertificateException, NoSuchAlgorithmException { + throws IOException, KeyStoreException, CertificateException, NoSuchAlgorithmException { try (InputStream in = Files.newInputStream(path)) { KeyStore store = KeyStore.getInstance(type); assert password != null; @@ -76,7 +75,7 @@ public static KeyStore readKeyStore(Path path, String type, char[] password) * @return an array of {@link Certificate} objects */ public static Certificate[] readCertificates(List certPaths, Environment environment) - throws CertificateException, IOException { + throws CertificateException, IOException { final List resolvedPaths = resolvePaths(certPaths, environment); return readCertificates(resolvedPaths); } @@ -121,7 +120,7 @@ public static List readCertificates(InputStream input) throws Certi * return the password for that key. If it returns {@code null}, then the key-pair for that alias is not read. */ public static Map readPkcs12KeyPairs(Path path, char[] password, Function keyPassword) - throws CertificateException, NoSuchAlgorithmException, KeyStoreException, IOException, UnrecoverableKeyException { + throws CertificateException, NoSuchAlgorithmException, KeyStoreException, IOException, UnrecoverableKeyException { final KeyStore store = readKeyStore(path, "PKCS12", password); final Enumeration enumeration = store.aliases(); final Map map = new HashMap<>(store.size()); @@ -139,7 +138,7 @@ public static Map readPkcs12KeyPairs(Path path, char[] passwor * Creates a {@link KeyStore} from a PEM encoded certificate and key file */ public static KeyStore getKeyStoreFromPEM(Path certificatePath, Path keyPath, char[] keyPassword) - throws IOException, CertificateException, KeyStoreException, NoSuchAlgorithmException { + throws IOException, CertificateException, KeyStoreException, NoSuchAlgorithmException { final PrivateKey key = PemUtils.readPrivateKey(keyPath, () -> keyPassword); final Certificate[] certificates = readCertificates(Collections.singletonList(certificatePath)); return getKeyStore(certificates, key, keyPassword); @@ -149,13 +148,13 @@ public static KeyStore getKeyStoreFromPEM(Path certificatePath, Path keyPath, ch * Returns a {@link X509ExtendedKeyManager} that is built from the provided private key and certificate chain */ public static X509ExtendedKeyManager keyManager(Certificate[] certificateChain, PrivateKey privateKey, char[] password) - throws NoSuchAlgorithmException, UnrecoverableKeyException, KeyStoreException, IOException, CertificateException { + throws NoSuchAlgorithmException, UnrecoverableKeyException, KeyStoreException, IOException, CertificateException { KeyStore keyStore = getKeyStore(certificateChain, privateKey, password); return keyManager(keyStore, password, KeyManagerFactory.getDefaultAlgorithm()); } private static KeyStore getKeyStore(Certificate[] certificateChain, PrivateKey privateKey, char[] password) - throws KeyStoreException, IOException, NoSuchAlgorithmException, CertificateException { + throws KeyStoreException, IOException, NoSuchAlgorithmException, CertificateException { KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType()); keyStore.load(null, null); // password must be non-null for keystore... @@ -167,7 +166,7 @@ private static KeyStore getKeyStore(Certificate[] certificateChain, PrivateKey p * Returns a {@link X509ExtendedKeyManager} that is built from the provided keystore */ public static X509ExtendedKeyManager keyManager(KeyStore keyStore, char[] password, String algorithm) - throws NoSuchAlgorithmException, UnrecoverableKeyException, KeyStoreException { + throws NoSuchAlgorithmException, UnrecoverableKeyException, KeyStoreException { KeyManagerFactory kmf = KeyManagerFactory.getInstance(algorithm); kmf.init(keyStore, password); KeyManager[] keyManagers = kmf.getKeyManagers(); @@ -206,7 +205,7 @@ static KeyConfig createKeyConfig(X509KeyPairSettings keyPair, Settings settings, String certPath = keyPair.certificatePath.get(settings).orElse(null); if (certPath == null) { throw new IllegalArgumentException("you must specify the certificates [" + keyPair.certificatePath.getKey() - + "] to use with the key [" + keyPair.keyPath.getKey() + "]"); + + "] to use with the key [" + keyPair.keyPath.getKey() + "]"); } return new PEMKeyConfig(keyPath, keyPassword, certPath); } @@ -219,7 +218,7 @@ static KeyConfig createKeyConfig(X509KeyPairSettings keyPair, Settings settings, keyStoreKeyPassword = keyStorePassword; } return new StoreKeyConfig(keyStorePath, keyStoreType, keyStorePassword, keyStoreKeyPassword, keyStoreAlgorithm, - trustStoreAlgorithm); + trustStoreAlgorithm); } return null; } @@ -231,13 +230,13 @@ static KeyConfig createKeyConfig(X509KeyPairSettings keyPair, Settings settings, * @return a trust manager that trusts the provided certificates */ public static X509ExtendedTrustManager trustManager(Certificate[] certificates) - throws NoSuchAlgorithmException, KeyStoreException, IOException, CertificateException { + throws NoSuchAlgorithmException, KeyStoreException, IOException, CertificateException { KeyStore store = trustStore(certificates); return trustManager(store, TrustManagerFactory.getDefaultAlgorithm()); } static KeyStore trustStore(Certificate[] certificates) - throws KeyStoreException, IOException, NoSuchAlgorithmException, CertificateException { + throws KeyStoreException, IOException, NoSuchAlgorithmException, CertificateException { assert certificates != null : "Cannot create trust store with null certificates"; KeyStore store = KeyStore.getInstance(KeyStore.getDefaultType()); store.load(null, null); @@ -260,7 +259,7 @@ static KeyStore trustStore(Certificate[] certificates) */ public static X509ExtendedTrustManager trustManager(String trustStorePath, String trustStoreType, char[] trustStorePassword, String trustStoreAlgorithm, Environment env) - throws NoSuchAlgorithmException, KeyStoreException, IOException, CertificateException { + throws NoSuchAlgorithmException, KeyStoreException, IOException, CertificateException { KeyStore trustStore = readKeyStore(env.configFile().resolve(trustStorePath), trustStoreType, trustStorePassword); return trustManager(trustStore, trustStoreAlgorithm); } @@ -269,7 +268,7 @@ public static X509ExtendedTrustManager trustManager(String trustStorePath, Strin * Creates a {@link X509ExtendedTrustManager} based on the trust material in the provided {@link KeyStore} */ public static X509ExtendedTrustManager trustManager(KeyStore keyStore, String algorithm) - throws NoSuchAlgorithmException, KeyStoreException { + throws NoSuchAlgorithmException, KeyStoreException { TrustManagerFactory tmf = TrustManagerFactory.getInstance(algorithm); tmf.init(keyStore); TrustManager[] trustManagers = tmf.getTrustManagers(); @@ -296,4 +295,5 @@ public static boolean isOrderedCertificateChain(List chain) { } return true; } + } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java index ec9f7a2d5a8f3..0602586ff01b0 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java @@ -5,8 +5,11 @@ */ package org.elasticsearch.xpack.core.ssl; +import org.apache.http.HttpHost; +import org.apache.http.conn.ssl.DefaultHostnameVerifier; import org.apache.http.conn.ssl.NoopHostnameVerifier; import org.apache.http.nio.conn.ssl.SSLIOSessionStrategy; +import org.apache.http.nio.reactor.IOSession; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.lucene.util.SetOnce; @@ -14,7 +17,10 @@ import org.elasticsearch.ElasticsearchSecurityException; import org.elasticsearch.common.CheckedSupplier; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.logging.LoggerMessageFormat; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.ssl.DiagnosticTrustManager; +import org.elasticsearch.common.ssl.SslDiagnostics; import org.elasticsearch.env.Environment; import org.elasticsearch.xpack.core.XPackSettings; import org.elasticsearch.xpack.core.common.socket.SocketAccess; @@ -25,7 +31,9 @@ import javax.net.ssl.KeyManagerFactory; import javax.net.ssl.SSLContext; import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLException; import javax.net.ssl.SSLParameters; +import javax.net.ssl.SSLPeerUnverifiedException; import javax.net.ssl.SSLSession; import javax.net.ssl.SSLSessionContext; import javax.net.ssl.SSLSocket; @@ -33,6 +41,7 @@ import javax.net.ssl.TrustManagerFactory; import javax.net.ssl.X509ExtendedKeyManager; import javax.net.ssl.X509ExtendedTrustManager; +import javax.security.auth.x500.X500Principal; import java.io.IOException; import java.net.InetAddress; import java.net.Socket; @@ -40,6 +49,8 @@ import java.security.KeyManagementException; import java.security.KeyStore; import java.security.NoSuchAlgorithmException; +import java.security.cert.Certificate; +import java.security.cert.X509Certificate; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -85,6 +96,7 @@ public class SSLService { } private final Settings settings; + private final boolean diagnoseTrustExceptions; /** * This is a mapping from "context name" (in general use, the name of a setting key) @@ -112,6 +124,7 @@ public class SSLService { public SSLService(Settings settings, Environment environment) { this.settings = settings; this.env = environment; + this.diagnoseTrustExceptions = true; // TODO, turn off in FIPS mode this.sslConfigurations = new HashMap<>(); this.sslContexts = loadSSLConfigurations(); } @@ -120,6 +133,7 @@ private SSLService(Settings settings, Environment environment, Map sslContexts) { this.settings = settings; this.env = environment; + this.diagnoseTrustExceptions = true; // TODO, turn off in FIPS mode this.sslConfigurations = sslConfigurations; this.sslContexts = sslContexts; } @@ -186,6 +200,14 @@ public SSLIOSessionStrategy sslIOSessionStrategy(SSLConfiguration config) { return sslIOSessionStrategy(sslContext, supportedProtocols, ciphers, verifier); } + public static HostnameVerifier getHostnameVerifier(SSLConfiguration sslConfiguration) { + if (sslConfiguration.verificationMode().isHostnameVerificationEnabled()) { + return new DefaultHostnameVerifier(); + } else { + return NoopHostnameVerifier.INSTANCE; + } + } + /** * The {@link SSLParameters} that are associated with the {@code sslContext}. *

@@ -209,7 +231,20 @@ SSLParameters sslParameters(SSLContext sslContext) { * @return Never {@code null}. */ SSLIOSessionStrategy sslIOSessionStrategy(SSLContext sslContext, String[] protocols, String[] ciphers, HostnameVerifier verifier) { - return new SSLIOSessionStrategy(sslContext, protocols, ciphers, verifier); + return new SSLIOSessionStrategy(sslContext, protocols, ciphers, verifier) { + @Override + protected void verifySession(HttpHost host, IOSession iosession, SSLSession session) throws SSLException { + if (verifier.verify(host.getHostName(), session) == false) { + final Certificate[] certs = session.getPeerCertificates(); + final X509Certificate x509 = (X509Certificate) certs[0]; + final X500Principal x500Principal = x509.getSubjectX500Principal(); + final String altNames = Strings.collectionToCommaDelimitedString(SslDiagnostics.describeValidHostnames(x509)); + throw new SSLPeerUnverifiedException(LoggerMessageFormat.format("Expected SSL certificate to be valid for host [{}]," + + " but it is only valid for subject alternative names [{}] and subject [{}]", + new Object[] { host.getHostName(), altNames, x500Principal.toString() })); + } + } + }; } /** @@ -391,6 +426,7 @@ private SSLContextHolder createSslContext(SSLConfiguration sslConfiguration) { */ private SSLContextHolder createSslContext(X509ExtendedKeyManager keyManager, X509ExtendedTrustManager trustManager, SSLConfiguration sslConfiguration) { + trustManager = wrapWithDiagnostics(trustManager, sslConfiguration); // Initialize sslContext try { SSLContext sslContext = SSLContext.getInstance(sslContextAlgorithm(sslConfiguration.supportedProtocols())); @@ -405,6 +441,33 @@ private SSLContextHolder createSslContext(X509ExtendedKeyManager keyManager, X50 } } + private X509ExtendedTrustManager wrapWithDiagnostics(X509ExtendedTrustManager trustManager, SSLConfiguration configuration) { + if (diagnoseTrustExceptions && trustManager instanceof DiagnosticTrustManager == false) { + final Logger diagnosticLogger = LogManager.getLogger(DiagnosticTrustManager.class); + // A single configuration might be used in many place, if there are multiple, we just list "shared" because + // that is better than the alternatives. Just listing would be misleading (it might not be the right one) + // but listing all of them would be confusing (e.g. some might be the default realms) + // This needs to be a supplier (deferred evaluation) because we might load more configurations after this context is built. + final Supplier contextName = () -> { + final List names = sslConfigurations.entrySet().stream() + .filter(e -> e.getValue().equals(configuration)) + .limit(2) + .map(Entry::getKey) + .collect(Collectors.toUnmodifiableList()); + switch (names.size()) { + case 0: + return "(unknown)"; + case 1: + return names.get(0); + default: + return "(shared)"; + } + }; + trustManager = new DiagnosticTrustManager(trustManager, contextName, diagnosticLogger::warn); + } + return trustManager; + } + /** * Parses the settings to load all SSLConfiguration objects that will be used. */ @@ -623,6 +686,7 @@ private void reloadSslContext() { orElse(getEmptyKeyManager()); X509ExtendedTrustManager loadedTrustManager = Optional.ofNullable(trustConfig.createTrustManager(env)). orElse(getEmptyTrustManager()); + loadedTrustManager = wrapWithDiagnostics(loadedTrustManager, sslConfiguration); SSLContext loadedSslContext = SSLContext.getInstance(sslContextAlgorithm(sslConfiguration.supportedProtocols())); loadedSslContext.init(new X509ExtendedKeyManager[]{loadedKeyManager}, new X509ExtendedTrustManager[]{loadedTrustManager}, null); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/test/http/MockWebServer.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/test/http/MockWebServer.java index 10ea0111b91aa..9de12ff7d75c5 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/test/http/MockWebServer.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/test/http/MockWebServer.java @@ -10,8 +10,8 @@ import com.sun.net.httpserver.HttpsConfigurator; import com.sun.net.httpserver.HttpsParameters; import com.sun.net.httpserver.HttpsServer; -import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.logging.log4j.util.Supplier; import org.elasticsearch.common.Strings; @@ -28,6 +28,9 @@ import java.io.OutputStream; import java.net.InetAddress; import java.net.InetSocketAddress; +import java.net.SocketAddress; +import java.net.URI; +import java.net.URISyntaxException; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; @@ -136,6 +139,18 @@ public void start() throws IOException { logger.info("bound HTTP mock server to [{}:{}]", getHostName(), getPort()); } + public SocketAddress getAddress() { + return new InetSocketAddress(hostname, port); + } + + public URI getUri(String path) throws URISyntaxException { + if (hostname == null) { + throw new IllegalStateException("Web server must be started in order to determine its URI"); + } + final String scheme = this.sslContext == null ? "http" : "https"; + return new URI(scheme, null, hostname, port, path, null, null); + } + /** * A custom HttpsConfigurator that takes the SSL context and the required client authentication into account * Also configured the protocols and cipher suites to match the security default ones diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java index 9657e4c31baa5..b7e4e2af8839f 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java @@ -53,8 +53,6 @@ import org.apache.http.concurrent.FutureCallback; import org.apache.http.config.Registry; import org.apache.http.config.RegistryBuilder; -import org.apache.http.conn.ssl.DefaultHostnameVerifier; -import org.apache.http.conn.ssl.NoopHostnameVerifier; import org.apache.http.entity.ContentType; import org.apache.http.impl.auth.BasicScheme; import org.apache.http.impl.nio.client.CloseableHttpAsyncClient; @@ -560,9 +558,7 @@ private CloseableHttpAsyncClient createHttpClient() { final String sslKey = RealmSettings.realmSslPrefix(realmConfig.identifier()); final SSLConfiguration sslConfiguration = sslService.getSSLConfiguration(sslKey); final SSLContext clientContext = sslService.sslContext(sslConfiguration); - boolean isHostnameVerificationEnabled = sslConfiguration.verificationMode().isHostnameVerificationEnabled(); - final HostnameVerifier verifier = isHostnameVerificationEnabled ? - new DefaultHostnameVerifier() : NoopHostnameVerifier.INSTANCE; + final HostnameVerifier verifier = SSLService.getHostnameVerifier(sslConfiguration); Registry registry = RegistryBuilder.create() .register("http", NoopIOSessionStrategy.INSTANCE) .register("https", new SSLIOSessionStrategy(clientContext, verifier)) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlRealm.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlRealm.java index 4c8cfa7c5fbda..b9508ecd97815 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlRealm.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlRealm.java @@ -10,8 +10,6 @@ import net.shibboleth.utilities.java.support.resolver.ResolverException; import net.shibboleth.utilities.java.support.xml.BasicParserPool; import org.apache.http.client.HttpClient; -import org.apache.http.conn.ssl.DefaultHostnameVerifier; -import org.apache.http.conn.ssl.NoopHostnameVerifier; import org.apache.http.conn.ssl.SSLConnectionSocketFactory; import org.apache.http.impl.client.HttpClientBuilder; import org.apache.logging.log4j.Logger; @@ -538,8 +536,7 @@ private static Tuple client.execute(request))) { + fail("Expected hostname verification exception"); + } catch (Exception e) { + assertThat(e, throwableWithMessage(containsStringIgnoringCase("Certificate"))); + assertThat(e, throwableWithMessage(containsString(request.getURI().getHost()))); + assertThat(e, throwableWithMessage(containsStringIgnoringCase("subject alternative names"))); + assertThat(e, throwableWithMessage(containsString("not.this.host"))); + } + } + } + + public void testMessageForRestClientHostnameVerificationFailure() throws IOException, URISyntaxException { + final Settings sslSetup = getPemSSLSettings(HTTP_SERVER_SSL, "not-this-host.crt", "not-this-host.key", + SSLClientAuth.NONE, VerificationMode.FULL, null) + // Client + .putList("xpack.http.ssl.certificate_authorities", getPath("ca1.crt")) + .build(); + final SSLService sslService = new SSLService(sslSetup, newEnvironment()); + try (MockWebServer webServer = initWebServer(sslService)) { + try (RestClient restClient = buildRestClient(sslService, webServer)) { + restClient.performRequest(new Request("GET", "/")); + fail("Expected hostname verification exception"); + } catch (Exception e) { + assertThat(e, throwableWithMessage(containsStringIgnoringCase("certificate"))); + assertThat(e, throwableWithMessage(containsString(webServer.getHostName()))); + assertThat(e, throwableWithMessage(containsStringIgnoringCase("subject alternative names"))); + assertThat(e, throwableWithMessage(containsString("not.this.host"))); + } + } + } + + public void testDiagnosticTrustManagerForHostnameVerificationFailure() throws Exception { + final Settings settings = getPemSSLSettings(HTTP_SERVER_SSL, "not-this-host.crt", "not-this-host.key", + SSLClientAuth.NONE, VerificationMode.FULL, null) + .putList("xpack.http.ssl.certificate_authorities", getPath("ca1.crt")) + .build(); + final SSLService sslService = new SSLService(settings, newEnvironment()); + final SSLConfiguration clientSslConfig = sslService.getSSLConfiguration(HTTP_CLIENT_SSL); + final SSLSocketFactory clientSocketFactory = sslService.sslSocketFactory(clientSslConfig); + + final Logger diagnosticLogger = LogManager.getLogger(DiagnosticTrustManager.class); + final MockLogAppender mockAppender = new MockLogAppender(); + mockAppender.start(); + + // Apache clients implement their own hostname checking, but we don't want that. + // We use a raw socket so we get the builtin JDK checking (which is what we use for transport protocol SSL checks) + try (MockWebServer webServer = initWebServer(sslService); + SSLSocket clientSocket = (SSLSocket) clientSocketFactory.createSocket()) { + Loggers.addAppender(diagnosticLogger, mockAppender); + + mockAppender.addExpectation(new MockLogAppender.PatternSeenEventExpectation( + "ssl diagnostic", + DiagnosticTrustManager.class.getName(), + Level.WARN, + "failed to establish trust with \\[SERVER\\] at \\[" + Pattern.quote(webServer.getHostName()) + "\\];" + + " the server provided a certificate with subject name \\[CN=not-this-host\\]" + + " and fingerprint \\[[0-9a-f]{40}\\];" + + " the certificate has subject alternative names \\[DNS:not\\.this\\.host\\];" + + " the certificate is issued by \\[CN=Certificate Authority 1,OU=ssl-error-message-test,DC=elastic,DC=co\\]" + + " but the server did not provide a copy of the issuing certificate;" + + " the issuing certificate is trusted in this ssl context " + Pattern.quote("([" + HTTP_CLIENT_SSL + "])") + + " with fingerprint \\[[0-9a-f]{40}\\]")); + enableHttpsHostnameChecking(clientSocket); + connect(clientSocket, webServer); + assertThat(clientSocket.isConnected(), is(true)); + final SSLHandshakeException handshakeException = expectThrows(SSLHandshakeException.class, + () -> clientSocket.getInputStream().read()); + assertThat(handshakeException, throwableWithMessage(containsStringIgnoringCase("subject alternative names"))); + assertThat(handshakeException, throwableWithMessage(containsString(webServer.getHostName()))); + mockAppender.assertAllExpectationsMatched(); + } finally { + Loggers.removeAppender(diagnosticLogger, mockAppender); + mockAppender.stop(); + } + } + + @SuppressForbidden(reason = "Allow opening socket for test") + private void connect(SSLSocket clientSocket, MockWebServer webServer) throws IOException { + SocketAccess.doPrivileged(() -> clientSocket.connect(webServer.getAddress())); + } + + private CloseableHttpClient buildHttpClient(SSLService sslService) { + final SSLConfiguration sslConfiguration = sslService.getSSLConfiguration(HTTP_CLIENT_SSL); + final HostnameVerifier verifier = SSLService.getHostnameVerifier(sslConfiguration); + final SSLSocketFactory socketFactory = sslService.sslSocketFactory(sslConfiguration); + final SSLConnectionSocketFactory connectionSocketFactory = new SSLConnectionSocketFactory(socketFactory, verifier); + return HttpClientBuilder.create().setSSLSocketFactory(connectionSocketFactory).build(); + } + + private RestClient buildRestClient(SSLService sslService, MockWebServer webServer) { + final SSLConfiguration sslConfiguration = sslService.getSSLConfiguration(HTTP_CLIENT_SSL); + final HttpHost httpHost = new HttpHost(webServer.getHostName(), webServer.getPort(), "https"); + return RestClient.builder(httpHost) + .setHttpClientConfigCallback(client -> client.setSSLStrategy(sslService.sslIOSessionStrategy(sslConfiguration))) + .build(); + } + + /** + * By default, JSSE doesn't actually do hostname checking as part of certificate verifications. + * It's possible to implement it yourself, or opt-in to have the TrustManager do it for you. + * However, just to make things difficult (ha!) the HTTP RFC and LDAP RFC have different rules for wildcard expansion in Certificate + * DNS SANs, which is why we need to enable "https" checking. + */ + private void enableHttpsHostnameChecking(SSLSocket clientSocket) { + final SSLParameters params = new SSLParameters(); + params.setEndpointIdentificationAlgorithm("HTTPS"); + clientSocket.setSSLParameters(params); + } + + private Settings.Builder getPemSSLSettings(String prefix, String certificatePath, String keyPath, SSLClientAuth clientAuth, + VerificationMode verificationMode, String caPath) throws FileNotFoundException { + final Settings.Builder builder = Settings.builder() + .put(prefix + ".enabled", true) + .put(prefix + ".certificate", getPath(certificatePath)) + .put(prefix + ".key", getPath(keyPath)) + .put(prefix + ".client_authentication", clientAuth.name()) + .put(prefix + ".verification_mode", verificationMode.name()); + if (caPath != null) { + builder.putList(prefix + ".certificate_authorities", getPath(caPath)); + } + return builder; + } + + private MockWebServer initWebServer(SSLService sslService) throws IOException { + final SSLConfiguration httpSslConfig = sslService.getSSLConfiguration(HTTP_SERVER_SSL); + final MockWebServer webServer = new MockWebServer(sslService.sslContext(httpSslConfig), false); + + webServer.enqueue(new MockResponse().setBody("{}").setResponseCode(200)); + webServer.start(); + return webServer; + } + + private String getPath(String fileName) throws FileNotFoundException { + final Path path = getDataPath("/org/elasticsearch/xpack/ssl/SSLErrorMessageTests/" + fileName); + if (Files.exists(path)) { + return path.toString(); + } else { + throw new FileNotFoundException("File " + path + " does not exist"); + } + } + +} diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLErrorMessageTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLErrorMessageFileTests.java similarity index 98% rename from x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLErrorMessageTests.java rename to x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLErrorMessageFileTests.java index 3f88e6ea84809..de12f2c3cf331 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLErrorMessageTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLErrorMessageFileTests.java @@ -45,9 +45,10 @@ import static org.hamcrest.Matchers.instanceOf; /** - * This is a suite of tests to ensure that meaningful error messages are generated for defined SSL configuration problems. + * This is a suite of tests to ensure that meaningful error messages are generated for defined SSL configuration problems due to file + * problems. */ -public class SSLErrorMessageTests extends ESTestCase { +public class SSLErrorMessageFileTests extends ESTestCase { private Environment env; private Map paths; @@ -272,7 +273,7 @@ private void checkBlockedResource(String sslManagerType, String fileType, String exception = exception.getCause(); assertThat(exception.getMessage(), containsString("failed to initialize SSL " + sslManagerType + " - access to read " + fileType + " file")); - assertThat(exception.getMessage(),containsString("file.error")); + assertThat(exception.getMessage(), containsString("file.error")); assertThat(exception, instanceOf(ElasticsearchException.class)); exception = exception.getCause(); @@ -360,7 +361,7 @@ private String resource(String fileName) { } private void requirePath(String fileName) throws FileNotFoundException { - final Path path = getDataPath("/org/elasticsearch/xpack/ssl/SSLServiceErrorMessageTests/" + fileName); + final Path path = getDataPath("/org/elasticsearch/xpack/ssl/SSLErrorMessageTests/" + fileName); if (Files.exists(path)) { paths.put(fileName, path); } else { diff --git a/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLServiceErrorMessageTests/README.txt b/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLErrorMessageTests/README.txt similarity index 79% rename from x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLServiceErrorMessageTests/README.txt rename to x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLErrorMessageTests/README.txt index bc788d59c18c9..1075612cd8160 100644 --- a/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLServiceErrorMessageTests/README.txt +++ b/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLErrorMessageTests/README.txt @@ -33,6 +33,32 @@ function new-p12-cert() { certutil cert --ca="${PWD}/$CaFile" --ca-pass="$CaPass" --days=5000 --out ${PWD}/$CertFile --pass="$CertPass" --name="$CertName" "$@" } +function new-pem-cert() { + local CrtFile="$1" + local KeyFile="$2" + local KeyPass="$3" + local CertName="$4" + local CaFile="$5" + local CaPass="$6" + shift 6 + + local ZipFile=${PWD}/$CertName.zip + local PassOpt="" + if [ -n "$KeyPass" ] + then + PassOpt="--pass=$KeyPass" + fi + + certutil cert --pem \ + --ca="${PWD}/$CaFile" --ca-pass="$CaPass" \ + --name="$CertName" --out $ZipFile \ + --days=5000 $PassOpt \ + "$@" + unzip -p $ZipFile "$CertName/$CertName.crt" > $CrtFile + unzip -p $ZipFile "$CertName/$CertName.key" > $KeyFile + rm $ZipFile +} + function p12-to-jks() { local P12File="$1" local P12Pass="$2" @@ -77,7 +103,7 @@ function p12-export-pair() { rm $TmpFile } - +function no-op() { # # Create a CA in PKCS#12 # @@ -103,4 +129,10 @@ p12-to-jks cert1a.p12 "cert1a-p12-password" cert1a.jks "cert1a-jks-password" # Convert to PEM # - "cert1a.key" is an (unprotected) PKCS#1 key p12-export-pair cert1a.p12 "cert1a-p12-password" "cert1a" cert1a.crt cert1a.key +} +# +# Create a Cert/Key Pair in PEM with a hostname "not.this.host" +# - "not_this_host.crt" is signed by "ca1" +# +new-pem-cert not-this-host.crt not-this-host.key "" "not-this-host" "ca1.p12" "ca1-p12-password" --dns not.this.host diff --git a/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLServiceErrorMessageTests/ca1.crt b/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLErrorMessageTests/ca1.crt similarity index 100% rename from x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLServiceErrorMessageTests/ca1.crt rename to x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLErrorMessageTests/ca1.crt diff --git a/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLServiceErrorMessageTests/ca1.jks b/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLErrorMessageTests/ca1.jks similarity index 100% rename from x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLServiceErrorMessageTests/ca1.jks rename to x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLErrorMessageTests/ca1.jks diff --git a/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLServiceErrorMessageTests/ca1.p12 b/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLErrorMessageTests/ca1.p12 similarity index 100% rename from x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLServiceErrorMessageTests/ca1.p12 rename to x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLErrorMessageTests/ca1.p12 diff --git a/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLServiceErrorMessageTests/cert1a.crt b/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLErrorMessageTests/cert1a.crt similarity index 100% rename from x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLServiceErrorMessageTests/cert1a.crt rename to x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLErrorMessageTests/cert1a.crt diff --git a/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLServiceErrorMessageTests/cert1a.jks b/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLErrorMessageTests/cert1a.jks similarity index 100% rename from x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLServiceErrorMessageTests/cert1a.jks rename to x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLErrorMessageTests/cert1a.jks diff --git a/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLServiceErrorMessageTests/cert1a.key b/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLErrorMessageTests/cert1a.key similarity index 100% rename from x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLServiceErrorMessageTests/cert1a.key rename to x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLErrorMessageTests/cert1a.key diff --git a/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLServiceErrorMessageTests/cert1a.p12 b/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLErrorMessageTests/cert1a.p12 similarity index 100% rename from x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLServiceErrorMessageTests/cert1a.p12 rename to x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLErrorMessageTests/cert1a.p12 diff --git a/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLErrorMessageTests/not-this-host.crt b/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLErrorMessageTests/not-this-host.crt new file mode 100644 index 0000000000000..e414b11013bfd --- /dev/null +++ b/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLErrorMessageTests/not-this-host.crt @@ -0,0 +1,21 @@ +-----BEGIN CERTIFICATE----- +MIIDfjCCAmagAwIBAgIVANLCn6aZ9aWqm76cjQas3ALTM5+5MA0GCSqGSIb3DQEB +CwUAMHAxEjAQBgoJkiaJk/IsZAEZFgJjbzEXMBUGCgmSJomT8ixkARkWB2VsYXN0 +aWMxHzAdBgNVBAsTFnNzbC1lcnJvci1tZXNzYWdlLXRlc3QxIDAeBgNVBAMTF0Nl +cnRpZmljYXRlIEF1dGhvcml0eSAxMB4XDTE5MTAxODA2NTkxNVoXDTMzMDYyNjA2 +NTkxNVowGDEWMBQGA1UEAxMNbm90LXRoaXMtaG9zdDCCASIwDQYJKoZIhvcNAQEB +BQADggEPADCCAQoCggEBAI+5PnEY4FJsAcojXAqrZZ7I0L0RSl093HAbWWKUtYii +5jcCl5Qz9czk0/kPXavkskzEaMKz4HzXKeHkQurSQa0bapUKO4TEdxJruPNTdLe1 +w+BOHiy/zmWZEZpWdpFqiFjzbz7ULrJVIGzTqURKMnhMziRZsCMveys5e/uZnTWD +no12fihq4jcuee5cNuK6LI/KKhUcd3qV063oe8j3tijsEphIFtU0hxwv9ORf4QO3 +vWDCRwumCRyUr0weMs3jxkMn2izfxYXn5RgVdElHAgiyFEbKiJ1FIjoc0D4xXeeZ +FVCYYXWSX9XThpD+BxUHpujccWdxW24uWNF3XEKHOosCAwEAAaNnMGUwHQYDVR0O +BBYEFMK4dB1CrrKcWLBblIhTES+SBqDEMB8GA1UdIwQYMBaAFKHpV7oSAMlY64s6 +FbHLPWYtoClTMBgGA1UdEQQRMA+CDW5vdC50aGlzLmhvc3QwCQYDVR0TBAIwADAN +BgkqhkiG9w0BAQsFAAOCAQEATBtfV/sedPn+kpEf+ZYOeEAiwre5tHgigpRuFnPA +tiTydY1b18vCo3Cw1lVul1WuZnU/gr7XUmcwcNGyRL/xEatKv5LXC7dRDKGyu4Xa +9p5yLVtROyO6eEqWhh4gHpJQqQ03rNKV0yaHa5Olm1srp28HVNZjQQmSopTcjDgZ +skjHPTyqbAXr3UwwnzACJEhDC7vezk8V7klZEaQxLkCqHqGXfsIQ/v2oQFrPHwAN +UzB+jH8XILODxXbShJ7eSBPrsUOampRrqRcQIwxIGtKJtAAU2T27ikCZl69i/fwQ +uLzB/PXgLYX7BerAaLDoHel1rgiAk0lMasJWEQqM3JLAAw== +-----END CERTIFICATE----- diff --git a/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLErrorMessageTests/not-this-host.key b/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLErrorMessageTests/not-this-host.key new file mode 100644 index 0000000000000..fe1d4e3a775ec --- /dev/null +++ b/x-pack/plugin/security/src/test/resources/org/elasticsearch/xpack/ssl/SSLErrorMessageTests/not-this-host.key @@ -0,0 +1,27 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEogIBAAKCAQEAj7k+cRjgUmwByiNcCqtlnsjQvRFKXT3ccBtZYpS1iKLmNwKX +lDP1zOTT+Q9dq+SyTMRowrPgfNcp4eRC6tJBrRtqlQo7hMR3Emu481N0t7XD4E4e +LL/OZZkRmlZ2kWqIWPNvPtQuslUgbNOpREoyeEzOJFmwIy97Kzl7+5mdNYOejXZ+ +KGriNy557lw24rosj8oqFRx3epXTreh7yPe2KOwSmEgW1TSHHC/05F/hA7e9YMJH +C6YJHJSvTB4yzePGQyfaLN/FheflGBV0SUcCCLIURsqInUUiOhzQPjFd55kVUJhh +dZJf1dOGkP4HFQem6NxxZ3Fbbi5Y0XdcQoc6iwIDAQABAoIBACsBSPfac7T18Isl +jXL72kpq2Frag5/m1MEOze47hR4/cBdhxS1pZtFKX0sVF/hJVDi4RIiN/kwcrcGT +5ZoSE+jSXU//YdPWXbK7XPoiLLiTwOqrNUg6lG2+IZBG8u+MKwRCwzTPlLFz22H0 +658tfHCWutARriy5FmfWqYydaHcMWRxommFkjfZzu2CCjVP8uz6a+UiPkpgysB3+ +l97vdRuTN++yBPzpERmqGNOrvKysWadd3eMnAECbD+khaxh909iC/25oYwZH0N6k +fmAiQQNOXsDwtGo6UNKhVxFu3y2o4DmJWs2nROlbrYOsD5Oi5acs+k2NlpGK7u7T +EqfeeQECgYEA3uPn/Ze4eKSzsxjgZesQ1woBh0ywkxmoaE29qRe1cDGwHLE8TBxd +TTwHTkqzi/0wcTUORCVSmmeR6tnq85Yf3a2IZJDSudHJeY/KRs6iRIisYqPpV20h +sMg+cegqZF9rqNlAJsKc3g0W5ROQwr5tXEBt+53p0t4c7wrGrhUN5csCgYEApRLH +uR4eLFVIpj8fQvPtVQ+wddQ82r9SP6+pCUryK4D0owUtM+ChXODfzMZdWPZukWPE +BqVoMm0dfxJWEtVfv+A7VOcQmp3CKVb7WUgxRUylxKcXHawfBFk4MBYw80JBqOdH +q/KB93nTzej50nr3bqKnM7llOXak7G3rRqszZkECgYAkRz4cVZSN9mjVTsg/bnpI +NfW3uvDGkJeLmpOzMQu0HjJHCUYCMV+yUYHy6U++ClDXLEaNKvH99buXWS7XxOic +4UDg9X2HBzFe2tuWmM1qkEBWsc0qELY6Gu2nBp3Xxnw0eF/rryNvNPwz/vJB8FLG +gCommTZUvxhAhCWAcibX7wKBgEWOX0f6j95AZWdfy61rmUKxZLqRnr7RxTd+rixz +Pw3jvbF/eeeVLIk1XDguEaFt3XM35Z6jTf+JiNdFg61V6Y2xT27cGlv8Q5clxq19 +RP2daXAutAfVwhAUBCAHCcNG1OH16nZiri74T65BEBuHowEkWm3qHeQTwTS6sFvL +wdHBAoGASUMH432pDkavryfl4l5BUQxef+kI4XqBzJAQ8dAyqp+4HIGYq23zsgda +FAiCN6xdRbVWhebWDAtuTzz5C8Hmzw2qc4aOYvx5YehVU0eP4uoskdQbKIGtGpte +Gyev2ZQfojCcIUU4HlB19nLnKvjLzgubTiLxz6G3xa5mrCHhJjE= +-----END RSA PRIVATE KEY----- diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpClient.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpClient.java index 49e27f51ea7c4..bd327f0ef53e9 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpClient.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpClient.java @@ -25,8 +25,6 @@ import org.apache.http.client.protocol.HttpClientContext; import org.apache.http.client.utils.URIBuilder; import org.apache.http.client.utils.URIUtils; -import org.apache.http.conn.ssl.DefaultHostnameVerifier; -import org.apache.http.conn.ssl.NoopHostnameVerifier; import org.apache.http.conn.ssl.SSLConnectionSocketFactory; import org.apache.http.entity.ByteArrayEntity; import org.apache.http.entity.ContentType; @@ -113,8 +111,7 @@ private CloseableHttpClient createHttpClient() { // ssl setup SSLConfiguration sslConfiguration = sslService.getSSLConfiguration(SETTINGS_SSL_PREFIX); - boolean isHostnameVerificationEnabled = sslConfiguration.verificationMode().isHostnameVerificationEnabled(); - HostnameVerifier verifier = isHostnameVerificationEnabled ? new DefaultHostnameVerifier() : NoopHostnameVerifier.INSTANCE; + HostnameVerifier verifier = SSLService.getHostnameVerifier(sslConfiguration); SSLConnectionSocketFactory factory = new SSLConnectionSocketFactory(sslService.sslSocketFactory(sslConfiguration), verifier); clientBuilder.setSSLSocketFactory(factory); From 55ddc9d662aafdbe204d4c18c1f3a72aaf66cd72 Mon Sep 17 00:00:00 2001 From: Tim Vernum Date: Tue, 19 Nov 2019 13:45:11 +1100 Subject: [PATCH 2/6] Address feedback --- .../common/ssl/SslDiagnostics.java | 101 ++++++++++-------- .../org/elasticsearch/common/ssl/SslUtil.java | 6 +- .../common/ssl/SslDiagnosticsTests.java | 92 ++++++++-------- .../xpack/core/ssl/CertParsingUtils.java | 1 - .../xpack/core/ssl/SSLService.java | 14 ++- .../xpack/security/Security.java | 1 + ...orMessageCertificateVerificationTests.java | 14 +-- 7 files changed, 130 insertions(+), 99 deletions(-) diff --git a/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/SslDiagnostics.java b/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/SslDiagnostics.java index 5100bf9614a5b..28c2d5e14f507 100644 --- a/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/SslDiagnostics.java +++ b/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/SslDiagnostics.java @@ -31,6 +31,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Optional; import java.util.stream.Collectors; public class SslDiagnostics { @@ -51,16 +52,10 @@ public static List describeValidHostnames(X509Certificate certificate) { } final int type = ((Integer) pair.get(0)).intValue(); final String name = (String) pair.get(1); - switch (type) { - case 2: - description.add("DNS:" + name); - break; - case 7: - description.add("IP:" + name); - break; - // Other type, we don't care. - default: - break; + if (type == 2) { + description.add("DNS:" + name); + } else if (type == 7) { + description.add("IP:" + name); } } return description; @@ -104,12 +99,16 @@ boolean foundCertificateForDn() { } private static class CertificateTrust { - private final List certificates; + /** + * This certificates are trusted in the relevant context. + * They might not match with the requested certificate (see {@link #match}) but will be for the requested DN. + */ + private final List trustedCertificates; private final boolean match; private final boolean identicalCertificate; private CertificateTrust(List certificates, boolean match, boolean identicalCertificate) { - this.certificates = certificates; + this.trustedCertificates = certificates; this.match = match; this.identicalCertificate = identicalCertificate; } @@ -118,20 +117,29 @@ private static CertificateTrust noMatchingIssuer() { return new CertificateTrust(null, false, false); } + /** + * We trust the provided certificates. + */ private static CertificateTrust sameCertificate(X509Certificate issuerCert) { return new CertificateTrust(List.of(issuerCert), true, true); } + /** + * Found trusted certificates with the same DN + same public keys, but different certificates + */ private static CertificateTrust samePublicKey(List issuerCerts) { return new CertificateTrust(issuerCerts, true, false); } + /** + * Found certificates for the requested DN, but they have different public keys + */ private static CertificateTrust nonMatchingCertificates(List certificates) { return new CertificateTrust(certificates, false, false); } boolean hasCertificates() { - return certificates != null && certificates.isEmpty() == false; + return trustedCertificates != null && trustedCertificates.isEmpty() == false; } boolean isTrusted() { @@ -150,11 +158,11 @@ boolean isSameCertificate() { */ public static String getTrustDiagnosticFailure(X509Certificate[] chain, PeerType peerType, SSLSession session, String contextName, @Nullable Map> trustedIssuers) { - final String peerAddress = session == null ? "" : session.getPeerHost(); + final String peerAddress = Optional.ofNullable(session).map(SSLSession::getPeerHost).orElse(""); - final StringBuilder message = new StringBuilder("failed to establish trust with [") - .append(peerType.name()) - .append("] at [") + final StringBuilder message = new StringBuilder("failed to establish trust with ") + .append(peerType.name().toLowerCase(Locale.ROOT)) + .append(" at [") .append(peerAddress) .append("]; "); @@ -201,7 +209,7 @@ public static String getTrustDiagnosticFailure(X509Certificate[] chain, PeerType if (chain.length == 1) { message.append(" but the ") .append(peerType.name().toLowerCase(Locale.ROOT)) - .append(" did not provide a copy of the issuing certificate") + .append(" did not provide a copy of the issuing certificate in the certificate chain") .append(describeIssuerTrust(contextName, trustedIssuers, peerCert, issuerName)); } } @@ -216,7 +224,7 @@ public static String getTrustDiagnosticFailure(X509Certificate[] chain, PeerType .append(fingerprintDescription(chain[i])); if (trustedIssuers != null) { - if (isTrusted(trustedIssuers, chain[i]).isTrusted()) { + if (resolveCertificateTrust(trustedIssuers, chain[i]).isTrusted()) { message.append(" {trusted issuer}"); } } @@ -245,10 +253,15 @@ private static CharSequence describeIssuerTrust(String contextName, @Nullable Ma StringBuilder message = new StringBuilder(); final IssuerTrust trust = checkIssuerTrust(trustedIssuers, certificate); if (trust.isVerified()) { - message.append("; the issuing certificate is trusted in this ssl context ([") + message.append("; the issuing ") + .append(trust.issuerCerts.size() == 1 ? "certificate": "certificates") + .append(" with ") + .append(fingerprintDescription(trust.issuerCerts)) + .append(" ") + .append(trust.issuerCerts.size() == 1 ? "is": "are") + .append(" trusted in this ssl context ([") .append(contextName) - .append("]) with ") - .append(fingerprintDescription(trust.issuerCerts)); + .append("])"); } else if (trust.foundCertificateForDn()) { message.append("; this ssl context ([") .append(contextName) @@ -271,58 +284,56 @@ private static CharSequence describeIssuerTrust(String contextName, @Nullable Ma private static CharSequence describeSelfIssuedCertificate(X509Certificate certificate, String contextName, @Nullable Map> trustedIssuers) { final StringBuilder message = new StringBuilder(); - final CertificateTrust trust = isTrusted(trustedIssuers, certificate); - message.append("self-issued [").append(certificate.getIssuerX500Principal().getName()).append("] and is ") - .append(trust.isTrusted() ? "trusted" : "not trusted") - .append(" in this ssl context ([") - .append(contextName) - .append("])"); + final CertificateTrust trust = resolveCertificateTrust(trustedIssuers, certificate); + message.append("self-issued; the [").append(certificate.getIssuerX500Principal().getName()).append("] certificate ") + .append(trust.isTrusted() ? "is" : "is not") + .append(" trusted in this ssl context ([").append(contextName).append("])"); if (trust.isTrusted()) { if (trust.isSameCertificate() == false) { - if (trust.certificates.size() == 1) { - message.append(" using a certificate with ") - .append(fingerprintDescription(trust.certificates.get(0))) - .append(" but the same public key"); + if (trust.trustedCertificates.size() == 1) { + message.append(" because we trust a certificate with ") + .append(fingerprintDescription(trust.trustedCertificates.get(0))) + .append(" for the same public key"); } else { - message.append(" using [") - .append(trust.certificates.size()) + message.append(" because we trust [") + .append(trust.trustedCertificates.size()) .append("] certificates with ") - .append(fingerprintDescription(trust.certificates)) - .append(" but the same public key"); + .append(fingerprintDescription(trust.trustedCertificates)) + .append(" for the same public key"); } } } else { if (trust.hasCertificates()) { - if (trust.certificates.size() == 1) { - final X509Certificate match = trust.certificates.get(0); + if (trust.trustedCertificates.size() == 1) { + final X509Certificate match = trust.trustedCertificates.get(0); message.append("; this ssl context does trust a certificate with subject [") .append(match.getSubjectX500Principal().getName()) - .append("] but it has ") + .append("] but the trusted certificate has ") .append(fingerprintDescription(match)); } else { message.append("; this ssl context does trust [") - .append(trust.certificates.size()) + .append(trust.trustedCertificates.size()) .append("] certificates with subject [") .append(certificate.getSubjectX500Principal().getName()) .append("] but those certificates have ") - .append(fingerprintDescription(trust.certificates)); + .append(fingerprintDescription(trust.trustedCertificates)); } } } return message; } - private static CertificateTrust isTrusted(Map> trustedIssuers, X509Certificate certificate) { - final List trustedCerts = trustedIssuers.get(certificate.getSubjectX500Principal().getName()); + private static CertificateTrust resolveCertificateTrust(Map> trustedIssuers, X509Certificate cert) { + final List trustedCerts = trustedIssuers.get(cert.getSubjectX500Principal().getName()); if (trustedCerts == null || trustedCerts.isEmpty()) { return CertificateTrust.noMatchingIssuer(); } - final int index = trustedCerts.indexOf(certificate); + final int index = trustedCerts.indexOf(cert); if (index != -1) { return CertificateTrust.sameCertificate(trustedCerts.get(index)); } final List sameKey = trustedCerts.stream() - .filter(c -> c.getPublicKey().equals(certificate.getPublicKey())) + .filter(c -> c.getPublicKey().equals(cert.getPublicKey())) .collect(Collectors.toList()); if (sameKey.isEmpty() == false) { return CertificateTrust.samePublicKey(sameKey); diff --git a/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/SslUtil.java b/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/SslUtil.java index 024c409330f28..d6d8330a85ba1 100644 --- a/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/SslUtil.java +++ b/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/SslUtil.java @@ -25,7 +25,11 @@ import java.security.cert.X509Certificate; import java.util.Objects; -public class SslUtil { +public final class SslUtil { + + private SslUtil() { + // utility class + } public static String calculateFingerprint(X509Certificate certificate) throws CertificateEncodingException { final MessageDigest sha1 = messageDigest("SHA-1"); diff --git a/libs/ssl-config/src/test/java/org/elasticsearch/common/ssl/SslDiagnosticsTests.java b/libs/ssl-config/src/test/java/org/elasticsearch/common/ssl/SslDiagnosticsTests.java index 26a6cad02fa2a..c22ab80496e85 100644 --- a/libs/ssl-config/src/test/java/org/elasticsearch/common/ssl/SslDiagnosticsTests.java +++ b/libs/ssl-config/src/test/java/org/elasticsearch/common/ssl/SslDiagnosticsTests.java @@ -61,13 +61,13 @@ public void testDiagnosticMessageWhenServerProvidesAFullCertChainThatIsTrusted() final Map> trustIssuers = trust("ca1/ca.crt", "ca2/ca.crt", "ca3/ca.crt"); final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, "xpack.http.ssl", trustIssuers); - assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.1.1];" + + assertThat(message, Matchers.equalTo("failed to establish trust with server at [192.168.1.1];" + " the server provided a certificate with subject name [CN=cert1] and fingerprint [3bebe388a66362784afd6c51a9000961a4e10050];" + " the certificate has subject alternative names [DNS:localhost,IP:127.0.0.1];" + " the certificate is issued by [CN=Test CA 1];" + " the certificate is signed by" + " (subject [CN=Test CA 1] fingerprint [2b7b0416391bdf86502505c23149022d2213dadc] {trusted issuer})" + - " which is self-issued [CN=Test CA 1] and is trusted in this ssl context ([xpack.http.ssl])")); + " which is self-issued; the [CN=Test CA 1] certificate is trusted in this ssl context ([xpack.http.ssl])")); } public void testDiagnosticMessageWhenServerProvidesAFullCertChainThatIsntTrusted() throws Exception { @@ -76,12 +76,12 @@ public void testDiagnosticMessageWhenServerProvidesAFullCertChainThatIsntTrusted final Map> trustIssuers = trust("ca2/ca.crt", "ca3/ca.crt"); final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, "xpack.http.ssl", trustIssuers); - assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.1.1];" + + assertThat(message, Matchers.equalTo("failed to establish trust with server at [192.168.1.1];" + " the server provided a certificate with subject name [CN=cert1] and fingerprint [3bebe388a66362784afd6c51a9000961a4e10050];" + " the certificate has subject alternative names [DNS:localhost,IP:127.0.0.1];" + " the certificate is issued by [CN=Test CA 1];" + " the certificate is signed by (subject [CN=Test CA 1] fingerprint [2b7b0416391bdf86502505c23149022d2213dadc])" + - " which is self-issued [CN=Test CA 1] and is not trusted in this ssl context ([xpack.http.ssl])")); + " which is self-issued; the [CN=Test CA 1] certificate is not trusted in this ssl context ([xpack.http.ssl])")); } public void testDiagnosticMessageWhenServerFullCertChainIsntTrustedButMimicIssuerExists() throws Exception { @@ -90,14 +90,14 @@ public void testDiagnosticMessageWhenServerFullCertChainIsntTrustedButMimicIssue final Map> trustIssuers = trust("ca1-b/ca.crt", "ca2/ca.crt", "ca3/ca.crt"); final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, "xpack.http.ssl", trustIssuers); - assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.1.1];" + + assertThat(message, Matchers.equalTo("failed to establish trust with server at [192.168.1.1];" + " the server provided a certificate with subject name [CN=cert1] and fingerprint [3bebe388a66362784afd6c51a9000961a4e10050];" + " the certificate has subject alternative names [DNS:localhost,IP:127.0.0.1];" + " the certificate is issued by [CN=Test CA 1];" + " the certificate is signed by (subject [CN=Test CA 1] fingerprint [2b7b0416391bdf86502505c23149022d2213dadc])" + - " which is self-issued [CN=Test CA 1] and is not trusted in this ssl context ([xpack.http.ssl]);" + + " which is self-issued; the [CN=Test CA 1] certificate is not trusted in this ssl context ([xpack.http.ssl]);" + " this ssl context does trust a certificate with subject [CN=Test CA 1]" + - " but it has fingerprint [b095bf2526be20783e1f26dfd69c7aae910e3663]")); + " but the trusted certificate has fingerprint [b095bf2526be20783e1f26dfd69c7aae910e3663]")); } public void testDiagnosticMessageWhenServerProvidesEndCertificateOnlyAndTheCertAuthIsTrusted() throws Exception { @@ -106,12 +106,13 @@ public void testDiagnosticMessageWhenServerProvidesEndCertificateOnlyAndTheCertA final Map> trustIssuers = trust("ca1/ca.crt", "ca2/ca.crt", "ca3/ca.crt"); final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, "xpack.http.ssl", trustIssuers); - assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.1.1];" + + assertThat(message, Matchers.equalTo("failed to establish trust with server at [192.168.1.1];" + " the server provided a certificate with subject name [CN=cert1] and fingerprint [3bebe388a66362784afd6c51a9000961a4e10050];" + " the certificate has subject alternative names [DNS:localhost,IP:127.0.0.1];" + - " the certificate is issued by [CN=Test CA 1] but the server did not provide a copy of the issuing certificate;" + - " the issuing certificate is trusted in this ssl context ([xpack.http.ssl])" + - " with fingerprint [2b7b0416391bdf86502505c23149022d2213dadc]")); + " the certificate is issued by [CN=Test CA 1]" + + " but the server did not provide a copy of the issuing certificate in the certificate chain;" + + " the issuing certificate with fingerprint [2b7b0416391bdf86502505c23149022d2213dadc]" + + " is trusted in this ssl context ([xpack.http.ssl])")); } public void testDiagnosticMessageWhenServerProvidesEndCertificateOnlyButTheCertAuthIsNotTrusted() throws Exception { @@ -120,10 +121,11 @@ public void testDiagnosticMessageWhenServerProvidesEndCertificateOnlyButTheCertA final Map> trustIssuers = trust("ca2/ca.crt", "ca3/ca.crt"); final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, "xpack.http.ssl", trustIssuers); - assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.1.1];" + + assertThat(message, Matchers.equalTo("failed to establish trust with server at [192.168.1.1];" + " the server provided a certificate with subject name [CN=cert1] and fingerprint [3bebe388a66362784afd6c51a9000961a4e10050];" + " the certificate has subject alternative names [DNS:localhost,IP:127.0.0.1];" + - " the certificate is issued by [CN=Test CA 1] but the server did not provide a copy of the issuing certificate;" + + " the certificate is issued by [CN=Test CA 1]" + + " but the server did not provide a copy of the issuing certificate in the certificate chain;" + " this ssl context ([xpack.http.ssl]) is not configured to trust that issuer")); } @@ -133,10 +135,11 @@ public void testDiagnosticMessageWhenServerProvidesEndCertificateOnlyWithMimicIs final Map> trustIssuers = trust("ca1-b/ca.crt", "ca2/ca.crt", "ca3/ca.crt"); final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, "xpack.http.ssl", trustIssuers); - assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.1.1];" + + assertThat(message, Matchers.equalTo("failed to establish trust with server at [192.168.1.1];" + " the server provided a certificate with subject name [CN=cert1] and fingerprint [3bebe388a66362784afd6c51a9000961a4e10050];" + " the certificate has subject alternative names [DNS:localhost,IP:127.0.0.1];" + - " the certificate is issued by [CN=Test CA 1] but the server did not provide a copy of the issuing certificate;" + + " the certificate is issued by [CN=Test CA 1]" + + " but the server did not provide a copy of the issuing certificate in the certificate chain;" + " this ssl context ([xpack.http.ssl]) trusts [1] certificate with subject name [CN=Test CA 1]" + " and fingerprint [b095bf2526be20783e1f26dfd69c7aae910e3663] but the signatures do not match")); } @@ -148,10 +151,11 @@ public void testDiagnosticMessageWhenServerProvidesEndCertificateWithMultipleMim final Map> trustIssuers = trust(ca1b, cloneCertificateAsMock(ca1b)); final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, "xpack.http.ssl", trustIssuers); - assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.1.9];" + + assertThat(message, Matchers.equalTo("failed to establish trust with server at [192.168.1.9];" + " the server provided a certificate with subject name [CN=cert1] and fingerprint [3bebe388a66362784afd6c51a9000961a4e10050];" + " the certificate has subject alternative names [DNS:localhost,IP:127.0.0.1];" + - " the certificate is issued by [CN=Test CA 1] but the server did not provide a copy of the issuing certificate;" + + " the certificate is issued by [CN=Test CA 1]" + + " but the server did not provide a copy of the issuing certificate in the certificate chain;" + " this ssl context ([xpack.http.ssl]) trusts [2] certificates with subject name [CN=Test CA 1]" + " and fingerprint [b095bf2526be20783e1f26dfd69c7aae910e3663], fingerprint [" + MOCK_FINGERPRINT_1 + "]" + " but the signatures do not match")); @@ -173,7 +177,7 @@ public void testDiagnosticMessageWhenServerProvidePartialChainFromTrustedCA() th final Map> trustIssuers = trust(issuingCA, rootCA); final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, "xpack.security.authc.realms.ldap.ldap1.ssl", trustIssuers); - assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.1.5];" + + assertThat(message, Matchers.equalTo("failed to establish trust with server at [192.168.1.5];" + " the server provided a certificate with subject name [CN=elastic1,OU=windows,DC=example,DC=com]" + " and fingerprint [" + MOCK_FINGERPRINT_4 + "];" + " the certificate does not have any subject alternative names;" + @@ -182,8 +186,8 @@ public void testDiagnosticMessageWhenServerProvidePartialChainFromTrustedCA() th " signed by (subject [CN=ca,OU=windows,DC=example,DC=com] fingerprint [" + MOCK_FINGERPRINT_3 + "])" + " signed by (subject [CN=issuing-ca,DC=example,DC=com] fingerprint [" + MOCK_FINGERPRINT_2 + "] {trusted issuer})" + " which is issued by [CN=root-ca,DC=example,DC=com] (but that issuer certificate was not provided in the chain);" + - " the issuing certificate is trusted in this ssl context ([xpack.security.authc.realms.ldap.ldap1.ssl])" + - " with fingerprint [" + MOCK_FINGERPRINT_1 + "]")); + " the issuing certificate with fingerprint [" + MOCK_FINGERPRINT_1 + "]" + + " is trusted in this ssl context ([xpack.security.authc.realms.ldap.ldap1.ssl])")); } public void testDiagnosticMessageWhenServerProvidePartialChainFromUntrustedCA() throws Exception { @@ -202,7 +206,7 @@ public void testDiagnosticMessageWhenServerProvidePartialChainFromUntrustedCA() final Map> trustIssuers = trust(Collections.emptyList()); final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, "xpack.security.authc.realms.ldap.ldap1.ssl", trustIssuers); - assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.1.6];" + + assertThat(message, Matchers.equalTo("failed to establish trust with server at [192.168.1.6];" + " the server provided a certificate with subject name [CN=elastic1,OU=windows,DC=example,DC=com]" + " and fingerprint [" + MOCK_FINGERPRINT_4 + "];" + " the certificate does not have any subject alternative names;" + @@ -220,11 +224,11 @@ public void testDiagnosticMessageWhenServerProvidesASelfSignedCertThatIsDirectly final Map> trustIssuers = trust("ca1/ca.crt", "ca2/ca.crt"); final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, "xpack.http.ssl", trustIssuers); - assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.1.1];" + + assertThat(message, Matchers.equalTo("failed to establish trust with server at [192.168.1.1];" + " the server provided a certificate with subject name [CN=Test CA 1]" + " and fingerprint [2b7b0416391bdf86502505c23149022d2213dadc];" + " the certificate does not have any subject alternative names;" + - " the certificate is self-issued [CN=Test CA 1] and is trusted in this ssl context ([xpack.http.ssl])")); + " the certificate is self-issued; the [CN=Test CA 1] certificate is trusted in this ssl context ([xpack.http.ssl])")); } public void testDiagnosticMessageWhenServerProvidesASelfSignedCertThatIsNotTrusted() throws Exception { @@ -233,11 +237,11 @@ public void testDiagnosticMessageWhenServerProvidesASelfSignedCertThatIsNotTrust final Map> trustIssuers = Collections.emptyMap(); final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, "xpack.http.ssl", trustIssuers); - assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.10.10];" + + assertThat(message, Matchers.equalTo("failed to establish trust with server at [192.168.10.10];" + " the server provided a certificate with subject name [CN=Test CA 1]" + " and fingerprint [2b7b0416391bdf86502505c23149022d2213dadc];" + " the certificate does not have any subject alternative names;" + - " the certificate is self-issued [CN=Test CA 1] and is not trusted in this ssl context ([xpack.http.ssl])")); + " the certificate is self-issued; the [CN=Test CA 1] certificate is not trusted in this ssl context ([xpack.http.ssl])")); } public void testDiagnosticMessageWhenServerProvidesASelfSignedCertWithMimicName() throws Exception { @@ -246,13 +250,13 @@ public void testDiagnosticMessageWhenServerProvidesASelfSignedCertWithMimicName( final Map> trustIssuers = trust("ca1-b/ca.crt", "ca2/ca.crt"); final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, "xpack.http.ssl", trustIssuers); - assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.1.1];" + + assertThat(message, Matchers.equalTo("failed to establish trust with server at [192.168.1.1];" + " the server provided a certificate with subject name [CN=Test CA 1]" + " and fingerprint [2b7b0416391bdf86502505c23149022d2213dadc];" + " the certificate does not have any subject alternative names;" + - " the certificate is self-issued [CN=Test CA 1] and is not trusted in this ssl context ([xpack.http.ssl]);" + + " the certificate is self-issued; the [CN=Test CA 1] certificate is not trusted in this ssl context ([xpack.http.ssl]);" + " this ssl context does trust a certificate with subject [CN=Test CA 1]" + - " but it has fingerprint [b095bf2526be20783e1f26dfd69c7aae910e3663]")); + " but the trusted certificate has fingerprint [b095bf2526be20783e1f26dfd69c7aae910e3663]")); } public void testDiagnosticMessageWithEmptyChain() throws Exception { @@ -261,7 +265,7 @@ public void testDiagnosticMessageWithEmptyChain() throws Exception { final Map> trustIssuers = Collections.emptyMap(); final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, "xpack.http.ssl", trustIssuers); - assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.1.2];" + + assertThat(message, Matchers.equalTo("failed to establish trust with server at [192.168.1.2];" + " the server did not provide a certificate")); } @@ -275,12 +279,12 @@ public void testDiagnosticMessageWhenServerProvidesAnEmailSubjAltName() throws E final Map> trustIssuers = trust(certificate); final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, "xpack.monitoring.exporters.elastic-cloud.ssl", trustIssuers); - assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.1.3];" + + assertThat(message, Matchers.equalTo("failed to establish trust with server at [192.168.1.3];" + " the server provided a certificate with subject name [CN=foo,DC=example,DC=com]" + " and fingerprint [" + MOCK_FINGERPRINT_1 + "];" + " the certificate does not have any DNS/IP subject alternative names;" + - " the certificate is self-issued [CN=foo,DC=example,DC=com]" + - " and is trusted in this ssl context ([xpack.monitoring.exporters.elastic-cloud.ssl])")); + " the certificate is self-issued;" + + " the [CN=foo,DC=example,DC=com] certificate is trusted in this ssl context ([xpack.monitoring.exporters.elastic-cloud.ssl])")); } public void testDiagnosticMessageWhenACertificateHasAnInvalidEncoding() throws Exception { @@ -293,12 +297,12 @@ public void testDiagnosticMessageWhenACertificateHasAnInvalidEncoding() throws E final Map> trustIssuers = trust(Collections.emptyList()); final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, "xpack.security.transport.ssl", trustIssuers); - assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.1.6];" + + assertThat(message, Matchers.equalTo("failed to establish trust with server at [192.168.1.6];" + " the server provided a certificate with subject name [CN=foo,DC=example,DC=com]" + " and invalid encoding [java.security.cert.CertificateEncodingException: MOCK INVALID ENCODING];" + " the certificate does not have any subject alternative names;" + - " the certificate is self-issued [CN=foo,DC=example,DC=com]" + - " and is not trusted in this ssl context ([xpack.security.transport.ssl])")); + " the certificate is self-issued;" + + " the [CN=foo,DC=example,DC=com] certificate is not trusted in this ssl context ([xpack.security.transport.ssl])")); } public void testDiagnosticMessageForClientCertificate() throws Exception { @@ -308,12 +312,13 @@ public void testDiagnosticMessageForClientCertificate() throws Exception { final Map> trustIssuers = trust("ca1/ca.crt"); final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.CLIENT, session, "xpack.security.transport.ssl", trustIssuers); - assertThat(message, Matchers.equalTo("failed to establish trust with [CLIENT] at [192.168.1.7];" + + assertThat(message, Matchers.equalTo("failed to establish trust with client at [192.168.1.7];" + " the client provided a certificate with subject name [CN=cert1]" + " and fingerprint [3bebe388a66362784afd6c51a9000961a4e10050];" + - " the certificate is issued by [CN=Test CA 1] but the client did not provide a copy of the issuing certificate;" + - " the issuing certificate is trusted in this ssl context ([xpack.security.transport.ssl])" + - " with fingerprint [2b7b0416391bdf86502505c23149022d2213dadc]")); + " the certificate is issued by [CN=Test CA 1]" + + " but the client did not provide a copy of the issuing certificate in the certificate chain;" + + " the issuing certificate with fingerprint [2b7b0416391bdf86502505c23149022d2213dadc]" + + " is trusted in this ssl context ([xpack.security.transport.ssl])")); } public void testDiagnosticMessageWhenCaHasNewIssuingCertificate() throws Exception { @@ -331,15 +336,16 @@ public void testDiagnosticMessageWhenCaHasNewIssuingCertificate() throws Excepti final Map> trustIssuers = trust(oldCaCert); final String message = SslDiagnostics.getTrustDiagnosticFailure(chain, SslDiagnostics.PeerType.SERVER, session, "xpack.security.authc.realms.saml.saml1.ssl", trustIssuers); - assertThat(message, Matchers.equalTo("failed to establish trust with [SERVER] at [192.168.1.4];" + + assertThat(message, Matchers.equalTo("failed to establish trust with server at [192.168.1.4];" + " the server provided a certificate with subject name [CN=cert1] and fingerprint [3bebe388a66362784afd6c51a9000961a4e10050];" + " the certificate has subject alternative names [DNS:localhost,IP:127.0.0.1];" + " the certificate is issued by [CN=Test CA 1];" + " the certificate is signed by (subject [CN=Test CA 1]" + " fingerprint [2b7b0416391bdf86502505c23149022d2213dadc] {trusted issuer})" + - " which is self-issued [CN=Test CA 1] and is trusted in this ssl context ([xpack.security.authc.realms.saml.saml1.ssl])" + - " using a certificate with fingerprint [1f8ac10f23c5b5bc1167bda84b833e5c057a77d2]" + - " but the same public key")); + " which is self-issued;" + + " the [CN=Test CA 1] certificate is trusted in this ssl context ([xpack.security.authc.realms.saml.saml1.ssl])" + + " because we trust a certificate with fingerprint [1f8ac10f23c5b5bc1167bda84b833e5c057a77d2]" + + " for the same public key")); } public X509Certificate cloneCertificateAsMock(X509Certificate clone) throws CertificateParsingException, CertificateEncodingException { diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/CertParsingUtils.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/CertParsingUtils.java index fac69fd972790..30b75aadf5fbb 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/CertParsingUtils.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/CertParsingUtils.java @@ -295,5 +295,4 @@ public static boolean isOrderedCertificateChain(List chain) { } return true; } - } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java index 3386e48e1e489..9bea45cc8e152 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java @@ -18,6 +18,7 @@ import org.elasticsearch.common.CheckedSupplier; import org.elasticsearch.common.Strings; import org.elasticsearch.common.logging.LoggerMessageFormat; +import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.ssl.DiagnosticTrustManager; import org.elasticsearch.common.ssl.SslDiagnostics; @@ -96,6 +97,9 @@ public class SSLService { ORDERED_PROTOCOL_ALGORITHM_MAP = Collections.unmodifiableMap(protocolAlgorithmMap); } + private static final Setting DIAGNOSE_TRUST_EXCEPTIONS_SETTING = Setting.boolSetting( + "xpack.security.ssl.diagnose_trust_failure", true, Setting.Property.NodeScope); + private final Settings settings; private final boolean diagnoseTrustExceptions; @@ -125,7 +129,7 @@ public class SSLService { public SSLService(Settings settings, Environment environment) { this.settings = settings; this.env = environment; - this.diagnoseTrustExceptions = true; // TODO, turn off in FIPS mode + this.diagnoseTrustExceptions = DIAGNOSE_TRUST_EXCEPTIONS_SETTING.get(settings); this.sslConfigurations = new HashMap<>(); this.sslContexts = loadSSLConfigurations(); } @@ -134,7 +138,7 @@ private SSLService(Settings settings, Environment environment, Map sslContexts) { this.settings = settings; this.env = environment; - this.diagnoseTrustExceptions = true; // TODO, turn off in FIPS mode + this.diagnoseTrustExceptions = DIAGNOSE_TRUST_EXCEPTIONS_SETTING.get(settings); this.sslConfigurations = sslConfigurations; this.sslContexts = sslContexts; } @@ -169,6 +173,10 @@ SSLContextHolder sslContextHolder(SSLConfiguration sslConfiguration) { }; } + public static void registerSettings(List> settingList) { + settingList.add(DIAGNOSE_TRUST_EXCEPTIONS_SETTING); + } + /** * Create a new {@link SSLIOSessionStrategy} based on the provided settings. The settings are used to identify the SSL configuration * that should be used to create the context. @@ -452,7 +460,7 @@ private X509ExtendedTrustManager wrapWithDiagnostics(X509ExtendedTrustManager tr final Supplier contextName = () -> { final List names = sslConfigurations.entrySet().stream() .filter(e -> e.getValue().equals(configuration)) - .limit(2) + .limit(2) // we only need to distinguishing between 0/1/many .map(Entry::getKey) .collect(Collectors.toUnmodifiableList()); switch (names.size()) { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java index a0e07907a8712..fe8ff8f5903b7 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java @@ -588,6 +588,7 @@ public static List> getSettings(List securityExten // The following just apply in node mode settingsList.add(XPackSettings.FIPS_MODE_ENABLED); + SSLService.registerSettings(settingsList); // IP Filter settings IPFilter.addSettings(settingsList); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLErrorMessageCertificateVerificationTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLErrorMessageCertificateVerificationTests.java index 3bbd7a036ec69..691b6501273a5 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLErrorMessageCertificateVerificationTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLErrorMessageCertificateVerificationTests.java @@ -20,6 +20,7 @@ import org.elasticsearch.common.SuppressForbidden; import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.ssl.DiagnosticTrustManager; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.MockLogAppender; import org.elasticsearch.test.http.MockResponse; @@ -29,7 +30,6 @@ import org.elasticsearch.xpack.core.ssl.SSLConfiguration; import org.elasticsearch.xpack.core.ssl.SSLService; import org.elasticsearch.xpack.core.ssl.VerificationMode; -import org.elasticsearch.common.ssl.DiagnosticTrustManager; import javax.net.ssl.HostnameVerifier; import javax.net.ssl.SSLHandshakeException; @@ -52,7 +52,6 @@ public class SSLErrorMessageCertificateVerificationTests extends ESTestCase { private static final String HTTP_SERVER_SSL = "xpack.security.http.ssl"; private static final String HTTP_CLIENT_SSL = "xpack.http.ssl"; - private static final String TRANSPORT_SSL = "xpack.security.transport.ssl"; public void testMessageForHttpClientHostnameVerificationFailure() throws IOException, URISyntaxException { final Settings sslSetup = getPemSSLSettings(HTTP_SERVER_SSL, "not-this-host.crt", "not-this-host.key", @@ -117,14 +116,14 @@ public void testDiagnosticTrustManagerForHostnameVerificationFailure() throws Ex "ssl diagnostic", DiagnosticTrustManager.class.getName(), Level.WARN, - "failed to establish trust with \\[SERVER\\] at \\[" + Pattern.quote(webServer.getHostName()) + "\\];" + + "failed to establish trust with server at \\[" + Pattern.quote(webServer.getHostName()) + "\\];" + " the server provided a certificate with subject name \\[CN=not-this-host\\]" + " and fingerprint \\[[0-9a-f]{40}\\];" + " the certificate has subject alternative names \\[DNS:not\\.this\\.host\\];" + " the certificate is issued by \\[CN=Certificate Authority 1,OU=ssl-error-message-test,DC=elastic,DC=co\\]" + - " but the server did not provide a copy of the issuing certificate;" + - " the issuing certificate is trusted in this ssl context " + Pattern.quote("([" + HTTP_CLIENT_SSL + "])") + - " with fingerprint \\[[0-9a-f]{40}\\]")); + " but the server did not provide a copy of the issuing certificate in the certificate chain;" + + " the issuing certificate with fingerprint \\[[0-9a-f]{40}\\]" + + " is trusted in this ssl context " + Pattern.quote("([" + HTTP_CLIENT_SSL + "])"))); enableHttpsHostnameChecking(clientSocket); connect(clientSocket, webServer); assertThat(clientSocket.isConnected(), is(true)); @@ -132,6 +131,9 @@ public void testDiagnosticTrustManagerForHostnameVerificationFailure() throws Ex () -> clientSocket.getInputStream().read()); assertThat(handshakeException, throwableWithMessage(containsStringIgnoringCase("subject alternative names"))); assertThat(handshakeException, throwableWithMessage(containsString(webServer.getHostName()))); + + // Logging message failures are tricky to debug because you just get a "didn't find match" assertion failure. + // You should be able to check the log output for the text that was logged and compare to the regex above. mockAppender.assertAllExpectationsMatched(); } finally { Loggers.removeAppender(diagnosticLogger, mockAppender); From b56ab6555e6b8602eddca9f39979d3cb135f46b5 Mon Sep 17 00:00:00 2001 From: Tim Vernum Date: Tue, 19 Nov 2019 16:06:59 +1100 Subject: [PATCH 3/6] Rename setting and add test --- .../xpack/core/ssl/SSLService.java | 4 +- .../xpack/core/ssl/SSLServiceTests.java | 102 +++++++++++------- .../security/qa/security-basic/build.gradle | 1 + 3 files changed, 66 insertions(+), 41 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java index 9bea45cc8e152..803f5fb856476 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java @@ -98,7 +98,7 @@ public class SSLService { } private static final Setting DIAGNOSE_TRUST_EXCEPTIONS_SETTING = Setting.boolSetting( - "xpack.security.ssl.diagnose_trust_failure", true, Setting.Property.NodeScope); + "xpack.security.ssl.diagnose.trust", true, Setting.Property.NodeScope); private final Settings settings; private final boolean diagnoseTrustExceptions; @@ -450,7 +450,7 @@ private SSLContextHolder createSslContext(X509ExtendedKeyManager keyManager, X50 } } - private X509ExtendedTrustManager wrapWithDiagnostics(X509ExtendedTrustManager trustManager, SSLConfiguration configuration) { + X509ExtendedTrustManager wrapWithDiagnostics(X509ExtendedTrustManager trustManager, SSLConfiguration configuration) { if (diagnoseTrustExceptions && trustManager instanceof DiagnosticTrustManager == false) { final Logger diagnosticLogger = LogManager.getLogger(DiagnosticTrustManager.class); // A single configuration might be used in many place, if there are multiple, we just list "shared" because diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLServiceTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLServiceTests.java index 45cb8f8d24075..67bb99e508af3 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLServiceTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLServiceTests.java @@ -22,6 +22,7 @@ import org.elasticsearch.common.SuppressForbidden; import org.elasticsearch.common.settings.MockSecureSettings; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.ssl.DiagnosticTrustManager; import org.elasticsearch.env.Environment; import org.elasticsearch.env.TestEnvironment; import org.elasticsearch.test.ESTestCase; @@ -40,6 +41,7 @@ import javax.net.ssl.SSLSessionContext; import javax.net.ssl.SSLSocket; import javax.net.ssl.SSLSocketFactory; +import javax.net.ssl.X509ExtendedTrustManager; import javax.security.cert.X509Certificate; import java.nio.file.Path; import java.security.AccessController; @@ -112,21 +114,21 @@ public void testThatCustomTruststoreCanBeSpecified() throws Exception { secureSettings.setString("xpack.security.transport.ssl.keystore.secure_password", "testnode"); secureSettings.setString("transport.profiles.foo.xpack.security.ssl.truststore.secure_password", "testclient"); Settings settings = Settings.builder() - .put("xpack.security.transport.ssl.enabled", true) - .put("xpack.security.transport.ssl.keystore.path", testnodeStore) - .put("xpack.security.transport.ssl.truststore.path", testnodeStore) - .put("xpack.security.transport.ssl.truststore.type", testnodeStoreType) - .setSecureSettings(secureSettings) - .put("transport.profiles.foo.xpack.security.ssl.truststore.path", testClientStore) - .build(); + .put("xpack.security.transport.ssl.enabled", true) + .put("xpack.security.transport.ssl.keystore.path", testnodeStore) + .put("xpack.security.transport.ssl.truststore.path", testnodeStore) + .put("xpack.security.transport.ssl.truststore.type", testnodeStoreType) + .setSecureSettings(secureSettings) + .put("transport.profiles.foo.xpack.security.ssl.truststore.path", testClientStore) + .build(); SSLService sslService = new SSLService(settings, env); MockSecureSettings secureCustomSettings = new MockSecureSettings(); secureCustomSettings.setString("truststore.secure_password", "testclient"); Settings customTruststoreSettings = Settings.builder() - .put("truststore.path", testClientStore) - .setSecureSettings(secureCustomSettings) - .build(); + .put("truststore.path", testClientStore) + .setSecureSettings(secureCustomSettings) + .build(); SSLConfiguration configuration = new SSLConfiguration(customTruststoreSettings); SSLEngine sslEngineWithTruststore = sslService.createSSLEngine(configuration, null, -1); @@ -146,11 +148,11 @@ public void testThatSslContextCachingWorks() throws Exception { MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("xpack.security.transport.ssl.secure_key_passphrase", "testnode"); Settings settings = Settings.builder() - .put("xpack.security.transport.ssl.enabled", true) - .put("xpack.security.transport.ssl.certificate", testnodeCert) - .put("xpack.security.transport.ssl.key", testnodeKey) - .setSecureSettings(secureSettings) - .build(); + .put("xpack.security.transport.ssl.enabled", true) + .put("xpack.security.transport.ssl.certificate", testnodeCert) + .put("xpack.security.transport.ssl.key", testnodeKey) + .setSecureSettings(secureSettings) + .build(); SSLService sslService = new SSLService(settings, env); final Settings transportSSLSettings = settings.getByPrefix("xpack.security.transport.ssl."); @@ -167,7 +169,7 @@ public void testThatSslContextCachingWorks() throws Exception { public void testThatKeyStoreAndKeyCanHaveDifferentPasswords() throws Exception { assumeFalse("Can't run in a FIPS JVM", inFipsJvm()); Path differentPasswordsStore = - getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode-different-passwords.jks"); + getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode-different-passwords.jks"); MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("xpack.security.transport.ssl.keystore.secure_password", "testnode"); secureSettings.setString("xpack.security.transport.ssl.keystore.secure_key_password", "testnode1"); @@ -185,7 +187,7 @@ public void testThatKeyStoreAndKeyCanHaveDifferentPasswords() throws Exception { public void testIncorrectKeyPasswordThrowsException() throws Exception { assumeFalse("Can't run in a FIPS JVM", inFipsJvm()); Path differentPasswordsStore = - getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode-different-passwords.jks"); + getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode-different-passwords.jks"); try { MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("xpack.security.transport.ssl.keystore.secure_password", "testnode"); @@ -346,7 +348,7 @@ public void testThatTruststorePasswordIsRequired() throws Exception { .put("xpack.security.transport.ssl.truststore.type", testnodeStoreType) .build(); ElasticsearchException e = - expectThrows(ElasticsearchException.class, () -> new SSLService(settings, env)); + expectThrows(ElasticsearchException.class, () -> new SSLService(settings, env)); assertThat(e, throwableWithMessage("failed to load SSL configuration [xpack.security.transport.ssl]")); assertThat(e.getCause(), throwableWithMessage(containsString("failed to initialize SSL TrustManager"))); } @@ -357,7 +359,7 @@ public void testThatKeystorePasswordIsRequired() throws Exception { .put("xpack.security.transport.ssl.keystore.type", testnodeStoreType) .build(); ElasticsearchException e = - expectThrows(ElasticsearchException.class, () -> new SSLService(settings, env)); + expectThrows(ElasticsearchException.class, () -> new SSLService(settings, env)); assertThat(e, throwableWithMessage("failed to load SSL configuration [xpack.security.transport.ssl]")); assertThat(e.getCause(), throwableWithMessage("failed to create trust manager")); } @@ -394,7 +396,7 @@ public void testInvalidCiphersOnlyThrowsException() throws Exception { .putList("xpack.security.transport.ssl.cipher_suites", new String[] { "foo", "bar" }) .build(); ElasticsearchException e = - expectThrows(ElasticsearchException.class, () -> new SSLService(settings, env)); + expectThrows(ElasticsearchException.class, () -> new SSLService(settings, env)); assertThat(e, throwableWithMessage("failed to load SSL configuration [xpack.security.transport.ssl]")); assertThat(e.getCause(), throwableWithMessage("none of the ciphers [foo, bar] are supported by this JVM")); } @@ -466,10 +468,10 @@ public void testSSLStrategy() { // this just exhaustively verifies that the right things are called and that it uses the right parameters VerificationMode mode = randomFrom(VerificationMode.values()); Settings settings = Settings.builder() - .put("supported_protocols", "protocols") - .put("cipher_suites", "") - .put("verification_mode", mode.name()) - .build(); + .put("supported_protocols", "protocols") + .put("cipher_suites", "") + .put("verification_mode", mode.name()) + .build(); SSLService sslService = mock(SSLService.class); SSLConfiguration sslConfig = new SSLConfiguration(settings); SSLParameters sslParameters = mock(SSLParameters.class); @@ -566,19 +568,19 @@ public void testReadCertificateInformation() throws Exception { secureSettings.setString("xpack.http.ssl.keystore.secure_password", "testnode"); final Settings settings = Settings.builder() - .put("xpack.security.transport.ssl.enabled", randomBoolean()) - .put("xpack.security.transport.ssl.keystore.path", jksPath) - .put("xpack.security.transport.ssl.truststore.path", jksPath) - .put("xpack.http.ssl.keystore.path", p12Path) - .put("xpack.security.authc.realms.active_directory.ad.ssl.certificate_authorities", pemPath) - .setSecureSettings(secureSettings) - .build(); + .put("xpack.security.transport.ssl.enabled", randomBoolean()) + .put("xpack.security.transport.ssl.keystore.path", jksPath) + .put("xpack.security.transport.ssl.truststore.path", jksPath) + .put("xpack.http.ssl.keystore.path", p12Path) + .put("xpack.security.authc.realms.active_directory.ad.ssl.certificate_authorities", pemPath) + .setSecureSettings(secureSettings) + .build(); final SSLService sslService = new SSLService(settings, env); final List certificates = new ArrayList<>(sslService.getLoadedCertificates()); assertThat(certificates, iterableWithSize(10)); Collections.sort(certificates, - Comparator.comparing((CertificateInfo c) -> c.alias() == null ? "" : c.alias()).thenComparing(CertificateInfo::path)); + Comparator.comparing((CertificateInfo c) -> c.alias() == null ? "" : c.alias()).thenComparing(CertificateInfo::path)); final Iterator iterator = certificates.iterator(); CertificateInfo cert = iterator.next(); @@ -742,9 +744,9 @@ public void testThatSSLContextTrustsJDKTrustedCAs() throws Exception { MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("xpack.security.transport.ssl.keystore.secure_password", "testclient"); Settings settings = Settings.builder() - .put("xpack.security.transport.ssl.keystore.path", testclientStore) - .setSecureSettings(secureSettings) - .build(); + .put("xpack.security.transport.ssl.keystore.path", testclientStore) + .setSecureSettings(secureSettings) + .build(); SSLService sslService = new SSLService(settings, env); SSLContext sslContext = sslService.sslContext(sslService.sslConfiguration(settings.getByPrefix("xpack.security.transport.ssl."))); try (CloseableHttpClient client = HttpClients.custom().setSSLContext(sslContext).build()) { @@ -775,9 +777,9 @@ public void testThatSSLIOSessionStrategyTrustsJDKTrustedCAs() throws Exception { MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("xpack.security.transport.ssl.keystore.secure_password", "testclient"); Settings settings = Settings.builder() - .put("xpack.security.transport.ssl.keystore.path", testclientStore) - .setSecureSettings(secureSettings) - .build(); + .put("xpack.security.transport.ssl.keystore.path", testclientStore) + .setSecureSettings(secureSettings) + .build(); final SSLService sslService = new SSLService(settings, env); SSLIOSessionStrategy sslStrategy = sslService.sslIOSessionStrategy(sslService.getSSLConfiguration("xpack.security.transport.ssl")); try (CloseableHttpAsyncClient client = getAsyncHttpClient(sslStrategy)) { @@ -789,6 +791,28 @@ public void testThatSSLIOSessionStrategyTrustsJDKTrustedCAs() throws Exception { } } + public void testWrapTrustManagerWhenDiagnosticsEnabled() { + final Settings.Builder builder = Settings.builder(); + if (randomBoolean()) { // randomly select between default, and explicit enabled + builder.put("xpack.security.ssl.diagnose.trust", true); + } + final SSLService sslService = new SSLService(builder.build(), env); + final X509ExtendedTrustManager baseTrustManager = TrustAllConfig.INSTANCE.createTrustManager(env); + final SSLConfiguration sslConfiguration = sslService.getSSLConfiguration("xpack.security.transport.ssl"); + final X509ExtendedTrustManager wrappedTrustManager = sslService.wrapWithDiagnostics(baseTrustManager, sslConfiguration); + assertThat(wrappedTrustManager, instanceOf(DiagnosticTrustManager.class)); + assertThat(sslService.wrapWithDiagnostics(wrappedTrustManager, sslConfiguration), sameInstance(wrappedTrustManager)); + } + + public void testDontWrapTrustManagerWhenDiagnosticsDisabled() { + final Settings.Builder builder = Settings.builder(); + builder.put("xpack.security.ssl.diagnose.trust", false); + final SSLService sslService = new SSLService(builder.build(), env); + final X509ExtendedTrustManager baseTrustManager = TrustAllConfig.INSTANCE.createTrustManager(env); + final SSLConfiguration sslConfiguration = sslService.getSSLConfiguration("xpack.security.transport.ssl"); + assertThat(sslService.wrapWithDiagnostics(baseTrustManager, sslConfiguration), sameInstance(baseTrustManager)); + } + class AssertionCallback implements FutureCallback { @Override @@ -812,7 +836,7 @@ public void cancelled() { private CloseableHttpAsyncClient getAsyncHttpClient(SSLIOSessionStrategy sslStrategy) throws Exception { try { return AccessController.doPrivileged((PrivilegedExceptionAction) - () -> HttpAsyncClientBuilder.create().setSSLStrategy(sslStrategy).build()); + () -> HttpAsyncClientBuilder.create().setSSLStrategy(sslStrategy).build()); } catch (PrivilegedActionException e) { throw (Exception) e.getCause(); } diff --git a/x-pack/plugin/security/qa/security-basic/build.gradle b/x-pack/plugin/security/qa/security-basic/build.gradle index 4fa19511797ef..0b51fba9667a6 100644 --- a/x-pack/plugin/security/qa/security-basic/build.gradle +++ b/x-pack/plugin/security/qa/security-basic/build.gradle @@ -16,6 +16,7 @@ testClusters.integTest { setting 'xpack.ml.enabled', 'false' setting 'xpack.license.self_generated.type', 'basic' setting 'xpack.security.enabled', 'true' + setting 'xpack.security.ssl.diagnose.trust', 'true' setting 'xpack.security.http.ssl.enabled', 'false' setting 'xpack.security.transport.ssl.enabled', 'false' setting 'xpack.security.authc.token.enabled', 'true' From e369c875fd5a8f20ae5f690458ac878425e7f278 Mon Sep 17 00:00:00 2001 From: Tim Vernum Date: Tue, 19 Nov 2019 17:28:45 +1100 Subject: [PATCH 4/6] Remove empty line --- .../org/elasticsearch/common/ssl/DiagnosticTrustManager.java | 1 - .../main/java/org/elasticsearch/common/ssl/SslDiagnostics.java | 1 - .../java/org/elasticsearch/common/ssl/SslDiagnosticsTests.java | 1 - 3 files changed, 3 deletions(-) diff --git a/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/DiagnosticTrustManager.java b/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/DiagnosticTrustManager.java index b38b5e4e284c6..31c15bd65cf8b 100644 --- a/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/DiagnosticTrustManager.java +++ b/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/DiagnosticTrustManager.java @@ -160,5 +160,4 @@ private SSLSession session(Socket socket) { private SSLSession session(SSLEngine engine) { return engine.getHandshakeSession(); } - } diff --git a/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/SslDiagnostics.java b/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/SslDiagnostics.java index 28c2d5e14f507..458d969f7f01a 100644 --- a/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/SslDiagnostics.java +++ b/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/SslDiagnostics.java @@ -380,5 +380,4 @@ private static boolean checkIssuer(X509Certificate certificate, X509Certificate private static boolean isSelfIssued(X509Certificate certificate) { return certificate.getIssuerX500Principal().equals(certificate.getSubjectX500Principal()); } - } diff --git a/libs/ssl-config/src/test/java/org/elasticsearch/common/ssl/SslDiagnosticsTests.java b/libs/ssl-config/src/test/java/org/elasticsearch/common/ssl/SslDiagnosticsTests.java index c22ab80496e85..883b71d1bc51b 100644 --- a/libs/ssl-config/src/test/java/org/elasticsearch/common/ssl/SslDiagnosticsTests.java +++ b/libs/ssl-config/src/test/java/org/elasticsearch/common/ssl/SslDiagnosticsTests.java @@ -418,5 +418,4 @@ private SSLSession session(String peerHost) { Mockito.when(mock.getPeerHost()).thenReturn(peerHost); return mock; } - } From 3ffb896bb16fda4563c3133c96c092fc6353acba Mon Sep 17 00:00:00 2001 From: Tim Vernum Date: Wed, 20 Nov 2019 14:15:51 +1100 Subject: [PATCH 5/6] Fix javadoc typo Co-Authored-By: Ioannis Kakavas --- .../main/java/org/elasticsearch/common/ssl/SslDiagnostics.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/SslDiagnostics.java b/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/SslDiagnostics.java index 458d969f7f01a..19765003c8afa 100644 --- a/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/SslDiagnostics.java +++ b/libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/SslDiagnostics.java @@ -100,7 +100,7 @@ boolean foundCertificateForDn() { private static class CertificateTrust { /** - * This certificates are trusted in the relevant context. + * These certificates are trusted in the relevant context. * They might not match with the requested certificate (see {@link #match}) but will be for the requested DN. */ private final List trustedCertificates; From 8944899a8f6417fb795dafcf6c4b38003da5796b Mon Sep 17 00:00:00 2001 From: Tim Vernum Date: Wed, 20 Nov 2019 16:25:16 +1100 Subject: [PATCH 6/6] Document new diagnose.trust setting --- docs/reference/settings/security-settings.asciidoc | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/reference/settings/security-settings.asciidoc b/docs/reference/settings/security-settings.asciidoc index 862d5efc13839..6fb5084b94a97 100644 --- a/docs/reference/settings/security-settings.asciidoc +++ b/docs/reference/settings/security-settings.asciidoc @@ -1492,6 +1492,16 @@ through the list of URLs will continue until a successful connection is made. [float] [[ssl-tls-settings]] +==== General TLS settings +`xpack.security.ssl.diagnose.trust`:: +Controls whether to output diagnostic messages for SSL/TLS trust failures. +If this is `true` (the default), a message will be printed to the Elasticsearch +log whenever an SSL connection (incoming or outgoing) is rejected due to a failure +to establish trust. +This diagnostic message contains information that can be used to determine the +cause of the failure and assist with resolving the problem. +Set to `false` to disable these messages. + ==== Default values for TLS/SSL settings In general, the values below represent the default values for the various TLS settings.