1035414 - 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

Status: RESOLVED FIXED

(Core :: Security: PSM, enhancement)

Description

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

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.

Comment 1

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Attachment: Part 1: Test that mozilla::pkix checks the issuer itself

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.

Comment 2

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Attachment: Part 2: Check that the subject's issuer field matches the potential issuer's subject field

Comment 3

[J.C. Jones [:jcj] (he/him)]

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.

Comment 4

[J.C. Jones [:jcj] (he/him)]

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.

Comment 5

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d96e3eaf20d9
https://hg.mozilla.org/integration/mozilla-inbound/rev/2018ebe53273

Thanks for the reviews!

Comment 6

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/d96e3eaf20d9
https://hg.mozilla.org/mozilla-central/rev/2018ebe53273

Comment 7

[[:mmc] Monica Chew (no longer reading bugmail)]

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...