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

Improve download task gpg verification #207

Merged
merged 1 commit into from
Sep 10, 2021
Merged

Conversation

reidmv
Copy link
Contributor

@reidmv reidmv commented Sep 10, 2021

Iterating on this task to ensure it behaves well in an air-gapped
environment by default.

  • Start by downloading the .asc file, always. If we don't re-download
    that file, we aren't really verifying anything.
  • Use the .asc file to try and verify existing content. Distinguish
    between service-not-available (such as gpg not present, or unable to
    reach keyserver) and verification failure.
  • If unable to retrieve the .asc file, or if service-not-available,
    fall back to size-verification.
  • Parameterize the key ID and key server values, to keep the download
    task plausibly generic, and not just for downloading the PE tarball.

@reidmv reidmv requested a review from a team as a code owner September 10, 2021 20:04
Iterating on this task to ensure it behaves well in an air-gapped
environment by default.

 * Start by downloading the .asc file, always. If we don't re-download
   that file, we aren't really verifying anything.
 * Use the .asc file to try and verify existing content. Distinguish
   between service-not-available (such as gpg not present, or unable to
   reach keyserver) and verification failure.
 * If unable to retrieve the .asc file, or if service-not-available,
   fall back to size-verification.
 * Parameterize the key ID and key server values, to keep the download
   task plausibly generic, and not just for downloading the PE tarball.
@reidmv reidmv force-pushed the iterate-on-download-verify branch from 23a208e to f31b6ae Compare September 10, 2021 20:13
Copy link
Member

@ody ody left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I expect the task to verify the download and am not aware that I can't fetch keys then the only way I know it skipped verification and fell back to old size comparison is by reading logs. My expectation is that it would fail if it couldn't verify and I the user would have to pass a "verify_fallback" option to explicitly accept old behavior or go through the work of manually importing the correct things ahead of time

@ody ody self-requested a review September 10, 2021 20:37
Copy link
Member

@ody ody left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After further discussion in Slack, that is good. The goal was to simply verify the success of the download and not validate the authenticity of the download

@reidmv reidmv merged commit ea5d1d8 into main Sep 10, 2021
@reidmv reidmv deleted the iterate-on-download-verify branch September 10, 2021 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants