Adds SecurityContext.usePrivateKeyBytes

Adds SecurityContext.usePrivateKeyBytes

This is the first step toward removing blocking IO calls from
the implementation of the SecurityContext API.

Using a buffer rather than a file name API will probably be needed
for implementing SecurityContext and SecureSocket with platform
specific built-in APIs on iOS/Mac/Windows rather than BoringSSL.

related #8227

R=whesse@google.com

Committed: https://github.com/dart-lang/sdk/commit/98771a083f13c1aebc5ebf599b52778b42b21f80

[zra, 2016-01-21 22:26:12]

Description was changed from

==========
Pass a buffer instead of a filename to SecurityContext.usePrivateKey

This is the first step toward removing synchronous IO calls from
the implementation of the SecurityContext API.

Changing to a buffer rather than a file API will also be needed
for implemeting SecurityContext and SecureSocket with platform
specific APIs rather than BoringSSL.
==========

to

==========
Pass a buffer instead of a filename to SecurityContext.usePrivateKey

This is the first step toward removing synchronous IO calls from
the implementation of the SecurityContext API.

Changing to a buffer rather than a file API will also be needed
for implemeting SecurityContext and SecureSocket with platform
specific APIs on iOS/Mac/Windows rather than BoringSSL.
==========

[zra, 2016-01-25 22:52:01]

Description was changed from

==========
Pass a buffer instead of a filename to SecurityContext.usePrivateKey

This is the first step toward removing synchronous IO calls from
the implementation of the SecurityContext API.

Changing to a buffer rather than a file API will also be needed
for implemeting SecurityContext and SecureSocket with platform
specific APIs on iOS/Mac/Windows rather than BoringSSL.
==========

to

==========
Adds SecurityContext.usePrivateKeyBytes

This is the first step toward removing synchronous IO calls from
the implementation of the SecurityContext API.

Using a buffer rather than a file name API will probably be needed
for implementing SecurityContext and SecureSocket with platform
specific APIs on iOS/Mac/Windows rather than BoringSSL.
==========

[zra, 2016-01-25 22:53:25]

Description was changed from

==========
Adds SecurityContext.usePrivateKeyBytes

This is the first step toward removing synchronous IO calls from
the implementation of the SecurityContext API.

Using a buffer rather than a file name API will probably be needed
for implementing SecurityContext and SecureSocket with platform
specific APIs on iOS/Mac/Windows rather than BoringSSL.
==========

to

==========
Adds SecurityContext.usePrivateKeyBytes

This is the first step toward removing blocking IO calls from
the implementation of the SecurityContext API.

Using a buffer rather than a file name API will probably be needed
for implementing SecurityContext and SecureSocket with platform
specific APIs on iOS/Mac/Windows rather than BoringSSL.
==========

[zra, 2016-01-25 22:53:55]

Description was changed from

==========
Adds SecurityContext.usePrivateKeyBytes

This is the first step toward removing blocking IO calls from
the implementation of the SecurityContext API.

Using a buffer rather than a file name API will probably be needed
for implementing SecurityContext and SecureSocket with platform
specific APIs on iOS/Mac/Windows rather than BoringSSL.
==========

to

==========
Adds SecurityContext.usePrivateKeyBytes

This is the first step toward removing blocking IO calls from
the implementation of the SecurityContext API.

Using a buffer rather than a file name API will probably be needed
for implementing SecurityContext and SecureSocket with platform
specific built-in APIs on iOS/Mac/Windows rather than BoringSSL.
==========

[zra, 2016-01-25 22:55:23]

zra@google.com changed reviewers:
+ iposva@google.com, whesse@google.com

[kevmoo, 2016-01-25 23:49:28]

kevmoo@google.com changed reviewers:
+ kevmoo@google.com

[kevmoo, 2016-01-25 23:49:29]

Is the breaking change strictly necessary? It's a bit annoying to break folks
who had to migrate just one release ago.

Is there a timeline for the deprecation?

https://codereview.chromium.org/1616073004/diff/20001/sdk/lib/io/security_con...
File sdk/lib/io/security_context.dart (right):

https://codereview.chromium.org/1616073004/diff/20001/sdk/lib/io/security_con...
sdk/lib/io/security_context.dart:36: /**
Update the changelog w/ both the deprecation and the new API

https://codereview.chromium.org/1616073004/diff/20001/sdk/lib/io/security_con...
sdk/lib/io/security_context.dart:44: * This function is deprecated. Use
[usePrivateKeyBytes] instead.
Please put the deprecation warning at the top of the doc comment.

https://codereview.chromium.org/1616073004/diff/20001/sdk/lib/io/security_con...
sdk/lib/io/security_context.dart:50: * Sets the private key for a server
certificate or client certificate.
Add a newline here -
https://www.dartlang.org/effective-dart/documentation/#do-make-the-first-sent...

[zra, 2016-01-25 23:59:39]

https://codereview.chromium.org/1616073004/diff/20001/sdk/lib/io/security_con...
File sdk/lib/io/security_context.dart (right):

https://codereview.chromium.org/1616073004/diff/20001/sdk/lib/io/security_con...
sdk/lib/io/security_context.dart:36: /**
On 2016/01/25 23:49:29, kevmoo wrote:
> Update the changelog w/ both the deprecation and the new API

I'll update the changelog when I've finished providing alternatives for all of
the calls.

https://codereview.chromium.org/1616073004/diff/20001/sdk/lib/io/security_con...
sdk/lib/io/security_context.dart:44: * This function is deprecated. Use
[usePrivateKeyBytes] instead.
On 2016/01/25 23:49:29, kevmoo wrote:
> Please put the deprecation warning at the top of the doc comment.

Done.

https://codereview.chromium.org/1616073004/diff/20001/sdk/lib/io/security_con...
sdk/lib/io/security_context.dart:50: * Sets the private key for a server
certificate or client certificate.
On 2016/01/25 23:49:29, kevmoo wrote:
> Add a newline here -
>
https://www.dartlang.org/effective-dart/documentation/#do-make-the-first-sent...

Done.

[zra, 2016-01-26 00:01:01]

On 2016/01/25 23:49:29, kevmoo wrote:
> Is the breaking change strictly necessary? It's a bit annoying to break folks
> who had to migrate just one release ago.

It's wrong for this API to force an implementation to do blocking IO. We should
fix it.

[Bill Hesse, 2016-01-26 12:54:48]

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

[Bill Hesse, 2016-01-26 12:54:49]

I am not OK with deprecating the existing usePrivateKey method, and also think
the tests should continue using this method.

The issue for adding use of in-memory keys and certificates is
https://github.com/dart-lang/sdk/issues/8227

This is great, implementing it for private keys, but we should come to consensus
on whether it takes bytes or a string, and whether the existing file api is
maintained.  The file api is what is implemented natively by the BoringSSL
library, and is the best fit for most users.

I didn't know there was a plan to try and use naive platform secure networking
on windows and mac.  What are the reasons?  Will we be able to make a common API
that works well with all libraries?

I will review the code implementing the new API in a later review.

[zra, 2016-01-26 17:40:38]

On 2016/01/26 12:54:49, Bill Hesse wrote:
> I am not OK with deprecating the existing usePrivateKey method, and also think
> the tests should continue using this method.

Sorry for being a bit too eager to mark it as deprecated. I've removed the
annotation, but implemented it instead in terms of usePrivateKeyAsBytes,
returning a Future. Since usePrivateKey calls usePrivateKeyAsBytes, I've left
the tests calling usePrivateKeyAsBytes.

> The issue for adding use of in-memory keys and certificates is
> https://github.com/dart-lang/sdk/issues/8227

Thanks!

> This is great, implementing it for private keys, but we should come to
consensus
> on whether it takes bytes or a string, and whether the existing file api is
> maintained.  The file api is what is implemented natively by the BoringSSL
> library, and is the best fit for most users.

Taking bytes will give us more flexibility going forward. Windows and Mac prefer
PKCS12-encoded certificate files, which use a binary encoding. The file-based
calls in BoringSSL are generally wrappers around other public API calls that use
in-memory buffers. We can provide the file-based wrappers in Dart instead.

> I will review the code implementing the new API in a later review.

Thanks!

[zra, 2016-01-26 17:41:06]

Description was changed from

==========
Adds SecurityContext.usePrivateKeyBytes

This is the first step toward removing blocking IO calls from
the implementation of the SecurityContext API.

Using a buffer rather than a file name API will probably be needed
for implementing SecurityContext and SecureSocket with platform
specific built-in APIs on iOS/Mac/Windows rather than BoringSSL.
==========

to

==========
Adds SecurityContext.usePrivateKeyBytes

This is the first step toward removing blocking IO calls from
the implementation of the SecurityContext API.

Using a buffer rather than a file name API will probably be needed
for implementing SecurityContext and SecureSocket with platform
specific built-in APIs on iOS/Mac/Windows rather than BoringSSL.

related #8227
==========

[zra, 2016-01-27 18:16:14]

Ping

[zra, 2016-01-28 19:01:44]

Ping

[Bill Hesse, 2016-01-28 19:06:56]

On 2016/01/28 19:01:44, zra wrote:
> Ping

I'm leaving now, but tomorrow I'll do it.

[zra, 2016-01-28 19:10:06]

On 2016/01/28 19:06:56, Bill Hesse wrote:
> On 2016/01/28 19:01:44, zra wrote:
> > Ping
> 
> I'm leaving now, but tomorrow I'll do it.

Thanks!

[Bill Hesse, 2016-01-29 14:41:50]

LGTM.

https://codereview.chromium.org/1616073004/diff/60001/runtime/bin/secure_sock...
File runtime/bin/secure_socket.cc (right):

https://codereview.chromium.org/1616073004/diff/60001/runtime/bin/secure_sock...
runtime/bin/secure_socket.cc:421: ThrowIfError(Dart_ListGetAsBytes(key_object,
0, key_bytes, key_bytes_len));
Leak of key_bytes in this line.  Perhaps this is acceptable, but I can imagine a
program continuing after this failure.

https://codereview.chromium.org/1616073004/diff/60001/sdk/lib/io/http.dart
File sdk/lib/io/http.dart (right):

https://codereview.chromium.org/1616073004/diff/60001/sdk/lib/io/http.dart#ne...
sdk/lib/io/http.dart:112: *       context.usePrivateKeyAsBytes(keyBytes,
password: 'dartdart');
Either load the file as bytes, or use usePrivateKey.  I would do the latter.

[zra, 2016-01-29 16:47:49]

https://codereview.chromium.org/1616073004/diff/60001/runtime/bin/secure_sock...
File runtime/bin/secure_socket.cc (right):

https://codereview.chromium.org/1616073004/diff/60001/runtime/bin/secure_sock...
runtime/bin/secure_socket.cc:421: ThrowIfError(Dart_ListGetAsBytes(key_object,
0, key_bytes, key_bytes_len));
On 2016/01/29 14:41:50, Bill Hesse wrote:
> Leak of key_bytes in this line.  Perhaps this is acceptable, but I can imagine
a
> program continuing after this failure.

deleted key_bytes on error.

https://codereview.chromium.org/1616073004/diff/60001/sdk/lib/io/http.dart
File sdk/lib/io/http.dart (right):

https://codereview.chromium.org/1616073004/diff/60001/sdk/lib/io/http.dart#ne...
sdk/lib/io/http.dart:112: *       context.usePrivateKeyAsBytes(keyBytes,
password: 'dartdart');
On 2016/01/29 14:41:50, Bill Hesse wrote:
> Either load the file as bytes, or use usePrivateKey.  I would do the latter.

`useCertificateChain` will also be changing to return a Future, so I made main
async, and changed the call here to `await context.usePrivateKey(...)`

[zra, 2016-01-29 16:52:21]

Description was changed from

==========
Adds SecurityContext.usePrivateKeyBytes

This is the first step toward removing blocking IO calls from
the implementation of the SecurityContext API.

Using a buffer rather than a file name API will probably be needed
for implementing SecurityContext and SecureSocket with platform
specific built-in APIs on iOS/Mac/Windows rather than BoringSSL.

related #8227
==========

to

==========
Adds SecurityContext.usePrivateKeyBytes

This is the first step toward removing blocking IO calls from
the implementation of the SecurityContext API.

Using a buffer rather than a file name API will probably be needed
for implementing SecurityContext and SecureSocket with platform
specific built-in APIs on iOS/Mac/Windows rather than BoringSSL.

related #8227

R=whesse@google.com

Committed:
https://github.com/dart-lang/sdk/commit/98771a083f13c1aebc5ebf599b52778b42b21f80
==========

[zra, 2016-01-29 16:52:23]

Committed patchset #5 (id:80001) manually as
98771a083f13c1aebc5ebf599b52778b42b21f80 (presubmit successful).