andreacavalli
/
netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

OpenSslEngine: Remove renegotiation support 

Motivation:

SSL.setState() has gone from openssl 1.1. Calling it is, and probably
always has been, incorrect. Doing renogitation in this manner is
potentially insecure. There have been at least two insecure
renegotiation vulnerabilities in users of the OpenSSL library.

Renegotiation is not necessary for correct operation of the TLS protocol.

BoringSSL has already eliminated this functionality, and the tests
(now deleted) were not running on BoringSSL.

Modifications:

If the connection setup has completed, always return that
negotiation is not supported. Previously this was done only if we were
the client.

Remove the tests for this functionality.

Fixes #6320.
This commit is contained in:
Chris West 2018-01-02 18:00:11 +00:00 committed by Scott Mitchell
parent c6a9c4d2bd
commit 8ffa828cbb
3 changed files with 1 additions and 108 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java
View File

@ -163,7 +163,6 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
}
private HandshakeState handshakeState = HandshakeState.NOT_STARTED;
private boolean renegotiationPending;
private boolean receivedShutdown;
private volatile int destroyed;
private volatile String applicationProtocol;
@ -633,17 +632,6 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
status = handshake();
if (renegotiationPending && status == FINISHED) {
// If renegotiationPending is true that means when we attempted to start renegotiation
// the BIO buffer didn't have enough space to hold the HelloRequest which prompts the
// client to initiate a renegotiation. At this point the HelloRequest has been written
// so we can actually start the handshake process.
renegotiationPending = false;
SSL.setState(ssl, SSL.SSL_ST_ACCEPT);
handshakeState = HandshakeState.STARTED_EXPLICITLY;
status = handshake();
}
// Handshake may have generated more data, for example if the internal SSL buffer is small
// we may have freed up space by flushing above.
bytesProduced = bioLengthBefore - SSL.bioLengthByteBuffer(networkBIO);
@ -1489,43 +1477,7 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
// Nothing to do as the handshake is not done yet.
break;
case FINISHED:
if (clientMode) {
// Only supported for server mode at the moment.
throw RENEGOTIATION_UNSUPPORTED;
}
// For renegotiate on the server side we need to issue the following command sequence with openssl:
//
// SSL_renegotiate(ssl)
// SSL_do_handshake(ssl)
// ssl->state = SSL_ST_ACCEPT
// SSL_do_handshake(ssl)
//
// Because of this we fall-through to call handshake() after setting the state, as this will also take
// care of updating the internal OpenSslSession object.
//
// See also:
// https://github.com/apache/httpd/blob/2.4.16/modules/ssl/ssl_engine_kernel.c#L812
// http://h71000.www7.hp.com/doc/83final/ba554_90007/ch04s03.html
int status;
if ((status = SSL.renegotiate(ssl)) != 1 || (status = SSL.doHandshake(ssl)) != 1) {
int err = SSL.getError(ssl, status);
if (err == SSL.SSL_ERROR_WANT_READ || err == SSL.SSL_ERROR_WANT_WRITE) {
// If the internal SSL buffer is small it is possible that doHandshake may "fail" because
// there is not enough room to write, so we should wait until the renegotiation has been.
renegotiationPending = true;
handshakeState = HandshakeState.STARTED_EXPLICITLY;
lastAccessed = System.currentTimeMillis();
return;
} else {
throw shutdownWithError("renegotiation failed");
}
}
SSL.setState(ssl, SSL.SSL_ST_ACCEPT);
lastAccessed = System.currentTimeMillis();
// fall-through
throw RENEGOTIATION_UNSUPPORTED;
case NOT_STARTED:
handshakeState = HandshakeState.STARTED_EXPLICITLY;
handshake();

23
handler/src/test/java/io/netty/handler/ssl/OpenSslRenegotiateSmallBIOTest.java
View File

@ -1,23 +0,0 @@
/*
* Copyright 2017 The Netty Project
*
* The Netty Project licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/
package io.netty.handler.ssl;
public class OpenSslRenegotiateSmallBIOTest extends OpenSslRenegotiateTest {
@Override
protected void initSslServerContext(SslContext context) {
((ReferenceCountedOpenSslContext) context).setBioNonApplicationBufferSize(1);
}
}

36
handler/src/test/java/io/netty/handler/ssl/OpenSslRenegotiateTest.java
View File

@ -1,36 +0,0 @@
/*
* Copyright 2015 The Netty Project
*
* The Netty Project licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/
package io.netty.handler.ssl;
import org.junit.BeforeClass;
import static org.junit.Assume.assumeFalse;
import static org.junit.Assume.assumeTrue;
public class OpenSslRenegotiateTest extends RenegotiateTest {
@BeforeClass
public static void checkOpenSsl() {
assumeTrue(OpenSsl.isAvailable());
// BoringSSL does not support renegotiation intentionally.
assumeFalse("BoringSSL".equals(OpenSsl.versionString()));
}
@Override
protected SslProvider serverSslProvider() {
return SslProvider.OPENSSL;
}
}