Last Comment Bug 473944 - Trust anchor is not trusted when requireFreshInfo flag is set.
: Trust anchor is not trusted when requireFreshInfo flag is set.
Status: RESOLVED FIXED
PKIX
:
Product: NSS
Classification: Components
Component: Tools (show other bugs)
: trunk
: All All
: -- normal (vote)
: 3.12.3
Assigned To: Alexei Volkov
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-01-16 06:12 PST by Slavomir Katuscak
Modified: 2009-04-01 13:33 PDT (History)
0 users
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Config - failing revocation tests. (1.06 KB, text/plain)
2009-01-19 08:36 PST, Slavomir Katuscak
no flags Details
Remove code dups that checks chain agains trust anchors(not for review) (12.30 KB, patch)
2009-01-20 16:16 PST, Alexei Volkov
no flags Details | Diff | Splinter Review
Patch v2 - simplify code by removing duplicated block of code. (13.86 KB, patch)
2009-01-20 16:43 PST, Alexei Volkov
no flags Details | Diff | Splinter Review
Patch v3 - code clean up (47.92 KB, patch)
2009-01-23 10:40 PST, Alexei Volkov
no flags Details | Diff | Splinter Review
Patch v4 - fixes for the problems found during the review (49.05 KB, patch)
2009-02-02 17:46 PST, Alexei Volkov
no flags Details | Diff | Splinter Review
Patch v5(v4 againt old file revision) - fixes for the problems found during the review (48.29 KB, patch)
2009-02-03 10:02 PST, Alexei Volkov
nelson: review+
Details | Diff | Splinter Review
Additional changes required by patch v5 (2.49 KB, patch)
2009-02-03 10:03 PST, Alexei Volkov
no flags Details | Diff | Splinter Review
Patch v2 - Additional changes required by patch v5 (1.88 KB, patch)
2009-02-04 10:14 PST, Alexei Volkov
no flags Details | Diff | Splinter Review
Patch v3 - Additional changes required by patch v5 (2.91 KB, patch)
2009-02-04 10:19 PST, Alexei Volkov
nelson: review+
Details | Diff | Splinter Review
Supl patch v1 - new changes required by main patch (3.77 KB, patch)
2009-03-02 12:32 PST, Alexei Volkov
nelson: review+
Details | Diff | Splinter Review

Description Slavomir Katuscak 2009-01-16 06:12:45 PST
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.
Comment 1 Slavomir Katuscak 2009-01-19 08:36:38 PST
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.
Comment 2 Alexei Volkov 2009-01-20 16:16:02 PST
Created attachment 357883 [details] [diff] [review]
Remove code dups that checks chain agains trust anchors(not for review)
Comment 3 Alexei Volkov 2009-01-20 16:43:04 PST
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.
Comment 4 Slavomir Katuscak 2009-01-21 08:14:56 PST
(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 ?
Comment 5 Alexei Volkov 2009-01-21 11:45:19 PST
(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.
Comment 6 Alexei Volkov 2009-01-23 10:40:01 PST
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.
Comment 7 Alexei Volkov 2009-02-02 17:46:28 PST
Created attachment 360215 [details] [diff] [review]
Patch v4 - fixes for the problems found during the review
Comment 8 Alexei Volkov 2009-02-03 10:02:13 PST
Created attachment 360333 [details] [diff] [review]
Patch v5(v4 againt old file revision) - fixes for the problems found during the review
Comment 9 Alexei Volkov 2009-02-03 10:03:41 PST
Created attachment 360334 [details] [diff] [review]
Additional changes required by patch v5
Comment 10 Nelson Bolyard (seldom reads bugmail) 2009-02-03 14:53:16 PST
Comment on attachment 360333 [details] [diff] [review]
Patch v5(v4 againt old file revision) - fixes for the problems found during the review

r=nelson
Comment 11 Alexei Volkov 2009-02-04 10:14:46 PST
Created attachment 360537 [details] [diff] [review]
Patch v2 - Additional changes required by patch v5
Comment 12 Alexei Volkov 2009-02-04 10:19:15 PST
Created attachment 360538 [details] [diff] [review]
Patch v3 - Additional changes required by patch v5
Comment 13 Nelson Bolyard (seldom reads bugmail) 2009-02-04 13:25:07 PST
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.
Comment 14 Alexei Volkov 2009-02-12 09:13:54 PST
committed.
Comment 15 Alexei Volkov 2009-03-02 12:32:21 PST
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.
Comment 16 Nelson Bolyard (seldom reads bugmail) 2009-03-03 16:29:00 PST
Comment on attachment 364982 [details] [diff] [review]
Supl patch v1 - new changes required by main patch

r=nelson

Note You need to log in before you can comment on or make changes to this bug.