Feature: Binary artifacts check should allow signed binaries

[jeffmendoza]

Feature request: in a similar vein as #1815 Scorecard Binary Artifacts should allow signed files. See below

---

@randomascii commented 23 minutes ago
My github project necessarily contains five binary files that come from a Microsoft SDK. They are needed for proper operation of the project. Removing them from the repo would require getting them from somewhere else which would be less secure.

These files are all signed. The allstar project could reduce it's "false positive" rate by treating signed files separately, either not warning on them or giving a different type of warning.

Right now if I add a signed binary file to my repo the warning will be identical to the warning I get when I add an unsigned file, despite the fact that the risk level is dramatically different between the two of them. This means that I have to do manual signature checking, which is something that allstar should be doing.

---

[laurentsimon]

sounds reasonable. @randomascii how does verification work? Could you give us some pointers how you verify it's a Microsoft-signed binary?

Also, are there conventions for the filename: .exe, .dll, others?

[randomascii]

Good question. I don't know details of how the signatures are stored. I know that .exe and .dll files are probably the two most common file extensions that would be signed, but other file types can also be signed (.msi at least). I use sigcheck.exe from sysinternals (https://docs.microsoft.com/en-us/sysinternals/downloads/sigcheck) to verify the signatures - it says whether they are signed, and who they are signed by.

It looks like sigcheck uses CryptAcquireContext, CertGetCertificateChain, CryptCATOpen, and a couple of dozen other Win32 cryptography related functions to do the signature verification.

[naveensrinivasan]

sounds reasonable. @randomascii how does verification work? Could you give us some pointers how you verify it's a Microsoft-signed binary?

Also, are there conventions for the filename: .exe, .dll, others?

There is lot to unpack here with validation of Signed Windows binaries without Windows https://blog.trailofbits.com/2020/05/27/verifying-windows-binaries-without-windows/

cc @woodruffw

[woodruffw]

Yep. Windows binaries are usually signed with Authenticode, which is PKCS#7 formatted with some vendor-specific tweaks.

uthenticode works, but an important caveat: it doesn't do full chain verification. It could, in theory, if someone were to reverse engineer Windows's trust store and export whatever default root certs are in it, but I haven't undertaken that task 🙂. For the time being, all uthenticode does is verify the internal consistency of the embedded certificates, the cryptographic hash, and whatever other invariants are expected that aren't predicated on the full chain.

[naveensrinivasan]

Yep. Windows binaries are usually signed with Authenticode, which is PKCS#7 formatted with some vendor-specific tweaks.

uthenticode works, but an important caveat: it doesn't do full chain verification. It could, in theory, if someone were to reverse engineer Windows's trust store and export whatever default root certs are in it, but I haven't undertaken that task 🙂. For the time being, all uthenticode does is verify the internal consistency of the embedded certificates, the cryptographic hash, and whatever other invariants are expected that aren't predicated on the full chain.

Great! Thanks! Does uthenticode have an API that we can call from the go code?

[woodruffw]

Great! Thanks! Does uthenticode have an API that we can call from the go code?

I don't know very much about Go -- can you call C++ APIs from it? If so, then yes. Otherwise, you might need to write a small C API shim around the public C++ API and use cgo (I think that's what it's called?)

[woodruffw]

Specifically, the C++ API is currently documented here: https://trailofbits.github.io/uthenticode/

[naveensrinivasan]

Specifically, the C++ API is currently documented here: https://trailofbits.github.io/uthenticode/

👍

[laurentsimon]

Thanks @naveensrinivasan and @woodruffw I wish there was a GHA that validates these, like for Java wrapper #2048

Maybe that'd be easier? Would @trailofbits be interested in packaging up the code into a GHA?

For root CAs, any ideas how often I don't know how often these CA keys are rotated; I assume not very often...?

/cc @di

@raghavkaul there is some structured results we could add here in terms of binary type / signature / wrapper action, etc

[woodruffw]

Maybe that'd be easier? Would @trailofbits be interested in packaging up the code into a GHA?

We have one here: https://github.com/trailofbits/winchecksec-scan

That action runs all of Winchecksec, which includes a bunch of other property tests on the binary (e.g. for ASLR, DEP, etc.).

We don't have a specific use case that would require use to break just uthenticode out into its own GitHub Action, though, and that isn't scoped in any current work we have.

For root CAs, any ideas how often I don't know how often these CA keys are rotated; I assume not very often...?

I don't have any strong numbers/statistics on this, but my understanding is that the Windows codesigning trust store gets updated somewhat frequently -- vendors are constantly being added and removed from the trusted set, and any given Windows host may or may not have the same trusted set as others (because of different Windows versions, "flavors" like consumer vs. enterprise, etc.).

[laurentsimon]

Maybe that'd be easier? Would @trailofbits be interested in packaging up the code into a GHA?

We have one here: https://github.com/trailofbits/winchecksec-scan

That action runs all of Winchecksec, which includes a bunch of other property tests on the binary (e.g. for ASLR, DEP, etc.).

We don't have a specific use case that would require use to break just uthenticode out into its own GitHub Action, though, and that isn't scoped in any current work we have.

That's great. There is support for enabling authenticode on its own, so scorecard could check for the presence of this action and this argument passed. It does not solve the full chain validation. I suppose it's pretty hard to do anything beyond what your tool already does.

[woodruffw]

Yep -- I think the best we could do is opportunistically detect the local machine's trust store (assuming it's a Windows host) and figure out how to reuse it, but that presents its own problems (both in terms of UX and user expectations, as well as increased ambiguity about the verification we're performing).

[laurentsimon]

Got it. Do you know if the GitHub Windows runners have a "standard" CA store? Could we use one of the utilities shipped with Windows to perform the verification for us?

[woodruffw]

Could we use one of the utilities shipped with Windows to perform the verification for us?

Yep, I think so -- the builtin signtool or certutil should do the trick.

Edit: docs for both:

• https://docs.microsoft.com/en-us/dotnet/framework/tools/signtool-exe
• https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/certutil

[laurentsimon]

Brilliant, thanks for the information. Should this functionality live in the trailofbits action? It seems to be the best place, but I don't know how much work would be involved or who should do the implementation. Let us know, thanks

[woodruffw]

Brilliant, thanks for the information. Should this functionality live in the trailofbits action? It seems to be the best place, but I don't know how much work would be involved or who should do the implementation. Let us know, thanks

That action was mostly a proof-of-concept, so I'm wary of introducing too much more functionality to it 😅. It also runs on Linux with a Docker container, so retrofitting it for Windows would likely be roughly as much effort as doing things "right" from scratch (a Windows runner + using signtool directly instead of uthenticode).

I think this is probably a good candidate for a new GitHub Action, one that's constrained in domain to just a Windows runner and the tools that MS provides.

[laurentsimon]

Thanks for the detailed explanation. Totally understand.

Does everyone think the right direction is to have a separate GHA ,like the Java wrapper GHA?

Or do you think scorecard should handle it itself?

Please chime in with your opinion

/cc @ethanent

[naveensrinivasan]

Does everyone think the right direction is to have a separate GHA ,like the Java wrapper GHA?
👍 +1

[ethan7g]

Does everyone think the right direction is to have a separate GHA ,like the Java wrapper GHA?

Or do you think scorecard should handle it itself?

I think having Scorecard handle the checking of such binaries might too significantly expand the burden on Scorecard maintainers, but it is tempting because it doesn't require configuration on the part of the maintainers of checked repos.

For the GHA check solution, there is complexity that arises from figuring out which files have been verified. We could use the annotations provided by the relevant check run for the latest commit if they are in a common format.

The gradle/wrapper-validation-action, for example, uses a format that isn't extremely easy to parse.

Gradle Wrapper Validation Failed!
  See https://github.com/gradle/wrapper-validation-action#reporting-failures
✗ Found unknown Gradle Wrapper JAR files:
  3c56bb5e0c1016189ab1a7ba45b351136f2f7f5d3d53afa812c8479edf9fae30 gradle-wrapper/gradle-wrapper.jar

We could make a standard format for providing file verification results in annotations from other checks, something like...

{
  "results": [
    {
      "file": "bin/some_verified_executable.exe",
      "verified": true
    },
    {
      "file": "bin/some_tampered_executable.exe",
      "verified": false,

      // optional message
      "message": "File not signed by trusted party"
    }
  ]
}

To identify an annotation as one which we should use to see if files have been verified, we can add a prefix to the annotation.

Relevant GitHub API endpoints:

List check runs for a Git reference: /repos/{owner}/{repo}/commits/{ref}/check-runs
=> Take each check run ID and get annotations using the following endpoint.

List annotations for a check run: /repos/{owner}/{repo}/check-runs/{check_run_id}/annotations
=> Check annotation for each job, see if it starts with "File verification: ", and if so, parse the following JSON and apply results by allowing the verified files.

[ethan7g]

Or do you think scorecard should handle it itself?

Actually, on the topic of the solution whereby Scorecard verifies binaries itself, I don't think it's the best because it would mean Scorecard is just verifying the files at a point in time, rather than checking that the project has a process in place for verifying the files in the future.

Because, in my opinion, that doesn't fit with the "preventative" (reducing potential for attack) philosophy of the Binary Artifacts check, I would recommend a GHA solution like I described in my previous comment.

[laurentsimon]

If we create an Action, where should it leave? A new ossf repository? In scorecard repository?
What shall we call it?

Do we need a "meta" Action that contains signature verification, gradle wrapper, etc? (And the other ones we may need later)

[ethan7g]

If we create an Action, where should it leave? A new ossf repository? In scorecard repository? What shall we call it?

Probably in another ossf repo. Maybe something like "binary-verification-action"?

Do we need a "meta" Action that contains signature verification, gradle wrapper, etc? (And the other ones we may need later)

I like this idea! I think we should continue to allow a passing gradle/wrapper-validation-action to exempt gradle-wrapper.jar files, but similar verification could additionally be added to this Action (eventually) for the sake of simplicity.

[laurentsimon]

Let's add this for discussion next meeting. @ethanent are you interested ^^

[github-actions]

Stale issue message - this issue will be closed in 7 days

[woodruffw]

Not stale, I believe.

[github-actions]

This issue is stale because it has been open for 60 days with no activity.

[github-actions]

This issue is stale because it has been open for 60 days with no activity.

[github-actions]

This issue has been marked stale because it has been open for 60 days with no activity.