From 6bb54cbff3636e42ae2523afeee08079e5bd1d5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sun, 2 Nov 2014 13:23:32 +0100 Subject: [PATCH 1/8] Add a SecureTransport TLS channel As an alternative to OpenSSL when we're on OS X. This one can actually take advantage of stacking the streams. --- CMakeLists.txt | 30 +++- cmake/Modules/FindCoreFoundation.cmake | 9 + cmake/Modules/FindSecurity.cmake | 9 + src/settings.c | 2 +- src/stransport_stream.c | 219 +++++++++++++++++++++++++ src/stransport_stream.h | 14 ++ src/transport.c | 2 +- src/transports/http.c | 2 +- tests/core/features.c | 2 +- 9 files changed, 283 insertions(+), 6 deletions(-) create mode 100644 cmake/Modules/FindCoreFoundation.cmake create mode 100644 cmake/Modules/FindSecurity.cmake create mode 100644 src/stransport_stream.c create mode 100644 src/stransport_stream.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 5c3ba504c..2c1430356 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -36,7 +36,6 @@ OPTION( LIBGIT2_FILENAME "Name of the produced binary" OFF ) OPTION( ANDROID "Build for android NDK" OFF ) -OPTION( USE_OPENSSL "Link with and use openssl library" ON ) OPTION( USE_ICONV "Link with and use iconv library" OFF ) OPTION( USE_SSH "Link with libssh to enable SSH support" ON ) OPTION( USE_GSSAPI "Link with libgssapi for SPNEGO auth" OFF ) @@ -44,6 +43,8 @@ OPTION( VALGRIND "Configure build for valgrind" OFF ) IF(${CMAKE_SYSTEM_NAME} MATCHES "Darwin") SET( USE_ICONV ON ) + FIND_PACKAGE(Security) + FIND_PACKAGE(CoreFoundation REQUIRED) ENDIF() IF(MSVC) @@ -68,6 +69,7 @@ IF(MSVC) ADD_DEFINITIONS(-D_CRT_NONSTDC_NO_DEPRECATE) ENDIF() + IF(WIN32) # By default, libgit2 is built with WinHTTP. To use the built-in # HTTP transport, invoke CMake with the "-DWINHTTP=OFF" argument. @@ -79,6 +81,10 @@ IF(MSVC) OPTION(MSVC_CRTDBG "Enable CRTDBG memory leak reporting" OFF) ENDIF() +IF (NOT ${CMAKE_SYSTEM_NAME} MATCHES "Darwin") + OPTION( USE_OPENSSL "Link with and use openssl library" ON ) +ENDIF() + # This variable will contain the libraries we need to put into # libgit2.pc's Requires.private. That is, what we're linking to or # what someone who's statically linking us needs to link to. @@ -148,6 +154,15 @@ STRING(REGEX REPLACE "^.*LIBGIT2_SOVERSION ([0-9]+)$" "\\1" LIBGIT2_SOVERSION "$ # Find required dependencies INCLUDE_DIRECTORIES(src include) +IF (SECURITY_FOUND) + MESSAGE("-- Found Security ${SECURITY_DIRS}") +ENDIF() + +IF (COREFOUNDATION_FOUND) + MESSAGE("-- Found CoreFoundation ${COREFOUNDATION_DIRS}") +ENDIF() + + IF (WIN32 AND EMBED_SSH_PATH) FILE(GLOB SRC_SSH "${EMBED_SSH_PATH}/src/*.c") INCLUDE_DIRECTORIES("${EMBED_SSH_PATH}/include") @@ -415,12 +430,19 @@ ELSE() # that uses CMAKE_CONFIGURATION_TYPES and not CMAKE_BUILD_TYPE ENDIF() +IF (SECURITY_FOUND) + ADD_DEFINITIONS(-DGIT_SECURE_TRANSPORT) + INCLUDE_DIRECTORIES(${SECURITY_INCLUDE_DIR}) +ENDIF () + IF (OPENSSL_FOUND) ADD_DEFINITIONS(-DGIT_SSL) INCLUDE_DIRECTORIES(${OPENSSL_INCLUDE_DIR}) SET(SSL_LIBRARIES ${OPENSSL_LIBRARIES}) ENDIF() + + IF (THREADSAFE) IF (NOT WIN32) FIND_PACKAGE(Threads REQUIRED) @@ -459,6 +481,8 @@ ENDIF() # Compile and link libgit2 ADD_LIBRARY(git2 ${SRC_H} ${SRC_GIT2} ${SRC_OS} ${SRC_ZLIB} ${SRC_HTTP} ${SRC_REGEX} ${SRC_SSH} ${SRC_SHA1} ${WIN_RC}) +TARGET_LINK_LIBRARIES(git2 ${SECURITY_DIRS}) +TARGET_LINK_LIBRARIES(git2 ${COREFOUNDATION_DIRS}) TARGET_LINK_LIBRARIES(git2 ${SSL_LIBRARIES}) TARGET_LINK_LIBRARIES(git2 ${SSH_LIBRARIES}) TARGET_LINK_LIBRARIES(git2 ${GSSAPI_LIBRARIES}) @@ -527,6 +551,8 @@ IF (BUILD_CLAR) ADD_EXECUTABLE(libgit2_clar ${SRC_H} ${SRC_GIT2} ${SRC_OS} ${SRC_CLAR} ${SRC_TEST} ${SRC_ZLIB} ${SRC_HTTP} ${SRC_REGEX} ${SRC_SSH} ${SRC_SHA1}) + TARGET_LINK_LIBRARIES(libgit2_clar ${COREFOUNDATION_DIRS}) + TARGET_LINK_LIBRARIES(libgit2_clar ${SECURITY_DIRS}) TARGET_LINK_LIBRARIES(libgit2_clar ${SSL_LIBRARIES}) TARGET_LINK_LIBRARIES(libgit2_clar ${SSH_LIBRARIES}) TARGET_LINK_LIBRARIES(libgit2_clar ${GSSAPI_LIBRARIES}) @@ -540,7 +566,7 @@ IF (BUILD_CLAR) ENDIF () ENABLE_TESTING() - IF (WINHTTP OR OPENSSL_FOUND) + IF (WINHTTP OR OPENSSL_FOUND OR SECURITY_FOUND) ADD_TEST(libgit2_clar libgit2_clar -ionline) ELSE () ADD_TEST(libgit2_clar libgit2_clar -v) diff --git a/cmake/Modules/FindCoreFoundation.cmake b/cmake/Modules/FindCoreFoundation.cmake new file mode 100644 index 000000000..ebd619a53 --- /dev/null +++ b/cmake/Modules/FindCoreFoundation.cmake @@ -0,0 +1,9 @@ +IF (COREFOUNDATION_INCLUDE_DIR AND COREFOUNDATION_DIRS) + SET(COREFOUNDATION_FOUND TRUE) +ELSE () + FIND_PATH(COREFOUNDATION_INCLUDE_DIR NAMES CoreFoundation.h) + FIND_LIBRARY(COREFOUNDATION_DIRS NAMES CoreFoundation) + IF (COREFOUNDATION_INCLUDE_DIR AND COREFOUNDATION_DIRS) + SET(COREFOUNDATION_FOUND TRUE) + ENDIF () +ENDIF () diff --git a/cmake/Modules/FindSecurity.cmake b/cmake/Modules/FindSecurity.cmake new file mode 100644 index 000000000..0decdde92 --- /dev/null +++ b/cmake/Modules/FindSecurity.cmake @@ -0,0 +1,9 @@ +IF (SECURITY_INCLUDE_DIR AND SECURITY_DIRS) + SET(SECURITY_FOUND TRUE) +ELSE () + FIND_PATH(SECURITY_INCLUDE_DIR NAMES Security/Security.h) + FIND_LIBRARY(SECURITY_DIRS NAMES Security) + IF (SECURITY_INCLUDE_DIR AND SECURITY_DIRS) + SET(SECURITY_FOUND TRUE) + ENDIF () +ENDIF () diff --git a/src/settings.c b/src/settings.c index 971b50935..77a5f2ed1 100644 --- a/src/settings.c +++ b/src/settings.c @@ -28,7 +28,7 @@ int git_libgit2_features() #ifdef GIT_THREADS | GIT_FEATURE_THREADS #endif -#if defined(GIT_SSL) || defined(GIT_WINHTTP) +#if defined(GIT_SSL) || defined(GIT_WINHTTP) || defined(GIT_SECURE_TRANSPORT) | GIT_FEATURE_HTTPS #endif #if defined(GIT_SSH) diff --git a/src/stransport_stream.c b/src/stransport_stream.c new file mode 100644 index 000000000..09cc7cb00 --- /dev/null +++ b/src/stransport_stream.c @@ -0,0 +1,219 @@ +/* + * Copyright (C) the libgit2 contributors. All rights reserved. + * + * This file is part of libgit2, distributed under the GNU GPL v2 with + * a Linking Exception. For full terms see the included COPYING file. + */ + +#ifdef GIT_SECURE_TRANSPORT + +#include +#include +#include + +#include "git2/transport.h" + +#include "socket_stream.h" + +int stransport_error(OSStatus ret) +{ + switch (ret) { + case noErr: + giterr_clear(); + return 0; + case errSSLXCertChainInvalid: + case errSSLBadCert: + return GIT_ECERTIFICATE; + default: + giterr_set(GITERR_NET, "SecureTransport error %d", ret); + return -1; + } +} + +typedef struct { + git_stream parent; + git_stream *io; + SSLContextRef ctx; + CFDataRef der_data; + git_cert_x509 cert_info; +} stransport_stream; + +int stransport_connect(git_stream *stream) +{ + stransport_stream *st = (stransport_stream *) stream; + int error; + OSStatus ret; + + if ((error = git_stream_connect(st->io)) < 0) + return error; + + if ((ret = SSLHandshake(st->ctx)) != noErr) + return stransport_error(ret); + + return 0; +} + +int stransport_certificate(git_cert **out, git_stream *stream) +{ + stransport_stream *st = (stransport_stream *) stream; + SecTrustRef trust = NULL; + SecCertificateRef sec_cert; + SecTrustResultType sec_res; + OSStatus ret; + + if ((ret = SSLCopyPeerTrust(st->ctx, &trust)) != noErr) + return stransport_error(ret); + + if ((ret = SecTrustEvaluate(trust, &sec_res)) != noErr) + return stransport_error(ret); + + sec_cert = SecTrustGetCertificateAtIndex(trust, 0); + st->der_data = SecCertificateCopyData(sec_cert); + CFRelease(trust); + + if (st->der_data == NULL) { + giterr_set(GITERR_SSL, "retrieved invalid certificate data"); + return -1; + } + + st->cert_info.cert_type = GIT_CERT_X509; + st->cert_info.data = (void *) CFDataGetBytePtr(st->der_data); + st->cert_info.len = CFDataGetLength(st->der_data); + + *out = (git_cert *)&st->cert_info; + return 0; +} + +static OSStatus write_cb(SSLConnectionRef conn, const void *data, size_t *len) +{ + git_stream *io = (git_stream *) conn; + ssize_t ret; + + ret = git_stream_write(io, data, *len, 0); + if (ret < 0) { + *len = 0; + return -1; + } + + *len = ret; + + return noErr; +} + +ssize_t stransport_write(git_stream *stream, const char *data, size_t len, int flags) +{ + stransport_stream *st = (stransport_stream *) stream; + size_t data_len, processed; + OSStatus ret; + + GIT_UNUSED(flags); + + data_len = len; + if ((ret = SSLWrite(st->ctx, data, data_len, &processed)) != noErr) + return stransport_error(ret); + + return processed; +} + +static OSStatus read_cb(SSLConnectionRef conn, void *data, size_t *len) +{ + git_stream *io = (git_stream *) conn; + ssize_t ret; + size_t left, requested; + + requested = left = *len; + do { + ret = git_stream_read(io, data + (requested - left), left); + if (ret < 0) { + *len = 0; + return -1; + } + + left -= ret; + } while (left); + + *len = requested; + + if (ret == 0) + return errSSLClosedGraceful; + + return noErr; +} + +ssize_t stransport_read(git_stream *stream, void *data, size_t len) +{ + stransport_stream *st = (stransport_stream *) stream; + size_t processed; + OSStatus ret; + + if ((ret = SSLRead(st->ctx, data, len, &processed)) != noErr) + return stransport_error(ret); + + return processed; +} + +int stransport_close(git_stream *stream) +{ + stransport_stream *st = (stransport_stream *) stream; + OSStatus ret; + + if ((ret = SSLClose(st->ctx)) != noErr) + return stransport_error(ret); + + return git_stream_close(st->io); +} + +void stransport_free(git_stream *stream) +{ + stransport_stream *st = (stransport_stream *) stream; + + git_stream_free(st->io); + CFRelease(st->ctx); + if (st->der_data) + CFRelease(st->der_data); + git__free(st); +} + +int git_stransport_stream_new(git_stream **out, const char *host, const char *port) +{ + stransport_stream *st; + int error; + OSStatus ret; + + assert(out && host); + + st = git__calloc(1, sizeof(stransport_stream)); + GITERR_CHECK_ALLOC(st); + + if ((error = git_socket_stream_new(&st->io, host, port)) < 0){ + git__free(st); + return error; + } + + st->ctx = SSLCreateContext(NULL, kSSLClientSide, kSSLStreamType); + if (!st->ctx) { + giterr_set(GITERR_NET, "failed to create SSL context"); + return -1; + } + + if ((ret = SSLSetIOFuncs(st->ctx, read_cb, write_cb)) != noErr || + (ret = SSLSetConnection(st->ctx, st->io)) != noErr || + (ret = SSLSetPeerDomainName(st->ctx, host, strlen(host))) != noErr) { + git_stream_free((git_stream *)st); + return stransport_error(ret); + } + + st->parent.version = GIT_STREAM_VERSION; + st->parent.encrypted = 1; + st->parent.connect = stransport_connect; + st->parent.certificate = stransport_certificate; + st->parent.read = stransport_read; + st->parent.write = stransport_write; + st->parent.close = stransport_close; + st->parent.free = stransport_free; + + *out = (git_stream *) st; + return 0; +} + +#endif diff --git a/src/stransport_stream.h b/src/stransport_stream.h new file mode 100644 index 000000000..714f90273 --- /dev/null +++ b/src/stransport_stream.h @@ -0,0 +1,14 @@ +/* + * Copyright (C) the libgit2 contributors. All rights reserved. + * + * This file is part of libgit2, distributed under the GNU GPL v2 with + * a Linking Exception. For full terms see the included COPYING file. + */ +#ifndef INCLUDE_stransport_stream_h__ +#define INCLUDE_stransport_stream_h__ + +#include "git2/sys/stream.h" + +extern int git_stransport_stream_new(git_stream **out, const char *host, const char *port); + +#endif diff --git a/src/transport.c b/src/transport.c index fc9c692b8..6266fdd34 100644 --- a/src/transport.c +++ b/src/transport.c @@ -29,7 +29,7 @@ static transport_definition local_transport_definition = { "file://", git_transp static transport_definition transports[] = { { "git://", git_transport_smart, &git_subtransport_definition }, { "http://", git_transport_smart, &http_subtransport_definition }, -#if defined(GIT_SSL) || defined(GIT_WINHTTP) +#if defined(GIT_SSL) || defined(GIT_WINHTTP) || defined(GIT_SECURE_TRANSPORT) { "https://", git_transport_smart, &http_subtransport_definition }, #endif { "file://", git_transport_local, NULL }, diff --git a/src/transports/http.c b/src/transports/http.c index 6b100df7a..264c9c512 100644 --- a/src/transports/http.c +++ b/src/transports/http.c @@ -557,7 +557,7 @@ static int http_connect(http_subtransport *t) error = git_stream_connect(t->io); -#ifdef GIT_SSL +#if defined(GIT_SSL) || defined(GIT_SECURE_TRANSPORT) if ((!error || error == GIT_ECERTIFICATE) && t->owner->certificate_check_cb != NULL && git_stream_is_encrypted(t->io)) { git_cert *cert; diff --git a/tests/core/features.c b/tests/core/features.c index 3ce02f4d6..e6d0e0f51 100644 --- a/tests/core/features.c +++ b/tests/core/features.c @@ -17,7 +17,7 @@ void test_core_features__0(void) cl_assert((caps & GIT_FEATURE_THREADS) == 0); #endif -#if defined(GIT_SSL) || defined(GIT_WINHTTP) +#if defined(GIT_SSL) || defined(GIT_WINHTTP) || defined(GIT_SECURE_TRANSPORT) cl_assert((caps & GIT_FEATURE_HTTPS) != 0); #else cl_assert((caps & GIT_FEATURE_HTTPS) == 0); From 6946a3be953446b2838857de5e9c2002843499b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Thu, 19 Mar 2015 00:18:03 +0100 Subject: [PATCH 2/8] Abstract away the TLS stream implementation Instead, provide git_tls_stream_new() to ask for the most appropriate encrypted stream and use it in our HTTP transport. --- src/tls_stream.c | 28 ++++++++++++++++++++++++++++ src/tls_stream.h | 21 +++++++++++++++++++++ src/transports/http.c | 4 ++-- 3 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 src/tls_stream.c create mode 100644 src/tls_stream.h diff --git a/src/tls_stream.c b/src/tls_stream.c new file mode 100644 index 000000000..d44709af4 --- /dev/null +++ b/src/tls_stream.c @@ -0,0 +1,28 @@ +/* + * Copyright (C) the libgit2 contributors. All rights reserved. + * + * This file is part of libgit2, distributed under the GNU GPL v2 with + * a Linking Exception. For full terms see the included COPYING file. + */ + +#include "git2/errors.h" +#include "common.h" + +#include "openssl_stream.h" +#include "stransport_stream.h" + +int git_tls_stream_new(git_stream **out, const char *host, const char *port) +{ +#ifdef GIT_SECURE_TRANSPORT + return git_stransport_stream_new(out, host, port); +#elif defined(GIT_SSL) + return git_openssl_stream_new(out, host, port); +#else + GIT_UNUSED(out); + GIT_UNUSED(host); + GIT_UNUSED(port); + + giterr_set(GITERR_SSL, "there is no TLS stream available"); + return -1; +#endif +} diff --git a/src/tls_stream.h b/src/tls_stream.h new file mode 100644 index 000000000..98a704174 --- /dev/null +++ b/src/tls_stream.h @@ -0,0 +1,21 @@ +/* + * Copyright (C) the libgit2 contributors. All rights reserved. + * + * This file is part of libgit2, distributed under the GNU GPL v2 with + * a Linking Exception. For full terms see the included COPYING file. + */ +#ifndef INCLUDE_tls_stream_h__ +#define INCLUDE_tls_stream_h__ + +#include "git2/sys/stream.h" + +/** + * Create a TLS stream with the most appropriate backend available for + * the current platform. + * + * This allows us to ask for a SecureTransport or OpenSSL stream + * according to being on general Unix vs OS X. + */ +extern int git_tls_stream_new(git_stream **out, const char *host, const char *port); + +#endif diff --git a/src/transports/http.c b/src/transports/http.c index 264c9c512..bad7e2594 100644 --- a/src/transports/http.c +++ b/src/transports/http.c @@ -13,7 +13,7 @@ #include "smart.h" #include "auth.h" #include "auth_negotiate.h" -#include "openssl_stream.h" +#include "tls_stream.h" #include "socket_stream.h" git_http_auth_scheme auth_schemes[] = { @@ -545,7 +545,7 @@ static int http_connect(http_subtransport *t) } if (t->connection_data.use_ssl) { - error = git_openssl_stream_new(&t->io, t->connection_data.host, t->connection_data.port); + error = git_tls_stream_new(&t->io, t->connection_data.host, t->connection_data.port); } else { error = git_socket_stream_new(&t->io, t->connection_data.host, t->connection_data.port); } From 70b852cee2c9e87588d65581e13c9e65b255cecf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Thu, 19 Mar 2015 00:45:43 +0100 Subject: [PATCH 3/8] Silence unused warnings when not using OpenSSL --- src/openssl_stream.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/openssl_stream.c b/src/openssl_stream.c index 9ddf6e4be..8d79a9d84 100644 --- a/src/openssl_stream.c +++ b/src/openssl_stream.c @@ -374,6 +374,10 @@ int git_openssl_stream_new(git_stream **out, const char *host, const char *port) int git_openssl_stream_new(git_stream **out, const char *host, const char *port) { + GIT_UNUSED(out); + GIT_UNUSED(host); + GIT_UNUSED(port); + giterr_set(GITERR_SSL, "openssl is not supported in this version"); return -1; } From 24e53d2fba1ea10c27c3b19f202dc92cabedf0ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Thu, 19 Mar 2015 09:55:20 +0100 Subject: [PATCH 4/8] Rename GIT_SSL to GIT_OPENSSL This is what it's meant all along, but now we actually have multiple implementations, it's clearer to use the name of the library. --- CMakeLists.txt | 2 +- src/global.c | 8 ++++---- src/global.h | 2 +- src/netops.h | 4 ++-- src/openssl_stream.c | 2 +- src/settings.c | 6 +++--- src/tls_stream.c | 2 +- src/transport.c | 2 +- src/transports/http.c | 2 +- tests/core/features.c | 2 +- 10 files changed, 16 insertions(+), 16 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 2c1430356..5c8bf7ab8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -436,7 +436,7 @@ IF (SECURITY_FOUND) ENDIF () IF (OPENSSL_FOUND) - ADD_DEFINITIONS(-DGIT_SSL) + ADD_DEFINITIONS(-DGIT_OPENSSL) INCLUDE_DIRECTORIES(${OPENSSL_INCLUDE_DIR}) SET(SSL_LIBRARIES ${OPENSSL_LIBRARIES}) ENDIF() diff --git a/src/global.c b/src/global.c index f267fbd24..9f1a0bf10 100644 --- a/src/global.c +++ b/src/global.c @@ -17,7 +17,7 @@ git_mutex git__mwindow_mutex; #define MAX_SHUTDOWN_CB 8 -#ifdef GIT_SSL +#ifdef GIT_OPENSSL # include SSL_CTX *git__ssl_ctx; # ifdef GIT_THREADS @@ -57,7 +57,7 @@ static void git__shutdown(void) } } -#if defined(GIT_THREADS) && defined(GIT_SSL) +#if defined(GIT_THREADS) && defined(GIT_OPENSSL) void openssl_locking_function(int mode, int n, const char *file, int line) { int lock; @@ -89,7 +89,7 @@ static void shutdown_ssl_locking(void) static void init_ssl(void) { -#ifdef GIT_SSL +#ifdef GIT_OPENSSL long ssl_opts = SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3; /* Older OpenSSL and MacOS OpenSSL doesn't have this */ @@ -118,7 +118,7 @@ static void init_ssl(void) int git_openssl_set_locking(void) { -#ifdef GIT_SSL +#ifdef GIT_OPENSSL # ifdef GIT_THREADS int num_locks, i; diff --git a/src/global.h b/src/global.h index f56bec46c..fdad6ba89 100644 --- a/src/global.h +++ b/src/global.h @@ -17,7 +17,7 @@ typedef struct { char oid_fmt[GIT_OID_HEXSZ+1]; } git_global_st; -#ifdef GIT_SSL +#ifdef GIT_OPENSSL # include extern SSL_CTX *git__ssl_ctx; #endif diff --git a/src/netops.h b/src/netops.h index d5f0ca3f3..b7170a0f2 100644 --- a/src/netops.h +++ b/src/netops.h @@ -11,12 +11,12 @@ #include "common.h" #include "stream.h" -#ifdef GIT_SSL +#ifdef GIT_OPENSSL # include #endif typedef struct gitno_ssl { -#ifdef GIT_SSL +#ifdef GIT_OPENSSL SSL *ssl; #else size_t dummy; diff --git a/src/openssl_stream.c b/src/openssl_stream.c index 8d79a9d84..2ebfac738 100644 --- a/src/openssl_stream.c +++ b/src/openssl_stream.c @@ -5,7 +5,7 @@ * a Linking Exception. For full terms see the included COPYING file. */ -#ifdef GIT_SSL +#ifdef GIT_OPENSSL #include diff --git a/src/settings.c b/src/settings.c index 77a5f2ed1..2097ca314 100644 --- a/src/settings.c +++ b/src/settings.c @@ -5,7 +5,7 @@ * a Linking Exception. For full terms see the included COPYING file. */ -#ifdef GIT_SSL +#ifdef GIT_OPENSSL # include #endif @@ -28,7 +28,7 @@ int git_libgit2_features() #ifdef GIT_THREADS | GIT_FEATURE_THREADS #endif -#if defined(GIT_SSL) || defined(GIT_WINHTTP) || defined(GIT_SECURE_TRANSPORT) +#if defined(GIT_OPENSSL) || defined(GIT_WINHTTP) || defined(GIT_SECURE_TRANSPORT) | GIT_FEATURE_HTTPS #endif #if defined(GIT_SSH) @@ -138,7 +138,7 @@ int git_libgit2_opts(int key, ...) break; case GIT_OPT_SET_SSL_CERT_LOCATIONS: -#ifdef GIT_SSL +#ifdef GIT_OPENSSL { const char *file = va_arg(ap, const char *); const char *path = va_arg(ap, const char *); diff --git a/src/tls_stream.c b/src/tls_stream.c index d44709af4..39a8ce343 100644 --- a/src/tls_stream.c +++ b/src/tls_stream.c @@ -15,7 +15,7 @@ int git_tls_stream_new(git_stream **out, const char *host, const char *port) { #ifdef GIT_SECURE_TRANSPORT return git_stransport_stream_new(out, host, port); -#elif defined(GIT_SSL) +#elif defined(GIT_OPENSSL) return git_openssl_stream_new(out, host, port); #else GIT_UNUSED(out); diff --git a/src/transport.c b/src/transport.c index 6266fdd34..5c65c7c06 100644 --- a/src/transport.c +++ b/src/transport.c @@ -29,7 +29,7 @@ static transport_definition local_transport_definition = { "file://", git_transp static transport_definition transports[] = { { "git://", git_transport_smart, &git_subtransport_definition }, { "http://", git_transport_smart, &http_subtransport_definition }, -#if defined(GIT_SSL) || defined(GIT_WINHTTP) || defined(GIT_SECURE_TRANSPORT) +#if defined(GIT_OPENSSL) || defined(GIT_WINHTTP) || defined(GIT_SECURE_TRANSPORT) { "https://", git_transport_smart, &http_subtransport_definition }, #endif { "file://", git_transport_local, NULL }, diff --git a/src/transports/http.c b/src/transports/http.c index bad7e2594..89cbd17d7 100644 --- a/src/transports/http.c +++ b/src/transports/http.c @@ -557,7 +557,7 @@ static int http_connect(http_subtransport *t) error = git_stream_connect(t->io); -#if defined(GIT_SSL) || defined(GIT_SECURE_TRANSPORT) +#if defined(GIT_OPENSSL) || defined(GIT_SECURE_TRANSPORT) if ((!error || error == GIT_ECERTIFICATE) && t->owner->certificate_check_cb != NULL && git_stream_is_encrypted(t->io)) { git_cert *cert; diff --git a/tests/core/features.c b/tests/core/features.c index e6d0e0f51..5eeb05e81 100644 --- a/tests/core/features.c +++ b/tests/core/features.c @@ -17,7 +17,7 @@ void test_core_features__0(void) cl_assert((caps & GIT_FEATURE_THREADS) == 0); #endif -#if defined(GIT_SSL) || defined(GIT_WINHTTP) || defined(GIT_SECURE_TRANSPORT) +#if defined(GIT_OPENSSL) || defined(GIT_WINHTTP) || defined(GIT_SECURE_TRANSPORT) cl_assert((caps & GIT_FEATURE_HTTPS) != 0); #else cl_assert((caps & GIT_FEATURE_HTTPS) == 0); From b7e1c81d1bb779bf97a209f239388d1a1cd25509 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Thu, 19 Mar 2015 10:51:48 +0100 Subject: [PATCH 5/8] SecureTransport: allow overriding a bad certificate Do not automatically fail on a bad certificate, but let the caller decide. This means we don't need our switch on errors anymore but can return a string representation from the security framework. --- src/stransport_stream.c | 55 ++++++++++++++++++++++++++++++----------- 1 file changed, 41 insertions(+), 14 deletions(-) diff --git a/src/stransport_stream.c b/src/stransport_stream.c index 09cc7cb00..644a5a7c2 100644 --- a/src/stransport_stream.c +++ b/src/stransport_stream.c @@ -17,17 +17,19 @@ int stransport_error(OSStatus ret) { - switch (ret) { - case noErr: + CFStringRef message; + + if (ret == noErr) { giterr_clear(); return 0; - case errSSLXCertChainInvalid: - case errSSLBadCert: - return GIT_ECERTIFICATE; - default: - giterr_set(GITERR_NET, "SecureTransport error %d", ret); - return -1; } + + message = SecCopyErrorMessageString(ret, NULL); + GITERR_CHECK_ALLOC(message); + + giterr_set(GITERR_NET, "SecureTransport error: %s", CFStringGetCStringPtr(message, kCFStringEncodingUTF8)); + CFRelease(message); + return -1; } typedef struct { @@ -42,15 +44,43 @@ int stransport_connect(git_stream *stream) { stransport_stream *st = (stransport_stream *) stream; int error; + SecTrustRef trust = NULL; + SecTrustResultType sec_res; OSStatus ret; if ((error = git_stream_connect(st->io)) < 0) return error; - if ((ret = SSLHandshake(st->ctx)) != noErr) - return stransport_error(ret); + ret = SSLHandshake(st->ctx); + if (ret != errSSLServerAuthCompleted) { + giterr_set(GITERR_SSL, "unexpected return value from ssl handshake %d", ret); + return -1; + } + + if ((ret = SSLCopyPeerTrust(st->ctx, &trust)) != noErr) + goto on_error; + + if ((ret = SecTrustEvaluate(trust, &sec_res)) != noErr) + goto on_error; + + CFRelease(trust); + + if (sec_res == kSecTrustResultInvalid || sec_res == kSecTrustResultOtherError) { + giterr_set(GITERR_SSL, "internal security trust error"); + return -1; + } + + if (sec_res == kSecTrustResultDeny || sec_res == kSecTrustResultRecoverableTrustFailure || + sec_res == kSecTrustResultFatalTrustFailure) + return GIT_ECERTIFICATE; return 0; + +on_error: + if (trust) + CFRelease(trust); + + return stransport_error(ret); } int stransport_certificate(git_cert **out, git_stream *stream) @@ -58,15 +88,11 @@ int stransport_certificate(git_cert **out, git_stream *stream) stransport_stream *st = (stransport_stream *) stream; SecTrustRef trust = NULL; SecCertificateRef sec_cert; - SecTrustResultType sec_res; OSStatus ret; if ((ret = SSLCopyPeerTrust(st->ctx, &trust)) != noErr) return stransport_error(ret); - if ((ret = SecTrustEvaluate(trust, &sec_res)) != noErr) - return stransport_error(ret); - sec_cert = SecTrustGetCertificateAtIndex(trust, 0); st->der_data = SecCertificateCopyData(sec_cert); CFRelease(trust); @@ -198,6 +224,7 @@ int git_stransport_stream_new(git_stream **out, const char *host, const char *po if ((ret = SSLSetIOFuncs(st->ctx, read_cb, write_cb)) != noErr || (ret = SSLSetConnection(st->ctx, st->io)) != noErr || + (ret = SSLSetSessionOption(st->ctx, kSSLSessionOptionBreakOnServerAuth, true)) != noErr || (ret = SSLSetPeerDomainName(st->ctx, host, strlen(host))) != noErr) { git_stream_free((git_stream *)st); return stransport_error(ret); From 85247df0844501e8017f54bcbccfd3e72bb6c7c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Thu, 19 Mar 2015 14:26:07 +0100 Subject: [PATCH 6/8] Update THREADING and CHANGELOG with SecureTransport details --- CHANGELOG.md | 3 +++ THREADING.md | 18 ++++++++++++++---- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12c60532c..c97a18b50 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,9 @@ v0.22 + 1 * `git_rebase_commit` now returns `GIT_EUNMERGED` when you attempt to commit with unstaged changes. +* On Mac OS X, we now use SecureTransport to provide the cryptographic +support for HTTPS connections insead of OpenSSL. + ### API additions * The `git_merge_options` gained a `file_flags` member. diff --git a/THREADING.md b/THREADING.md index c9bee5184..cf7ac1ff0 100644 --- a/THREADING.md +++ b/THREADING.md @@ -41,12 +41,22 @@ both of which are thread-safe. You do not need to do anything special. When using libssh2 which itself uses WinCNG, there are no special steps necessary. If you are using a MinGW or similar environment where -libssh2 uses OpenSSL or libgcrypt, then the non-Windows case affects +libssh2 uses OpenSSL or libgcrypt, then the general case affects you. -Non-Windows +On Mac OS X ----------- +On OS X, the library makes use of CommonCrypto and SecureTransport for +cryptographic support. These are thread-safe and you do not need to do +anything special. + +Note that libssh2 may still use OpenSSL itself. In that case, the +general case still affects you if you use ssh. + +General Case +------------ + On the rest of the platforms, libgit2 uses OpenSSL to be able to use HTTPS as a transport. This library is made to be thread-implementation agnostic, and the users of the library must set which locking function @@ -71,8 +81,8 @@ See the [OpenSSL documentation](https://www.openssl.org/docs/crypto/threads.html) on threading for more details. -Be also aware that libgit2 may not always link against OpenSSL in the -future if there are alternatives provided by the system. +Be also aware that libgit2 does not always link against OpenSSL +if there are alternatives provided by the system. libssh2 may be linked against OpenSSL or libgcrypt. If it uses OpenSSL, you only need to set up threading for OpenSSL once and the From 65ac7ddcccbf28158d75cfa4e524500f5fdd5f4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 24 Mar 2015 16:31:51 +0100 Subject: [PATCH 7/8] SecureTransport: require TLS v1.x Anything SSL is deprecated. Let's make sure we don't try to use SSL v3 when talking to the server. --- src/stransport_stream.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/stransport_stream.c b/src/stransport_stream.c index 644a5a7c2..db993ffb7 100644 --- a/src/stransport_stream.c +++ b/src/stransport_stream.c @@ -225,6 +225,8 @@ int git_stransport_stream_new(git_stream **out, const char *host, const char *po if ((ret = SSLSetIOFuncs(st->ctx, read_cb, write_cb)) != noErr || (ret = SSLSetConnection(st->ctx, st->io)) != noErr || (ret = SSLSetSessionOption(st->ctx, kSSLSessionOptionBreakOnServerAuth, true)) != noErr || + (ret = SSLSetProtocolVersionMin(st->ctx, kTLSProtocol1)) != noErr || + (ret = SSLSetProtocolVersionMax(st->ctx, kTLSProtocol12)) != noErr || (ret = SSLSetPeerDomainName(st->ctx, host, strlen(host))) != noErr) { git_stream_free((git_stream *)st); return stransport_error(ret); From 44b769e497f9e972e398f98a3d80267ac03e3e1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Mon, 13 Apr 2015 15:39:58 +0200 Subject: [PATCH 8/8] SecureTransport: handle graceful closes On close, we might get a return code which looks like an error but just means that the other side closed gracefully. Handle that. --- src/stransport_stream.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/stransport_stream.c b/src/stransport_stream.c index db993ffb7..429aa2d55 100644 --- a/src/stransport_stream.c +++ b/src/stransport_stream.c @@ -19,7 +19,7 @@ int stransport_error(OSStatus ret) { CFStringRef message; - if (ret == noErr) { + if (ret == noErr || ret == errSSLClosedGraceful) { giterr_clear(); return 0; } @@ -183,7 +183,8 @@ int stransport_close(git_stream *stream) stransport_stream *st = (stransport_stream *) stream; OSStatus ret; - if ((ret = SSLClose(st->ctx)) != noErr) + ret = SSLClose(st->ctx); + if (ret != noErr && ret != errSSLClosedGraceful) return stransport_error(ret); return git_stream_close(st->io);