Commit 0119e57df028d2eb9ed6f80aa859bb4ed976bb2b
2020-02-11T10:37:32
streams: openssl: switch approach to silence Valgrind errors As OpenSSL loves using uninitialized bytes as another source of entropy, we need to mark them as defined so that Valgrind won't complain about use of these bytes. Traditionally, we've been using the macro `VALGRIND_MAKE_MEM_DEFINED` provided by Valgrind, but starting with OpenSSL 1.1 the code doesn't compile anymore due to `struct SSL` having become opaque. As such, we also can't set it as defined anymore, as we have no way of knowing its size. Let's change gears instead by just swapping out the allocator functions of OpenSSL with our own ones. The twist is that instead of calling `malloc`, we just call `calloc` to have the bytes initialized automatically. Next to soothing Valgrind, this approach has the benefit of being completely agnostic of the memory sanitizer and is neatly contained at a single place. Note that we shouldn't do this for non-Valgrind builds. As we cannot set up memory functions for a given SSL context, only, we need to swap them at a global context. Furthermore, as it's possible to call `OPENSSL_set_mem_functions` once only, we'd prevent users of libgit2 to set up their own allocators.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130
diff --git a/src/streams/openssl.c b/src/streams/openssl.c
index 98a3635..458810c 100644
--- a/src/streams/openssl.c
+++ b/src/streams/openssl.c
@@ -30,10 +30,6 @@
#include <openssl/x509v3.h>
#include <openssl/bio.h>
-#ifdef VALGRIND
-# include <valgrind/memcheck.h>
-#endif
-
SSL_CTX *git__ssl_ctx;
#define GIT_SSL_DEFAULT_CIPHERS "ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:DHE-DSS-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:DHE-DSS-AES256-GCM-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES256-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES256-SHA256:DHE-RSA-AES128-SHA:DHE-RSA-AES256-SHA:DHE-DSS-AES128-SHA256:DHE-DSS-AES256-SHA256:DHE-DSS-AES128-SHA:DHE-DSS-AES256-SHA:AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256:AES128-SHA:AES256-SHA"
@@ -200,16 +196,69 @@ static void shutdown_ssl(void)
}
}
+#ifdef VALGRIND
+#ifdef OPENSSL_LEGACY_API
+static void *git_openssl_malloc(size_t bytes)
+{
+ return git__calloc(1, bytes);
+}
+
+static void *git_openssl_realloc(void *mem, size_t size)
+{
+ return git__realloc(mem, size);
+}
+
+static void git_openssl_free(void *mem)
+{
+ return git__free(mem);
+}
+#else
+static void *git_openssl_malloc(size_t bytes, const char *file, int line)
+{
+ GIT_UNUSED(file);
+ GIT_UNUSED(line);
+ return git__calloc(1, bytes);
+}
+
+static void *git_openssl_realloc(void *mem, size_t size, const char *file, int line)
+{
+ GIT_UNUSED(file);
+ GIT_UNUSED(line);
+ return git__realloc(mem, size);
+}
+
+static void git_openssl_free(void *mem, const char *file, int line)
+{
+ GIT_UNUSED(file);
+ GIT_UNUSED(line);
+ return git__free(mem);
+}
+#endif
+#endif
+
int git_openssl_stream_global_init(void)
{
long ssl_opts = SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3;
const char *ciphers = git_libgit2__ssl_ciphers();
+#ifdef VALGRIND
+ static bool allocators_initialized = false;
+#endif
/* Older OpenSSL and MacOS OpenSSL doesn't have this */
#ifdef SSL_OP_NO_COMPRESSION
ssl_opts |= SSL_OP_NO_COMPRESSION;
#endif
+#ifdef VALGRIND
+ /* Swap in our own allocator functions that initialize allocated memory */
+ if (!allocators_initialized &&
+ CRYPTO_set_mem_functions(git_openssl_malloc,
+ git_openssl_realloc,
+ git_openssl_free) != 1)
+ goto error;
+ allocators_initialized = true;
+#endif
+
OPENSSL_init_ssl(0, NULL);
/*
@@ -314,11 +363,6 @@ static int bio_read(BIO *b, char *buf, int len)
static int bio_write(BIO *b, const char *buf, int len)
{
git_stream *io = (git_stream *) BIO_get_data(b);
-
-#ifdef VALGRIND
- VALGRIND_MAKE_MEM_DEFINED(buf, len);
-#endif
-
return (int) git_stream_write(io, buf, len, 0);
}
@@ -595,10 +639,6 @@ static int openssl_connect(git_stream *stream)
BIO_set_data(bio, st->io);
SSL_set_bio(st->ssl, bio, bio);
-#ifdef VALGRIND
- VALGRIND_MAKE_MEM_DEFINED(st->ssl, sizeof(SSL));
-#endif
-
/* specify the host in case SNI is needed */
#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
SSL_set_tlsext_host_name(st->ssl, st->host);
@@ -609,10 +649,6 @@ static int openssl_connect(git_stream *stream)
st->connected = true;
-#ifdef VALGRIND
- VALGRIND_MAKE_MEM_DEFINED(st->ssl, sizeof(SSL));
-#endif
-
return verify_server_cert(st->ssl, st->host);
}
@@ -679,10 +715,6 @@ static ssize_t openssl_read(git_stream *stream, void *data, size_t len)
if ((ret = SSL_read(st->ssl, data, len)) <= 0)
return ssl_set_error(st->ssl, ret);
-#ifdef VALGRIND
- VALGRIND_MAKE_MEM_DEFINED(data, ret);
-#endif
-
return ret;
}