Open thomasf opened 8 years ago
@thomasf Thanks for reporting this! I really appreciate it.
I will note that even when incorporating the version into the hash/signature calculations, an attacker who compromises the mechanism used to pull the metadata that describes 'the next available update' could still force a downgrade unless the second check for 'target version being higher than the current version' is implemented (which is difficult for a few reasons, timestamps are better). In short: compromising the update check could allow an attacker to return a valid tuple of [old version, old_signature, payload of old version] and circumvent this fix as well.
This kind of problem is avoided by using a more robust publishing/updating framework like TUF.
go-update's docs need to provide a security section explaining the threat model and the tradeoffs and compromises made in the design of the library.
In the meantime, this additional check will improve things, thanks again for sharing it.
I will implement the valid version check in my client using a semver library so that the client only accepts a semver version newer than itself.
Btw. I'm getting to this item some time tomorrow in the project I'm working on (which currently is being moved to https://github.com/alkasir/alkasir ).
Btw. would you like me to contribute https://github.com/alkasir/alkasir/blob/master/pkg/upgradebin/ed25519Verifier.go to go-update?
So.. this is how I solved in my use case.. It could possibly be a pattern of interest to other people...
Adding the version number to the verifier: https://github.com/alkasir/alkasir/commit/5d57cc6168f958775583a6a4be8c1d5e79d72369
And a Version number check in the client before even trying to accept a patch: https://github.com/alkasir/alkasir/commit/ca8a5c13f585f91135dc990de8c7020755fd46ac
Hi there.. An security audit of a software using this package revealed a possible problem with this package. I think that this is relevant also for upstream even If I can (and am) fixing this for my own usage as well.
Here's a a slightly edited section from the report, I have added the ED25519 hash myself:
Updater allows for a rogue Version Downgrade (Medium)
Update metadata (including a SHA256 hash over the updated binary and an ED25519 signature over the SHA256 hash
...
After that a patch dedicated to updating to the new version is downloaded over HTTPS.
...
The code in pkg/upgradebin/upgradebin.go then uses vendor/github.com/ inconshrevable/go-update /apply.go to verify and apply the update. Within this process the patch is firstly applied to the current binary in order to create a new binary. Before the old binary is replaced with the new one, the SHA256 checksum of the new binary is compared with the expected one as well as checked against the received ED25519 signature. Besides the usage of TLS as transport and the security provided by ..., there are no cryptographically protected version checks occurring throughout this process. Therefore it opens doors for an attacker without the ED25519 update signing key who nevertheless either has access to [server] or is otherwise in possession of a valid TLS certificate for [server] and MitM access 5 somewhere between the bridge and [server]. In effect, the attacker could deliver a malicious “update” to a victim, causing the binary to be downgraded to an earlier less- secure version. This could mean that if the victim has version 3 and the attacker wants to rollback to version 2, he requests the update metadata from version 1 to version 2 from the server, obtaining a signature over version 2. He or she would then need to calculate a patch from version 3 to version 2 and deliver it to the victim together with the obtained signature.
It is recommended to amend the updater so that the signature is calculated over the hash of the concatenation of the target version, a delimiter that cannot occur in a version number (e.g. a null byte) and the binary (or its hash). The updater should additionally verify that the target version is in fact higher than the current version.