Fix several issues with tls-boringssl.c.
An earlier CL added IPPS support via BoringSSL, but the change had many
issues. This fixes the following:
- random() with srandom(time(NULL)) is not an acceptable source of
entropy for a cryptographic library. Fortunately, BoringSSL ignores
calls to RAND_seed anyway, so delete it all.
- ERR_error_string(NULL) is not thread-safe. (We've really got to get
rid of that function, but there are a lot of callers to clear
through.) Use ERR_error_string_n with a local buffer.
- Using the version-specific APIs disables TLS 1.2. Instead, use
TLS_method (client and server distinctions on methods are ignored in
BoringSSL), and configure the minimum protocol version accordingly.
- SSL 3.0 is gone. Ignore _HTTP_TLS_ALLOW_SSL3 altogether. The old code
called SSLv3_*_method() which always fail in BoringSSL and, were they
not to fail, would have disabled all secure versions of TLS!
- The SSL_set_tlsext_host_name call was guarded by a
HAVE_SSL_SET_TLSEXT_HOST_NAME, but config.h was not updated. Remove
the guard.
- Server support was not added, so make the operation actually fail.
Remove the commented out code (which wouldn't work as calling
SSL_CTX_* functions after SSL_new doesn't do anything).
- The code to call SSL_connect vs SSL_accept had a typo and only called
SSL_accept. In case someone wants server support in the future, use
the generic SSL_do_handshake which is equivalent, provided the caller
uses SSL_set_{connect,accept}_state.
This also cleans a couple things up:
- SSL_load_error_strings is a no-op in BoringSSL. SSL_library_init is
also a no-op on Android, but there do exist configuations where it is
not, so I've left it in.
- SSL_write returns int, not ssize_t. The casts are unnecessary.
- Extracting the SSL_CTX from the SSL to free it is weird. One can just
free it earlier. The SSL owns a reference to the SSL_CTX and will
do the rest for you.
- Delete some unused functions whose comments even still say "gnutls" on
them.
IMPORTANT: This does NOT fix the following:
- This file does not verify peer certificates at all. This means any
network attacker could use a different certificate and break the
connection anyway. I do not know how printer certificates are
typically checked or how Android's trust store is set up, so someone
with more domain knowledge may need to help out here. (Are printer
certs typically checked at all? [0] suggests yes, amazingly.)
[0] https://support.microsoft.com/en-us/help/2021626/when-attempting-to-add-an-ipp-printer-over-https--you-receive-an-error
Test: mma. Additionally tested by Mopria folks. See review comments.
Change-Id: Ife007038290ff79f3413179a26c0d40c1bb2c85b
diff --git a/cups/tls-boringssl.c b/cups/tls-boringssl.c
index 6aec314..a8b7de5 100644
--- a/cups/tls-boringssl.c
+++ b/cups/tls-boringssl.c
@@ -23,7 +23,6 @@
#include "http.h"
#include "thread-private.h"
#include <openssl/err.h>
-#include <openssl/rand.h>
#include <openssl/ssl.h>
#include <sys/stat.h>
@@ -37,8 +36,6 @@
* Local functions...
*/
-static const char *http_bssl_default_path(char *buffer, size_t bufsize);
-static const char *http_bssl_make_path(char *buffer, size_t bufsize, const char *dirname, const char *filename, const char *ext);
static BIO_METHOD * _httpBIOMethods(void);
static int http_bio_write(BIO *h, const char *buf, int num);
static int http_bio_read(BIO *h, char *buf, int size);
@@ -123,88 +120,6 @@
/*
- * 'http_gnutls_default_path()' - Get the default credential store path.
- */
-
-static const char * /* O - Path or NULL on error */
-http_bssl_default_path(char *buffer,/* I - Path buffer */
- size_t bufsize)/* I - Size of path buffer */
-{
- const char *home = getenv("HOME"); /* HOME environment variable */
-
-
- if (getuid() && home)
- {
- snprintf(buffer, bufsize, "%s/.cups", home);
- if (access(buffer, 0))
- {
- DEBUG_printf(("1http_gnutls_default_path: Making directory \"%s\".", buffer));
- if (mkdir(buffer, 0700))
- {
- DEBUG_printf(("1http_gnutls_default_path: Failed to make directory: %s", strerror(errno)));
- return (NULL);
- }
- }
-
- snprintf(buffer, bufsize, "%s/.cups/ssl", home);
- if (access(buffer, 0))
- {
- DEBUG_printf(("1http_gnutls_default_path: Making directory \"%s\".", buffer));
- if (mkdir(buffer, 0700))
- {
- DEBUG_printf(("1http_gnutls_default_path: Failed to make directory: %s", strerror(errno)));
- return (NULL);
- }
- }
- }
- else
- strlcpy(buffer, CUPS_SERVERROOT "/ssl", bufsize);
-
- DEBUG_printf(("1http_gnutls_default_path: Using default path \"%s\".", buffer));
-
- return (buffer);
-}
-
-
-/*
- * 'http_gnutls_make_path()' - Format a filename for a certificate or key file.
- */
-
-static const char * /* O - Filename */
-http_bssl_make_path(
- char *buffer, /* I - Filename buffer */
- size_t bufsize, /* I - Size of buffer */
- const char *dirname, /* I - Directory */
- const char *filename, /* I - Filename (usually hostname) */
- const char *ext) /* I - Extension */
-{
- char *bufptr, /* Pointer into buffer */
- *bufend = buffer + bufsize - 1; /* End of buffer */
-
-
- snprintf(buffer, bufsize, "%s/", dirname);
- bufptr = buffer + strlen(buffer);
-
- while (*filename && bufptr < bufend)
- {
- if (_cups_isalnum(*filename) || *filename == '-' || *filename == '.')
- *bufptr++ = *filename;
- else
- *bufptr++ = '_';
-
- filename ++;
- }
-
- if (bufptr < bufend)
- *bufptr++ = '.';
-
- strlcpy(bufptr, ext, (size_t)(bufend - bufptr + 1));
-
- return (buffer);
-}
-
-
-/*
* '_httpBIOMethods()' - Get the OpenSSL BIO methods for HTTP connections.
*/
@@ -361,27 +276,7 @@
void
_httpTLSInitialize(void)
{
- int i; /* Looping var */
- unsigned char data[1024]; /* Seed data */
-
- /*
- * Initialize OpenSSL...
- */
-
- SSL_load_error_strings();
SSL_library_init();
-
- /*
- * Using the current time is a dubious random seed, but on some systems
- * it is the best we can do (on others, this seed isn't even used...)
- */
-
- CUPS_SRAND(time(NULL));
-
- for (i = 0; i < sizeof(data); i ++)
- data[i] = CUPS_RAND();
-
- RAND_seed(data, sizeof(data));
}
@@ -453,12 +348,9 @@
return (-1);
}
+ context = SSL_CTX_new(TLS_method());
if (tls_options & _HTTP_TLS_DENY_TLS10)
- context = SSL_CTX_new(http->mode == _HTTP_MODE_CLIENT ? TLSv1_1_client_method() : TLSv1_1_server_method());
- else if (tls_options & _HTTP_TLS_ALLOW_SSL3)
- context = SSL_CTX_new(http->mode == _HTTP_MODE_CLIENT ? SSLv3_client_method() : SSLv3_server_method());
- else
- context = SSL_CTX_new(http->mode == _HTTP_MODE_CLIENT ? TLSv1_client_method() : TLSv1_server_method());
+ SSL_CTX_set_min_proto_version(context, TLS1_1_VERSION);
bio = BIO_new(_httpBIOMethods());
BIO_ctrl(bio, BIO_C_SET_FILE_PTR, 0, (char *)http);
@@ -466,8 +358,13 @@
http->tls = SSL_new(context);
SSL_set_bio(http->tls, bio, bio);
+ /* http->tls retains an internal reference to the SSL_CTX. */
+ SSL_CTX_free(context);
+
if (http->mode == _HTTP_MODE_CLIENT)
{
+ SSL_set_connect_state(http->tls);
+
/*
* Client: get the hostname to use for TLS...
*/
@@ -487,30 +384,26 @@
*hostptr == '.')
*hostptr = '\0';
}
-# ifdef HAVE_SSL_SET_TLSEXT_HOST_NAME
SSL_set_tlsext_host_name(http->tls, hostname);
-# endif /* HAVE_SSL_SET_TLSEXT_HOST_NAME */
-
}
else
{
/* @@@ TODO @@@ */
-// SSL_CTX_use_PrivateKey_file(context, ServerKey, SSL_FILETYPE_PEM);
-// SSL_CTX_use_certificate_chain_file(context, ServerCertificate);
+ _cupsSetError(IPP_STATUS_ERROR_INTERNAL, "Server not supported", 0);
}
- if (http->mode == _HTTP_MODE_CLIENT ? SSL_connect(http->tls) != 1 :SSL_connect(http->tls) != 1)
+ if (SSL_do_handshake(http->tls) != 1)
{
unsigned long error; /* Error code */
+ char buf[256];
while ((error = ERR_get_error()) != 0)
{
- message = ERR_error_string(error, NULL);
- DEBUG_printf(("8http_setup_ssl: %s", message));
+ ERR_error_string_n(error, buf, sizeof(buf));
+ DEBUG_printf(("8http_setup_ssl: %s", buf));
}
- SSL_CTX_free(context);
SSL_free(http->tls);
http->tls = NULL;
@@ -536,11 +429,8 @@
void
_httpTLSStop(http_t *http) /* I - Connection to server */
{
- SSL_CTX *context; /* Context for encryption */
unsigned long error; /* Error code */
- context = SSL_get_SSL_CTX(http->tls);
-
switch (SSL_shutdown(http->tls))
{
case 1 :
@@ -551,12 +441,14 @@
"Fatal error during SSL shutdown!", 0);
default :
while ((error = ERR_get_error()) != 0)
- _cupsSetError(IPP_STATUS_ERROR_INTERNAL,
- ERR_error_string(error, NULL), 0);
+ {
+ char buf[256];
+ ERR_error_string_n(error, buf, sizeof(buf));
+ _cupsSetError(IPP_STATUS_ERROR_INTERNAL, buf, 0);
+ }
break;
}
- SSL_CTX_free(context);
SSL_free(http->tls);
http->tls = NULL;
}
@@ -570,14 +462,14 @@
const char *buf, /* I - Buffer holding data */
int len) /* I - Length of buffer */
{
- ssize_t result; /* Return value */
+ int result; /* Return value */
DEBUG_printf(("2http_write_ssl(http=%p, buf=%p, len=%d)", http, buf, len));
result = SSL_write((SSL *)(http->tls), buf, len);
- DEBUG_printf(("3http_write_ssl: Returning %d.", (int)result));
+ DEBUG_printf(("3http_write_ssl: Returning %d.", result));
- return ((int)result);
+ return result;
}
