Activate GPG verification upon install #8749
Conversation
and store the result in a field instead. Failure handling is delegated.
tapeinosyne
added
the
core
|
I would like to move forward and address the issue of user-facing UX separately. @phinze, this PR is tangled with parts of the codebase you may not be too familiar with (e.g. metadata), but perhaps you would like to review it anyway? Some segments could likely be clarified. |
|
This is great, @ndr-qef! I might suggest some rearrangement of the methods in Re: output - I think I fall on the side of making the output cleaner and maybe just outputting the fingerprint with an OK. But similarly I think that's something we can iterate on after this feature lands. I'm really happy to see this security-focused stuff moving forward - it's something I've wanted to do in the project for a long time. 🎉 🔐 |
Oh, there is no particular urgency to merge; my intention is to bring the core functionality to a working state, largely free from further tinkering. If you already have specific advice, I would welcome it. |
Specifically, handle cases where the most recent metadata folder was created by something other than GpgCheck, e.g. an `install --force`.
and require `gpg_check`, thereby introducing an entry point for GPG verification.
and associated support files: test Casks, binaries, signature.
|
|
||
| meta_dir = cached || @cask.metadata_subdir('gpg', :now, true) | ||
| sig_path = meta_dir.join("signature.asc") | ||
| sig_path = meta_dir.join('signature.asc') |
There was a problem hiding this comment.
Does this assume the signature is always in the signature.asc file? This may not always be the case—and actually isn't for a lot of software. The creators may have a signature with a different name, or (as is most common) may have clearsigned the sha256 hashes of all the executables they serve for their project.
There was a problem hiding this comment.
No, that is merely the path to our local copy of the signature.
There was a problem hiding this comment.
Right, sorry for not researching better what that was :)
|
@ndr-qef What is this waiting on? |
|
I'd really like to see this merged, @vitorgalvao given that @ndr-qef has gone quiet recently, how would you like to move forward here? |
|
Closing as #16090 builds off of this PR and thus includes the changes proposed here. |
miccal
removed
core
Refer to #5971.
This patchset lands GPG verification as a working feature, with a minimum of tests. GPG verification is automatically enabled if a working
gpgbinary is found on the system.Most notably, two decisions are open for debate.
Failure handling
Handling of failed GPG verification is delegated from the
Hbc::GpgCheckclass to the caller. (As of this PR, the only caller is intended to beHbc::Verify.gpg_signature.)This is largely a matter of internal design, with no functional consequences.
CLI output
Standard
gpgoutput is printed unmodified. Security features may benefit from a transparent UI, but I worry that the galore of lines and information is noise to most users, or otherwise irrelevant.As an example, consider the output of
brew cask install libre-office, sans our own GPG messages:(For the purpose of this discussion, ignore truncation as an unrelated issue.)
It is arguably desirable to silence
gpgcommands, as commonly done by other package managers, and favor more concise notifications for each step: key retrieval, signature download, and final check.