widevine-partner/android
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Improve SSL Error Handling in HttpSocket::Read()

Browse Source 
Merge from Widevine repo of http://go/wvgerrit/53640

While HttpSocket supports both secure and insecure requests, the
error-handling code in HttpSocket::Read() was written assuming that
the insecure code path was previously taken. This resulted in spurious
and misleading error messages being printed when an SSL error
occurred, and it also meant that retryable SSL responses were not
being retried. Also, the code for detecting a closed connection was
technically incorrect, although a quirk of BoringSSL meant that it
happened to work well enough to go unnoticed.

This patch adds separate SSL error handling from the non-secure error
handling. It correctly checks for a closed connection. It will retry
retryable errors after a delay. And it prints the correct BoringSSL
error when an unrecoverable error occurs. There should be no change in
behavior for insecure connections.

Bug: 77338045
Test: CE CDM Unit Tests
Test: tested as part of http://go/ag/4674759

Change-Id: I8c45ca5771f22c11716d2e3649de91ab1acc1954
This commit is contained in:
Fred Gylys-Colwell
2018-07-01 12:23:45 -07:00
parent 22d9160219
commit 0936f1b875
1 changed files with 45 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

56
libwvdrmengine/cdm/core/test/http_socket.cpp
View File

@@ -42,11 +42,7 @@ bool Tokenize(const std::string& source, const std::string& delim,
SSL_CTX* InitSslContext() {
OpenSSL_add_all_algorithms();
SSL_load_error_strings();
#if (OPENSSL_VERSION_NUMBER < 0x10100000L)
const SSL_METHOD* method = TLSv1_2_client_method();
#else
const SSL_METHOD* method = TLS_client_method();
#endif
SSL_CTX* ctx = SSL_CTX_new(method);
if (!ctx) LOGE("failed to create SSL context");
int ret = SSL_CTX_set_cipher_list(
@@ -55,6 +51,18 @@ SSL_CTX* InitSslContext() {
return ctx;
}
static int LogBoringSslError(
const char* message, size_t length, void* /* user_data */) {
LOGE(" BoringSSL Error: %s", message);
return length;
}
bool IsRetryableSslError(int ssl_error) {
return ssl_error != SSL_ERROR_ZERO_RETURN &&
ssl_error != SSL_ERROR_SYSCALL &&
ssl_error != SSL_ERROR_SSL;
}
#if 0
// unused, may be useful for debugging SSL-related issues.
void ShowServerCertificate(const SSL* ssl) {
@@ -312,22 +320,48 @@ int HttpSocket::Read(char* data, int len, int timeout_in_ms) {
return -1;
}
errno = 0; // Reset errno, as we will depend on its value shortly.
int read;
if (secure_connect_)
if (secure_connect_) {
read = SSL_read(ssl_, data, to_read);
else
} else {
read = recv(socket_fd_, data, to_read, 0);
}
if (read > 0) {
to_read -= read;
data += read;
total_read += read;
} else if (read == 0) {
// The connection has been closed. No more data.
break;
} else if (secure_connect_) {
// Secure read error
int ssl_error = SSL_get_error(ssl_, read);
if (ssl_error == SSL_ERROR_ZERO_RETURN ||
(ssl_error == SSL_ERROR_SYSCALL && errno == 0)) {
// The connection has been closed. No more data.
break;
} else if (IsRetryableSslError(ssl_error)) {
sleep(1);
// After sleeping, fall through to iterate the loop again and retry.
} else {
// Unrecoverable error. Log and abort.
LOGE("SSL_read returned %d, LibSSL Error = %d", read, ssl_error);
if (ssl_error == SSL_ERROR_SYSCALL) {
LOGE(" errno = %d = %s", errno, strerror(errno));
}
ERR_print_errors_cb(LogBoringSslError, NULL);
return -1;
}
} else {
LOGE("recv returned %d, errno = %d = %s", read, errno, strerror(errno));
return -1;
// Non-secure read error
if (read == 0) {
// The connection has been closed. No more data.
break;
} else {
// Log the error received
LOGE("recv returned %d, errno = %d = %s", read, errno, strerror(errno));
return -1;
}
}
}