1512471 - Proxy certificate validation from Socket Process/Socket Thread to Parent Process

Status: RESOLVED FIXED

(Core :: Security: PSM, enhancement, P1)

Comment 1

[Honza Bambas (:mayhemer)]

Attachment: wip, backup, needs rebasing

Comment 2

[Honza Bambas (:mayhemer)]

Attachment: wip, only for code bits reference

• please use this patch only for getting some base parts necessary for the IPC plumbing between the processes to carry on the cert verification
• there is PSMChild and *Parent added to manage the new PAuthCertificate protocol providing async messages for cert verification
• there are also some base tools for serialization added
• unfinished: we need to carry some more properties like the host name we verify against via the new PAuthCertificate protocol (host name - a property of the info object - is not serialized)
• definitely unfinished: this needs PBackground or end-points to send from the socket thread on the socket process to some thread on the parent process and back

I never made this work even slightly, so there is probably a lot of work to do...

Comment 3

[Dragana Damjanovic [:dragana]]

Attachment: 1512471 - Proxy the cert verification to the parent process. r=keeler

Comment 4

[Dragana Damjanovic [:dragana]]

Attachment: IPC protocol for the certificate verification. r=keeler

Comment 5

[Dragana Damjanovic [:dragana]]

Attachment: Perform cert verifications on the parent process. r=keeler

Comment 6

[Dragana Damjanovic [:dragana]]

Attachment: Part 2: Perform cert rebuilding on the parent process. r=keeler

Comment 7

[Dragana Damjanovic [:dragana]]

keeler, can you now review this patches?

Comment 8

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

I can start to look at these again, yes. Are you going to land the patches from bug 1546816 on mozilla-central?

Comment 9

[Dragana Damjanovic [:dragana]]

(In reply to Dana Keeler (she/her) (use needinfo) (:keeler for reviews) from comment #8)

I can start to look at these again, yes. Are you going to land the patches from bug 1546816 on mozilla-central?

I though of landing it on larch, but mozilla-central is fine too, probably better.

Comment 10

[Kershaw Chang [:kershaw]]

Hi Dana,

When I tested the patches in this bug, I found something strange. When I tried to connect to www.mysecurex.eu, I always got SEC_ERROR_UNKNOWN_ISSUER. This error returned from PathBuildingStep::CheckResult and the call stack is:

frame #0: 0x00000001153030ce XUL`mozilla::pkix::BuildForward(mozilla::pkix::TrustDomain&, mozilla::pkix::BackCert const&, mozilla::pkix::Time, mozilla::pkix::KeyUsage, mozilla::pkix::KeyPurposeId, mozilla::pkix::CertPolicyId const&, mozilla::pkix::Input const*, unsigned int, unsigned int&) [inlined] mozilla::pkix::PathBuildingStep::CheckResult() const at pkixbuild.cpp:135:8 [opt]
frame #1: 0x00000001153030ce XUL`mozilla::pkix::BuildForward(trustDomain=0x00007000113954b0, subject=0x00007000113951f8, time=(elapsedSecondsAD = 63703127818), requiredKeyUsageIfPresent=<unavailable>, requiredEKUIfPresent=id_kp_serverAuth, requiredPolicy=0x000000011877aa5e, stapledOCSPResponse=0x0000700011395550, subCACount=<unavailable>, buildForwardCallBudget=0x00007000113951f4) at pkixbuild.cpp:375 [opt]
frame #2: 0x00000001153032a5 XUL`mozilla::pkix::BuildCertChain(trustDomain=0x00007000113954b0, certDER=<unavailable>, time=(elapsedSecondsAD = 63703127818), endEntityOrCA=MustBeEndEntity, requiredKeyUsageIfPresent=digitalSignature, requiredEKUIfPresent=id_kp_serverAuth, requiredPolicy=0x000000011877aa5e, stapledOCSPResponse=0x0000700011395550) at pkixbuild.cpp:422:10 [opt]
frame #3: 0x000000011002424e XUL`mozilla::psm::BuildCertChainForOneKeyUsage(trustDomain=0x00007000113954b0, certDER=<unavailable>, time=(elapsedSecondsAD = 63703127818), ku1=digitalSignature, ku2=keyEncipherment, ku3=keyAgreement, eku=id_kp_serverAuth, requiredPolicy=0x000000011877aa5e, stapledOCSPResponse=0x0000700011395550, ocspStaplingStatus=0x0000700011395738) at CertVerifier.cpp:209:7 [opt]
frame #4: 0x00000001100237f0 XUL`mozilla::psm::CertVerifier::VerifyCert(this=0x0000000132148000, cert=<unavailable>, usage=<unavailable>, time=(elapsedSecondsAD = 63703127818), pinArg=0x0000000000000000, hostname="aus5.mozilla.org", builtChain=0x0000700011395750, flags=0, stapledOCSPResponseArg=0x0000000132591d10, sctsFromTLS=0x0000000132591d20, originAttributes=0x0000000132591d58, evOidPolicy=0x000070001139574c, ocspStaplingStatus=0x0000700011395738, keySizeStatus=0x0000700011395748, sha1ModeResult=0x0000700011395744, pinningTelemetryInfo=0x0000700011395758, ctInfo=0x0000700011395770) at CertVerifier.cpp:711:16 [opt]
frame #5: 0x000000011002447a XUL`mozilla::psm::CertVerifier::VerifySSLServerCert(this=0x0000000132148000, peerCert=0x0000000132591d08, stapledOCSPResponse=0x0000000132591d10, sctsFromTLS=<unavailable>, time=<unavailable>, pinarg=0x0000000000000000, hostname=0x0000000132591d38, builtChain=0x0000700011395750, saveIntermediatesInPermanentDatabase=<unavailable>, flags=0, originAttributes=0x0000000132591d58, evOidPolicy=0x000070001139574c, ocspStaplingStatus=0x0000700011395738, keySizeStatus=0x0000700011395748, sha1ModeResult=0x0000700011395744, pinningTelemetryInfo=0x0000700011395758, ctInfo=0x0000700011395770) at CertVerifier.cpp:893:7 [opt]
frame #6: 0x00000001152ad8ee XUL`mozilla::psm::VerifySSLServerCertParent::Run(this=<unavailable>) at VerifySSLServerCertParent.cpp:96:25 [opt]

In NSSCertDBTrustDomain::FindIssuer, I also found CERT_CreateSubjectCertList returns an empty cert list. But CERT_CreateSubjectCertList returns a cert list when socket process is disabled.
So, I compared the input encodedIssuerNameItem to CERT_CreateSubjectCertList with and without socket process, and it's the same.
Do you know what else could make CERT_CreateSubjectCertList return null? Is there anything I missed?

Thanks!

Comment 11

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

I'm assuming this is because the parent process can't find the intermediate certificates that the server sent in the TLS handshake, because only the socket process know about them. This is why it's necessary to call SSL_PeerCertificateChain(fd) in the socket process and send that data to the parent: https://phabricator.services.mozilla.com/D28743#1335802

Comment 12

[Kershaw Chang [:kershaw]]

(In reply to Dana Keeler (she/her) (use needinfo) (:keeler for reviews) from comment #11)

I'm assuming this is because the parent process can't find the intermediate certificates that the server sent in the TLS handshake, because only the socket process know about them. This is why it's necessary to call SSL_PeerCertificateChain(fd) in the socket process and send that data to the parent: https://phabricator.services.mozilla.com/D28743#1335802

Yes, you are right! After sending the peer cert chain to parent process and calling CERT_NewTempCertificate for each cert, it works.
Thanks for your help. I'll update the patch.

Comment 13

[Kershaw Chang [:kershaw]]

Attachment: Bug 1512471 - Add a helper function to convert CertificateTransparencyInfo to CertificateTransparencyStatus

Comment 14

[Kershaw Chang [:kershaw]]

Attachment: Bug 1512471 - Introduce PVerifySSLServerCert for the certificate verification

Comment 15

[Kershaw Chang [:kershaw]]

Attachment: Bug 1512471 - Refactor SSLServerCertVerificationJob for reusing the code

Comment 16

[Kershaw Chang [:kershaw]]

Attachment: Bug 1512471 - Using arrays of array bytes to replace UniqueCERTCertList

Comment 17

[Phabricator Automation]

Comment on attachment 9104698 [details]
Bug 1512471 - Add a helper function to convert CertificateTransparencyInfo to CertificateTransparencyStatus

Revision D50833 was moved to bug 1612362. Setting attachment 9104698 [details] to obsolete.

Comment 18

[Phabricator Automation]

Comment on attachment 9118508 [details]
Bug 1512471 - Using arrays of array bytes to replace UniqueCERTCertList

Revision D58608 was moved to bug 1612362. Setting attachment 9118508 [details] to obsolete.

Comment 19

[Phabricator Automation]

Comment on attachment 9118501 [details]
Bug 1512471 - Refactor SSLServerCertVerificationJob for reusing the code

Revision D58604 was moved to bug 1612362. Setting attachment 9118501 [details] to obsolete.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b5861f950f8f
Introduce PVerifySSLServerCert for the certificate verification r=keeler,jld

Comment 21

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/b5861f950f8f