-
Notifications
You must be signed in to change notification settings - Fork 33
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
Implement checking for the integrity of downloaded remote releases. #11
base: main
Are you sure you want to change the base?
Conversation
Thanks for your work! Could you explain why do we need these checks? |
@MakovWait said:
Checking the checksum of a file is good to make sure that the file hasn't been corrupted during transmission (https://en.wikipedia.org/wiki/Checksum). That's what the SHA512-SUMS.txt file, generally found alongside godot release zips, is for. If the checksum files were also to be signed (which they at the moment aren't), it would also allow to verify that the release was released by the godot developers and hasn't been tampered with by humans with malicious intentions. While the TCP protocol already performs some error-checking on the data delivered, it isn't infallible. An example of software that also does such integrity checks are package managers, such as |
I noticed on really slow systems, it is noticeable when OS.execute blocks the project to run the tool that verifies the checksum. I will need to change the code to use OS.create_process, to not freeze the program on those systems when the check is made. |
It should already work as it is now. However this is not implemented for apple systems (because I don't have one to test), so if anyone wants to help, feel free. |
btw found a way to calculate the sha256 sum: FileAccess.get_sha256(file_path) |
@MakovWait said
Godot releases use SHA512. |
I possess a Mac. I think the There is also a PR (godotengine/godot#79540) to add SHA512 to Godot's crypto utilities, which would likely make this easier, but the branch needs serious cleaning up and it's undecided how it would work when compiling sans crypto lib; so that might have to wait. |
Booted up the PR this morning and got this working on macOS: # res://src/components/editors/remote/remote_editor_download/remote_editor_download.gd
"macOS":
var command_template = "cd '%s' && shasum -a 512 -c --ignore-missing --status '%s'"
var command = command_template % [globalized_directory_path, _CHECKSUM_FILENAME]
var status = OS.execute("bash", ["-c", command])
call_deferred("emit_signal", "integrity_check_completed", status == 0 or status == 127)
I used a format string to make it more readable, adding single quotes around the file paths to handle any spaces (which should probably be done for the linux codepath as well). Tested in editor and exported build for both tuxfamily and github downloads. |
This PR implements checking the integrity of downloaded remote releases, by comparing their sha512 sum.
Note that this PR does not verify the authenticity of the release. Doing so is impossible to do, because Godot does not at this time sign their checksums. There is however a proposal to do so (godotengine/godot-proposals#6042). Therefore, the absence of a SHA512-SUMS.txt (and therefore the impossibility of checking the file's integrity) does not constitute a failure and is simply ignored.
Currently implemented for:
This was implemented using system commands, because Godot doesn't at this moment natively support SHA512.
I only have a Linux system to test on and therefore was only able to write the code for Linux and BSD (assuming those generally have the
sha512sum
binary installed). I think on Windows one could usecertutil
. And maybe Apple systems also havesha512sum
, but I don't know anyone with an Apple system.