[EG] Update the EG updater to allow sideloading #3938
Conversation
|
Here's a draft EG sideloading change, I'm still waiting on Kabuki to ack my bug for adding a user warning but figured we can start a review now and get any comments out of the way. I tested this and it works, but requires a server with SSL setup. I'm looking into a simple way to set that up in a Flask server but running into connection issues with self-signed certs. An alternative can be to also disable requiring HTTPS with a flag (or alongside one of the new flags I'm adding). |
|
I'm assuming the whole PR is blocked until webapp implements a dialog/message telling the user they are in debug mode . We could temporarily add another boolean which turns off this whole functionality get the PR submitted and turn it back on when webapp finishes its implementation. |
Yeah, it'll be blocked until then. I also need to add unit tests for the new h5vcc apis. To get this in so we don't end up with conflicts that's a good idea, to lock it behind a bool and make a new change once things are completed on the webapp. There may be further changes too depending on the webapp team's requirements. |
|
I should have time tomorrow to take a look at this. |
Unit tests are a good idea: to ensure this works for partners we might want to add YTS tests as well. |
|
I was pulled away by a few P0 issues this week. I'll review this PR Friday or early next week. @TyHolc Please take a look at the failed Evergreen checks first. Thanks. |
chrome/updater/updater_module.cc
Outdated
| #if defined(COBALT_BUILD_TYPE_GOLD) | ||
| bool skipVerifyPublicKeyHash = false; | ||
| #else // defined(COBALT_BUILD_TYPE_GOLD) | ||
| bool skipVerifyPublicKeyHash = GetAllowSelfSignedBuilds(); |
There was a problem hiding this comment.
Based on the design I thought we were adding a web API to set a public key to use for verification of the self-signed package. Did we decide instead to skip package verification altogether?
There was a problem hiding this comment.
Yeah, from the security review I go the impression that they don't see that approach as more secure than skipping the signature check, and it was more difficult for users.
If we agree as a team that we should do that instead I can update this, it would pretty much just be changing this to a string and instead of clearing the key list in the updater_module I'd instead add the given key hash. LMK what you think.
There was a problem hiding this comment.
That sounds good to me, I also don't see a significant difference in security between the two approaches.
It may be good to just capture that feedback from the security review in the design doc somewhere, maybe under Design Review Notes.
There was a problem hiding this comment.
The PR lgtm. The current approach is good to me, as long as the security review team and EG team decide that skipping verification is okay. I only have some nit comments. Other than that, please take a look at the failed checks and resolve them. Thanks Tyler!
Currently still discussing with the webapp team but so far their guidance is to have Cobalt render this message rather than have the webapp show it. That will make it so it's impossible to skirt around the message being displayed through redirects. I'm looking into how we can do this from our end. |
Add h5vcc API options to skip checking if package signature matches our key and to set a custom URL to check for package updates. Implement the above features into the updater for non-gold builds only. b/325626249 Change-Id: I594d04207d242dfbf69c7b6f734174ccb03a43cd
b/325626249 Change-Id: I30353d0559453a62793e3724c0152d2cadf97e64
Add an option to disable the network encryption check when checking for and downloading Evergreen updates on non-production builds. b/325626249 Change-Id: I9b5b402a5852bd183296d4406e88562db1ed662f
Add a minimal python server to act as a stub omaha server. It will serve serve an appropriate response for the file at `file_location` when queried for an update, then serve that file on the subsequent request. b/325626249 Change-Id: I2933e99b485d74581fd332763f05204be21f2c93
|
Made a couple of significant updates. I uploaded the python server I've been using to test and added a toggle for forced network encryption so we can use http. I looked at instead allowing https and ignoring ssl errors so we can have a self-signed cert for the server as well, but that was more difficult to implement and not really more secure. I also internally uploaded the scripts I made for creating a signed crx package and for rebuilding EG and the loader app if anyone is interested in re-using them. |
Add implementations of new virtual methods to test_configurator b/325626249 Change-Id: I000d8742475829bd396918f17cedb3450d52773a
Add h5vcc API options to skip checking if package signature matches our key and to set a custom URL to check for package updates.
Implement the above features into the updater for non-gold builds only.
b/325626249
Change-Id: I594d04207d242dfbf69c7b6f734174ccb03a43cd