-
Notifications
You must be signed in to change notification settings - Fork 88
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
Library builders do not verify integrity of downloaded files #308
Comments
Yes, sure, a PR would be good. But how will the hash get passed to fetch_unpack? |
I was planning adding arguments up the chain - in 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 |
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.
|
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 Maybe the solution will be just to add separate function |
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.
|
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:
As for now, I prefer option #2 above and to make it work, I would split |
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 |
Yes, that's my proposal. I've submitted a PR as it may be easier, what exactly I'm proposing |
For a previous implementation that does this functionality, you can take a look at |
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.
The text was updated successfully, but these errors were encountered: