Bug 99453 - Add new bool attribute, to distinguish between "is Mozilla CA cert" and "isn't Mozilla CA cert"
Summary: Add new bool attribute, to distinguish between "is Mozilla CA cert" and "isn'...
Status: RESOLVED MOVED
Alias: None
Product: p11-glue
Classification: Unclassified
Component: p11-kit (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Stef Walter
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-01-18 19:52 UTC by Kai Engert
Modified: 2017-02-06 10:29 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments
isrg.p11-kit (15.68 KB, text/plain)
2017-01-29 22:53 UTC, Kai Engert
Details
subset-isrg.p11-kit (2.88 KB, text/plain)
2017-01-29 23:53 UTC, Kai Engert
Details
patch v1, work in progress (3.43 KB, patch)
2017-01-30 08:43 UTC, Kai Engert
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kai Engert 2017-01-18 19:52:19 UTC
The Firefox application wishes to distinguish between root CA certificates that Mozilla has approved for inclusion in the public web PKI, and which Mozilla ships as part of Firefox,
and between other CAs that are installed locally.

In the past Mozilla used the token name to distinguish, but that had problems.

Recently, Mozilla has changed to check for the "has roots" attribute that libnssckbi.so contains, but that has problems, too.

(See also bug 66161 comment 6 for context.)

The new suggestion is as follows:

For each certificate that p11-kit-trust provides, it should provide a new boolean attribute, which we will define in this tracking bug.

For discussion purposes let's use the name:
  CKA_MOZILLA_CA_POLICY

The meaning shall be:

  If the attribute is set to TRUE, the application can assume it is one 
  of the CAs that Mozilla has included as part of their Mozilla CA policy.

  If the attribute is set to FALSE, it's *not* one of the CAs that 
  Mozilla includes.

For backwards compatibility, if an application uses an older version of p11-kit, which doesn't yet provide this new attribute:

  If the attribute is missing, treat the same as "exists and TRUE".

The justification is, that today, the most recent Firefox assumes that all CAs in the p11-kit-trust module are Mozilla CAs.


The code in p11-kit-trust must decide which certificates get this new flag set to TRUE, and which ones set to FALSE.

My suggestion is that we introduce a new rule for the filenames that p11-kit-trust dynamically reads:

  If a certificate is consumed from a source filename that has the prefix
    mozilla-ca-policy-
  then it gets the flag CKA_MOZILLA_CA_POLICY set to TRUE.

  If a certificate is consumed from a source filename, that doesn't match
  that prefix, the flag CKA_MOZILLA_CA_POLICY is set to FALSE.
Comment 1 Kai Engert 2017-01-18 20:27:42 UTC
In case you don't like the idea of using filename prefixes to distinguish between Mozilla-Policy-CAs and other CAs:

We could also require, that in order for this attribute to be set, a special file must be required on the filesystem, which specifially whitelists the certificates that should get the attribute set to TRUE.

I have a slight preference for the prefix solution, because it seems simpler to implement.

(Given that p11-kit-trust dynymically scans for new files, it seems like a burden to always have to check for that whitelist file to change, too, and go everywhere to update everything.)

If you think we shouldn't use filename prefixes, but that we should use the separate whitelist file, please give your arguments.
Comment 2 Stef Walter 2017-01-19 07:58:39 UTC
(In reply to Kai Engert from comment #1)
> In case you don't like the idea of using filename prefixes to distinguish
> between Mozilla-Policy-CAs and other CAs:

I agree that using the filename seems rather hacky.

Lets use a real attribute. p11-kit has support for implementing new attributes and loading those from .p11-kit files.

See the trust/input/verisign-v1.p11-kit file for such an example.

To add a new attribute the following files functions need to be modified. You can use 'git grep' to find them

common/pkcs11x.h   (add the new constant)
trust/persist.c    format_bool()
trust/constants.c  p11_constant_types
trust/constants.c  p11_constant_certs
trust/builder.c    certificate_schema

Hopefully that's enough to get you started.
Comment 3 Stef Walter 2017-01-19 08:05:32 UTC
In addition FWIW, the .p11-kit files a strictly a superset of the PEM file format which allows arbitrary content before/after the PEM blocks. The .p11-kit files can continue to be parsed as PEM files by other tooling.

The trust/input/verisign-v1.p11-kit is such an example.
Comment 4 Kai Engert 2017-01-27 10:39:00 UTC
Thanks Stef, your guidance is very helpful.

Two more questions:

(a)
Is the BEGIN TRUSTED format supported, too, for .p11-kit files?

[p11-kit-object-v1]
-----BEGIN TRUSTED CERTIFICATE-----
...


(b)
It isn't obvious why a change is necessary in
trust/constants.c  p11_constant_certs

Does this define different types of certificates? Do we need to add a new cert type?
Comment 5 Kai Engert 2017-01-27 12:41:04 UTC
(In reply to Kai Engert from comment #4)
> (a)
> Is the BEGIN TRUSTED format supported, too, for .p11-kit files?
> 
> [p11-kit-object-v1]
> -----BEGIN TRUSTED CERTIFICATE-----


No. I run into:
  trust/persist.c: p11_lexer_msg (pb->lexer, "unsupported pem block in store");

Currently, I use the trusted PEM file format as input to p11-kit-trust, which allows me to set the individual trust/distrust flags for tls/email/etc.

Your suggestion is that I use the p11-kit file format instead.

This means, we must:
- either enhance the p11-kit files to work with the 
  BEGIN TRUSTED CERTIFICATE format

- or switch to the regular BEGIN CERTIFICATE as the input format 
  provided by the ca-certificates package,
  and use a different way to set the trust/distrust flags,
  using p11-kit flags.
  (I'll try to find out if we already have those flags.)
Comment 6 Kai Engert 2017-01-27 12:48:26 UTC
(In reply to Kai Engert from comment #5)
> - or switch to the regular BEGIN CERTIFICATE as the input format 
>   provided by the ca-certificates package,
>   and use a different way to set the trust/distrust flags,
>   using p11-kit flags.
>   (I'll try to find out if we already have those flags.)

Looks like they exist already:
	CT (CKA_TRUST_SERVER_AUTH, "trust-server-auth")
	CT (CKA_TRUST_CODE_SIGNING, "trust-code-signing")
	CT (CKA_TRUST_EMAIL_PROTECTION, "trust-email-protection")

I'll try to switch to BEGIN CERTIFICATE.
Comment 7 Kai Engert 2017-01-27 13:11:20 UTC
Is my understanding correct, that setting:
  trust-server-auth: false
means "neutral trust for server auth"?

All I see it the x-distrusted attribute, which I guess is active for any use?

(If correct, then I don't see a way to individually distrust per type of use. We haven't yet required this level of control, but the begin-trusted format allows to transport this level of detail.)
Comment 8 Kai Engert 2017-01-27 19:52:40 UTC
The approach from comment 2 and comment 3 doesn't seem to be very straightforward. The code that parses a certificate inline inside a [p11-kit-object-v1] isn't connected to the code that creates all these trust objects when consuming an openssl trusted format file. A simply approach to read an ulong attribute probably doesn't work, because it seems you need code that goes indirectly through creating EKUs to make that work. It's not immediately obvious how that behaves internally.

An alternative approach, suggested by Daiki, could be to keep using the openssl trusted format, and use additional p11-kit files, that staple the remaining attributes to the certs. That would require to create the keys from the CAs, and create the separate files that add the attribute. But the existing file format doesn't support yet adding attributes, only extensions. How to add attributes also requires to learn more about the code. And I'm not happy that we'd have to create two separate sets of inputs that must be combined by p11-kit to create the desired result.

I wish we could use a simpler approach. You didn't like the filename hack, but could we parameterize on the input?

Could we use a variation of the BEGIN TRUSTED CERTIFICATE header?

For example, if the header is BEGIN TRUSTED MOZ_CA_CERTIFICATE, then p11-kit could handle exactly as with B.T.C., but add the new attribute to it with true, and when it's getting the regular B.T.C. header, it adds the new attribute with false?

I'm guessing that would be much simpler to implement. I see there's existing code to merge two sets of attributes, so in parse_openssl_trusted_certificate, after having created all the attributes, we could merge the new moz-policy-ca attribute into the attr variable?
Comment 9 Kai Engert 2017-01-28 08:53:26 UTC
Here's the latest episode of "Kai's has too many hacky ideas", but as usual, I hope that it could be acceptable.

We already use the concept of different subdirectories having different meaning for how the input is treated (in addition to the contents of the files), for:
- "anchors" that are trusted, without requiring trust attributes
- "blacklist" that are distrusted, without requiring distrust attributes

Could we have another subdirectory:
- "mozilla-ca-policy", which requires that trust flags are used in the
                       files, but all certificates found here get the
                       new attribute?


Also, earlier in this bug I said:
> For backwards compatibility, if an application uses an older version of
> p11-kit, which doesn't yet provide this new attribute:
>
>  If the attribute is missing, treat the same as "exists and TRUE".

I'm not longer sure that's a good idea. Given that we have many code paths that can create certificate objects, it seems easy to forget about this attribute, and get this treatment, without it being intended.

Also, if ever anyone implements other tokens having roots, and loads them into a Mozilla app (maybe that already exists in some deployment and we don't know about it), then those roots from those tokens would also get the Mozilla treatment.

It might be safer to declare that "missing" means "attribute is FALSE".
Comment 10 Kai Engert 2017-01-29 22:52:22 UTC
I met with Stef and we discussed this issue.

Stef's preference is not to use the filename/directory hacks, but to use the [p11-kit-object-v1] file format that allows to flexibly control all attributes.

After learning that I must create two separate objects (cert and trust), I tried but couldn't get it to work.

The problematic issue are the separate trust flags for "server-auth", "email" and "code-signing". While it's easy to import a CA that is trusted for all usages, I couldn't figure out how to make it trusted for a subset of usages, only.

(Which can be tested e.g. using "trust list --purpose=email" .)

Stef today created a patch to p11-kit's trust utility, that adds a "dump" functionality, which can completely dump all the attributes that are found inside the p11-kit-trust.so object, into the [p11-kit-object-v1] file format.

From that dump, I was able to understand what input is required.

It's a rather complicated input. It can require up to 6 separate p11-kit-object-v1 sections, containing information that needs to be extracted from an input certificate:
- the certificate
- an nss-trust object
- one, two or three trust assertion objects
- some certificates require a stapled extended-key-usage extension

As an example let's look at the Let's Encrypt CA named "ISRG Root X1".
I'll attach the subset related to that CA.

There are several attributes which are tricky to create. It isn't obvious which attributes could be omitted.

For example,
  trust-crl-sign: nss-trusted-delegator
does the tool creating the input really need to find out if the cert is allowed to do that, by extracing usages from the certificate, and set it correctly?

Another example is the decision, when is a stapled EKU object required, and when isn't it? The dump only adds it for a subset of certificates.

I'm a bit worried that things could go wrong when creating the input, and the the wrong trust could be produced. To avoid that, we'd have to carefully check that anything passed into p11-kit-trust is really producing the expected data.

I wish we'd simply have a way to reuse all the existing magic, which already works inside of p11-kit-trust, when it parses a BEGIN TRUSTED CERTIFICATE, and already knows how to create all those trust objects and extensions internally, and could allow us to simply stick additional attributes on top of it.


In other words, it would be nice if we could simply use the following, (without having to produce all those details seen in the attachment):

-----BEGIN TRUSTED CERTIFICATE-----
...
-----END TRUSTED CERTIFICATE-----
some-extra-flag: true
Comment 11 Kai Engert 2017-01-29 22:53:05 UTC
Created attachment 129219 [details]
isrg.p11-kit
Comment 12 Kai Engert 2017-01-29 23:52:21 UTC
Maybe it's simpler than I thought.

Apparently there's an x-certificate-extension 2.5.29.37 for every certificate, and the value is one out of 8 possible combinations for the email/server/code trust bits.

Passing in an nss-trust object and the trust-assertion objects seems unnecessary, the two objects certificate and extension seems sufficient.
Comment 13 Kai Engert 2017-01-29 23:53:04 UTC
Created attachment 129220 [details]
subset-isrg.p11-kit
Comment 14 Kai Engert 2017-01-30 08:43:22 UTC
Created attachment 129225 [details] [review]
patch v1, work in progress
Comment 15 Kai Engert 2017-02-02 15:07:55 UTC
The patch seems to work, when also using the proposed changes to NSS.

I've submitted a pull request:
https://github.com/p11-glue/p11-kit/pull/46
Comment 16 Stef Walter 2017-02-06 10:29:23 UTC
The work is moved to github: https://github.com/p11-glue/p11-kit/pull/46


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.