Trust anchor is not trusted when requireFreshInfo flag is set.

RESOLVED FIXED in 3.12.3

Status

NSS
Tools
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: Slavomir Katuscak, Assigned: Alexei Volkov)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: PKIX)

Attachments

(4 attachments, 6 obsolete attachments)

(Reporter)

Description

9 years ago
Root cert is NOT revoked - when I set leaf test and only check root cert I got 2 errors - one that root is revoked and one that it's not trusted (even if set as trust anchor). Both of these errors are wrong.

vfychain -d AllDB -pp -vv  -g leaf -h requireFreshInfo -m crl    Root.der  -t Root.der
Chain is bad, -8172 = Peer's certificate issuer has been marked as not trusted by the user.
PROBLEM WITH THE CERT CHAIN:
CERT 1. Root [Certificate Authority]:
  ERROR -8180: Peer's Certificate has been revoked.
  ERROR -8172: Peer's certificate issuer has been marked as not trusted by the user.
    CN=Root ROOT CA,O=Root,C=US

When I set chain instead of leaf then it's OK when I test only this one cert. But when there is a chain of more certs, I still receive message that root is not trusted:

vfychain -d AllDB -pp -vv  -g chain -h requireFreshInfo -m crl    EE12CA1.der  -t Root.der
Chain is bad, -8172 = Peer's certificate issuer has been marked as not trusted by the user.
PROBLEM WITH THE CERT CHAIN:
CERT 4. Root [Certificate Authority]:
  ERROR -8172: Peer's certificate issuer has been marked as not trusted by the user.
    CN=Root ROOT CA,O=Root,C=US

Without requireFreshInfo flag, CRL info from other CA databases than main DB set by -d parameter is not fetched and revoked certificates are not recognized as revoked.
(Reporter)

Comment 1

9 years ago
Created attachment 357667 [details]
Config - failing revocation tests.

Alexei, please try to run this tests config (would work with patch from bug 473790). There are 2 problems, trust anchor is not trusted (seems that message that Root is not trusted is there even if is not set as trust anchor) and certificate EE1 that is not revoked is marked as revoked.

I need to know if my steps are wrong, or if there is a bug in code. Thanks.
(Assignee)

Comment 2

9 years ago
Created attachment 357883 [details] [diff] [review]
Remove code dups that checks chain agains trust anchors(not for review)
(Assignee)

Comment 3

9 years ago
Created attachment 357888 [details] [diff] [review]
Patch v2 - simplify code by removing duplicated block of code.

Slavo, please try this patch. You should get cert revoked status in both cases.
The patch removed a bunch of duplicate code and makes sure libpkix returns a proper error.

The cert is considered to be revoked, because you requested to make it revoked if there is not fresh info is available in db. So, since you have not installed crl into it -> there is not fresh revocation info in the db -> cert reported as revoked.
Attachment #357883 - Attachment is obsolete: true
(Reporter)

Comment 4

9 years ago
(In reply to comment #3)
> The cert is considered to be revoked, because you requested to make it revoked
> if there is not fresh info is available in db. So, since you have not installed
> crl into it -> there is not fresh revocation info in the db -> cert reported as
> revoked.

I don't really understand this. Where should I install crl and how ? If I set some cert as trust anchor it should mean that I trust this cert, so how it's possible that it's reported as revoked ?
(Assignee)

Comment 5

9 years ago
(In reply to comment #4)
> (In reply to comment #3)
> I don't really understand this. Where should I install crl and how ? If I set
> some cert as trust anchor it should mean that I trust this cert, so how it's
> possible that it's reported as revoked ?

Have you applied the patch? I guess not.

By specifying "requireFreshInfo" flag, you have asked libpkix to revoke the cert if there is no fresh/current revocation information available. There was no crl and ocsp informaion, so the cert was considered to be revoked. It has nothing do to with trust.

The error message that you see is confusing. The patch should fix it.
(Assignee)

Updated

9 years ago
Attachment #357888 - Attachment description: Patch v2 - remove dup code(Not for review) → Patch v2 - simplify code by removing duplicated block of code.
Attachment #357888 - Flags: review?(nelson)
(Assignee)

Updated

9 years ago
Attachment #357888 - Flags: review?(nelson)
(Assignee)

Comment 6

9 years ago
Created attachment 358439 [details] [diff] [review]
Patch v3 - code clean up

This simplifies chain checks against user defined trust anchors. Patch v2 was based on assumption, that all certs as placed in trust domain temporary cache, which is not the case.

This patch fixes the problem that patch v2 has by appending trust anchor that passed validation criteria into the beginning of the candidate cert list.
Attachment #357888 - Attachment is obsolete: true
(Assignee)

Updated

9 years ago
Attachment #358439 - Flags: review?(nelson)
(Assignee)

Updated

9 years ago
Whiteboard: PKIX
(Assignee)

Comment 7

9 years ago
Created attachment 360215 [details] [diff] [review]
Patch v4 - fixes for the problems found during the review
Attachment #358439 - Attachment is obsolete: true
Attachment #360215 - Flags: review?(nelson)
Attachment #358439 - Flags: review?(nelson)
(Assignee)

Comment 8

9 years ago
Created attachment 360333 [details] [diff] [review]
Patch v5(v4 againt old file revision) - fixes for the problems found during the review
Attachment #360215 - Attachment is obsolete: true
Attachment #360333 - Flags: review?(nelson)
Attachment #360215 - Flags: review?(nelson)
(Assignee)

Comment 9

9 years ago
Created attachment 360334 [details] [diff] [review]
Additional changes required by patch v5
Attachment #360334 - Flags: review?(nelson)
Attachment #360333 - Flags: review?(nelson) → review+
Comment on attachment 360333 [details] [diff] [review]
Patch v5(v4 againt old file revision) - fixes for the problems found during the review

r=nelson
(Assignee)

Comment 11

9 years ago
Created attachment 360537 [details] [diff] [review]
Patch v2 - Additional changes required by patch v5
Attachment #360334 - Attachment is obsolete: true
Attachment #360537 - Flags: review?(nelson)
Attachment #360334 - Flags: review?(nelson)
(Assignee)

Comment 12

9 years ago
Created attachment 360538 [details] [diff] [review]
Patch v3 - Additional changes required by patch v5
Attachment #360537 - Attachment is obsolete: true
Attachment #360538 - Flags: review?(nelson)
Attachment #360537 - Flags: review?(nelson)
Comment on attachment 360538 [details] [diff] [review]
Patch v3 - Additional changes required by patch v5

r=nelson, assuming that all the NSS tests still pass with this change.
Attachment #360538 - Flags: review?(nelson) → review+
(Assignee)

Comment 14

9 years ago
committed.
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
(Assignee)

Comment 15

9 years ago
Created attachment 364982 [details] [diff] [review]
Supl patch v1 - new changes required by main patch

after two set of certs(trusted and not trusted) are found, need to make sure, that in the merged list we only have one copy of each certificate. The preference goes to a certificate from a list of user trusted anchors.
Attachment #364982 - Flags: review?(nelson)
(Assignee)

Updated

9 years ago
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 364982 [details] [diff] [review]
Supl patch v1 - new changes required by main patch

r=nelson
Attachment #364982 - Flags: review?(nelson) → review+
(Assignee)

Updated

8 years ago
Status: REOPENED → RESOLVED
Last Resolved: 9 years ago8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.