From 198282fc68a098c4b87c3e0805eb778b28d9f70c Mon Sep 17 00:00:00 2001 From: jmehrens Date: Thu, 8 Feb 2024 00:38:33 -0600 Subject: [PATCH] Illegal reflective access by com.sun.mail.util.SocketFetcher #124 Co-authored-by: jmehrens Co-authored-by: icu5545 Signed-off-by: jmehrens --- .../angus/mail/util/SocketFetcher.java | 95 +++++-------------- .../eclipse/angus/mail/imap/package-info.java | 18 ++-- .../eclipse/angus/mail/pop3/package-info.java | 18 ++-- .../eclipse/angus/mail/smtp/package-info.java | 18 ++-- 4 files changed, 44 insertions(+), 105 deletions(-) diff --git a/core/src/main/java/org/eclipse/angus/mail/util/SocketFetcher.java b/core/src/main/java/org/eclipse/angus/mail/util/SocketFetcher.java index 95e9e29..a09edc0 100644 --- a/core/src/main/java/org/eclipse/angus/mail/util/SocketFetcher.java +++ b/core/src/main/java/org/eclipse/angus/mail/util/SocketFetcher.java @@ -18,6 +18,8 @@ import javax.net.SocketFactory; import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.SNIHostName; +import javax.net.ssl.SNIServerName; import javax.net.ssl.SSLParameters; import javax.net.ssl.SSLPeerUnverifiedException; import javax.net.ssl.SSLSocket; @@ -635,6 +637,16 @@ private static void configureSSLSocket(Socket socket, String host, if (!eia.isEmpty()) { SSLParameters params = sslsocket.getSSLParameters(); params.setEndpointIdentificationAlgorithm(eia); + + SNIHostName shn = new SNIHostName(host); + List src = params.getServerNames(); + if (!src.contains(shn)) { + List copy = new ArrayList<>(src.size() + 1); + copy.addAll(src); + copy.add(shn); + params.setServerNames(copy); + } + sslsocket.setSSLParameters(params); logger.log(Level.FINER, "Endpoint identification algorithm {0}", eia); @@ -659,6 +671,12 @@ private static void configureSSLSocket(Socket socket, String host, * Check server identity and trust. */ try { + //Perform any custom hostname verification requested by the user + //so user can force legacy behavior on JDK9+ by setting this to + //HostnameChecker and checkserveridentity=true + checkServerIdentity(getHostnameVerifier(props, prefix), + host, sslsocket); + if (PropUtil.getBooleanProperty(props, prefix + ".ssl.checkserveridentity", true)) { /* Check the server from the Socket connection against the @@ -668,10 +686,6 @@ private static void configureSSLSocket(Socket socket, String host, checkServerIdentity(newHostnameVerifier(), host, sslsocket); } - - //Perform any custom hostname verification requested by the user. - checkServerIdentity(getHostnameVerifier(props, prefix), - host, sslsocket); } catch (IOException ioe) { throw cleanupAndThrow(sslsocket,ioe); } catch (ReflectiveOperationException | RuntimeException re) { @@ -759,7 +773,7 @@ private static HostnameVerifier newHostnameVerifier() { */ try { Class.forName("java.lang.Module"); - return TrustManagerHostnameVerifier.or(MailHostnameVerifier.of()); + return MailHostnameVerifier.of(); } catch (ClassNotFoundException preJdk9) { return HostnameChecker.or(MailHostnameVerifier.of()); } @@ -777,6 +791,7 @@ private static X509Certificate getX509Certificate( return (X509Certificate) first; } + //Only metadata about the cert is shown in the message throw new SSLPeerUnverifiedException(first == null ? "null" : (first.getClass().getName() + " " + first.getType())); } @@ -790,7 +805,7 @@ private static X509Certificate getX509Certificate( * * @param fqcn the class name of the {@link HostnameVerifier} * @return the {@link HostnameVerifier} or null - * @throws ClassCastException if assigned hostnameverifier is not a {@link HostnameVerifier} + * @throws ClassCastException if hostnameverifier is not a {@link HostnameVerifier} * @throws ReflectiveOperationException if unable to construct a {@link HostnameVerifier} */ private static HostnameVerifier getHostnameVerifier(Properties props, String prefix) @@ -809,10 +824,8 @@ private static HostnameVerifier getHostnameVerifier(Properties props, String pre } //Handle all aliases names - if ("any".equals(fqcn)) { - return HostnameChecker.or( - TrustManagerHostnameVerifier.or( - MailHostnameVerifier.of())); + if ("any".equals(fqcn)) { //legacy behavior + return HostnameChecker.or(MailHostnameVerifier.of()); } if ("sun.security.util.HostnameChecker".equals(fqcn) @@ -824,11 +837,6 @@ private static HostnameVerifier getHostnameVerifier(Properties props, String pre return MailHostnameVerifier.of(); } - if (TrustManagerHostnameVerifier.class.getSimpleName() - .equals(fqcn)) { - return TrustManagerHostnameVerifier.of(); - } - //Handle the fully qualified class name Class verifierClass = null; ClassLoader ccl = getContextClassLoader(); @@ -1087,63 +1095,6 @@ public boolean verify(String server, SSLSession ssls) { } } - private static final class TrustManagerHostnameVerifier - implements HostnameVerifier { - - private final HostnameVerifier or; - - static HostnameVerifier of() { - return or((n, s) -> { return false; }); - } - - static HostnameVerifier or(HostnameVerifier or) { - return new TrustManagerHostnameVerifier(or); - } - - private TrustManagerHostnameVerifier(final HostnameVerifier or) { - this.or = Objects.requireNonNull(or); - } - - @Override - public boolean verify(String server, SSLSession ssls) { - try { - X509Certificate cert = getX509Certificate( - ssls.getPeerCertificates()); - if (logger.isLoggable(Level.FINER)) { - logger.finer("matchCert server " - + server + ", cert " + cert); - } - KeyStore ks = KeyStore.getInstance("pkcs12"); - //TODO: Load single cert vs. load full chain - ks.setCertificateEntry("test", cert); - TrustManagerFactory tmf = TrustManagerFactory.getInstance("PKIX"); - tmf.init(ks); - TrustManager[] tms = tmf.getTrustManagers(); - if (tms == null || tms.length == 0) { - throw new SSLPeerUnverifiedException(Arrays.toString(tms)); - } - - TrustManager tm = tms[0]; - if (tm instanceof X509TrustManager) { - //TODO: checkServer vs. checkClient - ((X509TrustManager) tm).checkServerTrusted( - new X509Certificate[]{cert}, server); - return true; - } - } catch (IOException ioe) { - throw new UncheckedIOException(ioe); - } catch (GeneralSecurityException ex) { - logger.log(Level.FINER, "Unable to load TrustManager", ex); - } - return or.verify(server, ssls); - } - - @Override - public String toString() { - return "[" + getClass().getSimpleName() +", or=" + or + "]"; - } - } - /** * Check the server from the Socket connection against the server name(s) * as expressed in the server certificate (RFC 2595 check). This is a diff --git a/providers/imap/src/main/java/org/eclipse/angus/mail/imap/package-info.java b/providers/imap/src/main/java/org/eclipse/angus/mail/imap/package-info.java index 27ea1fa..128940f 100644 --- a/providers/imap/src/main/java/org/eclipse/angus/mail/imap/package-info.java +++ b/providers/imap/src/main/java/org/eclipse/angus/mail/imap/package-info.java @@ -681,17 +681,13 @@ * any {@link java.io.IOException}. The alias name any will * attempt find a built in hostname verifier that passes verification. * The alias name sun.security.util.HostnameChecker or - * org.eclipse.angus.mail.util.SocketFetcher$HostnameChecker will - * attempt to access the sun.security.util.HostnameChecker via - * reflection which will require an add-opens of 'java.base/sun.security.util'. - * The alias name - * org.eclipse.angus.mail.util.SocketFetcher$MailHostnameVerifier - * will use a basic hostname verification of the certificate. * - * The instantiated object will provide additional checks based on the content - * of the server's certificate are intended to prevent man-in-the-middle - * attacks. Defaults to any if the - * mail.imap.ssl.checkserveridentity is true (set or default). - * Otherwise the default is null. + * HostnameChecker will attempt to access the + * sun.security.util.HostnameChecker via reflection which will + * require an add-opens of 'java.base/sun.security.util'. The alias name + * MailHostnameVerifier will use a basic hostname verification of + * the certificate. The instantiated object will provide additional checks + * based on the content of the server's certificate are intended to prevent + * man-in-the-middle attacks. Defaults to null. * * * diff --git a/providers/pop3/src/main/java/org/eclipse/angus/mail/pop3/package-info.java b/providers/pop3/src/main/java/org/eclipse/angus/mail/pop3/package-info.java index 76f76b8..58c0966 100644 --- a/providers/pop3/src/main/java/org/eclipse/angus/mail/pop3/package-info.java +++ b/providers/pop3/src/main/java/org/eclipse/angus/mail/pop3/package-info.java @@ -521,17 +521,13 @@ * any {@link java.io.IOException}. The alias name any will * attempt find a built in hostname verifier that passes verification. * The alias name sun.security.util.HostnameChecker or - * org.eclipse.angus.mail.util.SocketFetcher$HostnameChecker will - * attempt to access the sun.security.util.HostnameChecker via - * reflection which will require an add-opens of 'java.base/sun.security.util'. - * The alias name - * org.eclipse.angus.mail.util.SocketFetcher$MailHostnameVerifier - * will use a basic hostname verification of the certificate. * - * The instantiated object will provide additional checks based on the content - * of the server's certificate are intended to prevent man-in-the-middle - * attacks. Defaults to any if the - * mail.pop3.ssl.checkserveridentity is true (set or default). - * Otherwise the default is null. + * HostnameChecker will attempt to access the + * sun.security.util.HostnameChecker via reflection which will + * require an add-opens of 'java.base/sun.security.util'. The alias name + * MailHostnameVerifier will use a basic hostname verification of + * the certificate. The instantiated object will provide additional checks + * based on the content of the server's certificate are intended to prevent + * man-in-the-middle attacks. Defaults to null. * * * diff --git a/providers/smtp/src/main/java/org/eclipse/angus/mail/smtp/package-info.java b/providers/smtp/src/main/java/org/eclipse/angus/mail/smtp/package-info.java index 08786a3..33afe9d 100644 --- a/providers/smtp/src/main/java/org/eclipse/angus/mail/smtp/package-info.java +++ b/providers/smtp/src/main/java/org/eclipse/angus/mail/smtp/package-info.java @@ -644,17 +644,13 @@ * any {@link java.io.IOException}. The alias name any will * attempt find a built in hostname verifier that passes verification. * The alias name sun.security.util.HostnameChecker or - * org.eclipse.angus.mail.util.SocketFetcher$HostnameChecker will - * attempt to access the sun.security.util.HostnameChecker via - * reflection which will require an add-opens of 'java.base/sun.security.util'. - * The alias name - * org.eclipse.angus.mail.util.SocketFetcher$MailHostnameVerifier - * will use a basic hostname verification of the certificate. * - * The instantiated object will provide additional checks based on the content - * of the server's certificate are intended to prevent man-in-the-middle - * attacks. Defaults to any if the - * mail.smtp.ssl.checkserveridentity is true (set or default). - * Otherwise the default is null. + * HostnameChecker will attempt to access the + * sun.security.util.HostnameChecker via reflection which will + * require an add-opens of 'java.base/sun.security.util'. The alias name + * MailHostnameVerifier will use a basic hostname verification of + * the certificate. The instantiated object will provide additional checks + * based on the content of the server's certificate are intended to prevent + * man-in-the-middle attacks. Defaults to null. * * *