[Security] Use SHA-256 checksums from trusted mirrors only #493
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![sonatype-lift[bot]](https://avatars.githubusercontent.com/in/9843?s=60&v=4){kind=small}
To keep this commit small, `hashurl` was removed from QtPackage, and `get_hash` constructs the hash url based on the url of the 7z archive to download. I think that in the future, QtArchive and QtPackage could be refactored to construct this url more appropriately. However, this would be a complicated change that doesn't belong in this commit.
This uses a fixture to load the default settings file before each test.
Tests that require a different settings file (ie test_settings) can be
configured to skip this step using this line of code:
@pytest.mark.load_default_settings(False)
This change is necessary to be able to run these tests in isolation from
each other. The `altlink`, `getUrl` and `downloadBinaryFile` functions
are all dependent on Settings, and you cannot test them without loading
a Settings file first. Without this change, when these tests ran, they
used a Settings object loaded up in some previous test, which could have
loaded some Settings file that we do not want. If we try to run these
tests without loading a Settings file, they will just fail to run
because Settings doesn't exist yet.
This splits QtArchive.archive_url into two new datamembers: archive_path and base_url. Ultimately, base_url should be removed from QtPackage entirely.
ddalcino
force-pushed
the
retry_checksums
branch
from
9387368 to
7fa7200
Compare
aqt.helper.MyConfigParser.getlist fails to retrieve a fallback list when the `section` parameter does not exist in the `settings.ini` file. This ensures that the fallback is used when that key is missing.
ddalcino
force-pushed
the
retry_checksums
branch
from
479233c to
12d20a3
Compare
`MetadataFactory.fetch_http` must often download HTML pages, not Updates.xml files. download.qt.io does not store checksums for these files, so this particular function must be allowed to download these pages without using a checksum.
|
The change looks good and all the installation/compilation test are passed, and I observed that these tests downloads SHA256 hash files for check. |
|
I can’t help but worry that there’s some security flaw that I’m overlooking here. This sonatype output shows several preexisting flaws that we should probably fix before a full v2.1.0 release (maybe this deserves a separate issue): On the other hand, I agree that this change is important enough to let users begin using it as soon as possible. I prefer to release it as v2.1.0rc1. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change causes
aqtto use SHA-256 checksums for all downloads, and only download them from a list of trusted mirrors. By default, this list includes only https://download.qt.io, but the settings.ini file may be used to modify this list. The settings.ini file can also be used to set the number of times to retry the failed download of a checksum.Partially addresses #488
Todo: