Skip to content

Commit

Permalink
Illegal reflective access by com.sun.mail.util.SocketFetcher #124
Browse files Browse the repository at this point in the history 
Co-authored-by: jmehrens <jason_mehrens@hotmail.com>
Co-authored-by: icu5545 <icu5545@github.com>
Signed-off-by: jmehrens <jason_mehrens@hotmail.com>
  • Loading branch information
@jmehrens
jmehrens committed Feb 8, 2024
1 parent 9073def commit 198282f
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 105 deletions.
95 changes: 23 additions & 72 deletions core/src/main/java/org/eclipse/angus/mail/util/SocketFetcher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<SNIServerName> src = params.getServerNames();
if (!src.contains(shn)) {
List<SNIServerName> 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);
Expand All @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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());
}
Expand All @@ -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()));
}
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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<? extends HostnameVerifier> verifierClass = null;
ClassLoader ccl = getContextClassLoader();
Expand Down Expand Up @@ -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
Expand Down
18 changes: 7 additions & 11 deletions providers/imap/src/main/java/org/eclipse/angus/mail/imap/package-info.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -681,17 +681,13 @@
* any {@link java.io.IOException}. The alias name <code>any</code> will
* attempt find a built in hostname verifier that passes verification.
* The alias name <code>sun.security.util.HostnameChecker</code> or
* <code>org.eclipse.angus.mail.util.SocketFetcher$HostnameChecker</code> will
* attempt to access the <code>sun.security.util.HostnameChecker</code> via
* reflection which will require an add-opens of 'java.base/sun.security.util'.
* The alias name
* <code>org.eclipse.angus.mail.util.SocketFetcher$MailHostnameVerifier</code>
* 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
* <code>mail.imap.ssl.checkserveridentity</code> is true (set or default).
* Otherwise the default is null.
* <code>HostnameChecker</code> will attempt to access the
* <code>sun.security.util.HostnameChecker</code> via reflection which will
* require an add-opens of 'java.base/sun.security.util'. The alias name
* <code>MailHostnameVerifier</code> 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.
* </TD>
* </TR>
*
Expand Down
18 changes: 7 additions & 11 deletions providers/pop3/src/main/java/org/eclipse/angus/mail/pop3/package-info.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -521,17 +521,13 @@
* any {@link java.io.IOException}. The alias name <code>any</code> will
* attempt find a built in hostname verifier that passes verification.
* The alias name <code>sun.security.util.HostnameChecker</code> or
* <code>org.eclipse.angus.mail.util.SocketFetcher$HostnameChecker</code> will
* attempt to access the <code>sun.security.util.HostnameChecker</code> via
* reflection which will require an add-opens of 'java.base/sun.security.util'.
* The alias name
* <code>org.eclipse.angus.mail.util.SocketFetcher$MailHostnameVerifier</code>
* 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
* <code>mail.pop3.ssl.checkserveridentity</code> is true (set or default).
* Otherwise the default is null.
* <code>HostnameChecker</code> will attempt to access the
* <code>sun.security.util.HostnameChecker</code> via reflection which will
* require an add-opens of 'java.base/sun.security.util'. The alias name
* <code>MailHostnameVerifier</code> 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.
* </TD>
* </TR>
*
Expand Down
18 changes: 7 additions & 11 deletions providers/smtp/src/main/java/org/eclipse/angus/mail/smtp/package-info.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -644,17 +644,13 @@
* any {@link java.io.IOException}. The alias name <code>any</code> will
* attempt find a built in hostname verifier that passes verification.
* The alias name <code>sun.security.util.HostnameChecker</code> or
* <code>org.eclipse.angus.mail.util.SocketFetcher$HostnameChecker</code> will
* attempt to access the <code>sun.security.util.HostnameChecker</code> via
* reflection which will require an add-opens of 'java.base/sun.security.util'.
* The alias name
* <code>org.eclipse.angus.mail.util.SocketFetcher$MailHostnameVerifier</code>
* 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
* <code>mail.smtp.ssl.checkserveridentity</code> is true (set or default).
* Otherwise the default is null.
* <code>HostnameChecker</code> will attempt to access the
* <code>sun.security.util.HostnameChecker</code> via reflection which will
* require an add-opens of 'java.base/sun.security.util'. The alias name
* <code>MailHostnameVerifier</code> 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.
* </TD>
* </TR>
*
Expand Down

0 comments on commit 198282f

Please sign in to comment.