Library builders do not verify integrity of downloaded files

[wiktorn]

Currently functions in library_builders.sh do not verify integrity of downloaded files.

The good practice would be to include SHA256 hashes of the files among the versions and verify the checksum and fail the build on mismatch.

This can be achieved by adding new optional argument to fetch_unpack in common_utils.sh, which would take sha256 sum to verify against downloaded file.

Is this something, you'd like to see PR on?

Originally reported as osmcode/pyosmium-wheel-build#2, but as we move towards manylinux2010 the only "unsafe" downloads will be those done by multibuild.

[matthew-brett]

Yes, sure, a PR would be good. But how will the hash get passed to fetch_unpack?

[wiktorn]

I was planning adding arguments up the chain - in build_simple and build_* functions for specific libraries. And for all versions referenced in beginning of the file would be extended with hash values.

Though, going through the code now I notice, that for openssl there is already a sha256 check after call to fetch_unpack.

I'm leaning towards my primary plan (verification in fetch_unpack) but maybe I should follow openssl way?

[matthew-brett]

My worry was that I didn't want to force the use of these hashes - and it may be difficult to make the hash arguments optional to the build calls. For the wheels I build - as long as I'm sure that openssl is OK, and that the library URL is https, that's good enough for me.

[wiktorn]

Just for your consideration, any other library (once maliciously rigged) may export the same symbols that openssl exports and if it's loaded before openssl, it will be it's function called and not openssl's. So guarding openssl works as long as openssl is the only library that you link against in your wheel.

As I understand, your concern is, that it might be difficult to have two optional arguments in fetch_unpack and have the possibility to provide only one of them.

Maybe the solution will be just to add separate function fetch_unpack_check that will do also hash checking. I'll play a bit and come back with possible approaches here.

[matthew-brett]

I'm probably not understanding your point, but what I meant was, once I can trust OpenSSL in the container doing the building, then I tend to trust libraries fetched via https from some standard URL. Of course it's possible that someone will hijack e.g. https://sourceware.org/pub/bzip2 - but relatively unlikely. Trusting OpenSSL depends on the OpenSSL built with the container of course, and not the OpenSSL built with multibuild. I wonder whether there would be a way to provide a grid of SHAs to fetch_unpack, maybe as a global, such that it would check the SHA if a relevant SHA was available. Then the user could fill in the grid for the libraries they were using.

…

On Mon, Feb 17, 2020 at 10:16 PM Wiktor Niesiobędzki ***@***.***> wrote: Just for your consideration, any other library (once maliciously rigged) may export the same symbols that openssl exports and if it's loaded before openssl, it will be it's function called and not openssl's. So guarding openssl works as long as openssl is the only library that you link against in your wheel. As I understand, your concern is, that it might be difficult to have two optional arguments in fetch_unpack and have the possibility to provide only one of them. Maybe the solution will be just to add separate function fetch_unpack_check that will do also hash checking. I'll play a bit and come back with possible approaches here. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

[wiktorn]

I misunderstood because openssl download is now verified against the hash, so I thought you meant openssl built within wheels.

I did a bit of research and:

1. hashtable is not possible as macOS is stuck with bash3 due to licensing
2. indirect variables ("${!var_name}") like ZLIB_HASH=... looks reasonable, if it would be used within build_simple and not within fetch_unpack, where we have URL and filename. <filename>_HASH=... will be for most cases improper variable name
3. create a file, e.g. archives/sha256.sums which would contain all hashes for all filenames. To check them, grep to separate file is necessary, as Centos 6's sha256sum doesn't support --ignore-missing option

As for now, I prefer option #2 above and to make it work, I would split fetch_unpack to two functions (fetch returning $out_archive and unpack), and call check_sha256sum in between them. There it will be easy to use specific variable or use indirect expansion, to get proper variable to get hash. This will also provide same interface to change version and hash value for wheel builders.

[matthew-brett]

Sorry - just checking - for option 2 - you are proposing that - say - in build_bzip - that after fetching the bzip archive, the code should check for a variable BZIP_HASH, which, if present would contain the correct hash for the downloaded file. The user would have to define BZIP_HASH ZLIB_HASH and so on for all packages they wanted to check, and create the hashes corresponding to the versions they plan to download.

[wiktorn]

Yes, that's my proposal. I've submitted a PR as it may be easier, what exactly I'm proposing

[native-api]

For a previous implementation that does this functionality, you can take a look at fetch_tarball() in Python-build. It consumes a URL that's optionally suffixed with #<hash>, either MD5 or SHA256. Quite a convenient way to specify checksums together with URLs if you ask me.