Skip to content

Commit

Permalink
Illegal reflective access by com.sun.mail.util.SocketFetcher eclipse-…
Browse files Browse the repository at this point in the history
…ee4j#124

Co-authored-by: jmehrens <[email protected]>
Co-authored-by: icu5545 <[email protected]>
Signed-off-by: jmehrens <[email protected]>
  • Loading branch information
jmehrens committed Feb 10, 2024
1 parent deee564 commit ff64ec9
Show file tree
Hide file tree
Showing 2 changed files with 146 additions and 56 deletions.
22 changes: 16 additions & 6 deletions core/src/main/java/org/eclipse/angus/mail/util/SocketFetcher.java
Original file line number Diff line number Diff line change
Expand Up @@ -631,8 +631,12 @@ private static void configureSSLSocket(Socket socket, String host,
SSLParameters params = sslsocket.getSSLParameters();
params.setEndpointIdentificationAlgorithm(eia);
sslsocket.setSSLParameters(params);
logger.log(Level.FINER,
"Endpoint identification algorithm {0}", eia);

if (logger.isLoggable(Level.FINER)) {
logger.log(Level.FINER,
"Checking {0} using endpoint identification algorithm {1}",
new Object[]{eia, params.getServerNames()});
}
}
} catch (RuntimeException re) {
throw cleanupAndThrow(sslsocket,
Expand Down Expand Up @@ -670,7 +674,7 @@ private static void configureSSLSocket(Socket socket, String host,
}
} catch (IOException ioe) {
throw cleanupAndThrow(sslsocket,ioe);
} catch (ReflectiveOperationException | RuntimeException re) {
} catch (ReflectiveOperationException | RuntimeException | LinkageError re) {
throw cleanupAndThrow(sslsocket,
new IOException("Unable to check server idenitity",re));
}
Expand Down Expand Up @@ -723,7 +727,13 @@ private static boolean isRecoverable(Throwable t) {
private static void checkServerIdentity(HostnameVerifier hnv,
String server, SSLSocket sslSocket)
throws IOException {
logger.log(Level.FINE, "Using HostnameVerifier: {0}", hnv);
if(logger.isLoggable(Level.FINER)) {
//Only expose the toString of the HostnameVerifier to the logger
//and not a direct reference to the HostnameVerifier
logger.log(Level.FINER, "Using HostnameVerifier: {0}",
Objects.toString(hnv));
}

if (hnv == null) {
return;
}
Expand Down Expand Up @@ -801,7 +811,7 @@ private static HostnameVerifier getHostnameVerifier(Properties props, String pre
}

String fqcn = props.getProperty(prefix + ".ssl.hostnameverifier.class");
if (fqcn == null) {
if (fqcn == null || fqcn.isEmpty()) {
return null;
}

Expand Down Expand Up @@ -1091,7 +1101,7 @@ public boolean verify(String server, SSLSession ssls) {
* It is preferred to set mail.<protocol>.ssl.endpointidentitycheck property
* to 'LDAPS' instead of using this verifier. This adapter will be removed
* in a future release of Angus Mail when there is no reason to keep this
* for compatiblity sake.
* for compatibility sake.
*
* See: JDK-8062515 - Migrate use of sun.security.** to supported API
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,22 @@
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.net.InetAddress;
import java.net.Socket;
import java.nio.charset.StandardCharsets;
import java.util.Base64;
import java.util.Properties;
import java.util.concurrent.atomic.AtomicInteger;
import javax.net.ssl.HostnameVerifier;
import javax.net.ssl.SSLHandshakeException;
import javax.net.ssl.SSLSession;
import org.eclipse.angus.mail.imap.IMAPHandler;
import org.eclipse.angus.mail.test.TestSSLSocketFactory;

import static org.junit.Assert.fail;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;

Expand Down Expand Up @@ -139,8 +142,8 @@ public void issue45Failure() throws IOException {
}

@Test
public void testSSLSocketFactoryHostnameVerifierAcceptsConnections() throws Exception {
testSSLSocketFactoryHostnameVerifier(true);
public void testSSLHostnameVerifierAcceptsConnections() throws Exception {
testSSLHostnameVerifier(true);
}

/**
Expand All @@ -150,13 +153,13 @@ public void testSSLSocketFactoryHostnameVerifierAcceptsConnections() throws Exce
* @throws Exception
*/
@Test
public void testSSLSocketFactoryHostnameVerifierRejectsConnections() throws Exception {
testSSLSocketFactoryHostnameVerifier(false);
public void testSSLHostnameVerifierRejectsConnections() throws Exception {
testSSLHostnameVerifier(false);
}

@Test
public void testSSLSocketFactoryHostnameVerifierInstantiatedByString() throws Exception {
testSSLSocketFactoryHostnameVerifierByName();
public void testSSLVerifierInstantiatedByString() throws Exception {
testSSLHostnameVerifierByName();
}

/**
Expand All @@ -165,7 +168,7 @@ public void testSSLSocketFactoryHostnameVerifierInstantiatedByString() throws Ex
* @param acceptConnections Whether the {@link HostnameVerifier} should accept or reject connections.
* @throws Exception
*/
private void testSSLSocketFactoryHostnameVerifier(boolean acceptConnections) throws Exception {
private void testSSLHostnameVerifier(boolean acceptConnections) throws Exception {
final Properties properties = new Properties();
properties.setProperty("mail.imap.host", "localhost");
properties.setProperty("mail.imap.ssl.enable", "true");
Expand All @@ -180,43 +183,37 @@ private void testSSLSocketFactoryHostnameVerifier(boolean acceptConnections) thr
properties.put("mail.imap.ssl.hostnameverifier", hnv);
properties.setProperty("mail.imap.ssl.checkserveridentity", "false");

ThrowingRunnable runnable = new ThrowingRunnable() {
@Override
public void run() throws Throwable {
TestServer server = null;
try {
server = new TestServer(new IMAPHandler(), true);
server.start();
ThrowingRunnable runnable = () -> {
TestServer server = null;
try {
server = new TestServer(new IMAPHandler(), true);
server.start();

properties.setProperty("mail.imap.port", "" + server.getPort());
final Session session = Session.getInstance(properties);
properties.setProperty("mail.imap.port",
Integer.toString(server.getPort()));
final Session session = Session.getInstance(properties);

try (Store store = session.getStore("imap")) {
store.connect("test", "test");
}
try (Store store = session.getStore("imap")) {
store.connect("test", "test");
}
finally {
if (server != null) {
server.quit();
}
} finally {
if (server != null) {
server.quit();
}
}
};

if (!acceptConnections) {
// When the hostname verifier refuses a connection, a MessagingException will be thrown.
assertThrows(MessagingException.class, runnable);
}
else {
assertNotNull(assertThrows(MessagingException.class, runnable));
} else {
// When the hostname verifier is not set to refuse connections, no exception should be thrown.
synchronized (TestHostnameVerifier.class) {
try {
runnable.run();
}
catch(Throwable t){
} catch(Throwable t){
throw new AssertionError(t);
}
finally{
} finally{
TestHostnameVerifier.reset();
}
}
Expand All @@ -226,7 +223,7 @@ public void run() throws Throwable {
assertTrue("Hostname verifier was not used.", hnv.hasInvokedVerify());
}

private void testSSLSocketFactoryHostnameVerifierByName() throws Exception {
private void testSSLHostnameVerifierByName() throws Exception {
final Properties properties = new Properties();
properties.setProperty("mail.imap.host", "localhost");
properties.setProperty("mail.imap.ssl.enable", "true");
Expand All @@ -240,25 +237,22 @@ private void testSSLSocketFactoryHostnameVerifierByName() throws Exception {
properties.setProperty("mail.imap.ssl.hostnameverifier.class", TestHostnameVerifier.class.getName());
properties.setProperty("mail.imap.ssl.checkserveridentity", "false");

ThrowingRunnable runnable = new ThrowingRunnable() {
@Override
public void run() throws Throwable {
TestServer server = null;
try {
server = new TestServer(new IMAPHandler(), true);
server.start();
ThrowingRunnable runnable = () -> {
TestServer server = null;
try {
server = new TestServer(new IMAPHandler(), true);
server.start();

properties.setProperty("mail.imap.port", "" + server.getPort());
final Session session = Session.getInstance(properties);
properties.setProperty("mail.imap.port",
Integer.toString(server.getPort()));
final Session session = Session.getInstance(properties);

try (Store store = session.getStore("imap")) {
store.connect("test", "test");
}
try (Store store = session.getStore("imap")) {
store.connect("test", "test");
}
finally {
if (server != null) {
server.quit();
}
} finally {
if (server != null) {
server.quit();
}
}
};
Expand All @@ -269,13 +263,99 @@ public void run() throws Throwable {
assertEquals("Expected the Default Constructor of the HostnameVerifier class to be invoked once.", 1, TestHostnameVerifier.getDefaultConstructorCount());
} catch (Throwable t) {
throw new AssertionError(t);
}
finally {
} finally {
TestHostnameVerifier.reset();
}
}
}

@Test
public void testSSLEndpointIdentityCheckEmptyString() throws Throwable {
testSSLEndpointIdentityCheck("");
}

@Test
public void testSSLEndpointIdentityCheckLdaps() {
try {
testSSLEndpointIdentityCheck("LDAPS");
throw new AssertionError();
} catch (Error | RuntimeException e) {
throw e;
} catch (MessagingException me) {
Throwable cause = me.getCause();
assertTrue(String.valueOf(cause),
cause instanceof SSLHandshakeException);
assertTrue(me.toString(), isFromTrustManager(me));
} catch (Throwable t) {
throw new AssertionError(t);
}
}

@Test
public void testSSLEndpointIdentityCheckHttps() {
try {
testSSLEndpointIdentityCheck("HTTPS");
throw new AssertionError();
} catch (Error | RuntimeException e) {
throw e;
} catch (MessagingException me) {
Throwable cause = me.getCause();
assertTrue(String.valueOf(cause),
cause instanceof SSLHandshakeException);
assertTrue(me.toString(), isFromTrustManager(me));
} catch (Throwable t) {
throw new AssertionError(t);
}
}

private boolean isFromTrustManager(Throwable thrown) {
for (Throwable t = thrown; t != null; t = t.getCause()) {
for (StackTraceElement s : t.getStackTrace()) {
if ("checkServerTrusted".equals(s.getMethodName())
&& s.getClassName().contains("TrustManager")) {
return true;
}
}
}
return false;
}


private void testSSLEndpointIdentityCheck(String algo) throws Throwable {
final Properties props = new Properties();
//String host = InetAddress.getLocalHost().getCanonicalHostName();
//properties.setProperty("mail.imap.host", host);
props.setProperty("mail.imap.host", "localhost");
props.setProperty("mail.imap.ssl.enable", "true");

TestSSLSocketFactory sf = new TestSSLSocketFactory();
props.put("mail.imap.ssl.socketFactory", sf);

// don't fall back to non-SSL
props.setProperty("mail.imap.socketFactory.fallback", "false");

props.setProperty("mail.imap.ssl.checkserveridentity", "false");
props.setProperty("mail.imap.ssl.endpointidentitycheck", algo);

TestServer server = null;
try {
server = new TestServer(new IMAPHandler(), true);
server.start();

props.setProperty("mail.imap.port",
Integer.toString(server.getPort()));
final Session session = Session.getInstance(props);

try (Store store = session.getStore("imap")) {
store.connect("test", "test");
}
} finally {
if (server != null) {
server.quit();
}
}
}

/**
*
*/
Expand Down Expand Up @@ -392,7 +472,7 @@ public static void reset() {
* and save user/password string;
*/
private static class ProxyHandler extends ProtocolHandler {
private boolean http;
private final boolean http;

// must be static because handler is cloned for each connection
private static volatile boolean connected;
Expand Down

0 comments on commit ff64ec9

Please sign in to comment.