Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pin code signing certificate #272

Closed
mherrmann opened this issue May 27, 2021 · 9 comments · Fixed by #296
Closed

Pin code signing certificate #272

mherrmann opened this issue May 27, 2021 · 9 comments · Fixed by #296

Comments

@mherrmann
Copy link
Contributor

mherrmann commented May 27, 2021

Thank you for your work on Omaha. My agency uses it to help many other companies implement automatic update solutions with it.

We would like to pin the code signing certificates accepted by the client. Could you give pointers what needs to be done in addition to changing the constants in const_code_signing.h? It feels like just changing those constants can't be enough, or else it would not be possible to update with open source Omaha at all, because outside organizations don't have the certificates mentioned there.

In this particular project, the update binary is an MSI. If any special logic is necessary for pinning MSI signatures, it would also be great if you could let me know.

I'm taking the liberty to CC you @roander-msft because you may not receive notifications but seem to be doing a lot of work with Omaha. I hope you don't mind the spam.

Thank you again,
Michael

@sorinj
Copy link
Collaborator

sorinj commented May 27, 2021

Right off, the certificate pin is not used at all for updates. Update payloads are not checked with Authenticode right now.

Authenticode is used just in two scenarios, which might not even occur in practice, for most users. It has to do with
verifying the integrity of a payload downloaded and executed outside the typical authenticated channel Omaha establishes with the server by using CUP and file hashes.

The scenarios are the following:

  • code red: this is a last resort mechanism to attempt recovering the updater if it gets bricked
  • app commands: if you don't know what they are, you probably don't care about them. It's a feature used by ChromeFrame long ago. Some app commands are used by Chrome during its setup and update but those app commands don't use Authenticode.

We could discuss further if you still want to do the pinning in your fork.

@mherrmann
Copy link
Contributor Author

Thank you very much for your reply.

Does this mean that also Google doesn't pin the code certificates for updates? If yes, how do you protect against an attacker taking over the update server?

@sorinj
Copy link
Collaborator

sorinj commented May 27, 2021

Considering the current Omaha implementation, it is important to secure both the signing and the update servers.

@mherrmann
Copy link
Contributor Author

mherrmann commented May 27, 2021

Thanks; what do you mean by "securing the signing"? Do you mean making sure that the code signing certificate does not fall into the wrong hands?

@sorinj
Copy link
Collaborator

sorinj commented May 27, 2021

Yes, that includes unauthorized access to the code signing certificate but probably there are other scenarios as well.

@mherrmann
Copy link
Contributor Author

I see. You offered we could discuss further if we want to pin in our fork. Do you think this would be difficult? It seems like the necessary code is there; It would just need to be called from the right places.

@roander-msft
Copy link
Contributor

@mherrmann Sorin is the right person to talk to here for sure. Most of my activity in this repo has been landing unit test reliability improvements that we've found useful downstream on Edge.

@sorinj
Copy link
Collaborator

sorinj commented Jun 1, 2021

I suggest the following to do signing certificate pinning:

  • call the function VerifyGoogleAuthenticodeSignature from DownloadManager::CachePackage. You could probably call this function from other places, but CachePackage stands out since that's where the payload hash is verified.
  • Introducing new pins in your fork requires changes to the base/const_code_signing.h, base/signaturevalidator_unittest.cc, common/google_signaturevalidator_unittest.cc, and possibly adding new unit test support files to verify the pins.
  • I suggest implementing the feature behind a conditional build flag, so that you could potentially upstream your change from your fork.
  • Unfortunately, the pin files and the program identifiers are hardcoded to Google. I suggest not making lexical renames, it is what it is. In your case, VerifyGoogleAuthenticodeSignature means verifying the payload with your company's pins, not Google's.
  • An issue to keep in mind: Authenticode requires a network check for its certificate revocation list.
  • At any rate, running the code under the debugger and testing the code is doing the right thing is critical. Obviously, such feature may result in preventing future updates to updaters or apps. It needs to be rolled out carefully, even if the code is correct.

This are the high level ideas I can come up right now. Please do ask questions, we are glad to help.

@mherrmann
Copy link
Contributor Author

Thank you very much @sorinj. We will look into this. I will try to report back here once I know more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants