From cdcc221d6020f545b8a99dc3a8a8c14e3eb61949 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Wed, 2 Mar 2022 19:50:50 +0000 Subject: [PATCH] Remove renegotiate. TESTED=deprecation does not break unit tests. Bug: https://github.com/dart-lang/sdk/issues/42771 Change-Id: I5c661ef91285a117c881cb15d26ab40cfd185a70 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/234880 Reviewed-by: Alexander Aprelev Commit-Queue: Brian Quinlan --- CHANGELOG.md | 3 + runtime/bin/io_natives.cc | 1 - runtime/bin/secure_socket_filter.cc | 22 ----- runtime/bin/secure_socket_filter.h | 3 - runtime/bin/secure_socket_unsupported.cc | 5 -- .../_internal/vm/bin/secure_socket_patch.dart | 11 +-- sdk/lib/io/secure_socket.dart | 22 ++--- .../io/secure_socket_renegotiate_client.dart | 72 --------------- .../io/secure_socket_renegotiate_test.dart | 85 ------------------ tests/standalone/standalone_kernel.status | 1 - .../io/secure_socket_renegotiate_client.dart | 75 ---------------- .../io/secure_socket_renegotiate_test.dart | 88 ------------------- tests/standalone_2/standalone_2_kernel.status | 1 - 13 files changed, 13 insertions(+), 376 deletions(-) delete mode 100644 tests/standalone/io/secure_socket_renegotiate_client.dart delete mode 100644 tests/standalone/io/secure_socket_renegotiate_test.dart delete mode 100644 tests/standalone_2/io/secure_socket_renegotiate_client.dart delete mode 100644 tests/standalone_2/io/secure_socket_renegotiate_test.dart diff --git a/CHANGELOG.md b/CHANGELOG.md index d06a6990f5b..70414c2b61f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,6 +60,9 @@ - Add a optional `keyLog` parameter to `SecureSocket.connect` and `SecureSocket.startConnect`. +- Deprecate `SecureSocket.renegotiate` and `RawSecureSocket.renegotiate`, + which were no-ops. + ### Tools #### Dart command line diff --git a/runtime/bin/io_natives.cc b/runtime/bin/io_natives.cc index ba4c45981af..6eddb5ab424 100644 --- a/runtime/bin/io_natives.cc +++ b/runtime/bin/io_natives.cc @@ -131,7 +131,6 @@ namespace bin { V(SecureSocket_RegisterBadCertificateCallback, 2) \ V(SecureSocket_RegisterKeyLogPort, 2) \ V(SecureSocket_RegisterHandshakeCompleteCallback, 2) \ - V(SecureSocket_Renegotiate, 4) \ V(SecurityContext_Allocate, 1) \ V(SecurityContext_UsePrivateKeyBytes, 3) \ V(SecurityContext_SetAlpnProtocols, 3) \ diff --git a/runtime/bin/secure_socket_filter.cc b/runtime/bin/secure_socket_filter.cc index 43e3a9d746a..9e1492e4ed8 100644 --- a/runtime/bin/secure_socket_filter.cc +++ b/runtime/bin/secure_socket_filter.cc @@ -175,17 +175,6 @@ void FUNCTION_NAME(SecureSocket_GetSelectedProtocol)( GetFilter(args)->GetSelectedProtocol(args); } -void FUNCTION_NAME(SecureSocket_Renegotiate)(Dart_NativeArguments args) { - bool use_session_cache = - DartUtils::GetBooleanValue(Dart_GetNativeArgument(args, 1)); - bool request_client_certificate = - DartUtils::GetBooleanValue(Dart_GetNativeArgument(args, 2)); - bool require_client_certificate = - DartUtils::GetBooleanValue(Dart_GetNativeArgument(args, 3)); - GetFilter(args)->Renegotiate(use_session_cache, request_client_certificate, - require_client_certificate); -} - void FUNCTION_NAME(SecureSocket_RegisterHandshakeCompleteCallback)( Dart_NativeArguments args) { Dart_Handle handshake_complete = @@ -662,17 +651,6 @@ void SSLFilter::GetSelectedProtocol(Dart_NativeArguments args) { } } -void SSLFilter::Renegotiate(bool use_session_cache, - bool request_client_certificate, - bool require_client_certificate) { - // The SSL_REQUIRE_CERTIFICATE option only takes effect if the - // SSL_REQUEST_CERTIFICATE option is also set, so set it. - request_client_certificate = - request_client_certificate || require_client_certificate; - // TODO(24070, 24069): Implement setting the client certificate parameters, - // and triggering rehandshake. -} - void SSLFilter::FreeResources() { if (ssl_ != NULL) { SSL_free(ssl_); diff --git a/runtime/bin/secure_socket_filter.h b/runtime/bin/secure_socket_filter.h index 56f7f582afd..fe3b77831c7 100644 --- a/runtime/bin/secure_socket_filter.h +++ b/runtime/bin/secure_socket_filter.h @@ -85,9 +85,6 @@ class SSLFilter : public ReferenceCounted { void MarkAsTrusted(Dart_NativeArguments args); int Handshake(Dart_Port reply_port); void GetSelectedProtocol(Dart_NativeArguments args); - void Renegotiate(bool use_session_cache, - bool request_client_certificate, - bool require_client_certificate); void RegisterHandshakeCompleteCallback(Dart_Handle handshake_complete); void RegisterBadCertificateCallback(Dart_Handle callback); void RegisterKeyLogPort(Dart_Port key_log_port); diff --git a/runtime/bin/secure_socket_unsupported.cc b/runtime/bin/secure_socket_unsupported.cc index 37c2972652a..f64203d573a 100644 --- a/runtime/bin/secure_socket_unsupported.cc +++ b/runtime/bin/secure_socket_unsupported.cc @@ -90,11 +90,6 @@ void FUNCTION_NAME(SecureSocket_FilterPointer)(Dart_NativeArguments args) { "Secure Sockets unsupported on this platform")); } -void FUNCTION_NAME(SecureSocket_Renegotiate)(Dart_NativeArguments args) { - Dart_ThrowException(DartUtils::NewDartArgumentError( - "Secure Sockets unsupported on this platform")); -} - void FUNCTION_NAME(SecureSocket_NewServicePort)(Dart_NativeArguments args) { Dart_ThrowException(DartUtils::NewDartArgumentError( "Secure Sockets unsupported on this platform")); diff --git a/sdk/lib/_internal/vm/bin/secure_socket_patch.dart b/sdk/lib/_internal/vm/bin/secure_socket_patch.dart index a2ce0f36373..680adcf3066 100644 --- a/sdk/lib/_internal/vm/bin/secure_socket_patch.dart +++ b/sdk/lib/_internal/vm/bin/secure_socket_patch.dart @@ -33,12 +33,7 @@ class _SecureSocket extends _Socket implements SecureSocket { void renegotiate( {bool useSessionCache: true, bool requestClientCertificate: false, - bool requireClientCertificate: false}) { - _raw!.renegotiate( - useSessionCache: useSessionCache, - requestClientCertificate: requestClientCertificate, - requireClientCertificate: requireClientCertificate); - } + bool requireClientCertificate: false}) {} X509Certificate? get peerCertificate { if (_raw == null) { @@ -165,10 +160,6 @@ class _SecureFilterImpl extends NativeFieldWrapperClass1 @pragma("vm:external-name", "SecureSocket_GetSelectedProtocol") external String? selectedProtocol(); - @pragma("vm:external-name", "SecureSocket_Renegotiate") - external void renegotiate(bool useSessionCache, bool requestClientCertificate, - bool requireClientCertificate); - @pragma("vm:external-name", "SecureSocket_Init") external void init(); diff --git a/sdk/lib/io/secure_socket.dart b/sdk/lib/io/secure_socket.dart index de267bbed6a..7dd311cb919 100644 --- a/sdk/lib/io/secure_socket.dart +++ b/sdk/lib/io/secure_socket.dart @@ -208,12 +208,11 @@ abstract class SecureSocket implements Socket { /// protocol between client and server. String? get selectedProtocol; - /// Renegotiates an existing secure connection. + /// Does nothing. /// - /// Renews the session keys and possibly changes the connection properties. - /// - /// This repeats the SSL or TLS handshake, with options that allow clearing - /// the session cache and requesting a client certificate. + /// The original intent was to allow TLS renegotiation of existing secure + /// connections. + @Deprecated("Not implemented") void renegotiate( {bool useSessionCache = true, bool requestClientCertificate = false, @@ -421,11 +420,11 @@ abstract class RawSecureSocket implements RawSocket { supportedProtocols: supportedProtocols); } - /// Renegotiate an existing secure connection, renewing the session keys - /// and possibly changing the connection properties. + /// Does nothing. /// - /// This repeats the SSL or TLS handshake, with options that allow clearing - /// the session cache and requesting a client certificate. + /// The original intent was to allow TLS renegotiation of existing secure + /// connections. + @Deprecated("Not implemented") void renegotiate( {bool useSessionCache = true, bool requestClientCertificate = false, @@ -924,6 +923,7 @@ class _RawSecureSocket extends Stream } } + @Deprecated("Not implemented") void renegotiate( {bool useSessionCache = true, bool requestClientCertificate = false, @@ -932,8 +932,6 @@ class _RawSecureSocket extends Stream throw new HandshakeException( "Called renegotiate on a non-connected socket"); } - _secureFilter!.renegotiate( - useSessionCache, requestClientCertificate, requireClientCertificate); _status = handshakeStatus; _filterStatus.writeEmpty = false; _scheduleFilter(); @@ -1342,8 +1340,6 @@ abstract class _SecureFilter { Future handshake(); String? selectedProtocol(); void rehandshake(); - void renegotiate(bool useSessionCache, bool requestClientCertificate, - bool requireClientCertificate); void init(); X509Certificate? get peerCertificate; int processBuffer(int bufferIndex); diff --git a/tests/standalone/io/secure_socket_renegotiate_client.dart b/tests/standalone/io/secure_socket_renegotiate_client.dart deleted file mode 100644 index 775891b2338..00000000000 --- a/tests/standalone/io/secure_socket_renegotiate_client.dart +++ /dev/null @@ -1,72 +0,0 @@ -// Copyright (c) 2013, the Dart project authors. Please see the AUTHORS file -// for details. All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. - -// Client for secure_socket_renegotiate_test, that runs in a subprocess. -// The test verifies that client certificates work, if the client and server -// are in separate processes, and that connection renegotiation can request -// a client certificate to be sent. - -import "dart:async"; -import "dart:convert"; -import "dart:io"; - -const HOST_NAME = "localhost"; -String localFile(path) => Platform.script.resolve(path).toFilePath(); - -SecurityContext clientContext = new SecurityContext() - ..setTrustedCertificates(localFile('certificates/trusted_certs.pem')); - -class ExpectException implements Exception { - ExpectException(this.message); - String toString() => message; - String message; -} - -void expectEquals(expected, actual) { - if (actual != expected) { - throw new ExpectException('Expected $expected, found $actual'); - } -} - -void expect(condition) { - if (!condition) { - throw new ExpectException(''); - } -} - -void runClient(int port) { - SecureSocket.connect(HOST_NAME, port, context: clientContext) - .then((SecureSocket socket) { - X509Certificate? certificate = socket.peerCertificate; - expect(certificate != null); - expectEquals('CN=localhost', certificate!.subject); - expectEquals('CN=myauthority', certificate.issuer); - StreamIterator input = new StreamIterator( - socket.transform(utf8.decoder).transform(new LineSplitter())); - socket.writeln('first'); - input.moveNext().then((success) { - expect(success); - expectEquals('first reply', input.current); - socket.renegotiate(); - socket.writeln('renegotiated'); - return input.moveNext(); - }).then((success) { - expect(success); - expectEquals('server renegotiated', input.current); - X509Certificate? certificate = socket.peerCertificate; - expect(certificate != null); - expectEquals("CN=localhost", certificate!.subject); - expectEquals("CN=myauthority", certificate.issuer); - socket.writeln('second'); - return input.moveNext(); - }).then((success) { - expect(success != true); - socket.close(); - }); - }); -} - -void main(List args) { - runClient(int.parse(args[0])); -} diff --git a/tests/standalone/io/secure_socket_renegotiate_test.dart b/tests/standalone/io/secure_socket_renegotiate_test.dart deleted file mode 100644 index 05aa2f19665..00000000000 --- a/tests/standalone/io/secure_socket_renegotiate_test.dart +++ /dev/null @@ -1,85 +0,0 @@ -// Copyright (c) 2013, the Dart project authors. Please see the AUTHORS file -// for details. All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. -// -// OtherResources=certificates/server_chain.pem -// OtherResources=certificates/server_key.pem -// OtherResources=secure_socket_renegotiate_client.dart - -// This test verifies that client certificates work, if the client and server -// are in separate processes, and that connection renegotiation works, and -// can request a client certificate to be sent. - -import "dart:async"; -import "dart:convert"; -import "dart:io"; - -import "package:expect/expect.dart"; -import "package:path/path.dart"; - -const HOST_NAME = "localhost"; -String localFile(path) => Platform.script.resolve(path).toFilePath(); - -SecurityContext serverContext = new SecurityContext() - ..useCertificateChain(localFile('certificates/server_chain.pem')) - ..usePrivateKey(localFile('certificates/server_key.pem'), - password: 'dartdart'); - -Future runServer() { - return SecureServerSocket.bind(HOST_NAME, 0, serverContext) - .then((SecureServerSocket server) { - server.listen((SecureSocket socket) { - Expect.isNull(socket.peerCertificate); - - StreamIterator input = new StreamIterator( - utf8.decoder.bind(socket).transform(new LineSplitter())); - input.moveNext().then((success) { - Expect.isTrue(success); - Expect.equals('first', input.current); - socket.writeln('first reply'); - return input.moveNext(); - }).then((success) { - Expect.isTrue(success); - Expect.equals('renegotiated', input.current); - Expect.isNull(socket.peerCertificate); - socket.renegotiate( - requestClientCertificate: true, - requireClientCertificate: true, - useSessionCache: false); - socket.writeln('server renegotiated'); - return input.moveNext(); - }).then((success) { - Expect.isTrue(success); - Expect.equals('second', input.current); - X509Certificate certificate = socket.peerCertificate!; - Expect.equals("CN=localhost", certificate.subject); - Expect.equals("CN=myauthority", certificate.issuer); - server.close(); - socket.close(); - }); - }); - return server; - }); -} - -void main() { - runServer().then((SecureServerSocket server) { - var clientScript = Platform.script - .resolve('secure_socket_renegotiate_client.dart') - .toFilePath(); - Process.run( - Platform.executable, - [] - ..addAll(Platform.executableArguments) - ..addAll([clientScript, server.port.toString()])) - .then((ProcessResult result) { - if (result.exitCode != 0) { - print("Client failed, stdout:"); - print(result.stdout); - print(" stderr:"); - print(result.stderr); - Expect.fail('Client subprocess exit code: ${result.exitCode}'); - } - }); - }); -} diff --git a/tests/standalone/standalone_kernel.status b/tests/standalone/standalone_kernel.status index 609c61b308c..41bcfb00b37 100644 --- a/tests/standalone/standalone_kernel.status +++ b/tests/standalone/standalone_kernel.status @@ -97,7 +97,6 @@ io/raw_socket_test: Crash io/secure_multiple_client_server_test: Skip # Flaky. io/secure_server_closing_test: Skip # Flaky. io/secure_server_socket_test: Skip # Flaky. -io/secure_socket_renegotiate_test: Crash io/socket_many_connections_test: Skip # Flaky io/web_socket_error_test: Skip # Flaky io/web_socket_ping_test: Skip # Flaky. diff --git a/tests/standalone_2/io/secure_socket_renegotiate_client.dart b/tests/standalone_2/io/secure_socket_renegotiate_client.dart deleted file mode 100644 index c8ae0bbeac7..00000000000 --- a/tests/standalone_2/io/secure_socket_renegotiate_client.dart +++ /dev/null @@ -1,75 +0,0 @@ -// Copyright (c) 2013, the Dart project authors. Please see the AUTHORS file -// for details. All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. - -// @dart = 2.9 - -// Client for secure_socket_renegotiate_test, that runs in a subprocess. -// The test verifies that client certificates work, if the client and server -// are in separate processes, and that connection renegotiation can request -// a client certificate to be sent. - -import "dart:async"; -import "dart:convert"; -import "dart:io"; - -const HOST_NAME = "localhost"; -String localFile(path) => Platform.script.resolve(path).toFilePath(); - -SecurityContext clientContext = new SecurityContext() - ..setTrustedCertificates(localFile('certificates/trusted_certs.pem')); - -class ExpectException implements Exception { - ExpectException(this.message); - String toString() => message; - String message; -} - -void expectEquals(expected, actual) { - if (actual != expected) { - throw new ExpectException('Expected $expected, found $actual'); - } -} - -void expect(condition) { - if (!condition) { - throw new ExpectException(''); - } -} - -void runClient(int port) { - SecureSocket - .connect(HOST_NAME, port, context: clientContext) - .then((SecureSocket socket) { - X509Certificate certificate = socket.peerCertificate; - expect(certificate != null); - expectEquals('CN=localhost', certificate.subject); - expectEquals('CN=myauthority', certificate.issuer); - StreamIterator input = new StreamIterator( - socket.transform(utf8.decoder).transform(new LineSplitter())); - socket.writeln('first'); - input.moveNext().then((success) { - expect(success); - expectEquals('first reply', input.current); - socket.renegotiate(); - socket.writeln('renegotiated'); - return input.moveNext(); - }).then((success) { - expect(success); - expectEquals('server renegotiated', input.current); - X509Certificate certificate = socket.peerCertificate; - expect(certificate != null); - expectEquals("CN=localhost", certificate.subject); - expectEquals("CN=myauthority", certificate.issuer); - socket.writeln('second'); - return input.moveNext(); - }).then((success) { - expect(success != true); - socket.close(); - }); - }); -} - -void main(List args) { - runClient(int.parse(args[0])); -} diff --git a/tests/standalone_2/io/secure_socket_renegotiate_test.dart b/tests/standalone_2/io/secure_socket_renegotiate_test.dart deleted file mode 100644 index 36cb3a2b50f..00000000000 --- a/tests/standalone_2/io/secure_socket_renegotiate_test.dart +++ /dev/null @@ -1,88 +0,0 @@ -// Copyright (c) 2013, the Dart project authors. Please see the AUTHORS file -// for details. All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. -// -// OtherResources=certificates/server_chain.pem -// OtherResources=certificates/server_key.pem -// OtherResources=secure_socket_renegotiate_client.dart - -// @dart = 2.9 - -// This test verifies that client certificates work, if the client and server -// are in separate processes, and that connection renegotiation works, and -// can request a client certificate to be sent. - -import "dart:async"; -import "dart:convert"; -import "dart:io"; - -import "package:expect/expect.dart"; -import "package:path/path.dart"; - -const HOST_NAME = "localhost"; -String localFile(path) => Platform.script.resolve(path).toFilePath(); - -SecurityContext serverContext = new SecurityContext() - ..useCertificateChain(localFile('certificates/server_chain.pem')) - ..usePrivateKey(localFile('certificates/server_key.pem'), - password: 'dartdart'); - -Future runServer() { - return SecureServerSocket.bind(HOST_NAME, 0, serverContext) - .then((SecureServerSocket server) { - server.listen((SecureSocket socket) { - Expect.isNull(socket.peerCertificate); - - StreamIterator input = new StreamIterator( - utf8.decoder.bind(socket).transform(new LineSplitter())); - input.moveNext().then((success) { - Expect.isTrue(success); - Expect.equals('first', input.current); - socket.writeln('first reply'); - return input.moveNext(); - }).then((success) { - Expect.isTrue(success); - Expect.equals('renegotiated', input.current); - Expect.isNull(socket.peerCertificate); - socket.renegotiate( - requestClientCertificate: true, - requireClientCertificate: true, - useSessionCache: false); - socket.writeln('server renegotiated'); - return input.moveNext(); - }).then((success) { - Expect.isTrue(success); - Expect.equals('second', input.current); - X509Certificate certificate = socket.peerCertificate; - Expect.isNotNull(certificate); - Expect.equals("CN=localhost", certificate.subject); - Expect.equals("CN=myauthority", certificate.issuer); - server.close(); - socket.close(); - }); - }); - return server; - }); -} - -void main() { - runServer().then((SecureServerSocket server) { - var clientScript = Platform.script - .resolve('secure_socket_renegotiate_client.dart') - .toFilePath(); - Process.run( - Platform.executable, - [] - ..addAll(Platform.executableArguments) - ..addAll([clientScript, server.port.toString()])) - .then((ProcessResult result) { - if (result.exitCode != 0) { - print("Client failed, stdout:"); - print(result.stdout); - print(" stderr:"); - print(result.stderr); - Expect.fail('Client subprocess exit code: ${result.exitCode}'); - } - }); - }); -} diff --git a/tests/standalone_2/standalone_2_kernel.status b/tests/standalone_2/standalone_2_kernel.status index afb2751bb45..90cc6f48121 100644 --- a/tests/standalone_2/standalone_2_kernel.status +++ b/tests/standalone_2/standalone_2_kernel.status @@ -99,7 +99,6 @@ io/raw_socket_test: Crash io/secure_multiple_client_server_test: Skip # Flaky. io/secure_server_closing_test: Skip # Flaky. io/secure_server_socket_test: Skip # Flaky. -io/secure_socket_renegotiate_test: Crash io/socket_many_connections_test: Skip # Flaky io/web_socket_error_test: Skip # Flaky io/web_socket_ping_test: Skip # Flaky.