#471 crossgcc downloads 120MB of sources over plain http without any verification

Closed
opened 1 year ago by im_a_bug · 11 comments
im_a_bug commented 1 year ago

The culprit is the util/crossgcc/buildgcc script in the crossgcc module. That script does some checksum verification, but the astute reader will note that the verification is only used to determine whether the sources should be re-downloaded. After the download, the checksum is never checked!

You can verify for yourself by lightly corrupting util/crossgcc/sum/gcc-5.3.0.tar.bz2.cksum, and checking that buildgcc happily downloads and builds gcc anyway.

The culprit is the `util/crossgcc/buildgcc` script in the `crossgcc` module. That script does some checksum verification, but the astute reader will note that the verification is only used to determine whether the sources should be re-downloaded. After the download, the checksum is never checked! You can verify for yourself by lightly corrupting `util/crossgcc/sum/gcc-5.3.0.tar.bz2.cksum`, and checking that `buildgcc` happily downloads and builds gcc anyway.
im_a_bug commented 1 year ago
Poster

Here's a patch to fix this. Should probably let upstream know about it too. It also wouldn't hurt to switch to a stronger hash, but that's a separate matter.

--- a/util/crossgcc/buildgcc
+++ b/util/crossgcc/buildgcc
@@ -304,11 +304,15 @@ download() {
 		printf "(downloading from $archive)"
 		rm -f tarballs/$FILE
 		cd tarballs
 		download_showing_percentage $archive
 		cd ..
-		compute_sum $FILE
+		# compute_sum $FILE # <-- uncomment this to save the new checksum
+		if ! check_sum $FILE ; then
+			printf "\n${RED}Checksum verification failed for $FILE.${NC}\n"
+			exit 1
+		fi
 	fi
 
 	if [ ! -f tarballs/$FILE ]; then
 		printf "\n${RED}Failed to download $FILE.${NC}\n"
 		exit 1
Here's a patch to fix this. Should probably let upstream know about it too. It also wouldn't hurt to switch to a stronger hash, but that's a separate matter. ``` --- a/util/crossgcc/buildgcc +++ b/util/crossgcc/buildgcc @@ -304,11 +304,15 @@ download() { printf "(downloading from $archive)" rm -f tarballs/$FILE cd tarballs download_showing_percentage $archive cd .. - compute_sum $FILE + # compute_sum $FILE # <-- uncomment this to save the new checksum + if ! check_sum $FILE ; then + printf "\n${RED}Checksum verification failed for $FILE.${NC}\n" + exit 1 + fi fi if [ ! -f tarballs/$FILE ]; then printf "\n${RED}Failed to download $FILE.${NC}\n" exit 1 ```

If that does indeed solve the issue (I am not sure that it does, currently investigating), this patch could be applied in resources/scripts/helpers/download/crossgcc

If that does indeed solve the issue (I am not sure that it does, currently investigating), this patch could be applied in resources/scripts/helpers/download/crossgcc
Leah Rowe commented 1 year ago
Owner

could you patch the libreboot sources appropriately, and send a pull request? since this applies to coreboot upstream too, you could also send a patch there

For libreboot: https://libreboot.org/git.html

For coreboot: https://www.coreboot.org/Git

could you patch the libreboot sources appropriately, and send a pull request? since this applies to coreboot upstream too, you could also send a patch there For libreboot: https://libreboot.org/git.html For coreboot: https://www.coreboot.org/Git

Yeah, that doesn't solve the underlying issue. It's certainly a step in the right direction, but is only half way there.

Directly above that function, in download_showing_percentage, wget is used with --no-check-certificate, which should be fixed.

Additionally, links from line 57 to 72, HTTPS should be used instead of HTTP where possible. Currently, there are several servers that the subdomain ftpmirror.gnu.org points to that do indeed serve content over HTTPS.

Yeah, that doesn't solve the underlying issue. It's certainly a step in the right direction, but is only half way there. Directly above that function, in download_showing_percentage, wget is used with --no-check-certificate, which should be fixed. Additionally, links from line 57 to 72, HTTPS should be used instead of HTTP where possible. Currently, there are several servers that the subdomain ftpmirror.gnu.org points to that do indeed serve content over HTTPS.
im_a_bug commented 1 year ago
Poster

Yeah, that doesn't solve the underlying issue.

I actually believe it does, at least if you trust the security of the cryptographic hash used (in this case, SHA1). If an active attacker swaps the file downloaded for a malicious one, you will detect it because the hash check will fail. Switching the downloads over to https is at most a privacy benefit as a passive attacker won't be able to tell you're downloading an ancient version of gcc :p

Note that the git:// protocol is also unencrypted, so if an attacker were able to break SHA1 (preimage attack), they could also just intercept the numerous git clone operations performed during a clean libreboot build, and swap a git object for a malicious one with the same hash.

(That being said, it would be nice to switch the checksum hash to SHA2-512 since SHA1 is being phased out everywhere. Maybe as a separate patch?)

@vimuser OK, I'll do those things soon. Thanks!

> Yeah, that doesn't solve the underlying issue. I actually believe it does, at least if you trust the security of the cryptographic hash used (in this case, SHA1). If an active attacker swaps the file downloaded for a malicious one, you will detect it because the hash check will fail. Switching the downloads over to https is at most a privacy benefit as a passive attacker won't be able to tell you're downloading an ancient version of gcc :p Note that the `git://` protocol is also unencrypted, so if an attacker were able to break SHA1 (preimage attack), they could also just intercept the numerous git clone operations performed during a clean libreboot build, and swap a git object for a malicious one with the same hash. (That being said, it would be nice to switch the checksum hash to SHA2-512 since SHA1 is being phased out everywhere. Maybe as a separate patch?) @vimuser OK, I'll do those things soon. Thanks!

At least in my opinion, privacy is most certainly security.

If you know of any git URIs using the git:// protocol in libreboot, please let me know - I'll fix it!

At least in my opinion, privacy is most certainly security. If you know of any git URIs using the git:// protocol in libreboot, please let me know - I'll fix it!
im_a_bug commented 1 year ago
Poster

Fair enough, I'll make the required modifications. I hope the generic auto-redirect https://ftpmirror.gnu.org URL never redirects to a http mirror.

Fair enough, I'll make the required modifications. I hope the generic auto-redirect `https://ftpmirror.gnu.org` URL never redirects to a `http` mirror.
SolidHal commented 1 year ago

@vimuser, @im_a_bug, Looks like this was fixed some time ago in upstream and is in the version downloaded by the libreboot build system:

commit 92d483a8927066ef16603023be05ad1fb7dea644
Author: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
Date:   Tue Sep 26 22:33:53 2017 +0200

    buildgcc: Implement simple tarball hash verification
    
    This patch implements a relatively simple hash-based verification scheme
    for downloaded files (tarballs):
    
    After buildgcc downloads a file or notices that it has already been
    downloaded, it hashes the file, and compares the hash against the known
    hash stored in util/crossgcc/sum/$filename.cksum. Two errors can occur:
    
    1. The hash file is missing. In this case, crossgcc asks the user to
       verify the authenticity of the downloaded file. It also calculates
       its hash and stores it in util/crossgcc/sum/$filename.cksum.calc.
       If the file is authentic, the user may rename the calculated hash
       file to $filename.cksum, so that it can be found the next time
       buildgcc is started.
    
    2. The known hash and the calculated hash differ. This is the case that
       this patch seeks to protect against, because it may imply that the
       downloaded file was unexpectedly changed, either in transit
       (Man-in-the-Middle attack) or on the file server that it was
       downloaded from. If buildgcc detects such a hash mismatch, it asks
       the user to delete the downloaded file and retry, because it can also
       be caused by a benign network error. If, however, the error persists,
       buildgcc can't continue without risking that the user runs malicious
       code, and it stops.
    
    Note: The hash algorithm may be changed in the future, but for now I
    left it at SHA-1, to avoid bloating this patch.
    
    Change-Id: I0d5d67b34684d02011a845d00f6f5b6769f43b4f
    Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
    Reviewed-on: https://review.coreboot.org/21592
    Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
    Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
    Reviewed-by: Patrick Georgi <pgeorgi@google.com>

Also, @johnmh, looking at buildgcc downloaded when I ran "./libreboot download coreboot", all of the mirrors on line 57 to 72 are set to use HTTPS

@vimuser, @im_a_bug, Looks like this was fixed some time ago in upstream and is in the version downloaded by the libreboot build system: ``` commit 92d483a8927066ef16603023be05ad1fb7dea644 Author: Jonathan Neuschäfer <j.neuschaefer@gmx.net> Date: Tue Sep 26 22:33:53 2017 +0200 buildgcc: Implement simple tarball hash verification This patch implements a relatively simple hash-based verification scheme for downloaded files (tarballs): After buildgcc downloads a file or notices that it has already been downloaded, it hashes the file, and compares the hash against the known hash stored in util/crossgcc/sum/$filename.cksum. Two errors can occur: 1. The hash file is missing. In this case, crossgcc asks the user to verify the authenticity of the downloaded file. It also calculates its hash and stores it in util/crossgcc/sum/$filename.cksum.calc. If the file is authentic, the user may rename the calculated hash file to $filename.cksum, so that it can be found the next time buildgcc is started. 2. The known hash and the calculated hash differ. This is the case that this patch seeks to protect against, because it may imply that the downloaded file was unexpectedly changed, either in transit (Man-in-the-Middle attack) or on the file server that it was downloaded from. If buildgcc detects such a hash mismatch, it asks the user to delete the downloaded file and retry, because it can also be caused by a benign network error. If, however, the error persists, buildgcc can't continue without risking that the user runs malicious code, and it stops. Note: The hash algorithm may be changed in the future, but for now I left it at SHA-1, to avoid bloating this patch. Change-Id: I0d5d67b34684d02011a845d00f6f5b6769f43b4f Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net> Reviewed-on: https://review.coreboot.org/21592 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net> Reviewed-by: Patrick Georgi <pgeorgi@google.com> ``` Also, @johnmh, looking at buildgcc downloaded when I ran "./libreboot download coreboot", all of the mirrors on line 57 to 72 are set to use HTTPS

@SolidHal I don't know what ./libreboot download is. The only download script I'm aware of is ./download. Perhaps "newbuild" has something that was not put into "oldbuild"?

@SolidHal I don't know what `./libreboot download` is. The only download script I'm aware of is `./download`. Perhaps "newbuild" has something that was not put into "oldbuild"?
SolidHal commented 1 year ago

@johnmh ./libreboot is the new build system, I'm on a c201 so its the only build system I'm very familiar with.

./download crossgcc gets an older version

# coreboot revisios used for crossgcc
crossgccrevision="35562d8b6477058e6bca31b5cedd9d4897124fc7"
vbootrevision="d187cd3fc792f8bcefbee4587c83eafbd08441fc"

rm -Rf "crossgcc/"
(
    git clone https://review.coreboot.org/coreboot crossgcc || git clone https://github.com/coreboot/coreboot.git crossgcc
    cd "crossgcc/"
    git reset --hard ${crossgccrevision}
    git submodule update --init --checkout -- 3rdparty/vboot/

So either a newer version needs to be tested, or a patch is required.

I made a pull request that backports the change as a patch and a modification to apply it on download: #487

@johnmh ./libreboot is the new build system, I'm on a c201 so its the only build system I'm very familiar with. ./download crossgcc gets an older version ``` # coreboot revisios used for crossgcc crossgccrevision="35562d8b6477058e6bca31b5cedd9d4897124fc7" vbootrevision="d187cd3fc792f8bcefbee4587c83eafbd08441fc" rm -Rf "crossgcc/" ( git clone https://review.coreboot.org/coreboot crossgcc || git clone https://github.com/coreboot/coreboot.git crossgcc cd "crossgcc/" git reset --hard ${crossgccrevision} git submodule update --init --checkout -- 3rdparty/vboot/ ``` So either a newer version needs to be tested, or a patch is required. I made a pull request that backports the change as a patch and a modification to apply it on download: #487
Andrew Robbins commented 1 year ago
Collaborator

Resolved by pull request #487. Closed.

Resolved by pull request #487. Closed.
Sign in to join this conversation.
Loading...
Cancel
Save
There is no content yet.