Skip to content

Commit

Permalink
Remove remnants of broken SSL_renegotiate tests. (google#229)
Browse files Browse the repository at this point in the history
See issue google#228. These don't work anymore and will need to be rewritten,
probably against a test implementation.
  • Loading branch information
davidben authored and Nathan Mittler committed Jun 30, 2017
1 parent 4d2796a commit a78a118
Show file tree
Hide file tree
Showing 4 changed files with 0 additions and 142 deletions.
30 changes: 0 additions & 30 deletions common/src/jni/main/cpp/NativeCrypto.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9133,35 +9133,6 @@ static jlong NativeCrypto_SSL_get1_session(JNIEnv* env, jclass, jlong ssl_addres
return reinterpret_cast<uintptr_t>(SSL_get1_session(ssl));
}

/**
* Perform SSL renegotiation
*/
static void NativeCrypto_SSL_renegotiate(JNIEnv* env, jclass, jlong ssl_address)
{
SSL* ssl = to_SSL(env, ssl_address, true);
JNI_TRACE("ssl=%p NativeCrypto_SSL_renegotiate", ssl);
if (ssl == nullptr) {
return;
}
int result = SSL_renegotiate(ssl);
if (result != 1) {
Errors::throwSSLExceptionStr(env, "Problem with SSL_renegotiate");
return;
}
// first call asks client to perform renegotiation
int ret = SSL_do_handshake(ssl);
if (ret != 1) {
OpenSslError sslError(ssl, ret);
Errors::throwSSLExceptionWithSslErrors(env, ssl, sslError.release(),
"Problem with SSL_do_handshake after SSL_renegotiate");
return;
}
// if client agrees, set ssl state and perform renegotiation
SSL_set_state(ssl, SSL_ST_ACCEPT);
SSL_do_handshake(ssl);
JNI_TRACE("ssl=%p NativeCrypto_SSL_renegotiate => OK", ssl);
}

// TESTING METHODS END

#define CONSCRYPT_NATIVE_METHOD(className, functionName, signature) \
Expand Down Expand Up @@ -9450,7 +9421,6 @@ static JNINativeMethod sNativeCryptoMethods[] = {
CONSCRYPT_NATIVE_METHOD(NativeCrypto, SSL_get_mode, "(J)J"),
CONSCRYPT_NATIVE_METHOD(NativeCrypto, SSL_get_options, "(J)J"),
CONSCRYPT_NATIVE_METHOD(NativeCrypto, SSL_get1_session, "(J)J"),
CONSCRYPT_NATIVE_METHOD(NativeCrypto, SSL_renegotiate, "(J)V"),
};

void NativeCrypto::registerNativeMethods(JNIEnv* env) {
Expand Down
1 change: 0 additions & 1 deletion common/src/main/java/org/conscrypt/NativeCrypto.java
Original file line number Diff line number Diff line change
Expand Up @@ -1270,5 +1270,4 @@ static native void BIO_write(long bioRef, byte[] buffer, int offset, int length)
static native long SSL_get_mode(long ssl);
static native long SSL_get_options(long ssl);
static native long SSL_get1_session(long ssl);
static native void SSL_renegotiate(long sslNativePointer) throws SSLException;
}
1 change: 0 additions & 1 deletion constants/src/gen/cpp/generate_constants.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ int main(int /* argc */, char ** /* argv */) {

CONST(SSL_OP_CIPHER_SERVER_PREFERENCE);
CONST(SSL_OP_NO_TICKET);
CONST(SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION);
CONST(SSL_OP_NO_SSLv3);
CONST(SSL_OP_NO_TLSv1);
CONST(SSL_OP_NO_TLSv1_1);
Expand Down
110 changes: 0 additions & 110 deletions openjdk/src/test/java/org/conscrypt/NativeCryptoTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@
import org.conscrypt.OpenSSLX509CertificateFactory.ParsingException;
import org.junit.After;
import org.junit.BeforeClass;
import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
Expand Down Expand Up @@ -1299,71 +1298,6 @@ public long beforeHandshake(long c) throws SSLException {
}
}

/**
* Usually if a RuntimeException is thrown by the
* clientCertificateRequestedCalled callback, the caller sees it
* during the call to NativeCrypto_SSL_do_handshake. However, IIS
* does not request client certs until after the initial
* handshake. It does an SSL renegotiation, which means we need to
* be able to deliver the callback's exception in cases like
* SSL_read, SSL_write, and SSL_shutdown.
*/
@Ignore("TODO(nathanmittler): Determine why this doesn't pass on openjdk")
@Test
public void test_SSL_do_handshake_clientCertificateRequested_throws_after_renegotiate()
throws Exception {
final ServerSocket listener = newServerSocket();

Hooks cHooks = new Hooks() {
@Override
public long beforeHandshake(long context) throws SSLException {
long s = super.beforeHandshake(context);
NativeCrypto.SSL_clear_mode(s, SSL_MODE_ENABLE_FALSE_START);
return s;
}
@Override
public void afterHandshake(long session, long s, long c, Socket sock, FileDescriptor fd,
SSLHandshakeCallbacks callback) throws Exception {
NativeCrypto.SSL_read(s, fd, callback, new byte[1], 0, 1, 0);
fail();
super.afterHandshake(session, s, c, sock, fd, callback);
}
@Override
public void clientCertificateRequested(long s) {
super.clientCertificateRequested(s);
throw new RuntimeException("expected");
}
};
Hooks sHooks = new ServerHooks(getServerPrivateKey(), getServerCertificates()) {
@Override
public void afterHandshake(long session, long s, long c, Socket sock, FileDescriptor fd,
SSLHandshakeCallbacks callback) throws Exception {
try {
NativeCrypto.SSL_set_verify(s, NativeCrypto.SSL_VERIFY_PEER);
NativeCrypto.SSL_set_options(
s, NativeConstants.SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION);
NativeCrypto.SSL_renegotiate(s);
NativeCrypto.SSL_write(s, fd, callback, new byte[] {42}, 0, 1,
(int) ((TIMEOUT_SECONDS * 1000) / 2));
} catch (IOException expected) {
// Ignored.
} finally {
super.afterHandshake(session, s, c, sock, fd, callback);
}
}
};
Future<TestSSLHandshakeCallbacks> client = handshake(listener, 0, true, cHooks, null);
Future<TestSSLHandshakeCallbacks> server = handshake(listener, 0, false, sHooks, null);
try {
client.get(TIMEOUT_SECONDS, TimeUnit.SECONDS);
} catch (ExecutionException e) {
if (!"expected".equals(e.getCause().getMessage())) {
throw e;
}
}
server.get(TIMEOUT_SECONDS, TimeUnit.SECONDS);
}

@Test
public void test_SSL_do_handshake_client_timeout() throws Exception {
// client timeout
Expand Down Expand Up @@ -2115,50 +2049,6 @@ public void SSL_get_servername_shouldReturnNull() throws Exception {
// additional positive testing by test_SSL_set_tlsext_host_name
}

@Test(expected = NullPointerException.class)
public void SSL_renegotiate_withNullShouldThrow() throws Exception {
NativeCrypto.SSL_renegotiate(NULL);
}

@Ignore("TODO(nathanmittler): Determine why this doesn't pass on openjdk")
@Test
public void test_SSL_renegotiate_fails() throws Exception {
final ServerSocket listener = newServerSocket();
Hooks cHooks = new Hooks() {
@Override
public void afterHandshake(long session, long s, long c, Socket sock, FileDescriptor fd,
SSLHandshakeCallbacks callback) throws Exception {
byte[] buffer = new byte[1];
NativeCrypto.SSL_read(s, fd, callback, buffer, 0, 1, 0);
assertEquals(42, buffer[0]);
super.afterHandshake(session, s, c, sock, fd, callback);
}
};
Hooks sHooks = new ServerHooks(getServerPrivateKey(), getServerCertificates()) {
@Override
public void afterHandshake(long session, long s, long c, Socket sock, FileDescriptor fd,
SSLHandshakeCallbacks callback) throws Exception {
try {
// Ensure that BoringSSL throws an exception if you try to manually trigger
// SSL renegotiation, which is what we expect. The fail() may not directly
// cause the test to fail, because we may be in a separate thread from the main
// test thread, but the SSL_write() call that follows is required for the
// test to pass.
NativeCrypto.SSL_renegotiate(s);
fail("Expected SSLException");
} catch (SSLException expected) {
// Expected.
}
NativeCrypto.SSL_write(s, fd, callback, new byte[] {42}, 0, 1, 0);
super.afterHandshake(session, s, c, sock, fd, callback);
}
};
Future<TestSSLHandshakeCallbacks> client = handshake(listener, 0, true, cHooks, null);
Future<TestSSLHandshakeCallbacks> server = handshake(listener, 0, false, sHooks, null);
client.get(TIMEOUT_SECONDS, TimeUnit.SECONDS);
server.get(TIMEOUT_SECONDS, TimeUnit.SECONDS);
}

@Test(expected = NullPointerException.class)
public void SSL_get_certificate_withNullShouldThrow() throws Exception {
NativeCrypto.SSL_get_certificate(NULL);
Expand Down

0 comments on commit a78a118

Please sign in to comment.