Skip to content

Commit

Permalink
Enhance CertificateException message when throw due hostname validati…
Browse files Browse the repository at this point in the history
…on (#13381)

Motivation:

It is hard for end-user to understand why the hostname validation
failed. Let's try to help them by including the SubjectAlternativeNames
if there are any.

Modifications:

- Wrap the X509ExtendedTrustManager and so be able to catch the
CertificateException and add more details if needed
- Add unit test

Result:

Easier to debug hostname validation problems
  • Loading branch information
normanmaurer committed Jul 19, 2023
1 parent 61c2e9f commit 4d50958
Show file tree
Hide file tree
Showing 4 changed files with 474 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
/*
* Copyright 2023 The Netty Project
*
* The Netty Project 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:
*
* https://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 io.netty5.handler.ssl;

import javax.net.ssl.SSLEngine;
import javax.net.ssl.X509ExtendedTrustManager;
import javax.net.ssl.X509TrustManager;
import java.net.Socket;
import java.security.cert.CertificateException;
import java.security.cert.X509Certificate;
import java.util.Collection;
import java.util.List;


/**
* Wraps an existing {@link X509ExtendedTrustManager} and enhances the {@link CertificateException} that is thrown
* because of hostname validation.
*/
final class EnhancingX509ExtendedTrustManager extends X509ExtendedTrustManager {
private final X509ExtendedTrustManager wrapped;

EnhancingX509ExtendedTrustManager(X509TrustManager wrapped) {
this.wrapped = (X509ExtendedTrustManager) wrapped;
}

@Override
public void checkClientTrusted(X509Certificate[] chain, String authType, Socket socket)
throws CertificateException {
wrapped.checkClientTrusted(chain, authType, socket);
}

@Override
public void checkServerTrusted(X509Certificate[] chain, String authType, Socket socket)
throws CertificateException {
try {
wrapped.checkServerTrusted(chain, authType, socket);
} catch (CertificateException e) {
throwEnhancedCertificateException(chain, e);
}
}

@Override
public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngine engine)
throws CertificateException {
wrapped.checkClientTrusted(chain, authType, engine);
}

@Override
public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngine engine)
throws CertificateException {
try {
wrapped.checkServerTrusted(chain, authType, engine);
} catch (CertificateException e) {
throwEnhancedCertificateException(chain, e);
}
}

@Override
public void checkClientTrusted(X509Certificate[] chain, String authType)
throws CertificateException {
wrapped.checkClientTrusted(chain, authType);
}

@Override
public void checkServerTrusted(X509Certificate[] chain, String authType)
throws CertificateException {
try {
wrapped.checkServerTrusted(chain, authType);
} catch (CertificateException e) {
throwEnhancedCertificateException(chain, e);
}
}

@Override
public X509Certificate[] getAcceptedIssuers() {
return wrapped.getAcceptedIssuers();
}

private static void throwEnhancedCertificateException(X509Certificate[] chain, CertificateException e)
throws CertificateException {
// Matching the message is the best we can do sadly.
String message = e.getMessage();
if (message != null && e.getMessage().startsWith("No subject alternative DNS name matching")) {
StringBuilder names = new StringBuilder(64);
for (int i = 0; i < chain.length; i++) {
X509Certificate cert = chain[i];
Collection<List<?>> collection = cert.getSubjectAlternativeNames();
if (collection != null) {
for (List<?> altNames : collection) {
// 2 is dNSName. See X509Certificate javadocs.
if (altNames.size() >= 2 && ((Integer) altNames.get(0)).intValue() == 2) {
names.append((String) altNames.get(1)).append(",");
}
}
}
}
if (names.length() != 0) {
// Strip of ,
names.setLength(names.length() - 1);
throw new CertificateException(message +
" Subject alternative DNS names in the certificate chain of " + chain.length +
" certificate(s): " + names, e);
}
}
throw e;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLException;
import javax.net.ssl.SSLSessionContext;
import javax.net.ssl.TrustManager;
import javax.net.ssl.TrustManagerFactory;
import javax.net.ssl.X509ExtendedTrustManager;
import java.io.File;
import java.security.KeyStore;
import java.security.PrivateKey;
Expand Down Expand Up @@ -82,6 +84,11 @@ private static SSLContext newSSLContext(Provider sslContextProvider, X509Certifi
try {
if (trustCertCollection != null) {
trustManagerFactory = buildTrustManagerFactory(trustCertCollection, trustManagerFactory, keyStore);
} else if (trustManagerFactory == null) {
// Mimic the way SSLContext.getInstance(KeyManager[], null, null) works
trustManagerFactory = TrustManagerFactory.getInstance(
TrustManagerFactory.getDefaultAlgorithm());
trustManagerFactory.init((KeyStore) null);
}
if (key != null) {
keyManagerFactory = buildKeyManagerFactory(keyCertChain, null,
Expand All @@ -92,8 +99,7 @@ private static SSLContext newSSLContext(Provider sslContextProvider, X509Certifi
SSLContext ctx = sslContextProvider == null ? SSLContext.getInstance(PROTOCOL)
: SSLContext.getInstance(PROTOCOL, sslContextProvider);
ctx.init(keyManagerFactory.getKeyManagers(),
trustManagerFactory == null ? null : trustManagerFactory.getTrustManagers(),
null);
wrapTrustManagerIfNeeded(trustManagerFactory.getTrustManagers()), null);

SSLSessionContext sessCtx = ctx.getServerSessionContext();
if (sessionCacheSize > 0) {
Expand All @@ -110,4 +116,16 @@ private static SSLContext newSSLContext(Provider sslContextProvider, X509Certifi
throw new SSLException("failed to initialize the server-side SSL context", e);
}
}

private static TrustManager[] wrapTrustManagerIfNeeded(TrustManager[] trustManagers) {
for (int i = 0; i < trustManagers.length; i++) {
TrustManager tm = trustManagers[i];
if (tm instanceof X509ExtendedTrustManager) {
// Wrap the TrustManager to provide a better exception message for users to debug hostname
// validation failures.
trustManagers[i] = new EnhancingX509ExtendedTrustManager((X509ExtendedTrustManager) tm);
}
}
return trustManagers;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,13 @@ protected static X509Certificate[] certificates(byte[][] chain) {
protected static X509TrustManager chooseTrustManager(TrustManager[] managers) {
for (TrustManager m : managers) {
if (m instanceof X509TrustManager) {
return OpenSslX509TrustManagerWrapper.wrapIfNeeded((X509TrustManager) m);
X509TrustManager tm = OpenSslX509TrustManagerWrapper.wrapIfNeeded((X509TrustManager) m);
if (useExtendedTrustManager(tm)) {
// Wrap the TrustManager to provide a better exception message for users to debug hostname
// validation failures.
tm = new EnhancingX509ExtendedTrustManager(tm);
}
return tm;
}
}
throw new IllegalStateException("no X509TrustManager found");
Expand Down

0 comments on commit 4d50958

Please sign in to comment.