Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nss: load libnssckbi.so if no other trust is specified #1414

Closed
wants to merge 2 commits into from

Conversation

kdudka
Copy link
Contributor

@kdudka kdudka commented Apr 13, 2017

The module provides more advanced validation features.

Test plan is available at https://kuix.de/misc/test-distrust/

No change of behavior is intended by this commit.
@mention-bot
Copy link

@kdudka, thanks for your PR! By analyzing the history of the files in this pull request, we identified @bagder, @captain-caveman2k and @rousskov to be potential reviewers.

@kaie
Copy link

kaie commented Apr 13, 2017

Clarification Nit: You said "supports extended validation". Note that the term "extended validation" has a special meaning (https://en.wikipedia.org/wiki/Extended_Validation_Certificate ), and p11-kit doesn't implement that. This is just to make sure that nobody reading this ticket draws incorrect conclusions. I think your intention was to simply say that it provides "additional", or "more advanced" validation features.

Also note that NSS and p11-kit are separate projects. Your patch directly refers to the p11-kit-trust.so module.

I don't know if this works for everyone who uses curl with NSS. There might potentially be environments that don't make use of p11-kit-trust as a system-wide trust store.

Traditionally, the trust store library that is used by NSS is used libnssckbi.so, which is a library that is shipped as part of the NSS distribution. It contains both positive trust and negative distrust information.

p11-kit-trust.so is a fully compatible drop-in replacement for libnssckbi.so

On Fedora Linux specifically, we use the "update-alternatives" mechanism, to provide p11-kit-trust.so as a higher prioritized alternative to libnssckbi.so.

If you want this patch to work, if p11-kit-trust.so isn't installed, but only NSS is installed, then it might make sense to change your patch to load the library named "libnssckbi.so". On Fedora Linux, this should then automatically load the "p11-kit-trust.so" instead.

The module provides more advanced validation features.

Reviewed-by: Kai Engert
@kdudka
Copy link
Contributor Author

kdudka commented Apr 13, 2017

@kaie The above resubmission should address your remarks. Thanks for review!

@kdudka kdudka changed the title nss: load p11-kit-trust.so if no other trust is specified nss: load libnssckbi.so if no other trust is specified Apr 13, 2017
@bagder
Copy link
Member

bagder commented Apr 13, 2017

It's unclear to me what these "extra validation" steps are as this documentation only says how to enable them but not what benefit they provide - and what if you want them and provide a custom cacert file, shouldn't that be possible?

@kaie
Copy link

kaie commented Apr 13, 2017

It's about distrusted certificates.

@kdudka
Copy link
Contributor Author

kdudka commented Apr 13, 2017

Unfortunately, it is not possible with CA certificates loaded directly from files. Neither nss-pem, nor OpenSSL supports it. So this feature gets enabled only if no CA bundle is specified by the user.

@bagder
Copy link
Member

bagder commented Apr 13, 2017

So this is similar to CURLOPT_CRLFILE ? Where are those blacklisted certs, in the standard NSS cert db? (I'm sorry but I'm really bad at NSS internals)

@kdudka
Copy link
Contributor Author

kdudka commented Apr 13, 2017

I should have better explained the idea behind. On systems where p11-kit is configured as a reliable source of trust, one can build curl with --without-ca-bundle to have a more secure default. If someone either specifies a CA bundle at build time or uses CURLOPT_CAINFO at run time, he/she usually wants to use it as the only source of trust (as replacement of the system default).

I guess that @kaie could better explain how it works internally...

@kaie
Copy link

kaie commented Apr 13, 2017

No, it's different from CURLOPT_CRLFILE.
A CRL usually only covers only one CA, and is provided by the CA.

The CA trust that Mozilla provides isn't limited to positive trust. It also covers negative trust, when a CA needs to be distrusted.

There are scenarios when removing a CA isn't sufficient/possible. An example is when someone has been given an intermediate CA, and only that intermediate CA needs to be distrusted. DigiNotar was such an example. Another are mis-issued end entity certificates for high value domains.

In practice, the dynamic revocation mechanisms don't work well, because CRL/OCSP checkins is optional, and failure to dynamically check will be ignored (otherwise you'd be locked out from captive portals).

Thus, when there's a good reason to block some CAs or specific certificates, Mozilla may add distrust information to the Mozilla CA bundle (in certdata.txt).

As of today, curl only loads the positive trust. The suggestion is to also load the distrust.

Although there's a file format available with distrust information in a file format that openssl loads, it seems that often that file isn't used. And nss-pem doesn't support this file format that contains distrust information (Files with the BEGIN TRUSTED CERTIFICATE header is the format that may contain both positive and negative trust.)That means, this approach doesn't seem ideal for curl when using NSS.

The native way for NSS to load both positive and negative trust, which is also the way that Firefox loads it, is by loading the libnssckbi.so module provided by NSS itself. (Or alternative by loading p11-kit-trust.so which is a more advanced dynamic mechanism to configure additional trust/distrust on top of the static lists.)

@bagder bagder added the TLS label Apr 13, 2017
@bagder
Copy link
Member

bagder commented Apr 15, 2017

My only request here is that you try to include some of that explanation in the documentation so that users have a chance of understanding what they're asking for/getting!

@kdudka
Copy link
Contributor Author

kdudka commented Apr 20, 2017

@kaie Could you please suggest how the documentation could be updated?

I am afraid that my wording would not be accurate enough...

@kaie
Copy link

kaie commented Apr 24, 2017

@kdudka how about this:

Starting with curl-7.55.0, if both \fICURLOPT_CAINFO(3)\fP and
\fICURLOPT_CAPATH(3)\fP are unset, NSS-linked libcurl tries to load
libnssckbi.so, which contains a more comprehensive set of trust information
than supported by nss-pem, because libnssckbi.so also includes information
about distrusted certificates.

@kdudka
Copy link
Contributor Author

kdudka commented Apr 24, 2017

@kaie Looks good to me. Thanks!

@kdudka kdudka closed this in e3e8d02 Apr 25, 2017
@kdudka kdudka deleted the nss-trust branch April 25, 2017 12:02
@lock lock bot locked as resolved and limited conversation to collaborators May 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

None yet

4 participants