Skip to content

Api/clients#1787

Merged
technoweenie merged 8 commits intoapi-masterfrom
api/clients
Dec 20, 2016
Merged

Api/clients#1787
technoweenie merged 8 commits intoapi-masterfrom
api/clients

Conversation

@technoweenie
Copy link
Contributor

Adds support for custom ssl certs, http proxies, and timeouts.

@ttaylorr ttaylorr mentioned this pull request Dec 20, 2016
17 tasks
@technoweenie technoweenie merged commit 3cfdfee into api-master Dec 20, 2016
@technoweenie technoweenie deleted the api/clients branch December 20, 2016 21:52
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request May 5, 2025
In commit 8009d17 of PR git-lfs#1787, which
was then merged into our main development branch in PR git-lfs#1839, we added
support for the use of custom TLS/SSL certificates as may be specified
with the "http.<url>.sslCAInfo" and "http.sslCAPath" Git configuration
options or the GIT_SSL_CAINFO and GIT_SSL_CAPATH environment variables.

In the same commit we also introduced logic to ensure that on macOS,
TLS/SSL certificates were validated against a set of root certificate
authorities which included those registered in the System keychain
and not just those in the SystemRootCertificates keychain, as these
may contain distinct sets of root certificate authorities.

The HttpClient() method of our Client structure in our "lfshttp" package
initializes a Client structure from the Go library's "net/http" package,
which we then use to make HTTP requests to a given remote host.  We
populate the Transport element of the "net/http" package's Client
structure with a Transport structure we initialize in the Transport()
method of our own Client structure.

Our Transport() method sets the TLSClientConfig element of the new
Transport structure to a new Config structure, from the Go library's
"crypto/tls" package, which we also initialize in our Transport()
method.  This TLS configuration structure then controls how the TLS/SSL
verification is performed for the HTTP requests we make to a given
URL with the "net/http" package's Client structure.

In particular, the RootCAs element of the "crypto/tls" package's Config
structure defines the pool of root certificate authorities which will be
used to verify the TLS/SSL certificate presented by the HTTP server.
Our Transport() method sets the RootCAs element to the value returned
by the getRootCAsForHost() function, which in turn calls two functions,
appendRootCAsForHostFromGitconfig() and appendRootCAsForHostFromPlatform().

The first of these two functions, appendRootCAsForHostFromGitconfig(),
searches for valid TLS/SSL certificate files in any of the locations
specified by the GIT_SSL_CAINFO environment variable, the
"http.<url>.sslCAInfo" Git configuration option, the GIT_SSL_CAPATH
environment variable, and the "http.sslCAPath" Git configuration option,
in that order.  If one of these settings has a defined value, then it
preempts the others, even if no valid certificate files are found at
the location it specifies.  If valid certificates are found, their
contents are returned in PEM format, which our getRootCAsForHost()
function adds to the pool of certificate it will return.

In addition to the foregoing, however, the getRootCAsForHost() function
in our "lfshttp" package then calls appendRootCAsForHostFromPlatform(),
which may add other certificates to the pool that will be returned to
the Transport() method.  As noted, these will be used as the value of
the RootCAs element of the Config structure from the "crypto/tls" package,
and are therefore included in the set of root certificate authorities
allowed to verify the TLS/SSL certificate offered by the server in
response to our HTTP requests.

The appendRootCAsForHostFromPlatform() function is platform-specific,
with three different versions, but only the one for macOS systems is
non-trivial; the others simply return their input without alteration.

On macOS, the appendRootCAsForHostFromPlatform() function runs the
"/usr/bin/security list-keychains" command, extracts the path to the
System keychain, and the runs the "/usr/bin/security find-certificate"
command to look for certificates in the System keychain whose names
match the hostname from the URL for which we are constructing an
HTTP client.  If any are found, their PEM data is appended to that
found by the appendRootCAsForHostFromGitconfig() function (if any),
and so these certificates are also included among those used to
verify the server certificate in our HTTP requests.

This extra step, which is unique to macOS, was included in commit
8009d17 because at the time of PRs git-lfs#1787
and git-lfs#1839 we built the Git LFS client with Go v1.7.4, and depending on
the version of macOS in use and how Go was compiled, the Verify() method
of the Certificate structure in the "crypto/x509" package did not always
read root certificates from the System keychain.

The Go standard library's "crypto/tls" package performs the required
steps in a TLS/SSL handshake when an HTTP request is made with a
Client from the "net/http" package.  One of these steps involves the
verification of the server's certificate, which is done with a call
to the Verify() method of the "crypto/x509" package's Certificate
structure, passing in the root certificate authorities from the
RootCAs element of the Config structure.  The Verify() method, in
turn, depends on the pool of root certificates loaded by the internal
loadSystemRoots() function:

  https://github.com/golang/go/blob/go1.7.4/src/crypto/tls/handshake_client.go#L287-L304
  https://github.com/golang/go/blob/go1.7.4/src/crypto/x509/verify.go#L247
  https://github.com/golang/go/blob/go1.7.4/src/crypto/x509/root.go#L15-L22

Until Go v1.16, the "crypto/x509" package contained two separate
implementations of its loadSystemRoots() function.  When Go was compiled
with "cgo" support, so as to permit the use of C code, the function called
a C language function named FetchPEMRoots(), which could retrieve
certificates from the SystemRootCertificates keychain but also the
System and "login" keychains (known as the System, Admin, and User
trust setting domains, respectively), but only on systems with macOS
10.9 (Mavericks) and higher installed:

  https://github.com/golang/go/blob/go1.7.4/src/crypto/x509/root_cgo_darwin.go#L212
  https://github.com/golang/go/blob/go1.7.4/src/crypto/x509/root_cgo_darwin.go#L85-L90

On older macOS systems, the FetchPEMRoots() function simply called the
FetchPEMRoots_MountainLion() function, which only returned certificates
from the SystemRootCertificates keychain:

  https://github.com/golang/go/blob/go1.7.4/src/crypto/x509/root_cgo_darwin.go#L81-L83
  https://github.com/golang/go/blob/go1.7.4/src/crypto/x509/root_cgo_darwin.go#L19-L55

Alternatively, if Go was compiled without "cgo" support, then the
loadSystemRoots() function called an execSecurityRoots() function, which
invoked the "/usr/bin/security find-certificate" command on the
SystemRootCertificates keychain:

  https://github.com/golang/go/blob/go1.7.4/src/crypto/x509/root_nocgo_darwin.go#L10
  https://github.com/golang/go/blob/go1.7.4/src/crypto/x509/root_darwin.go#L31

These divergent implementations emerged from initial efforts to support
the verification of server TLS/SSL certificates against the System and
"login" keychains as well as the SystemRootCertificates keychain.  The
golang/go#14514 and golang/go#16532 issues reported the lack of support
for the System and "login" keychains in Go v1.6.x, and so in commit
golang/go@6cd698d the "cgo" implementation
was updated to load certificates from all three keychains.

Unfortunately, as reported in golang/go#16473, this led to crashes on
older macOS systems, and so the FetchPEMRoots_MountainLion() function was
introduced in commit golang/go@ff60da6
to restore the previous behaviour on systems with macOS 10.8 (Mountain
Lion) or earlier versions of macOS.  As a result, on these systems,
even if Go was compiled with "cgo" enabled, only root certificate
authorities from the SystemRootCertificates keychain would be checked
when attempting to verify a server's TLS/SSL certificate.

Both changes to the "cgo" version of the "crypto/x509" package were
included in Go v1.7.  However, the non-"cgo" version of the package still
only called the "/usr/bin/security find-certificate" command on the
SystemRootCertificates keychain.  The execSecurityRoots() function was
not updated to also run the "/usr/bin/security find-certificate" command
on the System and "login" keychains until commit
golang/go@4dbcacd, which was included
in the Go v1.9 release.

Subsequently, these two versions were refactored extensively in commit
golang/go@6f52790, which was part
of the Go v1.15 release.  The non-"cgo" version was removed entirely,
meaning the Go standard library no longer called the
"/usr/bin/security find-certificate" command at all.  The "cgo"
implementation was retained for testing purposes but ultimately also
removed in commit golang/go@5e18135 of
Go v1.16.  In this revised implementation, the loadSystemRoots() function
continued to load certificates from all three keychains, as before.

Further refinements have been made to this code since then, including
the consolidation of AMD64 and ARM64 versions, and another major
refactoring in commit golang/go@feb024f,
which followed the design from golang/go#46287.  In that commit, the
loadSystemRoots() function to a simple wrapper, the Verify() method was
altered to just run the systemVerify() method for macOS systems when no
custom root certificates were supplied by the caller, as was already the
case for Windows systems.  As well, the systemVerify() method was
expanded from a no-op call into the method which performs the principal
certificate verification steps, making use of the new TLS/SSL verification
APIs available with all the versions of macOS supported by Go at the time.
One of these API calls, in particular, is the SecTrustEvaluateWithError()
function, which ultimately determines if a given certificate is accepted
or not, per Apple's documentation:

  https://developer.apple.com/documentation/security/sectrustevaluatewitherror(_:_:)

We now build the Git LFS client using at least Go v1.23.x, and in
most cases we benefit from these improvements to the Go standard
library's TLS/SSL verification logic on macOS systems.  If the
user provides their own certificates to be used as root certificate
authorities with either the GIT_SSL_* environment variables or the
"http.<url>.sslCAInfo" or "http.sslCAPath" configuration options,
these will be checked by the Verify() method of the "crypto/x509"
package's Certificate structure in preference to any certificates
provided by the system, which is our desired behaviour.

In this case, as noted above, because we have defined the RootCAs element
of the Config structure from the "crypto/tls" package that we set as the
TLSClientConfig element of the Transport structure for our HTTP client,
the RootCAs element will be passed to the Verify() method of the
"crypto/x509" package's Certificate structure as the Roots element
of the VerifyOptions structure.  However, this Roots element, which is
a CertPool structure, will have its internal systemPool flag set to
"false".  As a consequence, because the VerifyOptions structure's Roots
element is non-nil but that Roots element's systemPool flag is "false",
the Verify() method will not call the platform-specific systemVerify()
method at all, so only the certificates the user provided will be used
to verify the server's TLS/SSL certificate:

  https://github.com/golang/go/blob/go1.23.0/src/crypto/tls/handshake_client.go#L1099-L1114
  https://github.com/golang/go/blob/go1.23.0/src/crypto/x509/cert_pool.go#L31-L35
  https://github.com/golang/go/blob/go1.23.0/src/crypto/x509/verify.go#L771-L785

If the user does not specify any of the GIT_SSL_* environment variables
or the "http.<url>.sslCAInfo" or "http.sslCAPath" configuration options,
then in general we will not define the RootCAs element of the "crypto/tls"
package's Config structure, and the Verify() method of the "crypto/x509"
package's Certificate structure will proceed to use its platform-specific
systemVerify() method on macOS and Windows.

Even if a macOS user's System keychain contains certificates, as is
common in enterprise environments, so long as none of them contain a
certificate whose name matches the server hostname from the HTTP
request's URL, our appendRootCAsForHostFromPlatform() function will
not append any of the keychain's certificates to the CertPool it returns,
and so we will not define the RootCAs element, and hence the "crypto/x509"
package's platform- specific certificate verification logic will be used.

However, as reported in git-lfs#6036, if a macOS user's System keychain does
happen to contain a certificate which matches the server hostname from
the HTTP request's URL, our appendRootCAsForHostFromPlatform() function
will append that certificate to the CertPool it returns, which will
then cause the Verify() method of the "crypto/x509" package's Certificate
structure to skip the platform-specific verification logic and try to
verify the server's TLS/SSL certificate against just the one we have
provided from the System keychain.  Should that not actually be a
root certificate authority which can verify the server's certificate,
the HTTP request will be rejected, which is not the behaviour we
expect or intend.

To resolve this problem and also simplify our HTTP client setup code
on macOS, we can just remove the workaround introduced in commit
8009d17 back in 2016.  We first
eliminate our appendRootCAsForHostFromPlatform() functions entirely,
including both the no-op versions for Linux and Windows, and the
macOS version which invoked the "/usr/bin/security list-keychains"
and "/usr/bin/security find-certificate" commands.

We then drop the getRootCAsForHost() function in our "lfshttp" package,
rename the appendRootCAsForHostFromGitconfig() function to
getRootCAsForHostFromGitconfig(), and revise its input arguments to
match those of the former getRootCAsForHost() function.

As well as making our code simpler, these changes mean that we will
also no longer run the "/usr/bin/security" command at all on macOS
systems, which should improve our performance on those systems.

Our Go tests which validate our support of the GIT_SSL_* environment
variables and the "http.<url>.sslCAInfo" or "http.sslCAPath" configuration
options still pass as expected, once we adjust the name of the function
they call.

Designing a test which could validate the Git LFS client's behaviour
on macOS when certificates are installed in the System keychain,
though, would be very challenging.  To quote from commit
golang/go@feb024f, which implemented
the Go language's proposal from golang/go#46287:

  Unfortunately there is not a clean way to programmatically add test
  roots to the system trust store that the builders would tolerate. The
  most obvious solution, using 'security add-trusted-cert' requires human
  interaction for authorization.

Instead, we rely on the Go project's own testing of their support for
system verification of HTTP server's TLS/SSL certificates, which should
be sufficient for our project as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant