Allow setting GIT_OPT_SET_SSL_CERT_LOCATIONS option in libgit2

[webknjaz]

Apparently, there's GIT_OPT_SET_SSL_CERT_LOCATIONS (ref: https://libgit2.org/libgit2/#HEAD/group/libgit2/git_libgit2_opts) option available in libgit2 (src: https://github.com/libgit2/libgit2/blob/12c6e1f/src/settings.c#L180-L186).

OpenSSL we have compiled under manylinux1 image has the following completely non-default address for OPENSSLDIR config value:

[root@7a356be85129 ~]# /opt/pyca/cryptography/openssl/bin/openssl version -d
OPENSSLDIR: "/opt/pyca/cryptography/openssl"

But even if we wanted to set something more generic we simply wouldn't be able to agree on what such generic value would be. Every OS has it's own "ideal" location in their minds: http://gagravarr.org/writing/openssl-certs/others.shtml

That's why it's important to expose this option so that it could be specified by users who could also have some non-default places for certificates for hardening reasons, for example.

Going forward, Python itself has ssl module which might have a better idea of where the current installation stores its certificates. This is the location default of my Gentoo Linux based laptop, for example:

➜ ipython
Python 3.7.1 (default, Jan 28 2019, 08:25:13) 
Type 'copyright', 'credits' or 'license' for more information
IPython 7.1.1 -- An enhanced Interactive Python. Type '?' for help.

In [1]: import ssl                                                                                                                                                                             

In [2]: ssl.get_default_verify_paths()                                                                                                                                                         
Out[2]: DefaultVerifyPaths(cafile=None, capath='/etc/ssl/certs', openssl_cafile_env='SSL_CERT_FILE', openssl_cafile='/etc/ssl/cert.pem', openssl_capath_env='SSL_CERT_DIR', openssl_capath='/etc/ssl/certs')

So in order to provide better consistency with the runtime, I suggest to not only expose this option but also set the location to match Python's defaults, during initialization.

[imbuedhope]

Should be able to extend src/options.c and pygit2/settings.py to add support for it as long as libgit2 is being built with GIT_OPENSSL or GIT_MBEDTLS (can someone double check that the wheels we're shipping do? Not sure how / what cmake sets by default)

Not quite sure what the most pythonic way to handle Either parameter may be 'NULL', but not both. would be. It doesn't make sense to make parameters that have defaults as required.

Maybe something like this in the Settings class?

def set_ssl_cert_locations(self, cert_file=None, cert_path=None):
    if not cert_file or cert_path:
         # set cert_file and cert_path to be values from Default 
    # call libgit2 hooks

Calling it with no paramters would set try to set the defaults from the ssl module, but if there were any parameters it would run it based on those.

[webknjaz]

can someone double check that the wheels we're shipping do?

Currently, it looks like Windows wheels are using WinHTTP: https://ci.appveyor.com/project/jdavid/pygit2/branch/master/job/8p52oln33yo6n41t#L70

For manylinux1 wheels it's going to be statically compiled OpenSSL once #875 is merged. But that's not yet shipped.

Not quite sure what the most pythonic way to handle Either parameter may be 'NULL', but not both. would be. It doesn't make sense to make parameters that have defaults as required.

It's usually:

if param1 is None and param2 is None:
    raise TypeError('Please provide at least one of param1 or param2')

While I like your suggestion to pull in ssl defaults for this case it can be confusing for end-users as it's not an obvious behavior. I'd rather provide another method reset_ssl_cert_locations for that. Explicit is better than implicit.

[webknjaz]

We could also emulate kwargs-only signature to enforce explicitly stating what arg users provide which is better readable for everyone seeing the calling code for the first time.

[imbuedhope]

Digging around a bit more, it looks like https://github.com/libgit2/pygit2/blob/master/pygit2/settings.py#L47 treats the options as properties of the Settings object. I think it's probably best to stay consistent with that scheme.

Probably best to just make Settings.ssl_cert_file and Settings.ssl_cert_path as unreadable properties. libgit2 doesn't seem to support a GIT_OPT_GET_SSL_CERT_LOCATIONS either.

If the user wants to set it to the paths from ssl it should be up to them to do. Adding a reset_ssl_cert_locations would be deceptive since that might not actually be what we're doing, depending on what libgit2 is pointing to.

Are there scenarios where setting both cert_file and cert_path at the same time makes sense? The libgit2 API reference doesn't seem to describe what happens if a file and a path are both specificed. It also feels like bad certificate organizing to need to both in the first place, not sure if we need to support that scenario.

[webknjaz]

properties is a way to go. I like it. We still need to set values from ssl on load, otherwise, users would be having broken https out of the box.

I don't know about scenarios with both cert_file and cert_path set but I'd assume that it would be case "use this file if it exists, otherwise look at this folder" which might make sense in case of installing things onto third-party user machines with too many unknowns. This could also be a mechanism of easy override for defaults...

[webknjaz]

@imbuedhope are you planning to send a PR?

[imbuedhope]

I was going to take a shot at it this weekend. I'm pretty swamped during the work week.

[webknjaz]

Ah, okay. Thanks!