mozilla::pkix should explicitly check that the child cert's issuer name matches the potential issuer's subject name, instead of relying on the TrustDomain to do the check correctly

RESOLVED FIXED in mozilla37

Status

()

enhancement
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: briansmith, Assigned: briansmith, Mentored)

Tracking

(Blocks 1 bug)

Trunk
mozilla37
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

The documentation for TrustDomain::FindPotentialIssuers (or whatever is is about to be renamed) says:

  // * The subject name of the certificate given to checker.Check must be equal
  //   to encodedIssuerName.

It would be much safer if we didn't rely on the TrustDomain to do that check. Instead, in pkixbuild.cpp we should verify that that condition is true.

Besides being safer, this would allow simpler implementations of the TrustDomain; See the bug I am about to file that depends on this, for an example.
Mentor: brian
Whiteboard: [good next bug]
Assignee: nobody → brian
Whiteboard: [good next bug]
Target Milestone: --- → mozilla37
Since keeler is on vacation, I'm asking jcj to do this review and for mmc to double-check the review. I set sr?keeler on this so that he can check it when he gets back.
Attachment #8541767 - Flags: superreview?(mmc)
Attachment #8541767 - Flags: review?(jjones)
Comment on attachment 8541767 [details] [diff] [review]
Part 1: Test that mozilla::pkix checks the issuer itself

Review of attachment 8541767 [details] [diff] [review]:
-----------------------------------------------------------------

I don't see any problems with the test. LGTM.
Attachment #8541767 - Flags: review?(jjones) → review+
Comment on attachment 8541768 [details] [diff] [review]
Part 2: Check that the subject's issuer field matches the potential issuer's subject field

Review of attachment 8541768 [details] [diff] [review]:
-----------------------------------------------------------------

Given the behavior of NSSCertDBTrustDomain, it seems likely that you planned for this functionality. It looks good to me.
Attachment #8541768 - Flags: review?(jjones) → review+
Comment on attachment 8541767 [details] [diff] [review]
Part 1: Test that mozilla::pkix checks the issuer itself

Review of attachment 8541767 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the delay on this! It wasn't showing up on my bugzilla notifications...
Attachment #8541767 - Flags: superreview?(mmc) → superreview+
Attachment #8541768 - Flags: superreview?(mmc) → superreview+
You need to log in before you can comment on or make changes to this bug.