Closed Bug 1817026 Opened 1 year ago Closed 1 year ago

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

Categories

(Core :: DLL Services, defect, P3)

Firefox 112
defect

Tracking

()

RESOLVED FIXED
116 Branch
Tracking Status
firefox112 --- wontfix
firefox116 --- fixed

People

(Reporter: gstoll, Assigned: gstoll)

References

Details

Attachments

(3 files, 4 obsolete files)

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.

Assignee: nobody → gstoll
Severity: -- → S3
Status: NEW → ASSIGNED
Priority: -- → P3

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.

Assignee: gstoll → nobody
Status: ASSIGNED → NEW

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.

Assignee: nobody → gstoll
Status: NEW → ASSIGNED

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.

Attachment #9321122 - Attachment is obsolete: true

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

Assignee: gstoll → nobody
Status: ASSIGNED → NEW
See Also: → 1829992
Assignee: nobody → gstoll
Status: NEW → ASSIGNED

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.

Depends on D179553

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

Attachment #9336759 - Attachment is obsolete: true
Attachment #9336760 - Attachment is obsolete: true
Attachment #9336761 - Attachment is obsolete: true
Attachment #9338175 - Attachment description: Bug 1817026 part 2: Set whether there's a nested Microsoft signature in GetBinaryOrgNames() r=yjuglaret → Bug 1817026 part 2: Calculate whether there's a nested Microsoft signature r=yjuglaret
Pushed by gstoll@mozilla.com:
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

Apologies, I'll take a look.

Flags: needinfo?(gstoll)
Pushed by gstoll@mozilla.com:
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
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 116 Branch
See Also: → 1808439
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: