Support for the ALPN extension of the TLS protocol for Client and Server

Support for the ALPN extension of the TLS protocol for Client and Server

This CL uses the NSS support for the ALPN extension of the TLS protocol
(see RFC 7301).

R=sgjesse@google.com

Committed: https://code.google.com/p/dart/source/detail?r=41628

[kustermann, 2014-10-03 13:25:26]

kustermann@google.com changed reviewers:
+ sgjesse@google.com

[kustermann, 2014-10-03 13:25:26]

You could take a look and give feedback. [First time I've touched used the
Embedder API]

This can obviously not land before the NSS roll is in.

https://codereview.chromium.org/625953002/diff/20001/dart/runtime/bin/secure_...
File dart/runtime/bin/secure_socket.cc (right):

https://codereview.chromium.org/625953002/diff/20001/dart/runtime/bin/secure_...
dart/runtime/bin/secure_socket.cc:178: protocol_string_len);
If GetFilter(args)->Connect() fails, can it unwind the stack without releasing
the TypedData?

Will the TypedData be released automatically?

[I've never worked with this C api, so I rely on you known what to do here].

[Søren Gjesse, 2014-10-03 15:47:04]

LGTM with comments

https://codereview.chromium.org/625953002/diff/20001/dart/runtime/bin/secure_...
File dart/runtime/bin/secure_socket.cc (right):

https://codereview.chromium.org/625953002/diff/20001/dart/runtime/bin/secure_...
dart/runtime/bin/secure_socket.cc:178: protocol_string_len);
On 2014/10/03 13:25:26, kustermann wrote:
> If GetFilter(args)->Connect() fails, can it unwind the stack without releasing
> the TypedData?
> 
> Will the TypedData be released automatically?
> 
> [I've never worked with this C api, so I rely on you known what to do here].

As discussed off-line you need to release it before calling Connect, that is
make a copy somehow.

https://codereview.chromium.org/625953002/diff/20001/dart/runtime/bin/secure_...
dart/runtime/bin/secure_socket.cc:890: "Protocol selected via ALPN was
non-UTF8");
In principle this could also be an OOM i think, or maybe they never return.

https://codereview.chromium.org/625953002/diff/20001/dart/sdk/lib/io/secure_s...
File dart/sdk/lib/io/secure_socket.dart (right):

https://codereview.chromium.org/625953002/diff/20001/dart/sdk/lib/io/secure_s...
dart/sdk/lib/io/secure_socket.dart:592: List<int> bytes = [];
You could allocate a 255 byte Uint8List here and return a view on the actual
data, or loop twice and first calculate the length (maybe with a fastcase for
ASCII). That will avoid the growable integer list.

https://codereview.chromium.org/625953002/diff/20001/dart/sdk/lib/io/secure_s...
dart/sdk/lib/io/secure_socket.dart:970: 'xConnection terminated during
handshake');
Accidental edit.

[Søren Gjesse, 2014-11-06 08:02:35]

On 2014/10/03 15:47:04, Søren Gjesse wrote:
> LGTM with comments
> 
>
https://codereview.chromium.org/625953002/diff/20001/dart/runtime/bin/secure_...
> File dart/runtime/bin/secure_socket.cc (right):
> 
>
https://codereview.chromium.org/625953002/diff/20001/dart/runtime/bin/secure_...
> dart/runtime/bin/secure_socket.cc:178: protocol_string_len);
> On 2014/10/03 13:25:26, kustermann wrote:
> > If GetFilter(args)->Connect() fails, can it unwind the stack without
releasing
> > the TypedData?
> > 
> > Will the TypedData be released automatically?
> > 
> > [I've never worked with this C api, so I rely on you known what to do here].
> 
> As discussed off-line you need to release it before calling Connect, that is
> make a copy somehow.
> 
>
https://codereview.chromium.org/625953002/diff/20001/dart/runtime/bin/secure_...
> dart/runtime/bin/secure_socket.cc:890: "Protocol selected via ALPN was
> non-UTF8");
> In principle this could also be an OOM i think, or maybe they never return.
> 
>
https://codereview.chromium.org/625953002/diff/20001/dart/sdk/lib/io/secure_s...
> File dart/sdk/lib/io/secure_socket.dart (right):
> 
>
https://codereview.chromium.org/625953002/diff/20001/dart/sdk/lib/io/secure_s...
> dart/sdk/lib/io/secure_socket.dart:592: List<int> bytes = [];
> You could allocate a 255 byte Uint8List here and return a view on the actual
> data, or loop twice and first calculate the length (maybe with a fastcase for
> ASCII). That will avoid the growable integer list.
> 
>
https://codereview.chromium.org/625953002/diff/20001/dart/sdk/lib/io/secure_s...
> dart/sdk/lib/io/secure_socket.dart:970: 'xConnection terminated during
> handshake');
> Accidental edit.

Martin,

Are you landing this for 1.8?

[kustermann, 2014-11-06 09:00:46]

On 2014/11/06 08:02:35, Søren Gjesse wrote:
> On 2014/10/03 15:47:04, Søren Gjesse wrote:
> > LGTM with comments
> > 
> >
>
https://codereview.chromium.org/625953002/diff/20001/dart/runtime/bin/secure_...
> > File dart/runtime/bin/secure_socket.cc (right):
> > 
> >
>
https://codereview.chromium.org/625953002/diff/20001/dart/runtime/bin/secure_...
> > dart/runtime/bin/secure_socket.cc:178: protocol_string_len);
> > On 2014/10/03 13:25:26, kustermann wrote:
> > > If GetFilter(args)->Connect() fails, can it unwind the stack without
> releasing
> > > the TypedData?
> > > 
> > > Will the TypedData be released automatically?
> > > 
> > > [I've never worked with this C api, so I rely on you known what to do
here].
> > 
> > As discussed off-line you need to release it before calling Connect, that is
> > make a copy somehow.
> > 
> >
>
https://codereview.chromium.org/625953002/diff/20001/dart/runtime/bin/secure_...
> > dart/runtime/bin/secure_socket.cc:890: "Protocol selected via ALPN was
> > non-UTF8");
> > In principle this could also be an OOM i think, or maybe they never return.
> > 
> >
>
https://codereview.chromium.org/625953002/diff/20001/dart/sdk/lib/io/secure_s...
> > File dart/sdk/lib/io/secure_socket.dart (right):
> > 
> >
>
https://codereview.chromium.org/625953002/diff/20001/dart/sdk/lib/io/secure_s...
> > dart/sdk/lib/io/secure_socket.dart:592: List<int> bytes = [];
> > You could allocate a 255 byte Uint8List here and return a view on the actual
> > data, or loop twice and first calculate the length (maybe with a fastcase
for
> > ASCII). That will avoid the growable integer list.
> > 
> >
>
https://codereview.chromium.org/625953002/diff/20001/dart/sdk/lib/io/secure_s...
> > dart/sdk/lib/io/secure_socket.dart:970: 'xConnection terminated during
> > handshake');
> > Accidental edit.
> 
> Martin,
> 
> Are you landing this for 1.8?

I think the value of it right now is limited as long as we don't work actively
on SPDY/HTTP2.0 (and maybe we should?!).

But I could finish & commit the CL if you want.

[kustermann, 2014-11-07 16:12:33]

Patchset #3 (id:40001) has been deleted

[kustermann, 2014-11-07 16:20:03]

PTAL

Thanks to lrn@ :)

https://codereview.chromium.org/625953002/diff/20001/dart/runtime/bin/secure_...
File dart/runtime/bin/secure_socket.cc (right):

https://codereview.chromium.org/625953002/diff/20001/dart/runtime/bin/secure_...
dart/runtime/bin/secure_socket.cc:178: protocol_string_len);
On 2014/10/03 15:47:04, Søren Gjesse wrote:
> On 2014/10/03 13:25:26, kustermann wrote:
> > If GetFilter(args)->Connect() fails, can it unwind the stack without
releasing
> > the TypedData?
> > 
> > Will the TypedData be released automatically?
> > 
> > [I've never worked with this C api, so I rely on you known what to do here].
> 
> As discussed off-line you need to release it before calling Connect, that is
> make a copy somehow.

Instead of making more calls into the VM & copying the string, I give the handle
to the filter method, where I can guarantee that nothing unwinds the stack.

https://codereview.chromium.org/625953002/diff/20001/dart/runtime/bin/secure_...
dart/runtime/bin/secure_socket.cc:890: "Protocol selected via ALPN was
non-UTF8");
On 2014/10/03 15:47:04, Søren Gjesse wrote:
> In principle this could also be an OOM i think, or maybe they never return.

Rephrased message.

https://codereview.chromium.org/625953002/diff/20001/dart/sdk/lib/io/secure_s...
File dart/sdk/lib/io/secure_socket.dart (right):

https://codereview.chromium.org/625953002/diff/20001/dart/sdk/lib/io/secure_s...
dart/sdk/lib/io/secure_socket.dart:592: List<int> bytes = [];
On 2014/10/03 15:47:04, Søren Gjesse wrote:
> You could allocate a 255 byte Uint8List here and return a view on the actual
> data, or loop twice and first calculate the length (maybe with a fastcase for
> ASCII). That will avoid the growable integer list.

Did something similar. Had some discussions with Lasse about it and he  made a
faster implementation :)

https://codereview.chromium.org/625953002/diff/20001/dart/sdk/lib/io/secure_s...
dart/sdk/lib/io/secure_socket.dart:970: 'xConnection terminated during
handshake');
On 2014/10/03 15:47:04, Søren Gjesse wrote:
> Accidental edit.

Done.

[Søren Gjesse, 2014-11-10 11:38:00]

lgtm

https://codereview.chromium.org/625953002/diff/60001/dart/sdk/lib/io/secure_s...
File dart/sdk/lib/io/secure_socket.dart (right):

https://codereview.chromium.org/625953002/diff/60001/dart/sdk/lib/io/secure_s...
dart/sdk/lib/io/secure_socket.dart:609: /// We therefore put the protocol least
desired tothe front, to make it the
tothe -> to the

https://codereview.chromium.org/625953002/diff/60001/dart/tests/standalone/io...
File dart/tests/standalone/io/secure_socket_alpn_test.dart (right):

https://codereview.chromium.org/625953002/diff/60001/dart/tests/standalone/io...
dart/tests/standalone/io/secure_socket_alpn_test.dart:1: // Copyright (c) 2013,
the Dart project authors.  Please see the AUTHORS file
2014

https://codereview.chromium.org/625953002/diff/60001/dart/tests/standalone/io...
dart/tests/standalone/io/secure_socket_alpn_test.dart:5: // This test tests TLS
session resume, by making multiple client connections
This description does not fit with what is being tested. Remove?

https://codereview.chromium.org/625953002/diff/60001/dart/tests/standalone/io...
dart/tests/standalone/io/secure_socket_alpn_test.dart:72:
server.first.catchError((error, stack) {
Add a test of the error type?

https://codereview.chromium.org/625953002/diff/60001/dart/tests/standalone/io...
dart/tests/standalone/io/secure_socket_alpn_test.dart:80: .catchError((error,
stack) {
Ditto.

[kustermann, 2014-11-10 12:30:22]

https://codereview.chromium.org/625953002/diff/60001/dart/sdk/lib/io/secure_s...
File dart/sdk/lib/io/secure_socket.dart (right):

https://codereview.chromium.org/625953002/diff/60001/dart/sdk/lib/io/secure_s...
dart/sdk/lib/io/secure_socket.dart:609: /// We therefore put the protocol least
desired tothe front, to make it the
On 2014/11/10 11:37:59, Søren Gjesse wrote:
> tothe -> to the

Done.

https://codereview.chromium.org/625953002/diff/60001/dart/tests/standalone/io...
File dart/tests/standalone/io/secure_socket_alpn_test.dart (right):

https://codereview.chromium.org/625953002/diff/60001/dart/tests/standalone/io...
dart/tests/standalone/io/secure_socket_alpn_test.dart:1: // Copyright (c) 2013,
the Dart project authors.  Please see the AUTHORS file
On 2014/11/10 11:38:00, Søren Gjesse wrote:
> 2014

Done.

https://codereview.chromium.org/625953002/diff/60001/dart/tests/standalone/io...
dart/tests/standalone/io/secure_socket_alpn_test.dart:5: // This test tests TLS
session resume, by making multiple client connections
On 2014/11/10 11:38:00, Søren Gjesse wrote:
> This description does not fit with what is being tested. Remove?

Done.

https://codereview.chromium.org/625953002/diff/60001/dart/tests/standalone/io...
dart/tests/standalone/io/secure_socket_alpn_test.dart:72:
server.first.catchError((error, stack) {
On 2014/11/10 11:38:00, Søren Gjesse wrote:
> Add a test of the error type?

Added

Expect.isTrue(error is HandshakeException);

https://codereview.chromium.org/625953002/diff/60001/dart/tests/standalone/io...
dart/tests/standalone/io/secure_socket_alpn_test.dart:80: .catchError((error,
stack) {
On 2014/11/10 11:38:00, Søren Gjesse wrote:
> Ditto.

ditto

[kustermann, 2014-11-10 12:31:22]

Committed patchset #4 (id:80001) manually as 41628 (presubmit successful).