Disable SSL session cache to fix issue with SecureSocket connections. (issue 11478049)

45 views
Skip to first unread message

whe...@google.com

unread,
Dec 10, 2012, 5:37:31 AM12/10/12
to ag...@google.com, rev...@dartlang.org
Reviewers: Mads Ager,

Description:
Disable SSL session cache to fix issue with SecureSocket connections.


BUG=dart:7208
TEST=standalone/io/secure_session_resume_test.dart


Please review this at https://codereview.chromium.org/11478049/

SVN Base: https://dart.googlecode.com/svn/branches/bleeding_edge/dart

Affected files:
M runtime/bin/secure_socket.cc
A + tests/standalone/io/secure_session_resume_test.dart


Index: runtime/bin/secure_socket.cc
diff --git a/runtime/bin/secure_socket.cc b/runtime/bin/secure_socket.cc
index
1e28e529b0df9e20cbadb7744daaf648d27c9f29..44158b524b3f011da520907ac2d19e5b32944c62
100644
--- a/runtime/bin/secure_socket.cc
+++ b/runtime/bin/secure_socket.cc
@@ -450,6 +450,14 @@ void SSLFilter::Connect(const char* host_name,
if (SSL_SetURL(filter_, host_name) == -1) {
ThrowPRException("Unsuccessful SetURL call");
}
+
+ // This disables the SSL session cache for client connections.
+ // This resolves issue 7208, but degrades performance.
+ // TODO(7230): Reenable session cache, without breaking client
connections.
+ status = SSL_OptionSet(filter_, SSL_NO_CACHE, PR_TRUE);
+ if (status != SECSuccess) {
+ ThrowPRException("Failed SSL_OptionSet(NO_CACHE) call");
+ }
}

// Install bad certificate callback, and pass 'this' to it if it is
called.
Index: tests/standalone/io/secure_session_resume_test.dart
diff --git a/tests/standalone/io/secure_server_test.dart
b/tests/standalone/io/secure_session_resume_test.dart
similarity index 79%
copy from tests/standalone/io/secure_server_test.dart
copy to tests/standalone/io/secure_session_resume_test.dart
index
89415483b9542d8d49b105715aacb69944818f6f..c70720ffb7216cb2fabe47f8e4b106d19e875814
100644
--- a/tests/standalone/io/secure_server_test.dart
+++ b/tests/standalone/io/secure_session_resume_test.dart
@@ -2,6 +2,15 @@
// 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.

+// This test tests TLS session resume, by making multiple client
connections
+// on the same port to the same server, with a delay of 200 ms between
them.
+// The unmodified secure_server_test creates all sessions simultaneously,
+// which means that no handshake completes and caches its keys in the
session
+// cache in time for other connections to use it.
+//
+// Session resume is currently disabled - see issue
+// https://code.google.com/p/dart/issues/detail?id=7230
+
import "dart:io";
import "dart:isolate";

@@ -101,7 +110,8 @@ void main() {
Path scriptDir = new Path.fromNative(new Options().script).directoryPath;
Path certificateDatabase = scriptDir.append('pkcert');
SecureSocket.initialize(database: certificateDatabase.toNativePath(),
- password: 'dartdart');
+ password: 'dartdart',
+ useBuiltinRoots: false);

var server = new SecureTestServer();
int port = server.start();
@@ -112,7 +122,13 @@ void main() {
keepAlive.close();
};

+ int delay = 0;
+ int delay_between_connections = 200; // Milliseconds.
+
for (var x in CLIENT_NAMES) {
- new SecureTestClient(port, x);
+ new Timer(delay, (_) {
+ new SecureTestClient(port, x);
+ });
+ delay += delay_between_connections;
}
}


ag...@google.com

unread,
Dec 10, 2012, 6:28:50 AM12/10/12
to whe...@google.com, rev...@dartlang.org
LGTM, but let's make the test as fast as we can.


https://codereview.chromium.org/11478049/diff/1/tests/standalone/io/secure_session_resume_test.dart
File tests/standalone/io/secure_session_resume_test.dart (right):

https://codereview.chromium.org/11478049/diff/1/tests/standalone/io/secure_session_resume_test.dart#newcode126
tests/standalone/io/secure_session_resume_test.dart:126: int
delay_between_connections = 200; // Milliseconds.
Does this fail with a smaller timeout as well? Can you delay only one
and get the issue? We should make this test as fast as we can. Maybe we
should make a specific test for this issue instead of putting it into
the basic test?

https://codereview.chromium.org/11478049/

whe...@google.com

unread,
Dec 10, 2012, 8:13:20 AM12/10/12
to ag...@google.com, rev...@dartlang.org
Made test run faster, added FAIL to status of bad_certificate_test on
Windows.


https://codereview.chromium.org/11478049/diff/1/tests/standalone/io/secure_session_resume_test.dart
File tests/standalone/io/secure_session_resume_test.dart (right):

https://codereview.chromium.org/11478049/diff/1/tests/standalone/io/secure_session_resume_test.dart#newcode126
tests/standalone/io/secure_session_resume_test.dart:126: int
delay_between_connections = 200; // Milliseconds.
On 2012/12/10 11:28:50, Mads Ager wrote:
> Does this fail with a smaller timeout as well? Can you delay only one
and get
> the issue? We should make this test as fast as we can. Maybe we should
make a
> specific test for this issue instead of putting it into the basic
test?

The timeout needs to be at least 200 ms, and more if we only do one
delayed one. I can change it to 400ms, and only do one delayed. So
that is only 500ms for the entire test.

https://codereview.chromium.org/11478049/
Reply all
Reply to author
Forward
0 new messages