1. 28 Feb, 2018 1 commit
    • curl: explicitly initialize and cleanup global curl state · 2022b004
      Our curl-based streams make use of the easy curl interface. This
      interface automatically initializes and de-initializes the global curl
      state by calling out to `curl_global_init` and `curl_global_cleanup`.
      Thus, all global state will be repeatedly re-initialized when creating
      multiple curl streams in succession. Despite being inefficient, this is
      not thread-safe due to `curl_global_init` being not thread-safe itself.
      Thus a multi-threaded programing handling multiple curl streams at the
      same time is inherently racy.
      
      Fix the issue by globally initializing and cleaning up curl's state.
      Patrick Steinhardt committed
  2. 16 Feb, 2018 1 commit
    • streams: openssl: fix use of uninitialized variable · 84f03b3a
      When verifying the server certificate, we do try to make sure that the
      hostname actually matches the certificate alternative names. In cases
      where the host is either an IPv4 or IPv6 address, we have to compare the
      binary representations of the hostname with the declared IP address of
      the certificate. We only do that comparison in case we were successfully
      able to parse the hostname as an IP, which would always result in the
      memory region being initialized. Still, GCC 6.4.0 was complaining about
      usage of non-initialized memory.
      
      Fix the issue by simply asserting that `addr` needs to be initialized.
      This shuts up the GCC warning.
      Patrick Steinhardt committed
  3. 03 Jan, 2018 1 commit
    • streams: openssl: fix thread-safety for OpenSSL error messages · ba56f781
      The function `ERR_error_string` can be invoked without providing a
      buffer, in which case OpenSSL will simply return a string printed into a
      static buffer. Obviously and as documented in ERR_error_string(3), this
      is not thread-safe at all. As libgit2 is a library, though, it is easily
      possible that other threads may be using OpenSSL at the same time, which
      might lead to clobbered error strings.
      
      Fix the issue by instead using a stack-allocated buffer. According to
      the documentation, the caller has to provide a buffer of at least 256
      bytes of size. While we do so, make sure that the buffer will never get
      overflown by switching to `ERR_error_string_n` to specify the buffer's
      size.
      Patrick Steinhardt committed
  4. 15 Dec, 2017 2 commits
  5. 14 Dec, 2017 1 commit
  6. 30 Nov, 2017 1 commit
    • openssl: fix thread-safety on non-glibc POSIX systems · 2d2e70f8
      While the OpenSSL library provides all means to work safely in a
      multi-threaded application, we fail to do so correctly. Quoting from
      crypto_lock(3):
      
          OpenSSL can safely be used in multi-threaded applications provided
          that at least two callback functions are set, locking_function and
          threadid_func.
      
      We do in fact provide the means to set up the locking function via
      `git_openssl_set_locking()`, where we initialize a set of locks by using
      the POSIX threads API and set the correct callback function to lock and
      unlock them.
      
      But what we do not do is setting the `threadid_func` callback. This
      function is being used to correctly locate thread-local data of the
      OpenSSL library and should thus return per-thread identifiers. Digging
      deeper into OpenSSL's documentation, the library does provide a fallback
      in case that locking function is not provided by the user. On Windows
      and BeOS we should be safe, as it simply "uses the system's default
      thread identifying API". On other platforms though OpenSSL will fall
      back to using the address of `errno`, assuming it is thread-local.
      
      While this assumption holds true for glibc-based systems, POSIX in fact
      does not specify whether it is thread-local or not. Quoting from
      errno(3p):
      
          It is unspecified whether errno is a macro or an identifier declared
          with external linkage.
      
      And in fact, with musl there is at least one libc implementation which
      simply declares `errno` as a simple `int` without being thread-local. On
      those systems, the fallback threadid function of OpenSSL will not be
      thread-safe.
      
      Fix this by setting up our own callback for this setting. As users of
      libgit2 may want to set it themselves, we obviously cannot always set
      that function on initialization. But as we already set up primitives for
      threading in `git_openssl_set_locking()`, this function becomes the
      obvious choice where to implement the additional setup.
      Patrick Steinhardt committed
  7. 23 Oct, 2017 2 commits