Add check for password length in SecurityContext. Check return values from BoringSSL functions, thr…

Add check for password length in SecurityContext. Check return values from BoringSSL functions, throw exceptions.

BUG=https://github.com/dart-lang/sdk/issues/24182
R=sgjesse@google.com

Committed: https://github.com/dart-lang/sdk/commit/0c2bea237f75330c0bed4dafcabe846b535fe3ad

[Bill Hesse, 2015-10-01 16:48:35]

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

[Søren Gjesse, 2015-10-05 09:13:40]

lgtm, with comments

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

https://codereview.chromium.org/1384533002/diff/40001/runtime/bin/secure_sock...
runtime/bin/secure_socket.cc:338: return strlen(password);
Always add a terminating \0 to password.

https://codereview.chromium.org/1384533002/diff/40001/runtime/bin/secure_sock...
runtime/bin/secure_socket.cc:373: assert(PEM_BUFSIZE == 1024);
Why this assert - is it for the error message? If so use snprintf to generate
the message.

Also use the ASSERT macro.

https://codereview.chromium.org/1384533002/diff/40001/runtime/bin/secure_sock...
runtime/bin/secure_socket.cc:961: }
Before there was a status check here as well.

https://codereview.chromium.org/1384533002/diff/40001/runtime/bin/secure_sock...
runtime/bin/secure_socket.cc:981: printf("Peer certificate SN: ");
Debug print.

https://codereview.chromium.org/1384533002/diff/40001/runtime/bin/secure_sock...
runtime/bin/secure_socket.cc:983: printf("\n");
ditto.

[Bill Hesse, 2015-10-05 11:28:31]

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

https://codereview.chromium.org/1384533002/diff/40001/runtime/bin/secure_sock...
runtime/bin/secure_socket.cc:338: return strlen(password);
On 2015/10/05 09:13:40, Søren Gjesse wrote:
> Always add a terminating \0 to password.

WE check in the caller that strlen(password) is < PEM_BUFSIZE, so there will
always be a \0 written.  We can do it again, but it is redundant.

https://codereview.chromium.org/1384533002/diff/40001/runtime/bin/secure_sock...
runtime/bin/secure_socket.cc:373: assert(PEM_BUFSIZE == 1024);
On 2015/10/05 09:13:40, Søren Gjesse wrote:
> Why this assert - is it for the error message? If so use snprintf to generate
> the message.
> 
> Also use the ASSERT macro.

Yes, it is for the error message. Removed it.

https://codereview.chromium.org/1384533002/diff/40001/runtime/bin/secure_sock...
runtime/bin/secure_socket.cc:961: }
On 2015/10/05 09:13:40, Søren Gjesse wrote:
> Before there was a status check here as well.

There still is.  The CheckStatus call below absorbed it.

https://codereview.chromium.org/1384533002/diff/40001/runtime/bin/secure_sock...
runtime/bin/secure_socket.cc:981: printf("Peer certificate SN: ");
On 2015/10/05 09:13:40, Søren Gjesse wrote:
> Debug print.

This is a logging print, guarded by SSL_LOG_STATUS.  It cannot use Log::Print
because the way to print a subject name in BoringSSL prints it to a file
descriptor, not to a string buffer or other place we can grab it from.

[Søren Gjesse, 2015-10-05 11:38:46]

lgtm

[Bill Hesse, 2015-10-05 11:51:07]

Committed patchset #4 (id:60001) manually as
0c2bea237f75330c0bed4dafcabe846b535fe3ad (presubmit successful).