1817026 - Consider anything signed by "Microsoft Corporation" to not be an untrusted module

Status: RESOLVED FIXED

(Core :: DLL Services, defect, P3)

Description

[Greg Stoll]

We attempt to do this already in ModuleEvaluator::GetTrust(), but on my system I see the following three DLLs on about:third-party -

• msvcp140.dll
• vcruntime140.dll
• vcruntime140_1.dll

All of these DLLs have two digital signatures: one by "Microsoft Windows Software Compatibility Publisher" and one by "Microsoft Corporation". On about:third-party, the "Microsoft Windows Software Compatibility Publisher" is the one that shows up.

Fixing this would mean less clutter for about:third-party (and less irrelevant telemetry as well)

Note that I'm assuming that this is two separate signatures; if there's some sort of chain here where "real" third-party DLLs can have a signature by "Microsoft Corporation" then we should reevaluate this.

Comment 1

[Greg Stoll]

After digging into this a bit, the data we're getting back from the Windows API CryptMsgGetParam() (in the SignedBinary constructor) doesn't match what Windows Explorer is showing. For msvcp140.dll, for example, the API only shows the "Microsoft Windows Software Compatibility Publisher" signer. I'm not sure if this is a limitation of the API or if I'm not querying it in the right way or something. Regardless, I'm going to shelve this work for now since it's a fairly minor issue.

Comment 2

[Greg Stoll]

OK, I poked around some more. The "extra" signatures are coming from various catalog files.

Ideally, we would look in the catalog files to see if there are any signatures by Microsoft and, if so, treat the DLL as not third-party. We already have some code to do something like this, but for some reason it's not finding the catalog signatures that sigcheck from Sysinternals is finding.

I'm going to keep trying to make this work. A backup plan would be to add "Microsoft Windows Software Compatibility Publisher" to the list of signatures that count as MIcrosoft ones, but I think that would be a bit less reliable than the first plan.

Comment 3

[Greg Stoll]

I've spent some more time on this and am stuck getting the catalog files. It's also weird that the way that we get the hash of the file (CryptCATAdminCalcHashFromFileHandle2) doesn't seem to match what I get when I actually hash the file contents with certutil.exe.

Anyway, after thinking about it some more I think extending the check to "Microsoft Windows Software Compatibility Publisher" is good enough for our purposes; it still indicates an ownership by Microsoft. So I'm going to do that.

Comment 4

[Greg Stoll]

Attachment: Bug 1817026 - count more DLLs as not third-party if they're signed by Microsoft r=yjuglaret

Comment 5

[Greg Stoll]

After some discussion on Phabricator it seems like some third-party DLLs on some other systems have their first signature as "Microsoft Windows Hardware Compatibility Publisher". This makes me uneasy about my proposed solution. I think if we want to fix this we have to figure out how to get the signature from the catalog file and compare it with "Microsoft Corporation".

Comment 6

[Greg Stoll]

To answer some of my past questions:

• Looking at msvcp140.dll specifically, it has multiple signatures in the file; there is no signature for it in the catalog file, so that's not the problem. You can see this by running sigcheck -i c:\windows\system32\msvcp140.dll and seeing that none of the Catalog: lines end with .cat - instead they all just list the path to the file.
• Given that, this isn't really relevant anymore, but the hash algorithm that CryptCATAdminCalcHashFromFileHandle2 uses by default is PESHA1, which certutil doesn't support anyhow.

Comment 7

[Greg Stoll]

Attachment: Bug 1817026 part 1: Allow returning multiple certificate signers from GetBinaryOrgNames() r=yjuglaret

Comment 8

[Greg Stoll]

Attachment: Bug 1817026 part 2: Returning nested certificate signers from GetBinaryOrgNames() r=yjuglaret

Depends on D179552

Comment 9

[Greg Stoll]

Attachment: Bug 1817026 part 3: Add test for GetBinaryOrgNames() r=yjuglaret

Depends on D179553

Comment 10

[Greg Stoll]

For the record, the "Microsoft Corporation" certificate was nested inside the first certificate - see part 2 of the patch for more details.

Comment 11

[Greg Stoll]

Attachment: Bug 1817026 part 1: Add a return parameter for whether a binary has a nested Microsoft signature to GetBinaryOrgNames() r=yjuglaret

Comment 12

[Greg Stoll]

Attachment: Bug 1817026 part 2: Calculate whether there's a nested Microsoft signature r=yjuglaret

Depends on D180386

Comment 13

[Greg Stoll]

Attachment: Bug 1817026 part 3: Add test for GetBinaryOrgNames() r=yjuglaret

Depends on D180387

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/da980372e28c
part 1: Add a return parameter for whether a binary has a nested Microsoft signature to GetBinaryOrgNames() r=yjuglaret
https://hg.mozilla.org/integration/autoland/rev/ba0f7a8ab8c0
part 2: Calculate whether there's a nested Microsoft signature r=yjuglaret
https://hg.mozilla.org/integration/autoland/rev/02929f9e3e89
part 3: Add test for GetBinaryOrgNames() r=yjuglaret

Comment 15

[Sandor Molnar[:smolnar]]

Backed out for causing build bustages in toolkit/xre/dllservices/mozglue/Authenticode.cpp

Backout link: https://hg.mozilla.org/integration/autoland/rev/a07bfd2d12e37ba92fd66a563b49fd9feb7e0357

Push with failures

Failure log -> /builds/worker/checkouts/gecko/toolkit/xre/dllservices/mozglue/Authenticode.cpp:186:32: error: use of undeclared identifier 'szOID_NESTED_SIGNATURE'

Comment 16

[Greg Stoll]

Apologies, I'll take a look.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4c5de376b6dd
part 1: Add a return parameter for whether a binary has a nested Microsoft signature to GetBinaryOrgNames() r=yjuglaret
https://hg.mozilla.org/integration/autoland/rev/61c1cf6e1be4
part 2: Calculate whether there's a nested Microsoft signature r=yjuglaret
https://hg.mozilla.org/integration/autoland/rev/eb926a42fef7
part 3: Add test for GetBinaryOrgNames() r=yjuglaret

Comment 18

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/4c5de376b6dd
https://hg.mozilla.org/mozilla-central/rev/61c1cf6e1be4
https://hg.mozilla.org/mozilla-central/rev/eb926a42fef7