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

Implement checking for the integrity of downloaded remote releases. #11

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Festerdam
Copy link
Contributor

@Festerdam Festerdam commented Aug 22, 2023

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:

  • Linux and BSD
  • Windows
  • The Apple operating system

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 use certutil. And maybe Apple systems also have sha512sum, but I don't know anyone with an Apple system.

@MakovWait
Copy link
Owner

Thanks for your work! Could you explain why do we need these checks?

@Festerdam
Copy link
Contributor Author

@MakovWait said:

Thanks for your work! Could you explain why do we need these checks?

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 apt. They also automatically do such integrity checks when downloading packages.

@Festerdam
Copy link
Contributor Author

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.

@Festerdam
Copy link
Contributor Author

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.
I might in the future check it out on a virtual machine.

@MakovWait
Copy link
Owner

btw found a way to calculate the sha256 sum:

FileAccess.get_sha256(file_path)

@Festerdam
Copy link
Contributor Author

@MakovWait said

btw found a way to calculate the sha256 sum

Godot releases use SHA512.

@MakovWait MakovWait added the help wanted Extra attention is needed label Mar 15, 2024
@denovodavid
Copy link

I possess a Mac. I think the shasum util comes installed by default, which I believe could be used to check a SHA512. I'll try and have a look at this at some point soon.

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.

@denovodavid
Copy link

denovodavid commented Mar 17, 2024

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)

shasum should be installed by default, and according to the man page: "...shasum mimics the behavior of the combined GNU sha1sum, sha224sum, sha256sum, sha384sum, and sha512sum programs..."

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants