Skip to content

Commit

Permalink
Address feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@normanmaurer
normanmaurer committed May 22, 2023
1 parent ffaeb6d commit 23e6fde
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 40 deletions.
30 changes: 15 additions & 15 deletions handler/src/main/java/io/netty/handler/ssl/EnhancingX509ExtendedTrustManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,51 +42,51 @@ final class EnhancingX509ExtendedTrustManager extends X509ExtendedTrustManager {
@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.checkClientTrusted(chain, authType, socket);
wrapped.checkServerTrusted(chain, authType, socket);
} catch (CertificateException e) {
throwEnhancedCertificateException(chain, e);
}
}

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

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

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

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

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

@Override
public X509Certificate[] getAcceptedIssuers() {
return wrapped.getAcceptedIssuers();
Expand Down
57 changes: 32 additions & 25 deletions handler/src/test/java/io/netty/handler/ssl/EnhancedX509ExtendedTrustManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package io.netty.handler.ssl;

import io.netty.util.internal.EmptyArrays;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.function.Executable;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
Expand All @@ -36,6 +37,7 @@
import java.util.List;
import java.util.Set;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
Expand Down Expand Up @@ -184,38 +186,38 @@ public byte[] getExtensionValue(String oid) {
private static final EnhancingX509ExtendedTrustManager MATCHING_MANAGER =
new EnhancingX509ExtendedTrustManager(new X509ExtendedTrustManager() {
@Override
public void checkClientTrusted(X509Certificate[] chain, String authType, Socket socket)
public void checkClientTrusted(X509Certificate[] chain, String authType, Socket socket) {
fail();
}

@Override
public void checkServerTrusted(X509Certificate[] chain, String authType, Socket socket)
throws CertificateException {
throw new CertificateException("No subject alternative DNS name matching netty.io.");
}

@Override
public void checkServerTrusted(X509Certificate[] chain, String authType, Socket socket) {
public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngine engine) {
fail();
}

@Override
public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngine engine)
public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngine engine)
throws CertificateException {
throw new CertificateException("No subject alternative DNS name matching netty.io.");
}

@Override
public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngine engine) {
public void checkClientTrusted(X509Certificate[] chain, String authType) {
fail();
}

@Override
public void checkClientTrusted(X509Certificate[] chain, String authType)
public void checkServerTrusted(X509Certificate[] chain, String authType)
throws CertificateException {
throw new CertificateException("No subject alternative DNS name matching netty.io.");
}

@Override
public void checkServerTrusted(X509Certificate[] chain, String authType) {
fail();
}

@Override
public X509Certificate[] getAcceptedIssuers() {
return new X509Certificate[0];
Expand All @@ -226,17 +228,17 @@ static List<Executable> throwingMatchingExecutables() {
return Arrays.asList(new Executable() {
@Override
public void execute() throws Throwable {
MATCHING_MANAGER.checkClientTrusted(new X509Certificate[] { TEST_CERT }, null);
MATCHING_MANAGER.checkServerTrusted(new X509Certificate[] { TEST_CERT }, null);
}
}, new Executable() {
@Override
public void execute() throws Throwable {
MATCHING_MANAGER.checkClientTrusted(new X509Certificate[] { TEST_CERT }, null, (SSLEngine) null);
MATCHING_MANAGER.checkServerTrusted(new X509Certificate[] { TEST_CERT }, null, (SSLEngine) null);
}
}, new Executable() {
@Override
public void execute() throws Throwable {
MATCHING_MANAGER.checkClientTrusted(new X509Certificate[] { TEST_CERT }, null, (SSLSocket) null);
MATCHING_MANAGER.checkServerTrusted(new X509Certificate[] { TEST_CERT }, null, (SSLSocket) null);
}
});
}
Expand All @@ -246,36 +248,39 @@ public void execute() throws Throwable {
@Override
public void checkClientTrusted(X509Certificate[] chain, String authType, Socket socket)
throws CertificateException {
throw new CertificateException();
}

@Override
public void checkServerTrusted(X509Certificate[] chain, String authType, Socket socket) {
fail();
}

@Override
public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngine engine)
public void checkServerTrusted(X509Certificate[] chain, String authType, Socket socket)
throws CertificateException {
throw new CertificateException();
}

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

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

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

@Override
public void checkServerTrusted(X509Certificate[] chain, String authType)
throws CertificateException {
throw new CertificateException();
}

@Override
public X509Certificate[] getAcceptedIssuers() {
return new X509Certificate[0];
Expand All @@ -286,17 +291,17 @@ static List<Executable> throwingNonMatchingExecutables() {
return Arrays.asList(new Executable() {
@Override
public void execute() throws Throwable {
NON_MATCHING_MANAGER.checkClientTrusted(new X509Certificate[] { TEST_CERT }, null);
NON_MATCHING_MANAGER.checkServerTrusted(new X509Certificate[] { TEST_CERT }, null);
}
}, new Executable() {
@Override
public void execute() throws Throwable {
NON_MATCHING_MANAGER.checkClientTrusted(new X509Certificate[] { TEST_CERT }, null, (SSLEngine) null);
NON_MATCHING_MANAGER.checkServerTrusted(new X509Certificate[] { TEST_CERT }, null, (SSLEngine) null);
}
}, new Executable() {
@Override
public void execute() throws Throwable {
NON_MATCHING_MANAGER.checkClientTrusted(new X509Certificate[] { TEST_CERT }, null, (SSLSocket) null);
NON_MATCHING_MANAGER.checkServerTrusted(new X509Certificate[] { TEST_CERT }, null, (SSLSocket) null);
}
});
}
Expand All @@ -307,6 +312,7 @@ void testEnhanceException(Executable executable) {
CertificateException exception = assertThrows(CertificateException.class, executable);
// We should wrap the original cause with our own.
assertInstanceOf(CertificateException.class, exception.getCause());
assertThat(exception.getMessage(), Matchers.containsString("some.netty.io"));
}

@ParameterizedTest
Expand All @@ -315,5 +321,6 @@ void testNotEnhanceException(Executable executable) {
CertificateException exception = assertThrows(CertificateException.class, executable);
// We should not wrap the original cause with our own.
assertNull(exception.getCause());
assertThat(exception.getMessage(), Matchers.not(Matchers.containsString("some.netty.io")));
}
}

0 comments on commit 23e6fde

Please sign in to comment.