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

Update notification #478

Merged
merged 3 commits into from
Jun 22, 2022
Merged

Conversation

rithvikvibhu
Copy link
Collaborator

Checks for updates on Bob Wallet start, and if a new stable release is found, shows a notification in sidebar and clicking on it opens the GitHub release page for that version:

image

Uses semver to compare, so RCs and others are handled properly. Just need to make sure that release tags are in format v${version} (like v0.9.0), which we've already been doing.

@rithvikvibhu rithvikvibhu added the needs translation PR updates locale strings and have not yet been translated label Feb 28, 2022
@pinheadmz
Copy link
Contributor

Concept ACK! Would it be crazy to help users verify the PGP sig on new dowloads as well? I dunno if that makes sense, theoretically a compromised verion could convince a user to download additional compromised versions.... but maybe there is something we can do? Im curious if other crypto-sensitive software handles releases in any special way like that.

@rithvikvibhu
Copy link
Collaborator Author

Would it be crazy to help users verify the PGP sig on new dowloads as well?

I think we can add more info in release docs and on the website on how to verify checksum and gpg easily (#436, #214).

Making the software itself verify an upgrade will complicate things imo - this would mean moving the download/install/update process inside the wallet which is messy when cross-platform.

try {
const releases = await (
await fetch(
'https://api.github.com/repos/kyokan/bob-wallet/releases'
Copy link
Contributor

@pinheadmz pinheadmz Apr 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mine as well check some kind of signature, even if its a brand new bob-release-update published by kyokan. Especially because you use url: latestRelease.html_url, down below. Giving users a link to click is a target for attacks, treat it like a nuclear bomb.

https://bitcoinist.com/electrum-wallet-phishing-bitcoin/

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow didn't think of that. Unlikely, but def possible.

How about always linking to https://github.com/kyokan/bob-wallet/releases/tag/{TAG} instead of latestRelease.html_url?

Not sure what signature can be checked here. Do you mean like with PGP? It's just a link so no info to verify.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look around how other popular wallets like Electrum and Wasabi handle this. Hard coding the link feels good. As far as signature what I meant was that Kyokan should publish a new key (hey maybe an HNS address so we can use rpc verify message) and Bob will verify an update notification from whatever source. I dunno if it's possible to check that the release signature on github is correct before notifying the user. Whatever works out, just keep in mind attackers would love to get Bob users to download malware with one button click, so line up the troops.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to prefix url and only use tag. imo the only way to break this would be to MITM a HTTPS req to github.com, which if someone can do, there are bigger problems already.

maybe an HNS address so we can use rpc verify message

Requires the node to be synced :|

I dunno if it's possible to check that the release signature on github is correct before notifying the user.

Afaik, no. We could download another file and check that, but again relies on an external source, mostly over https, with the same attack vectors as https://github.com. Additionally, verifying a file would mean downloading the actual binary from within Bob which feels more dangerous.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requires the node to be synced :|

no, just the address, not the NAME.

@Falci
Copy link
Contributor

Falci commented Apr 20, 2022

Checks for updates on Bob Wallet start.

I keep Bob running forever. Does it mean I won't be notified?

@rithvikvibhu
Copy link
Collaborator Author

@Falci heh. Releases aren't really frequent (2 per year?). Having a setInterval seems like overkill imo. Wdyt?

@@ -28,7 +28,7 @@ export const checkForUpdates = () => async (dispatch) => {
type: SET_UPDATE_AVAILABLE,
payload: {
version: latestRelease.tag_name,
url: latestRelease.html_url,
url: `https://github.com/kyokan/bob-wallet/releases/tag/${latestRelease.tag_name}`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, good change.

@rithvikvibhu rithvikvibhu merged commit d130b37 into kyokan:master Jun 22, 2022
@rithvikvibhu rithvikvibhu deleted the check-for-updates branch June 22, 2022 06:29
Falci pushed a commit to Falci/bob-wallet that referenced this pull request Jun 22, 2022
@rithvikvibhu rithvikvibhu removed the needs translation PR updates locale strings and have not yet been translated label Jul 27, 2022
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 this pull request may close these issues.

3 participants