Git: Add support for reading and validating HTTPS certificates via Windows Certificate Store

[drakino]

To aid in enterprise rollouts of git, supporting the Windows Certificate Store would be useful. Many enterprises have the ability to distribute trusted certificates as part of their Active Directory setups for their Windows users.

Similar support was added for git on OS X (at 1.8.0 I believe) to read the system Keychain for certificate validation when using HTTPS, and has been useful for deployments.

[dscho]

Are you talking about the credential store? As far as I understand that does not validate HTTPS certificates but only remembers (and offers) user/password combinations.

[dscho]

@drakino please note that I am waiting for your answer, and for clarification in general.

[dra27]

It sounds more like a request to have the WinSSL feature of Curl - I've only had a quick glance, so sorry if it's off-track, but Git's (lib)curl seems to be compiled to use a ca-bundle.crt file only. The WinSSL feature of CURL (configure with --with-winssl) allows it to use the Windows Certificate Store root certificates (including, critically, domain certificates).

[dscho]

@dra27 oh, that makes sense (I actually did not know that WinSSL is supported).

Apparently this is already kinda supported: https://github.com/Alexpux/MINGW-packages/blob/983ea7944e735459e5b48c4b0ecd09cdc9fe2d9e/mingw-w64-curl/PKGBUILD#L57-L62

But the variant is openssl, not winssl: https://github.com/Alexpux/MINGW-packages/blob/983ea7944e735459e5b48c4b0ecd09cdc9fe2d9e/mingw-w64-curl/PKGBUILD#L2

I guess what we want is openssl+winssl or some such?

[drakino]

Appologies for the late response, and thanks for following up @dscho . Wanted to confirm that what @dra27 indicated is what I'm looking for.

The personal reason for this request is from an observation of the behavior differences of Git on Windows and OS X. On Windows with the 2.5.0 git release, it won't verify a valid certificate from a root CA included in the ca-bundle.crt if an intermediate certificate is in the chain. I still need to verify the behavior on a Linux distro to compare. On OS X, it looks like Git uses the Keychain feature of the OS to validate certificates possibly via a feature similar to winssl in Curl.

I generalized the request a bit to also support enterprises possibly using their own internal CA instead of an external one, something I've found in common use at a few companies. They frequently will use a feature of Active Directory to distribute their internal CA cert to Windows boxes, and it ends up being stored in the Windows Certificate Store.

[dscho]

@drakino happily, with the information I gave you, you can go wild and experiment -- with your existing setup -- how to rebuild the mingw-w64-curl package best. All you have to do is read up on rebuilding packages in the Git for Windows SDK, then follow the pointers I gave above. That will enable you to

• build cURL with WinSSL support,
• install the package in the Git SDK,
• test whether, say, git clone respects the certificates you have in your Windows Certificate Manager, and
• verify that cloning from GitHub still works (i.e. the CA bundle is still respected, too).

That process might take an hour in total, although only 15 minutes of your time (most of the time will be spent by the SDK installer to download and install the SDK packages, and to clone and build Git for Windows itself).

[dra27]

There's a security argument for not enabling openssl by default (though by now that's probably a huge regression!) - ca-bundle.crt is potentially a security hole if the user messes up file permissions, where the Windows Certificate Store is slightly harder to attack.

[dscho]

@drakino please note that once again, I wait for your feedback.

[fliiiix]

Since I'm interested in this I have started working on this.

build cURL with WinSSL support

Done. Works fine. But if I understand it right this is 64bit only?
I need to rebuild this https://github.com/git-for-windows/MSYS2-packages/tree/master/curl for 32bit?

install the package in the Git SDK

Here I'm stuck I'm not really sure how I achieve this.
Is installing it with `pacman -U enough?

test whether, say, git clone respects the certificates you have in your Windows Certificate Manager, and

I tested that for now just with CA Cert which is per default not trusted, so any one can verify it.
You need to import the root cert and the revoke list in your in Trusted Root Certification Authorities.

Maybe we can disable the revoke list since my understanding is that this hasn't worked with the openssl backend. --ssl-no-revoke source: https://raw.githubusercontent.com/bagder/curl/master/docs/SSL-PROBLEMS

$ pkg/mingw-w64-x86_64-curl/mingw64/bin/curl.exe  https://www.cacert.org/
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd">
<html>
<head>

verify that cloning from GitHub still works (i.e. the CA bundle is still respected, too

Something like github works with winssl only because they have official trusted certificates.

$ pkg/mingw-w64-x86_64-curl/mingw64/bin/curl.exe  https://www.cacert.org/
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd">
<html>
<head>

I guess what we want is openssl+winssl or some such?

I don't think so. Or what is the reason you would support both? (except maybe regression for some users)

But just for the argument: I checked building it with openssl and winssl and it works (I need to do more testing which certificates are actually used)

[dscho]

build cURL with WinSSL support

Done. Works fine. But if I understand it right this is 64bit only?

Well, your rebuild might be 64-bit only but the idea is to contribute a patch to https://github.com/Alexpux/MINGW-packages/blob/master/mingw-w64-curl/PKGBUILD and have it fixed for _everybody_.

install the package in the Git SDK

Here I'm stuck I'm not really sure how I achieve this.
Is installing it with `pacman -U enough?

Yes.

I guess what we want is openssl+winssl or some such?

I don't think so. Or what is the reason you would support both? (except maybe regression for some users)

Indeed, it would be a regression to break something that worked before.

Remember: there are many, many users. Even if only 0.1% of them use a certain feature, that would still be a lot.

For reference: Git for Windows 2.6.1 has been out _just_ one week. It has been downloaded 250,000 times.

[dscho]

@fliiiix Oh, darn, I meant to write "Good work!" on top of this and below my comment, I wanted to suggest to craft a nice commit message and submit a Pull Request to upstream's MINGW-packages repository, mentioning this ticket. What do you think?

[fliiiix]

Remember: there are many, many users. Even if only 0.1% of them use a certain feature, that would still be a lot.

Wooaa, ok I didn't thought about that.

So I this case I understand why winssl + openssl is the best solution. But this is also the part I tested the least. So I will test tomorrow (or maybe to night) a few additional things and if things work as expect I create a pull request.

[dscho]

:+1:

[fliiiix]

Okey I just tested winssl + openssl and it's useless. If I understand this right you can't have your ca-bundle and the windows cert story.

http://curl.haxx.se/docs/sslcerts.html

If libcurl was built with Schannel (Microsoft's native TLS engine) or Secure Transport (Apple's native TLS engine) support, then libcurl will still perform peer certificate verification, but instead of using a CA cert bundle, it will use the certificates that are built into the OS.

And I found a discussion that they tried winssl in msysgit and rolled it back. But this was ~2 years ago so I will go ahead an create a pull request on the master repository to discus If it's worth the regression or not.
https://groups.google.com/forum/#!topic/msysgit/VqWArHFoeYU

[fliiiix]

I guess we can close the discussion here, since @Alexpux decided to stay on openssl.

[dscho]

As I mentioned in Alexpux/MINGW-packages#823, I would prefer to patch cURL to look up _both_ Windows Certificate Store and OpenSSL's ca-bundle.

[mxk]

How hard would it be to package two different cURL binaries and select which one to use based on some config option?

[dscho]

@mxk your guess is as good as mine. I would have to actually do it to determine how hard it is.

[dscho]

It's been over one year since anybody showed any interest in this. I'll wait a week or two, and if nothing changes regarding said interest, this ticket will be closed.

[Radrik5]

What's the right thing to do here? Patch curl to look up in both or patch installer to copy certs from Windows Certificate Store to ca-bundle.crt?

[shiftkey]

Patch curl to look up in both

I believe this is a non-trivial task, and might require the blessing of the curl maintainers before they'll even consider accepting the work. This is something I'm interested in, but I don't have the bandwidth currently to push this through, nor the familiarity with the curl source to figure out the best way to achieve this.

patch installer to copy certs from Windows Certificate Store to ca-bundle.crt?

Copying certs at install time is a partial solution, but it wouldn't include certificates added by the user afterwards.

[dscho]

So the biggest obstacle to supporting both ca-cert and the Windows Certificate Store is that you essentially have to use OpenSSL if you want to use the ca-cert file, and you have to use SChannel if you want to use the Windows Certificate Store, as the data structures are pretty different, and therefore the functions using them are, too.

Have a look here: https://github.com/curl/curl/search?utf8=%E2%9C%93&q=USE_SCHANNEL

The most problematic aspects are the different fields for OpenSSL vs SChannel in the ssl_connect_data struct, and after that the global functions that target either OpenSSL or SChannel (i.e. the curlssl_* functions and the X509-related functions).

It will require substantial refactoring to teach cURL to allow for optionally overriding which system to use, as it is currently a compile-time option with ramifications not only what libraries are linked and not only what type certain structs' fields have, but there are also implications on other compile-time options (it is not possible to use SChannel without SSPI support, for example, while it is possible to use OpenSSL without SSPI).

The biggest bang for the buck may be to work on bundling mingw-w64-curl-winssl in addition to mingw-w64-curl and marking these packages as mutually incompatible. It would then require only a little bit of installer magic to include the -winssl files in a special location, and to replace the regular mingw-w64-curl files upon an installer option.

This would side-step the challenge of refactoring cURL, even if it still would take a couple of days to accomplish. Meaning: I won't have time to do this, although I'd be eager to help any competent volunteer stepping forward.

[fourpastmidnight]

Or, create an adapter interface that cURL would use to talk to the certificate stores, which generalizes the interaction with them; and then implementations are provided for OpenSsl and SChannel. That may get more blessing from the cURL maintainers because the adapter implementations could be in separate modules. For example, *nix users of cURL have no use for a SChannel adapter.

[Radrik5]

I don't think that many users want to use both the ca-cert file and the Windows Certificate Store at the same time, so having an installer option that selects one or another seems to be a reasonable compromise. The default option for an upgrade should be "use the ca-cert file" to keep it backwards compatible, the default option for a fresh installation is not clear. I prefer the Windows Certificate Store.

I'm going to try to implement this sulution this week.

[drakino]

I think the path @Radrik5 is describing would have suited the environment that led to the enhancement request. It's been a while since I worked in it, moved on to other contract jobs since. I'll reach out to that team to see if it's still a need and if they can help contribute by testing and looking through the alternate ideas here.

[bbolli]

It might almost make sense to add a Windows Certificate Store backend to OpenSSL. The OpenSSL CAPI engine that supports the Windows CryptoAPI does this for the client certificates; unfortunately not for root or intermediate server certificates.

[Radrik5]

I created pull request with the mingw-w64-curl-winssl package: https://github.com/Alexpux/MINGW-packages/pull/2206.

@dscho could you suggest how to do "a little bit of installer magic to include the -winssl files in a special location, and to replace the regular mingw-w64-curl files upon an installer option"?

[dscho]

I created pull request with the mingw-w64-curl-winssl package: Alexpux/MINGW-packages#2206.

Okay.

The way I imagined this to work was actually more along the lines of this:

#!/bin/sh

die () {
    echo "$*" >&2
    exit 1
}

cd "$(dirname "$0")" ||
die "Could not change directory to mingw-w64-curl"

sed -e 's/^_variant=-openssl/#&/' -e 's/^#\(_variant=-winssl\)/\1/' \
    -e 's/^pkgname=.*/&-winssl/' \
    -e '/^depends=/iconflicts=("${MINGW_PACKAGE_PREFIX}-${_realname}")' \
    <PKGBUILD >PKGBUILD.winssl ||
die "Could not copy-edit PKGBUILD"

makepkg-mingw -s -p PKGBUILD.winssl ||
die "Could not build mingw-w64-curl-winssl"

i.e. a script in mingw-w64-curl/ that copy-edits the current PKGBUILD to generate a -winssl package.

could you suggest how to do "a little bit of installer magic to include the -winssl files in a special location, and to replace the regular mingw-w64-curl files upon an installer option"?

As far as I can tell, the only two files from the mingw-w64-curl package that actually make it into the installer are: /mingw64/bin/curl.exe and /mingw64/bin/libcurl-4.dll.

So one idea would be to extend the git-extra package to

1. download the current -winssl package with pacman -Syw mingw-w64-curl-winssl
2. unpack the files into /mingw64/bin/winssl/ via something like rm -rf /mingw64/bin/winssl && mkdir -p /mingw64/bin/winssl && cd /mingw64/bin/winssl && tar -xvf --strip-components=2 $(ls -t /var/cache/pacman/pkg/mingw-w64-x86_64-curl-winssl-*.pkg.tar.xz | head -n 1) mingw64/bin/\*)

then teach /usr/src/build-extra/make-file-list.sh to include the two files in place, and finally teach /usr/src/installer/install.iss to optionally create the /mingw64/bin/openssl/ subdirectory, move the curl.exe and libcurl-4.dll from the /mingw64/bin/ directory and move the winssl/ variants there instead.

[Radrik5]

I've added curl.exe and libcurl-4.dll from curl-winssl to the git-extra package: https://github.com/Radrik5/build-extra/commit/bbaf5563237eb75e7867e511b4b806d5e3cf5924.

Since the files are now part of installed package, make-file-list.sh outputs them as other installed files:

$ ARCH=x86_64 BITNESS=64 ./make-file-list.sh | grep curl
mingw64/bin/curl.exe
mingw64/bin/curl-winssl/curl.exe
mingw64/bin/curl-winssl/libcurl-4.dll
mingw64/bin/libcurl-4.dll
usr/bin/msys-curl-4.dll
usr/lib/gnupg/gnupg/gpgkeys_curl.exe

The next step is to change installer. And we probably need to teach please.sh to build both curl packages in order to avoid discrepancies in openssl and winssl versions.

[dscho]

Since the files are now part of installed package

This comment just gave me a new idea: how about changing the sed script to generate a PKGBUILD definition of mingw-w64-curl-winssl that does not conflict with mingw-w64-curl, but instead depends on it and only adds the /mingw64/bin/winssl/ files? That way, we could install both, bundle the contents of both in the installer, and everything becomes so much more elegant?

[Radrik5]

I like the idea. How about using a .patch file to modify the PKGBUILD definition instead of a complicated sed script?

[dscho]

Even better!

Maybe call the new package mingw-w64-curl-winssl-bin or something like that?

[Radrik5]

Sounds sensible, I'll add renaming to that .patch file.

@dscho, could you pull changes from Alexpux/MINGW-packages to git-for-windows/MINGW-packages?
Tomorrow I'm going to create a pull request in git-for-windows/MINGW-packages with changes in mingw-w64-curl and it will conflict with my previously merged PR https://github.com/Alexpux/MINGW-packages/pull/2206.

[dscho]

could you pull changes from Alexpux/MINGW-packages to git-for-windows/MINGW-packages?

Done.

[Radrik5]

I've added the "Use Windows Certificate Store to validate HTTPS certificates" option to the list of available components. It was easier than to add new "extra options" custom page with a single check-box.

However, I'm not sure about the wording. I'm not an expert in SSL certificates, so the wording may be technically not correct. Microsoft calls the store "Trusted Root Certification Authorities store" or "Trusted Root CA store": https://technet.microsoft.com/en-us/library/cc754841(v=ws.11).aspx

[dra27]

"Windows Certificate Store" sounds dangerously close to "Windows Store" to me. How about "Validate HTTPS certificates using Windows Trusted Root CA store"?

Equally, if I understand how this is implemented correctly, wouldn't this be better on a page on its own, not as a checkbox, but as two option boxes? Isn't the choice "Use cabundle.crt (OpenSSL)" vs "Use Windows Trusted Root CA root (WinSSL)"? That's just like the plink/ssh dialogue, and would allow more text to be included explaining the implications of each choice.

[fourpastmidnight]

Hmm, I think this probably should have it's own page, much like choosing how the PATH environment variable or the choice of terminal host is done. So, something like the following:

Choose cURL Certificate Store

Choose the source for the certificates cURL will use when validating SSL/TLS connections:

• ( x ) Use the OpenSSL cabundle.crt certificates bundle file

       Some explanatory text about selecting this option

• (    ) Use Windows' Certificate Stores

       Some explanatory text about selecting this option

---

Cancel                                                                      Back   Next

[fourpastmidnight]

@dra27 The name of the Windows certificate database _is_ Windows Certificate Store, and the _Trusted Root Store_ is but one of many containers within the store which are used in performing certificate validation.

[dra27]

@fourpastmidnight - searching MSDN, there is no technology which Microsoft refers to as "Windows Certificate Store" (even the snap-in to manage it is called Certificates). The docs fairly consistently use the plural - "Certificate Stores", so perhaps the option is better named "Use Windows Certificate Stores", correctly encompassing the use of stores beyond the Trusted Root CA store.

[fourpastmidnight]

@dra27 Agreed, each container is a "store"; I stand corrected. I'll update the wireframe.

[jberezanski]

I think it should be taken into account that the option is not merely about choosing the certificate validation source, but it in fact determines which SSL/TLS library will be used for HTTPS - and that has many more consequences (such as the set of supported cryptographic algorithms, vulnerability to certain bugs, compatibility with misbehaving servers, manageability by enterprise admins, patching method).

How about this:

Choose HTTPS implementation

Choose the SSL/TLS library which will be used by Git for HTTPS connections

• (*) Use the native Windows SChannel library

  Server certificates will be validated against Windows Certificate Stores.

• ( ) Use the OpenSSL library

  Server certificates will be validated using the cabundle.crt file.

(Personally, I'm very excited by this feature, as I much prefer SChannel to OpenSSL, both as end user and as admin.)

[dra27]

That looks good - the only change I'd propose would be to choose 'validated against' or 'validated using' (I'd go for the second)

[jberezanski]

Yes, 'validated using' looks better.

[fourpastmidnight]

@jberezanski Wow, yes, I like that much better. Thanks for the feedback! But perhaps even more clearer:

Choose cURL HTTPS Implementation

Choose the cURL SSL/TLS library to install with Git for Windows for performing
HTTPS certificate validation:

_the rest of the content_

After all, it's not Git making the https connection, but Git is being configured to install cURL with a particular https validation implementation.

[Radrik5]

@jberezanski, I like the wordings. Thank you for your help!

Here is how it looks in the installer:

I slightly modified the header because all the installer pages use gerund in the first line and question in the second:

I also changed the order of the options because I think OpenSSL should be the default one:

1. WinSSL version wasn't tested as much as the OpenSSL one and I'm not sure if WinSSL version works as fast and reliable as the OpenSSL one.
2. WinSSL version lacks some features: https://github.com/git-for-windows/build-extra/pull/135#issuecomment-282564353.
3. Most Git for Windows users will be just fine with the OpenSSL version. WinSSL version is needed only for those who install custom root certificates (usually for a domain). I really don't know how many people have their own Git server with custom HTTPS certificate and prefer HTTPS over SSH, so my estimate here may be incorrect.

[Radrik5]

@fourpastmidnight, from the implementation perspective, of course, it's about cURL version but from the user perspective I personally didn't know that Git uses cURL for HTTP/HTTPS until I started working on this issue. I don't think we need to show such implementation details in the header. Maybe we could describe it in the text under the options.

[dra27]

On balance, I agree with your reasoning for OpenSSL being the default - I wonder, therefore, if an extra sentence for the SChannel option might make its appearance clearer: "This option allows you to use Active Directory signed certificates, for example."?

[jberezanski]

@Radrik5 Right, that's precisely why I deliberately omitted any mention of cURL. The user invokes git.exe and may not know or care what cURL is (I know I didn't).

Yes, I agree that OpenSSL should remain the default, at least for now.

@dra27 Active Directory does not sign certificates, a Certificate Authority does. AD may be used to distribute certificates of Root CAs (internal or external).
How about: "This option allows you to use your company's internal Root CA certificates distributed via Active Directory."?

[dra27]

@jberezanski - the service is called "Active Directory Certificate Services". I was specifically pointing to certificates signed by the Domain CA, which are much harder (though obviously not impossible) to distribute other than via Active Directory. I'm not particularly sure which choice of wording is clearer to the less administrative user!

[fourpastmidnight]

@dra27 Agreed about mentioning cURL may be too implementation specific.

@jberezanski I like the extra text you suggested with respect to Active Directory Certificate Services. I would simply add this explanatory text to the explanatory text @Radrik5 already has for the WinSsl option. I would just add the word _also_:

This option also allows you to....

@Radrik5 I think the dialog looks great so far! I too, can't wait to see this make it into Git for Windows.

[jberezanski]

@dra27 OK, in all contexts I encountered, plain "Active Directory" was always understood to refer to the LDAP directory service, i.e. Active Directory Domain Services (which in fact was originally named simply "Active Directory" and only later received the "Domain Services" suffix). On the other hand, AD CS was earlier named just "Certificate Services". The two technologies interact, but do not depend on each other: it is trivial to distribute via AD DS certificates of CAs built using other technologies than AD CS (we have one in our company, in fact), it is also easy to manually distribute certificates of CAs implemented using AD CS (we are talking about certificates of the CAs themselves, not end-entity certificates issued by those CAs) without using AD DS (they are single .cer files, after all). Anyway, it is AD DS which populates certificate stores on domain-joined machines, not AD CS.

@fourpastmidnight "also" would be fine.

So, taking both clarifications into account:

This option also allows you to use your company's internal Root CA certificates distributed via Active Directory Domain Services.

[fliiiix]

I liked the first draft better, since the focus is now on how it's distributed which is not really important at least in my opinion since there are other ways to distribute a CA than AD DS. Also I would guess to a less administrative user "Active Directory Domain Services" and "Windows Certificate Stores".

So I would propose something like this

(*) Use the OpenSSL library

Server certificates will be validated using the cabundle.crt file. (If you are not sure use this)

( ) Use the WinSSL library

Server certificates will be validated against Windows Certificate Stores. This options allows you to use your company's internal Root CA certificates, which is distributed via Active Directory Domain Services for example.

[jberezanski]

Looks fine, apart from one issue: the proper name for the native Windows SSL/TLS library is Secure Channel, or SChannel for short. "WinSSL" is only how cURL developers chose to call their build option.

[splatteredbits]

Is there a way to change this after installation if a user chose the incorrect option? Or does it require a reinstall?

[fourpastmidnight]

I believe it will require a reinstall.

[dscho]

@splatteredbits if you want to become so familiar as to become a regular contributor to the Git for Windows project, it will be easy to do without a reinstall.

Otherwise, I would strongly suggest to simply reinstall; the installer will have remembered all your previous choices and all you need to do is to correct your mistake.

[jaylemur]

Does this new option only affect the GIT connection ?
or also the SVN connection in case of command "git svn clone" ?

[dscho]

This option affects only the Git connection for now. I fear that git svn (by virtue of being a Perl script, using Subversion's Perl bindings) may be unaffected. Maybe you can find out how Subversion accesses HTTPS (I.e. whether it uses cURL)?