Skip to content

Commit

Permalink
implement ECH Retry Config handling
Browse files Browse the repository at this point in the history
This introduces a new Exception so that clients can respond only to
the ECH retry request without having to parse SSLHandshakeExceptions
in general.  This exception should probably be implemented in
boringssl or native_crypto.cc.
  • Loading branch information
eighthave committed Nov 11, 2021
1 parent eadc7ad commit 591113a
Show file tree
Hide file tree
Showing 13 changed files with 265 additions and 1 deletion.
46 changes: 46 additions & 0 deletions common/src/jni/main/cpp/conscrypt/native_crypto.cc
Expand Up @@ -10474,6 +10474,50 @@ static jboolean NativeCrypto_SSL_set1_ech_config_list(JNIEnv* env, jclass, jlong
return !!ret;
}

static jstring NativeCrypto_SSL_get0_ech_name_override(JNIEnv* env, jclass, jlong ssl_address,
CONSCRYPT_UNUSED jobject ssl_holder) {
CHECK_ERROR_QUEUE_ON_RETURN;
SSL* ssl = to_SSL(env, ssl_address, true);
JNI_TRACE("ssl=%p NativeCrypto_SSL_get0_ech_name_override()", ssl);
if (ssl == nullptr) {
JNI_TRACE("ssl=%p NativeCrypto_SSL_get0_ech_name_override() => nullptr", ssl);
return nullptr;
}
const char* ech_name_override;
size_t ech_name_override_len;
SSL_get0_ech_name_override(ssl, &ech_name_override, &ech_name_override_len);
if (ech_name_override_len > 0) {
jstring name = env->NewStringUTF(ech_name_override);
return name;
}
return nullptr;
}

static jbyteArray NativeCrypto_SSL_get0_ech_retry_configs(JNIEnv* env, jclass, jlong ssl_address,
CONSCRYPT_UNUSED jobject ssl_holder) {
CHECK_ERROR_QUEUE_ON_RETURN;
SSL* ssl = to_SSL(env, ssl_address, true);
JNI_TRACE("ssl=%p NativeCrypto_SSL_get0_ech_retry_configs()", ssl);
if (ssl == nullptr) {
return nullptr;
}

const uint8_t *retry_configs;
size_t retry_configs_len;
SSL_get0_ech_retry_configs(ssl, &retry_configs, &retry_configs_len);

jbyteArray result = env->NewByteArray(static_cast<jsize>(retry_configs_len));
if (result == nullptr) {
JNI_TRACE("ssl=%p NativeCrypto_SSL_get0_ech_retry_configs() => creating byte array failed",
ssl);
return nullptr;
}
env->SetByteArrayRegion(result, 0, static_cast<jsize>(retry_configs_len),
(const jbyte*) retry_configs);
JNI_TRACE("ssl=%p NativeCrypto_SSL_get0_ech_retry_configs(%p) => %p", ssl, ssl, result);
return result;
}

/**
* public static native long SSL_ech_accepted(long ssl);
*/
Expand Down Expand Up @@ -10851,6 +10895,8 @@ static JNINativeMethod sNativeCryptoMethods[] = {
CONSCRYPT_NATIVE_METHOD(usesBoringSsl_FIPS_mode, "()Z"),
CONSCRYPT_NATIVE_METHOD(SSL_set_enable_ech_grease, "(J" REF_SSL "Z)V"),
CONSCRYPT_NATIVE_METHOD(SSL_set1_ech_config_list, "(J" REF_SSL "[B)Z"),
CONSCRYPT_NATIVE_METHOD(SSL_get0_ech_name_override, "(J" REF_SSL ")Ljava/lang/String;"),
CONSCRYPT_NATIVE_METHOD(SSL_get0_ech_retry_configs, "(J" REF_SSL ")[B"),
CONSCRYPT_NATIVE_METHOD(SSL_ech_accepted, "(J" REF_SSL ")Z"),
CONSCRYPT_NATIVE_METHOD(SSL_CTX_ech_enable_server, "(J" REF_SSL_CTX "[B[B)Z"),
Expand Down
Expand Up @@ -100,6 +100,10 @@ abstract class AbstractConscryptEngine extends SSLEngine {

public abstract byte[] getEchConfigList();

public abstract String getEchNameOverride();

public abstract byte[] getEchRetryConfig();

public abstract boolean echAccepted();

/* @Override */
Expand Down
Expand Up @@ -761,5 +761,9 @@ abstract byte[] exportKeyingMaterial(String label, byte[] context, int length)

public abstract byte[] getEchConfigList();

public abstract String getEchNameOverride();

public abstract byte[] getEchRetryConfig();

public abstract boolean echAccepted();
}
8 changes: 8 additions & 0 deletions common/src/main/java/org/conscrypt/Conscrypt.java
Expand Up @@ -510,6 +510,14 @@ public static byte[] getEchConfigList(SSLSocket socket) {
return toConscrypt(socket).getEchConfigList();
}

public static String getEchNameOverride(SSLSocket socket) {
return toConscrypt(socket).getEchNameOverride();
}

public static byte[] getEchRetryConfig(SSLSocket socket) {
return toConscrypt(socket).getEchConfigList();
}

public static boolean echAccepted(SSLSocket socket) {
return toConscrypt(socket).echAccepted();
}
Expand Down
10 changes: 10 additions & 0 deletions common/src/main/java/org/conscrypt/ConscryptEngine.java
Expand Up @@ -415,6 +415,16 @@ public byte[] getEchConfigList() {
return sslParameters.getEchConfigList();
}

@Override
public String getEchNameOverride() {
return ssl.getEchNameOverride();
}

@Override
public byte[] getEchRetryConfig() {
return ssl.getEchRetryConfig();
}

@Override
public boolean echAccepted() {
return ssl.echAccepted();
Expand Down
10 changes: 10 additions & 0 deletions common/src/main/java/org/conscrypt/ConscryptEngineSocket.java
Expand Up @@ -420,6 +420,16 @@ public byte[] getEchConfigList() {
return engine.getEchConfigList();
}

@Override
public String getEchNameOverride() {
return engine.getEchNameOverride();
}

@Override
public byte[] getEchRetryConfig() {
return engine.getEchRetryConfig();
}

@Override
public boolean echAccepted() {
return engine.echAccepted();
Expand Down
Expand Up @@ -931,6 +931,16 @@ public byte[] getEchConfigList() {
return sslParameters.getEchConfigList();
}

@Override
public String getEchNameOverride() {
return ssl.getEchNameOverride();
}

@Override
public byte[] getEchRetryConfig() {
return ssl.getEchRetryConfig();
}

@Override
public boolean echAccepted() {
return ssl.echAccepted();
Expand Down
18 changes: 18 additions & 0 deletions common/src/main/java/org/conscrypt/EchRejectedException.java
@@ -0,0 +1,18 @@
package org.conscrypt;

import javax.net.ssl.SSLHandshakeException;

/**
* The server rejected the ECH Config List, and might have supplied an ECH
* Retry Config.
*
* @see NativeCrypto#SSL_get0_ech_retry_configs(long, NativeSsl)
*/
public class EchRejectedException extends SSLHandshakeException {
private static final long serialVersionUID = 98723498273473923L;

EchRejectedException(String message) {
super(message);
}
}

10 changes: 10 additions & 0 deletions common/src/main/java/org/conscrypt/Java8EngineWrapper.java
Expand Up @@ -136,6 +136,16 @@ public byte[] getEchConfigList() {
return delegate.getEchConfigList();
}

@Override
public String getEchNameOverride() {
return delegate.getEchNameOverride();
}

@Override
public byte[] getEchRetryConfig() {
return delegate.getEchRetryConfig();
}

@Override
public boolean echAccepted() {
return delegate.echAccepted();
Expand Down
4 changes: 4 additions & 0 deletions common/src/main/java/org/conscrypt/NativeCrypto.java
Expand Up @@ -1454,6 +1454,10 @@ static native void ENGINE_SSL_shutdown(long ssl, NativeSsl ssl_holder, SSLHandsh

static native boolean SSL_set1_ech_config_list(long ssl, NativeSsl ssl_holder, byte[] echConfig);

static native String SSL_get0_ech_name_override(long ssl, NativeSsl ssl_holder);

static native byte[] SSL_get0_ech_retry_configs(long ssl, NativeSsl ssl_holder);

static native boolean SSL_ech_accepted(long ssl, NativeSsl ssl_holder);

static native boolean SSL_CTX_ech_enable_server(long sslCtx, AbstractSessionContext holder,
Expand Down
8 changes: 8 additions & 0 deletions common/src/main/java/org/conscrypt/NativeSsl.java
Expand Up @@ -279,6 +279,14 @@ byte[] getTlsChannelId() throws SSLException {
return NativeCrypto.SSL_get_tls_channel_id(ssl, this);
}

String getEchNameOverride() {
return NativeCrypto.SSL_get0_ech_name_override(ssl, this);
}

byte[] getEchRetryConfig() {
return NativeCrypto.SSL_get0_ech_retry_configs(ssl, this);
}

boolean echAccepted() {
return NativeCrypto.SSL_ech_accepted(ssl, this);
}
Expand Down
5 changes: 5 additions & 0 deletions common/src/main/java/org/conscrypt/SSLUtils.java
Expand Up @@ -359,6 +359,11 @@ static SSLHandshakeException toSSLHandshakeException(Throwable e) {
return (SSLHandshakeException) e;
}

if (e.getMessage().contains(":ECH_REJECTED ")) {
// TODO this should probably be implemented in boringssl
return (SSLHandshakeException) new EchRejectedException(e.getMessage()).initCause(e);
}

return (SSLHandshakeException) new SSLHandshakeException(e.getMessage()).initCause(e);
}

Expand Down
129 changes: 128 additions & 1 deletion openjdk/src/test/java/org/conscrypt/NativeCryptoTest.java
Expand Up @@ -29,6 +29,7 @@
import static org.conscrypt.NativeConstants.TLS1_VERSION;
import static org.conscrypt.TestUtils.openTestFile;
import static org.conscrypt.TestUtils.readTestFile;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
Expand Down Expand Up @@ -528,6 +529,9 @@ public long beforeHandshake(long c) throws SSLException {
public void afterHandshake(long session, long ssl, long context, Socket socket,
FileDescriptor fd, SSLHandshakeCallbacks callback) throws Exception {
assertFalse(NativeCrypto.SSL_ech_accepted(ssl, null));
assertNull(NativeCrypto.SSL_get0_ech_name_override(ssl, null));
byte[] retryConfigs = NativeCrypto.SSL_get0_ech_retry_configs(ssl, null);
assertEquals(5, retryConfigs.length); // should be the invalid ECH Config List
super.afterHandshake(session, ssl, context, socket, fd, callback);
}
};
Expand Down Expand Up @@ -626,6 +630,104 @@ public void afterHandshake(long session, long ssl, long context, Socket socket,
assertTrue(serverCallback.serverCertificateRequestedInvoked);
}

@Test
public void test_SSL_do_handshake_ech_retry_configs() throws Exception {
System.out.println("test_SSL_do_handshake_ech_client_server");
final ServerSocket listener = newServerSocket();

final byte[] key = readTestFile("boringssl-ech-private-key.bin");
final byte[] serverConfig = readTestFile("boringssl-server-ech-config.bin");
final byte[] originalClientConfigList = readTestFile("boringssl-ech-config-list.bin");
final byte[] clientConfigList = originalClientConfigList.clone();
clientConfigList[20] = (byte) (clientConfigList[20] % 255 + 1); // corrupt it

Hooks cHooks = new ClientHooks() {
@Override
public long beforeHandshake(long c) throws SSLException {
long ssl = super.beforeHandshake(c);
assertEquals(1, NativeCrypto.SSL_set_protocol_versions(ssl, null, TLS1_VERSION, TLS1_3_VERSION));
assertTrue(NativeCrypto.SSL_set1_ech_config_list(ssl, null, clientConfigList));
return ssl;
}

@Override
public void afterHandshake(long session, long ssl, long context, Socket socket,
FileDescriptor fd, SSLHandshakeCallbacks callback) {
fail();
}
};
Hooks sHooks = new ServerHooks(getServerPrivateKey(), getEncodedServerCertificates()) {
@Override
public long beforeHandshake(long c) throws SSLException {
long ssl = super.beforeHandshake(c);
assertEquals(1, NativeCrypto.SSL_set_protocol_versions(ssl, null, TLS1_VERSION, TLS1_3_VERSION));
assertTrue(NativeCrypto.SSL_CTX_ech_enable_server(c, null, key, serverConfig));
return ssl;
}

@Override
public void afterHandshake(long session, long ssl, long context, Socket socket,
FileDescriptor fd, SSLHandshakeCallbacks callback) throws Exception {
assertTrue(NativeCrypto.SSL_ech_accepted(ssl, null));
super.afterHandshake(session, ssl, context, socket, fd, callback);
}
};
Future<TestSSLHandshakeCallbacks> client = handshake(listener, 0, true, cHooks, null, null, true);
Future<TestSSLHandshakeCallbacks> server = handshake(listener, 0, false, sHooks, null, null, true);
TestSSLHandshakeCallbacks clientCallback = null;
TestSSLHandshakeCallbacks serverCallback = null;
try {
clientCallback = client.get(TIMEOUT_SECONDS, TimeUnit.SECONDS);
serverCallback = server.get(TIMEOUT_SECONDS, TimeUnit.SECONDS);
} catch (ExecutionException e) {
// caused by SSLProtocolException
}
assertNull(clientCallback);
assertNull(serverCallback);
assertArrayEquals(originalClientConfigList, cHooks.echRetryConfigs);
assertEquals("example.com", cHooks.echNameOverride);
assertNotNull(cHooks.echRetryConfigs);
assertNull(sHooks.echNameOverride);
assertNull(sHooks.echRetryConfigs);

final byte[] echRetryConfigsFromPrevious = cHooks.echRetryConfigs;
cHooks = new ClientHooks() {
@Override
public long beforeHandshake(long c) throws SSLException {
long ssl = super.beforeHandshake(c);
assertEquals(1, NativeCrypto.SSL_set_protocol_versions(ssl, null, TLS1_VERSION, TLS1_3_VERSION));
assertTrue(NativeCrypto.SSL_set1_ech_config_list(ssl, null, echRetryConfigsFromPrevious));
return ssl;
}

@Override
public void afterHandshake(long session, long ssl, long context, Socket socket,
FileDescriptor fd, SSLHandshakeCallbacks callback) throws Exception {
assertTrue(NativeCrypto.SSL_ech_accepted(ssl, null));
super.afterHandshake(session, ssl, context, socket, fd, callback);
}
};

client = handshake(listener, 0, true, cHooks, null, null);
server = handshake(listener, 0, false, sHooks, null, null);
clientCallback = client.get(TIMEOUT_SECONDS, TimeUnit.SECONDS);
serverCallback = server.get(TIMEOUT_SECONDS, TimeUnit.SECONDS);
assertTrue(clientCallback.verifyCertificateChainCalled);
assertEqualCertificateChains(
getServerCertificateRefs(), clientCallback.certificateChainRefs);
assertFalse(serverCallback.verifyCertificateChainCalled);
assertFalse(clientCallback.clientCertificateRequestedCalled);
assertFalse(serverCallback.clientCertificateRequestedCalled);
assertFalse(clientCallback.clientPSKKeyRequestedInvoked);
assertFalse(serverCallback.clientPSKKeyRequestedInvoked);
assertFalse(clientCallback.serverPSKKeyRequestedInvoked);
assertFalse(serverCallback.serverPSKKeyRequestedInvoked);
assertTrue(clientCallback.handshakeCompletedCalled);
assertTrue(serverCallback.handshakeCompletedCalled);
assertFalse(clientCallback.serverCertificateRequestedInvoked);
assertTrue(serverCallback.serverCertificateRequestedInvoked);
}

@Test
public void test_SSL_set_enable_ech_grease() throws Exception {
long c = NativeCrypto.SSL_CTX_new();
Expand Down Expand Up @@ -688,6 +790,11 @@ public void test_SSL_CTX_ech_enable_server() throws Exception {
NativeCrypto.SSL_CTX_free(c, null);
}

@Test(expected = NullPointerException.class)
public void test_SSL_get0_ech_retry_configs_withNullShouldThrow() throws Exception {
NativeCrypto.SSL_get0_ech_retry_configs(NULL, null);
}

@Test(expected = NullPointerException.class)
public void test_SSL_CTX_ech_enable_server_NULL_SSL_CTX() throws Exception {
NativeCrypto.SSL_CTX_ech_enable_server(NULL, null, null, null);
Expand Down Expand Up @@ -947,6 +1054,8 @@ public static class Hooks {
boolean pskEnabled;
byte[] pskKey;
List<String> enabledCipherSuites;
byte[] echRetryConfigs;
String echNameOverride;

/**
* @throws SSLException if an error occurs creating the context.
Expand Down Expand Up @@ -1268,6 +1377,12 @@ public void clientCertificateRequested(long s) {
public static Future<TestSSLHandshakeCallbacks> handshake(final ServerSocket listener,
final int timeout, final boolean client, final Hooks hooks, final byte[] alpnProtocols,
final ApplicationProtocolSelectorAdapter alpnSelector) {
return handshake(listener, timeout, client, hooks, alpnProtocols, alpnSelector, false);
}

public static Future<TestSSLHandshakeCallbacks> handshake(final ServerSocket listener,
final int timeout, final boolean client, final Hooks hooks, final byte[] alpnProtocols,
final ApplicationProtocolSelectorAdapter alpnSelector, final boolean useEchRetryConfig) {
ExecutorService executor = Executors.newSingleThreadExecutor();
Future<TestSSLHandshakeCallbacks> future =
executor.submit(new Callable<TestSSLHandshakeCallbacks>() {
Expand Down Expand Up @@ -1308,7 +1423,19 @@ public TestSSLHandshakeCallbacks call() throws Exception {
if (!client && alpnSelector != null) {
NativeCrypto.setHasApplicationProtocolSelector(s, null, true);
}
NativeCrypto.SSL_do_handshake(s, null, fd, callback, timeout);
if (useEchRetryConfig) {
try {
NativeCrypto.SSL_do_handshake(s, null, fd, callback, timeout);
} catch (SSLProtocolException e) {
hooks.echRetryConfigs =
NativeCrypto.SSL_get0_ech_retry_configs(s, null);
hooks.echNameOverride =
NativeCrypto.SSL_get0_ech_name_override(s, null);
throw e;
}
} else {
NativeCrypto.SSL_do_handshake(s, null, fd, callback, timeout);
}
session = NativeCrypto.SSL_get1_session(s, null);
if (DEBUG) {
System.out.println("ssl=0x" + Long.toString(s, 16)
Expand Down

0 comments on commit 591113a

Please sign in to comment.