[WIP] Add an update checker #17132

1. 
   emilengler commented at 2:56 am on October 14, 2019: contributor

   About

   This adds a cryptography system to check for newer versions of Bitcoin Core. It will never update binaries itself. It adds a new RPC command called check-update and displays a message in the GUI if a new version is available.

   How does this work

   There is a list of sources where a file is hosted which says the newest version + a digital signature of it. When checking for a new version it does the following steps:

   1. Checks if the client is a release and not a dirty build (disabled right now for testing purpose)
   2. It will choose a random source from where to get the versions file
   3. It will try to download the versions file, if it fails it will select a new source. If no source is found, it will fail
   4. It will parse the downloaded file
   5. It will verify if the signature matches one of the hard coded public keys
   6. It will check if the version is newer than the current one (disabled because of testing)
   7. It will check if the new version string only contains numbers, dots and ‘v’

   Q&A

   Q: Alert Key system remastered? A: No, the alert key system was a bitcoin specific thing which happened over the protocol, this happens over HTTP and is only related to Bitcoin Core and will only give a new version (or anything that can be done with just numbers and dots) Q: What happens if someone gets a public version signing key? A: Nothing. Unless he does not control a source, he can’t do anything with it. If he controls a source he could give a message with just numbers, dots and ‘v’ and it would be only visible to these peers who randomly select his source Q: What happens when an attacker gets the control over a source? A: Nothing, as long as he doesn’t have the private key of a version signing address. He could just remove the version or install an older Q: Is it centralization? A: Well yes, but Bitcoin Core is a centralized project (it has maintainers, governance, etc.). Like I said, the system has nothing to do with Bitcoin directly

   Version Signing Address

   A Version Signing Address needs to be legacy to create and verify signatures

   Source

   The source is just a txt file hosted on a web server. It needs to be exactly two lines long. The first line contains the text and the second one the message. Example:

   0v0.99.0
   1IEHPu8Bc609G9wxc0TPOfGlnSbtEcvoZA68e/pi5pEZqeqegvovwqcNtXfU50ju+hD5OIJWsuOTjHISLV+KprVs=
   

   Testing

   Because it uses libcurl and I am a complete disaster when it comes to build systems it only works with libcurl installed and I only tested it on (g++ (Debian 8.3.0-6) 8.3.0)

   First install libcurl On Debian 10

   0apt install libcurl4-openssl
   

   When you build this PR and run check-update you will get

   0v0.99.0
   

   When you run bitcoin-qt you will get It will use a source on my webserver with a public key that only I have (The server might be down around 4am german time every day)

   Testing with own source or address

   Source

   1. Setup a web server
   2. Copy a valid source file (you can use mine) to the webserver
   3. Add the URL of the file to the class constructor with adding the function addSource("http://yourhost");

   Address

   1. Create a new legacy address (can be done over getnewaddress)
   2. Copy this address to the class contructor with addKey("YOURADDRESS");
   3. Create a source file with signing the version text from line 1

   To-Do before merging

   • Get address from laanwj and others (just as backups)
   • Integrate libcurl better to the build system and if it’s not available disable this feature.
   • Setup multiple sources for the case that one is down and to add decentralization.
   • Add options to the RPC call to use a custom source and address (for testing)

   Because this PR uses new dependencies a compilation might not work Contact me whenever you have questions or problems of any kind

2. Add an update checker

   This adds a system to check for updates

   a7534a2260
3. 
   achow101 commented at 3:28 am on October 14, 2019: member

   Instead of adding a new dependency, I would suggest just using what bitcoin-cli uses for sending http requests for the json-rpc interface.
4. 
   harding commented at 3:39 am on October 14, 2019: contributor

   I haven’t tested, but the message shown in the GUI looks like one of the messages that the user can’t dismiss. I think that, if we want to allow people to continue running old versions (i.e. not force them to upgrade), that message should be something the user can dismiss (at least until the version number increments again).
5. DrahtBot added the label Build system on Oct 14, 2019
6. DrahtBot added the label RPC/REST/ZMQ on Oct 14, 2019
7. 
   emilengler commented at 4:44 am on October 14, 2019: contributor

   @achow101 Does it support https? Well it probably isn’t that necessary because the version numbers are signed anyways but still most webservers are configured to accept https only
8. 
   emilengler commented at 4:45 am on October 14, 2019: contributor

   @harding I agree, however I didn’t found a way to add a message that the user can dismiss. Is there an object for this?
9. 
   ariard commented at 4:57 am on October 14, 2019: member

   “It will verify if the signature matches one of the hard coded public keys” NACK, introducing hardcoded pubkeys or sources is just going to be another key management burden plus a vector of project centralization.. We shouldn’t nudge user vendor choice..

   Further, doesn’t seem a real problem users face, you can also write a bash script to do the same based on websites TLS certificates

10. 
    emilengler commented at 5:01 am on October 14, 2019: contributor

    @ariard Bash is platform incompatible and signatures are a better way for authentication
11. 
    DrahtBot commented at 6:24 am on October 14, 2019: member

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #17129 (tests: Add fuzzing harness for miniscript::FromScript(…) by practicalswift)
    • #16889 (Add some general std::vector utility functions by sipa)
    • #16365 (Log RPC parameters (arguments) if -debug=rpcparams by LarryRuane)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

12. 
    laanwj commented at 8:20 am on October 14, 2019: member

    Tend toward concept NACK. For the node software, I don’t think we want to add any functionality that phones home.

    The main threat scenario is that someone could take over the server used for checking, fake a release, virtually taking over the entire network. I know this is “optional”, but users are so trained to automatically update when they see a window like that. With a consensus system like bitcoin this should never be an automatic decision.

    And sure, you can make it more difficult by adding signing keys, possibly requiring multiple developer’s keys for signing, and so on. But the essential remains the same.

    It’s generally fine to keep running older versions until you need to upgrade.

13. laanwj added the label Needs Conceptual Review on Oct 14, 2019
14. 
    hebasto commented at 8:43 am on October 14, 2019: member

    Concept NACK. This feature will allow adversaries to raise false propaganda wave about the Bitcoin centralization.
15. 
    fanquake commented at 1:30 pm on October 14, 2019: member

    Concept NACK - For the same reasons as above and that have been discussed the previous times this has come up. See #1453 #2626 etc.
16. 
    MarcoFalke commented at 2:05 pm on October 14, 2019: member

    NACK. Too much complexity to bake into Bitcoin Core. Apart from the technical vulnerabilities in the new code and dependencies, the maintenance and testing burden on the Core project, as well as the centralizing effect, this is politically too controversial. (All points already mentioned in other comments)

    I think the existing solutions are more than sufficient. See #2626 (comment)

17. 
    emilengler commented at 3:25 pm on October 14, 2019: contributor

    Concept NACK. This feature will allow adversaries to raise false propaganda wave about the Bitcoin centralization.

    How? Only dots, numbers and ‘v’ are permitted in the update text. What could you make with these? My only suggestion would IPv4 address

18. 
    emilengler commented at 3:25 pm on October 14, 2019: contributor

    Ok, will close it now
19. emilengler closed this on Oct 14, 2019

    ---

20. emilengler deleted the branch on Oct 14, 2019
21. 
    laanwj commented at 6:11 am on October 15, 2019: member

    How? Only dots, numbers and ‘v’ are permitted in the update text.

    The problem is that technical mitigations like that do nothing to quell the potential outcry about things like this.

    The parsing, verification or network code might still contain a bug; if all clients automatically poll one endpoint, there’s potential for accidentally turning it into a botnet.

    Sorry for the wasted effort; for larger changes, it might be better to discuss your intent first before starting work on it.

22. 
    emilengler commented at 2:01 pm on October 15, 2019: contributor

    Ok, thanks for the advice. Anyways it was still fun working on it :smile:
23. MarcoFalke locked this on Dec 16, 2021

Contributors
emilengler achow101 harding ariard DrahtBot laanwj hebasto fanquake MarcoFalke

Labels
Build system RPC/REST/ZMQ Needs Conceptual Review