0003267: Auto-updates issued over insecure HTTP, risk of arbitrary code execution

Notes
(0018338) WubTheCaptain (reporter) 2017-09-19 08:08 edited on: 2017-09-19 08:10	Patch by Zalewa:'https://bitbucket.org/Doomseeker/doomseeker/commits/6e4d1b8a6b7339a353619b3f740e972f57187068 [^]' update-info.js files need a change too.

(0018339) WubTheCaptain (reporter) 2017-09-19 08:15	Can anyone tell if Doomseeker 1.0 is still supported or vulnerable?

(0018340) WubTheCaptain (reporter) 2017-09-19 08:17	I also have a question, is the X.509 certificate over HTTPS verified to be valid?

(0018343) Zalewa (developer) 2017-09-19 10:24	Quote from "WubTheCaptain" update-info.js files need a change too. I have already updated them on the server.

(0018405) WubTheCaptain (reporter) 2017-09-27 22:45	Could we release a security patch for Doomseeker? Old versions still attempt to browse the directory for JSON update files over HTTP, which poses the same risks for MITM. Every day that passes, bad guys may learn about exploiting this while no public announcement exists for the general public to be aware of.

(0018410) Blzut3 (administrator) 2017-10-01 20:04	Zalewa, I'll leave the decision of if we should put out a patch release to you. Personally I don't think this attack is very practical to warrant a hot fix considering it wasn't all that long ago that serving updates over HTTPS would have been impractical for us. Many other small programs were in the same boat and it's not like you hear about this kind of attack actually happening. On the other hand I don't really want to look like we don't take security seriously, so there is that merit to hot fixing.

(0018411) Zalewa (developer) 2017-10-01 22:29	One thing to consider here is that the benefit of attacking a handful of Doom players would be minimal in comparison to the required effort, given that a malicious host would probably need to come from the ISPs for many users. I could imagine a public WiFi access point injecting all downloaded executables with viruses, but in such case any insecure site can be used as an attack vector. However, given how everyone switches to HTTPS nowadays, there wouldn't be many opportunities to utilize such attack. Moreover, any sane Windows user will have an anti-virus installed, which will hopefully curb known malicious patterns. On the other hand, it's not unheard of for exploits to go undetected but be in use to attack people for years, and code is just code - anti-viruses won't detect everything. Ransomware, for example, can go undetected because it's just a piece of software that lists files, reads them and writes them back to disk (in a "slightly" different form.) I don't think we should trivialize this, though, even if it can be reasonably suspected that no one would take the required effort for the benefit of potentially infecting a few hosts. Since we have also done some other work lately, release 1.1.1 or 1.2 might be in order soon. There's no other way to tell Doomseeker to download the update-info.json from a HTTPS rather than HTTP, and also Doomseeker currently doesn't care if the URLs inside that file are secure or not. I'm not sure, however, how to protect the users from the case where someone hacks Doomseeker's website and replaces the files there. If they can do that, they can also replace any checksums that we can place there. One way to minimize the risk would be to put checksums on a separate host, with as much different configuration as possible. Mendeley also seems to already do checksum verification, unless that 'hash' field in that XML is just for show, so if we can ensure the security of those XML files then we should get that functionality for free. Another thing completely are Zandronum testing releases. Zandronum hosting can also get hacked and the game binaries can be replaced, and Doomseeker also installs and then executes those without a second thought.

(0018412) Blzut3 (administrator) 2017-10-01 22:59	My plan was to propose a 1.2 release after most of WubTheCaptain's tickets were resolved. Although I don't know how many weeks that will take. The only sane option for making the updates safer would be to add PCKS7 or GPG signatures and embedding the public key into the updater (similar to the request in ticket 0003275). We don't have the resources to really host hashes on a different server and honestly I don't really think that would actually add any more security.

(0018415) WubTheCaptain (reporter) 2017-10-02 22:23 edited on: 2017-10-02 22:33	Do Windows and macOS binaries of Doomseeker and Zandronum use code signing, or does this step of verification ever happen at all? What I mean by this is at least in Windows Vista and later, you may get an "unknown publisher" warning or error on execution depending on User Account Control (UAC) setting when they are not signed and make changes to the system. If this happens, it may be a first layer of security. (My guess is this may not happen, because the releases are portable with no shared installer wizard for portable/non-portable installs. Some wizards can be automated silently too. No idea about macOS, probably much easier to install stuff from the App Store or Homebrew on that operating system.) Quote from Zalewa Doomseeker currently doesn't care if the URLs inside that file are secure or not. It may not have to. If you make a request to a HTTPS site, without HTTP redirect in between or have HSTS preloaded, and verify the X.509 certificate from response, the files or hashes are good enough to trust to have originated from that server. (This relies on security of DNS and trust of X.509 CAs that we build upon our HTTPS sites, admitted.) To prevent the situation of replaced files through compromised server, OpenPGP signatures would sign the hashes (0003275). This also works over insecure HTTP/FTP, if the public key is known/trusted (web of trust); You get enough people to confirm "this public key/signature came from a Doomseeker developer" in person face-to-face (at key signing parties). If Doomseeker used code signed installer wizards to perform updates, Doomseeker's built-in code may not have to care at all where the files came from. The updates would then be verified at the operating system level. (I don't have an answer what would prevent a malicious author from replacing the code signatures in those installers with something trusted, as I'm not familiar with this.) Zandronum (and Freedoom WADs to an extent) rely more on trust of HTTPS for now. We don't have much control over those, unless upstreams make a change. Old versions use HTTP to discover them, again. 1.2 release may be a bit far off at current pace, so 1.1.1 seems more time-relevant to backport at least until a better solution is in place.

(0018416) WubTheCaptain (reporter) 2017-10-02 22:30 edited on: 2017-10-02 23:33	To add to the installer wizard proposal (if replacing Mendeley or distributed binaries is desired): WiX, Inno Setup and NSIS are popular libre wizards. (Windows Installers (MSI) are complex but the official Microsoft supported installation mechanism.)

(0018434) Zalewa (developer) 2017-10-04 16:27	Auto-updates need to remain as headless as possible. They need to be smooth, quick, transparent to the user and not install anything that wasn't installed already. I took example from Firefox. I don't think we should replace Mendeley.

(0018459) Zalewa (developer) 2017-10-07 10:09 edited on: 2017-10-07 10:09	By going full-HTTPS we have inadvertedly broken Windows XP support - or at least, an unupdated Windows XP SP3 support. I keep such VM to verify if Doomseeker runs and works properly on Win XP. Because I don't update this machine whatsoever, it has outdated CA root certs. Due to this, some of the HTTPS sites that we try to contact fail to respond. One of those sites was'https://doomseeker.drdteam.org [^]' although I cannot reproduce that particular connection failure anymore. Solutions: 1. Not care. People should not use systems that are past their official support date. 2. Dump CA certs from a contemporary OS into a .pem file, redistribute it with Doomseeker and instruct Qt to load certs from this file. I explored 2. a bit and conclusions are as follows: 1. Qt code needed to dump cacerts to a .pem file is trivial. Just foreach on the QSslConfiguration::systemCaCertificates() and write them all to one cacerts.pem file using'http://doc.qt.io/qt-5/qsslcertificate.html#toPem [^]' . 2. The resulting cacerts.pem file is only 50kB, and this is bigger than it should be because I have certs from Kaspersky AV. A "clean" OS will produce an even smaller file. We can put this file into the repository and distribute in doomseeker-core auto-update package. 3. The code to load additional CA certs from this file is also trivial: QFile certsFile("cacerts.pem"); if (certsFile.exists()) { certsFile.open(QIODevice::ReadOnly); QSslConfiguration sslConf = QSslConfiguration::defaultConfiguration(); QList<QSslCertificate> cacerts = sslConf.caCertificates(); cacerts.append(QSslCertificate::fromDevice(&certsFile)); sslConf.setCaCertificates(cacerts); QSslConfiguration::setDefaultConfiguration(sslConf); certsFile.close(); } Comments/suggestions?

(0018460) WubTheCaptain (reporter) 2017-10-07 10:30	Drop support, I say. Maintaining root certificates is a great burden and userspace applications should not do that (especially on GNU/Linux, nevermind how autoupdates don't happen there). I mean, you can still access it over HTTP I suppose:'http://doomseeker.drdteam.org/ [^]' – there's no HSTS or 301 redirects forcing you to use HTTPS. It's just you won't get automatic updates anymore on Windows XP. Ideally I would even go full HTTPS some day. I can't support this idea of loading a PEM file even conditionally on Windows XP only. Sorry.

(0018461) WubTheCaptain (reporter) 2017-10-07 10:38 edited on: 2017-10-07 10:41	Oh, I almost forgot the WAD archives were also switched to HTTPS by default. So were Zandronum releases and Freedoom game data downloads. ...tough shit. It's comparable with asking to weaken security for everyone (every secure system) with 3DES or RC4 ciphersuites only to support Windows XP with TLS. No. But thank you for taking the time to investigate, anyway! It was interesting to read.

(0018478) Blzut3 (administrator) 2017-10-07 23:16	Given that pretty much everyone in this community will be using Lets Encrypt, it'd probably be enough to add their CA cert.

(0018517) WubTheCaptain (reporter) 2017-10-10 09:49 edited on: 2017-10-10 09:50	Can I get a timeline update on this issue, please? I'd like this issue to be disclosed and communicated to the public by the next two weeks if possible. If it needs more time, please tell.

(0018532) Zalewa (developer) 2017-10-11 17:29	I plan on dealing with this properly once we resolve the ticket on licensing.

(0018534) WubTheCaptain (reporter) 2017-10-11 18:34 edited on: 2017-10-11 18:35	I acknowledge the need to resolve 0003237 before publishing any new releases on Doomseeker's website, and it's reasonable. But could we communicate this to the public with a patch/diff any sooner, please? More than a commit existing in the repository unknown to the public.

(0018857) Blzut3 (administrator) 2017-11-11 20:14	As far as I'm concerned you can disclose if you want. This issue is incredibly difficult to exploit with any reasonable expectation of success without specifically targeting someone.

(0018864) WubTheCaptain (reporter) 2017-11-12 17:14	Issue made public.

(0018957) Zalewa (developer) 2017-12-12 14:38	This commit enabled loading of cacert.pem file:'https://bitbucket.org/Doomseeker/doomseeker/commits/bffbf95cd7b38fbfa03bc763ef27dbd581cb99ad [^]' This commit actually adds the cacert.pem file to Windows release:'https://bitbucket.org/Doomseeker/doomseeker/commits/83cff7c9f1ace55672b2da50a50009e8bc7d6a48 [^]' cacerts.pem contains CA for Let's Encrypt and the one used by github.io so that Freedoom can be installed on Win XP.

(0018959) Zalewa (developer) 2017-12-12 17:25	The move to HTTPS is complete (unless I missed something), and despite Wub's advice I also included the CA cert database for github.io's cert and for "Let's Encrypt", as Blzut3 suggested. Now, we can take it up a notch and PGP the packages. Qt doesn't come with PGP features, but QCA tool exists ('https://userbase.kde.org/QCA [^]' ) which seems to do the job. Its license is LGPLv2.1+, so it's compatible with ours. We could even try and figure out how to compile its code into Doomseeker statically. Naturally, QCA code would be added to Doomseeker's repo like Mendeley Updater was. One PITA is that it apparently needs 'gpg' binary to run, which on Linux is granted and on Windows it's the opposite (and also Linux is the only platform where we don't need it). Embedding gnupg library directly is impossible due to conflicting licenses (GPLv3). This, of course, means more work, more maintenance, bigger Doomseeker package size and all the other imaginable and unimaginable overheads. If we were to do this (though I find it unlikely given the problematic implementation), let me know if I get the procedure right: 1. We (ie. The Doomseeker Team) create our own PGP key pair and embed the public key in Doomseeker executable. 2. The update archives and other software packages are created as usual. 3. We calculate hashes (SHA256) from the archives and encrypt those hashes using our private key. 4a. Update packages are made available for download from our server. 4b. PGP'd hashes are also made available for download and sit next to the update packages. 5. Doomseeker downloads the update package and its PGP'd hash. 6a. Doomseeker computes the hash of the downloaded update package. 6b. Doomseeker decrypts the PGP'd hash. 7. Doomseeker does computedHash == decryptedHash. 8. If match - install, if no match - scream in terror.

(0018961) WubTheCaptain (reporter) 2017-12-13 00:51	I believe cacert.pem is not a creative work in the sense of copyright (at least in the United States), and therefore believe it to be entirely of information that is common property and in public domain. Doomseeker code doesn't need to know about OpenPGP and all that, if the installer executable is code signed. See 0003292. As long as Mendeley can fetch an MSI file over HTTPS and execute it with silent upgrade option, all good? MSI installers refuse to install on Windows 10 without code signing. Unrelated: OpenPGP signatures for source tarball releases would be optional, but preferred.

(0018962) Blzut3 (administrator) 2017-12-13 04:38	The path of least resistance on signing the update packages would probably to use x509 since the libraries for it are already there for HTTPS. You should be able to use OpenSSL to verify PKCS#7/CMS signatures.

(0019036) Zalewa (developer) 2018-02-14 11:15	I did some additional investigation into using OpenSSL to sign & verify. For creating the packages I think we can safely assume that "openssl" executable will be somewhere in the path, even on Windows. "COMPILING.Windows.txt" already explains where to get the precompiled package. So, Ruby scripts can try to use OpenSSL to create the .sig files for each .zip. User will know that OpenSSL is missing when .sig files don't get created. Command lines to do this: # Spawn private key. openssl genpkey -algorithm RSA -out private_key.pem -pkeyopt rsa_keygen_bits:2048 # Get public key from private key. openssl rsa -pubout -in private_key.pem -out public_key.pem # Create package signature. openssl dgst -sha256 -sign private_key.pem update_package.zip Verification is another can of worms. Public key can be embedded as a Qt resource. However, unless I mislooked something, I couldn't find any way to do the verification itself directly from Qt. Instead, we could LoadLibrary the libeay32.dll and do the verification by using OpenSSL's API directly. Relevant tutorial:'https://wiki.openssl.org/index.php/EVP_Signing_and_Verifying [^]' I haven't checked yet if this is feasible. We will also need to create .sig files for the current stable release or otherwise downgrading from beta to stable won't be possible anymore because auto updater must refuse to work when .sig files are missing.

(0020263) WubTheCaptain (reporter) 2018-12-17 05:00	Fyi: We have .sig files since Doomseeker 1.2 release now.

(0021158) WubTheCaptain (reporter) 2020-01-30 13:46	I propose some kind of resolution to this issue; this was from a time when auto-updates were downloaded over HTTP, which now happens over HTTPS. For cryptographic signature validation, create a new issue for enhancement? Quote from WubTheCaptain I also have a question, is the X.509 certificate over HTTPS verified to be valid? remains a valid objection/question, which I've not tested.

Only registered users can voice their support. Click here to register, or here to log in.
Supporters:	Korshun
Opponents:	No one explicitly opposes this issue yet.

Issue History
Date Modified	Username	Field	Change
2017-09-19 08:08	WubTheCaptain	New Issue
2017-09-19 08:08	WubTheCaptain	Note Added: 0018338
2017-09-19 08:10	WubTheCaptain	Note Edited: 0018338	View Revisions
2017-09-19 08:15	WubTheCaptain	Note Added: 0018339
2017-09-19 08:17	WubTheCaptain	Note Added: 0018340
2017-09-19 10:24	Zalewa	Note Added: 0018343
2017-09-27 22:45	WubTheCaptain	Note Added: 0018405
2017-10-01 20:04	Blzut3	Note Added: 0018410
2017-10-01 22:29	Zalewa	Note Added: 0018411
2017-10-01 22:59	Blzut3	Note Added: 0018412
2017-10-02 22:23	WubTheCaptain	Note Added: 0018415
2017-10-02 22:30	WubTheCaptain	Note Added: 0018416
2017-10-02 22:33	WubTheCaptain	Note Edited: 0018415	View Revisions
2017-10-02 23:23	WubTheCaptain	Note Edited: 0018416	View Revisions
2017-10-02 23:33	WubTheCaptain	Note Edited: 0018416	View Revisions
2017-10-04 16:27	Zalewa	Note Added: 0018434
2017-10-05 02:49	WubTheCaptain	Status	new => acknowledged
2017-10-07 10:09	Zalewa	Note Added: 0018459
2017-10-07 10:09	Zalewa	Note Edited: 0018459	View Revisions
2017-10-07 10:30	WubTheCaptain	Note Added: 0018460
2017-10-07 10:38	WubTheCaptain	Note Added: 0018461
2017-10-07 10:41	WubTheCaptain	Note Edited: 0018461	View Revisions
2017-10-07 23:16	Blzut3	Note Added: 0018478
2017-10-10 09:49	WubTheCaptain	Note Added: 0018517
2017-10-10 09:50	WubTheCaptain	Note Edited: 0018517	View Revisions
2017-10-10 09:55	WubTheCaptain	Summary	Auto-updates issued over insecure HTTP, risk of arbitrary arbitrary code execution => Auto-updates issued over insecure HTTP, risk of arbitrary code execution
2017-10-11 17:29	Zalewa	Note Added: 0018532
2017-10-11 18:34	WubTheCaptain	Note Added: 0018534
2017-10-11 18:35	WubTheCaptain	Note Edited: 0018534	View Revisions
2017-11-11 11:26	Zalewa	Assigned To	=> Zalewa
2017-11-11 11:26	Zalewa	Status	acknowledged => assigned
2017-11-11 20:14	Blzut3	Note Added: 0018857
2017-11-12 17:14	WubTheCaptain	Note Added: 0018864
2017-11-12 17:14	WubTheCaptain	View Status	private => public
2017-12-12 14:38	Zalewa	Note Added: 0018957
2017-12-12 17:25	Zalewa	Note Added: 0018959
2017-12-12 17:25	Zalewa	Status	assigned => feedback
2017-12-13 00:51	WubTheCaptain	Note Added: 0018961
2017-12-13 00:51	WubTheCaptain	Status	feedback => assigned
2017-12-13 04:38	Blzut3	Note Added: 0018962
2017-12-13 18:37	Filystea	Note Added: 0018963
2017-12-14 00:48	Blzut3	Note Deleted: 0018963
2018-02-14 11:00	Zalewa	Assigned To	Zalewa =>
2018-02-14 11:15	Zalewa	Note Added: 0019036
2018-02-14 11:53	Zalewa	Status	assigned => acknowledged
2018-12-04 00:12	WubTheCaptain	Category	Bug => Security
2018-12-17 05:00	WubTheCaptain	Note Added: 0020263
2019-06-13 13:30	WubTheCaptain	Priority	urgent => high
2020-01-30 13:46	WubTheCaptain	Note Added: 0021158

Relationships