From 2505cbfc48a9500606d3bc087f0c8c4d1d46b720 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 3 Apr 2018 11:40:39 +0100 Subject: [PATCH 1/4] streams: openssl: move OpenSSL compat layer into implementation OpenSSL version 1.1 has broken its API in quite a few ways. To avoid having to use ifdef's everywhere, we have implemented the BIO functions added in version 1.1 ourselves in case we are using the legacy API. We were implementing them in the header file, though, which doesn't make a lot of sense, since these functions are only ever being used the the openssl stream implementation. Move these functions to the implementation file and mark them static. --- src/streams/openssl.c | 98 ++++++++++++++++++++++++++++++++++++++ src/streams/openssl.h | 107 ------------------------------------------ 2 files changed, 98 insertions(+), 107 deletions(-) diff --git a/src/streams/openssl.c b/src/streams/openssl.c index adcb7f14ef6..4b71050b165 100644 --- a/src/streams/openssl.c +++ b/src/streams/openssl.c @@ -38,6 +38,104 @@ 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" +/* + * OpenSSL 1.1 made BIO opaque so we have to use functions to interact with it + * which do not exist in previous versions. We define these inline functions so + * we can program against the interface instead of littering the implementation + * with ifdefs. + */ +#if OPENSSL_VERSION_NUMBER < 0x10100000L || \ + (defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < 0x20700000L) + +static BIO_METHOD* BIO_meth_new(int type, const char *name) +{ + BIO_METHOD *meth = git__calloc(1, sizeof(BIO_METHOD)); + if (!meth) { + return NULL; + } + + meth->type = type; + meth->name = name; + + return meth; +} + +static void BIO_meth_free(BIO_METHOD *biom) +{ + git__free(biom); +} + +static int BIO_meth_set_write(BIO_METHOD *biom, int (*write) (BIO *, const char *, int)) +{ + biom->bwrite = write; + return 1; +} + +static int BIO_meth_set_read(BIO_METHOD *biom, int (*read) (BIO *, char *, int)) +{ + biom->bread = read; + return 1; +} + +static int BIO_meth_set_puts(BIO_METHOD *biom, int (*puts) (BIO *, const char *)) +{ + biom->bputs = puts; + return 1; +} + +static int BIO_meth_set_gets(BIO_METHOD *biom, int (*gets) (BIO *, char *, int)) + +{ + biom->bgets = gets; + return 1; +} + +static int BIO_meth_set_ctrl(BIO_METHOD *biom, long (*ctrl) (BIO *, int, long, void *)) +{ + biom->ctrl = ctrl; + return 1; +} + +static int BIO_meth_set_create(BIO_METHOD *biom, int (*create) (BIO *)) +{ + biom->create = create; + return 1; +} + +static int BIO_meth_set_destroy(BIO_METHOD *biom, int (*destroy) (BIO *)) +{ + biom->destroy = destroy; + return 1; +} + +static int BIO_get_new_index(void) +{ + /* This exists as of 1.1 so before we'd just have 0 */ + return 0; +} + +static void BIO_set_init(BIO *b, int init) +{ + b->init = init; +} + +static void BIO_set_data(BIO *a, void *ptr) +{ + a->ptr = ptr; +} + +static void *BIO_get_data(BIO *a) +{ + return a->ptr; +} + +static const unsigned char *ASN1_STRING_get0_data(const ASN1_STRING *x) +{ + return ASN1_STRING_data((ASN1_STRING *)x); +} + +#endif + #if defined(GIT_THREADS) && OPENSSL_VERSION_NUMBER < 0x10100000L static git_mutex *openssl_locks; diff --git a/src/streams/openssl.h b/src/streams/openssl.h index 44329ec904d..496d7efbf65 100644 --- a/src/streams/openssl.h +++ b/src/streams/openssl.h @@ -17,111 +17,4 @@ extern int git_openssl_stream_new(git_stream **out, const char *host, const char extern int git_openssl__set_cert_location(const char *file, const char *path); -/* - * OpenSSL 1.1 made BIO opaque so we have to use functions to interact with it - * which do not exist in previous versions. We define these inline functions so - * we can program against the interface instead of littering the implementation - * with ifdefs. - */ -#ifdef GIT_OPENSSL -# include -# include -# include -# include - - - -# if OPENSSL_VERSION_NUMBER < 0x10100000L || \ - (defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < 0x20700000L) - -GIT_INLINE(BIO_METHOD*) BIO_meth_new(int type, const char *name) -{ - BIO_METHOD *meth = git__calloc(1, sizeof(BIO_METHOD)); - if (!meth) { - return NULL; - } - - meth->type = type; - meth->name = name; - - return meth; -} - -GIT_INLINE(void) BIO_meth_free(BIO_METHOD *biom) -{ - git__free(biom); -} - -GIT_INLINE(int) BIO_meth_set_write(BIO_METHOD *biom, int (*write) (BIO *, const char *, int)) -{ - biom->bwrite = write; - return 1; -} - -GIT_INLINE(int) BIO_meth_set_read(BIO_METHOD *biom, int (*read) (BIO *, char *, int)) -{ - biom->bread = read; - return 1; -} - -GIT_INLINE(int) BIO_meth_set_puts(BIO_METHOD *biom, int (*puts) (BIO *, const char *)) -{ - biom->bputs = puts; - return 1; -} - -GIT_INLINE(int) BIO_meth_set_gets(BIO_METHOD *biom, int (*gets) (BIO *, char *, int)) - -{ - biom->bgets = gets; - return 1; -} - -GIT_INLINE(int) BIO_meth_set_ctrl(BIO_METHOD *biom, long (*ctrl) (BIO *, int, long, void *)) -{ - biom->ctrl = ctrl; - return 1; -} - -GIT_INLINE(int) BIO_meth_set_create(BIO_METHOD *biom, int (*create) (BIO *)) -{ - biom->create = create; - return 1; -} - -GIT_INLINE(int) BIO_meth_set_destroy(BIO_METHOD *biom, int (*destroy) (BIO *)) -{ - biom->destroy = destroy; - return 1; -} - -GIT_INLINE(int) BIO_get_new_index(void) -{ - /* This exists as of 1.1 so before we'd just have 0 */ - return 0; -} - -GIT_INLINE(void) BIO_set_init(BIO *b, int init) -{ - b->init = init; -} - -GIT_INLINE(void) BIO_set_data(BIO *a, void *ptr) -{ - a->ptr = ptr; -} - -GIT_INLINE(void*) BIO_get_data(BIO *a) -{ - return a->ptr; -} - -GIT_INLINE(const unsigned char *) ASN1_STRING_get0_data(const ASN1_STRING *x) -{ - return ASN1_STRING_data((ASN1_STRING *)x); -} - -# endif // OpenSSL < 1.1 -#endif // GIT_OPENSSL - #endif From ede63b99ce5f080e32865918eab0d59083505ee7 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 3 Apr 2018 11:45:00 +0100 Subject: [PATCH 2/4] streams: openssl: unify version checks into single define By now, we have several locations where we are checking the version of OpenSSL to determine whether we can use the new "modern" API or need to use the pre-1.1 legacy API. As we have multiple implementations of OpenSSL with the rather recent libressl implementation, these checks need to honor versions of both implementations, which is rather tedious. Instead, we can just check once for the correct versions and define `OPENSSL_LEGACY_API` in case we cannot use the modern API. --- src/streams/openssl.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/streams/openssl.c b/src/streams/openssl.c index 4b71050b165..31c0824c62c 100644 --- a/src/streams/openssl.c +++ b/src/streams/openssl.c @@ -38,15 +38,18 @@ 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" +#if (defined(OPENSSL_VERSION_NUMBER) && OPENSSL_VERSION_NUMBER < 0x10100000L) || \ + (defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < 0x20700000L) +# define OPENSSL_LEGACY_API +#endif + /* * OpenSSL 1.1 made BIO opaque so we have to use functions to interact with it * which do not exist in previous versions. We define these inline functions so * we can program against the interface instead of littering the implementation * with ifdefs. */ -#if OPENSSL_VERSION_NUMBER < 0x10100000L || \ - (defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < 0x20700000L) - +#if defined(OPENSSL_LEGACY_API) static BIO_METHOD* BIO_meth_new(int type, const char *name) { BIO_METHOD *meth = git__calloc(1, sizeof(BIO_METHOD)); @@ -134,10 +137,7 @@ static const unsigned char *ASN1_STRING_get0_data(const ASN1_STRING *x) return ASN1_STRING_data((ASN1_STRING *)x); } -#endif - -#if defined(GIT_THREADS) && OPENSSL_VERSION_NUMBER < 0x10100000L - +# if defined(GIT_THREADS) static git_mutex *openssl_locks; static void openssl_locking_function( @@ -168,8 +168,8 @@ static void shutdown_ssl_locking(void) git_mutex_free(&openssl_locks[i]); git__free(openssl_locks); } - -#endif /* GIT_THREADS && OPENSSL_VERSION_NUMBER < 0x10100000L */ +# endif /* GIT_THREADS */ +#endif /* OPENSSL_LEGACY_API */ static BIO_METHOD *git_stream_bio_method; static int init_bio_method(void); @@ -202,8 +202,7 @@ int git_openssl_stream_global_init(void) ssl_opts |= SSL_OP_NO_COMPRESSION; #endif -#if OPENSSL_VERSION_NUMBER < 0x10100000L || \ - (defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < 0x20700000L) +#if defined(OPENSSL_LEGACY_API) SSL_load_error_strings(); OpenSSL_add_ssl_algorithms(); #else @@ -258,7 +257,7 @@ static void threadid_cb(CRYPTO_THREADID *threadid) int git_openssl_set_locking(void) { -#if defined(GIT_THREADS) && OPENSSL_VERSION_NUMBER < 0x10100000L +#if defined(GIT_THREADS) && defined(OPENSSL_LEGACY_API) int num_locks, i; CRYPTO_THREADID_set_callback(threadid_cb); @@ -277,7 +276,7 @@ int git_openssl_set_locking(void) CRYPTO_set_locking_callback(openssl_locking_function); git__on_shutdown(shutdown_ssl_locking); return 0; -#elif OPENSSL_VERSION_NUMBER >= 0x10100000L +#elif !defined(OPENSSL_LEGACY_API) return 0; #else giterr_set(GITERR_THREAD, "libgit2 was not built with threads"); From dd9de88aec3437d9c673398dee42f2f82cf468ad Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 3 Apr 2018 11:57:45 +0100 Subject: [PATCH 3/4] streams: openssl: provide `OPENSSL_init_ssl` for legacy API In order to further avoid using ifdef's in our code flow, provide the function `OPENSSL_init_ssl` in case we are using the legacy OpenSSL API. --- src/streams/openssl.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/streams/openssl.c b/src/streams/openssl.c index 31c0824c62c..8223d58bdc2 100644 --- a/src/streams/openssl.c +++ b/src/streams/openssl.c @@ -47,9 +47,18 @@ SSL_CTX *git__ssl_ctx; * OpenSSL 1.1 made BIO opaque so we have to use functions to interact with it * which do not exist in previous versions. We define these inline functions so * we can program against the interface instead of littering the implementation - * with ifdefs. + * with ifdefs. We do the same for OPENSSL_init_ssl. */ #if defined(OPENSSL_LEGACY_API) +static int OPENSSL_init_ssl(int opts, void *settings) +{ + GIT_UNUSED(opts); + GIT_UNUSED(settings); + SSL_load_error_strings(); + OpenSSL_add_ssl_algorithms(); + return 0; +} + static BIO_METHOD* BIO_meth_new(int type, const char *name) { BIO_METHOD *meth = git__calloc(1, sizeof(BIO_METHOD)); @@ -202,12 +211,7 @@ int git_openssl_stream_global_init(void) ssl_opts |= SSL_OP_NO_COMPRESSION; #endif -#if defined(OPENSSL_LEGACY_API) - SSL_load_error_strings(); - OpenSSL_add_ssl_algorithms(); -#else OPENSSL_init_ssl(0, NULL); -#endif /* * Load SSLv{2,3} and TLSv1 so that we can talk with servers From 173a0375a8eec02c1b40e73187347ca958e42b84 Mon Sep 17 00:00:00 2001 From: Etienne Samson Date: Thu, 8 Feb 2018 23:50:14 +0100 Subject: [PATCH 4/4] openssl: remove leftover #ifdef This is the "OpenSSL available" global init function after all --- src/streams/openssl.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/streams/openssl.c b/src/streams/openssl.c index 8223d58bdc2..d1bcbf2e105 100644 --- a/src/streams/openssl.c +++ b/src/streams/openssl.c @@ -202,7 +202,6 @@ static void shutdown_ssl(void) int git_openssl_stream_global_init(void) { -#ifdef GIT_OPENSSL long ssl_opts = SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3; const char *ciphers = git_libgit2__ssl_ciphers(); @@ -245,8 +244,6 @@ int git_openssl_stream_global_init(void) return -1; } -#endif - git__on_shutdown(shutdown_ssl); return 0;