[WIP] Activate GPG verification upon install #16090

jawshooah · 2015-12-24T07:00:46Z

Picking up where @ndr-qef left off with #8749.

This is still a work-in-progress, so please feel free to throw your feedback at me.

fanquake · 2015-12-24T13:29:45Z

Nice work. Glad this is getting some love again.

tapeinosyne · 2015-12-24T18:41:36Z

Thanks for taking up the patchset, @jawshooah.

Beyond refactoring, it might be best to improve the CLI output —that is, hide the rather verbose one of GPG— in this PR, rather than postponing UX changes to a later date.

adidalal · 2015-12-24T18:49:30Z

A (rather simple) idea would just be to dump the output to dev/null altogether (unless the --verbose flag is used) and have a single line, similar to the shasum check, i.e. "GPG verification of Cask succeded/failed"

tapeinosyne · 2015-12-24T19:01:19Z

Even simpler would be to set :print_stdout => false when invoking system gpg with our system_command wrapper.

jawshooah · 2015-12-24T19:03:05Z

@ndr-qef I'm not very familiar with GPG, but should we be using gnupg2 instead of gnupg? Is the legacy version still supported?

jawshooah · 2015-12-24T19:05:33Z

Also should we just add depends_on :formula => 'gpg' to casks with a gpg stanza?

adidalal · 2015-12-24T19:41:45Z

I don't like adding that dependency to Casks, as gpg verification should be "best effort" - the notification that gpg verification was skipped (and to install gpg to activate it) should be enough

tapeinosyne · 2015-12-26T20:50:21Z

should we be using gnupg2 instead of gnupg? Is the legacy version still supported?

GnuPG 1 is stable and maintained, as well as widely deployed — notably in the Debian package manager, where it is the standard version to this day.

In any case, the gpg command is meant to call the standard system GPG, whichever that may be. (Much like git, ruby, and so on.) As it happens, many OS X users install GPG through GPGTools, which ships version 2.

jawshooah · 2015-12-26T21:01:30Z

Gotcha. Just wanted to make sure we weren't dating ourselves. I'll work on this more tonight.

dconnolly · 2016-02-16T17:36:12Z

What's the plan for getting this merged? I'm looking forward to checking signatures without local patches on top.

jawshooah · 2016-02-16T18:38:47Z

There's still a bit more work to be done before merging. I've been swamped for a while, but I'll try to get this out within the next few days.

dconnolly · 2016-02-16T20:17:03Z

Awesome, thank you!

fanquake · 2016-03-05T15:39:25Z

@jawshooah Any update on this?

fanquake · 2016-05-10T05:35:03Z

@jawshooah ping

jawshooah · 2016-05-18T14:07:01Z

Sorry for the huge delay on this. Life has been dropping a hammer on me the last few months. I should have some time this evening to work on this, though.

and store the result in a field instead. Failure handling is delegated.

Specifically, handle cases where the most recent metadata folder was created by something other than GpgCheck, e.g. an `install --force`.

and associated support files: test Casks, binaries, signature.

fanquake · 2016-06-15T02:31:29Z

@caskroom/maintainers Does anyone want to give any feedback here?

vitorgalvao · 2016-06-15T19:58:13Z

Hadn’t noticed there were commits after the last comment. Is this ready @jawshooah?

jawshooah · 2016-06-15T21:32:57Z

Should be functional, I've just been meaning to add more tests but haven't gotten around to it. Also needs a rebase.

claui · 2016-06-16T16:46:00Z

lib/hbc/verify/gpg.rb


-  def initialize(cask, downloaded_path, command=Hbc::SystemCommand)
+  def initialize(cask, downloaded_path, force_fetch=false, command=Hbc::SystemCommand)


I think we should change the force_fetch and command parameters inside the signature into an options hash. Options hashes clean up signatures and result in code that is easier to read.

That way, we would also stay consistent to existing, similar initializers (example 1 2 3).

Since we now assume users are running Ruby >=2.0, why not use keyword arguments?

@jawshooah Thanks for pointing that out. Those would be an even better choice!

claui · 2016-06-16T20:59:04Z

One more issue that needs to be discussed before we merge is error handling.

For example, as of right now

https://get.videolan.org/vlc/2.2.4/macosx/vlc-2.2.4.dmg

is up, while at the same time

https://get.videolan.org/vlc/2.2.4/macosx/vlc-2.2.4.dmg.asc

returns a HTTP 500 status:

We're very sorry but...
none of our mirrors are able to handle your request, please try again later

When I rebase this PR and then do brew cask install vlc, the following happens:

I think it’s perfectly acceptable for the whole installation to fail when the signature is unavailable. However, this should result in a clear error message indicating what happened, rather than just bailing out with a stack trace.

fanquake · 2016-07-03T06:44:48Z

This needs a rebase.

fanquake · 2016-08-08T04:01:41Z

ping @jawshooah Any chance you could rebase and update the error handling?

jawshooah · 2016-08-08T14:51:58Z

@fanquake Hopefully I can get to this later tonight.

jawshooah · 2016-10-17T19:20:02Z

Leaving this open since there is still interest, I just haven't gotten around to cleaning it up yet. Will try to migrate to Homebrew/brew later this evening.

jawshooah · 2016-10-20T05:25:03Z

Continued in Homebrew/brew#1335.

jawshooah added enhancement core Issue with Homebrew itself rather than with a specific cask. labels Dec 24, 2015

jawshooah self-assigned this Dec 31, 2015

adidalal mentioned this pull request Jan 3, 2016

Activate GPG verification upon install #8749

Closed

kleetus mentioned this pull request Apr 6, 2016

Copay added to mac homebrew cask system bitpay/wallet#4007

Closed

ndr and others added 10 commits May 18, 2016 12:12

Do not abort on failed GPG check

89d6811

and store the result in a field instead. Failure handling is delegated.

Style nits in Hbc::GpgCheck

d1ca936

Expose more fields in Hbc::GpgCheck

cd9ddd1

Fix caching of gpg signatures

d4b737f

Specifically, handle cases where the most recent metadata folder was created by something other than GpgCheck, e.g. an `install --force`.

Wrap GPG key import failure with CaskError

7a5c698

Add exception for failed GPG verification

c45c56d

Do not purge files upon failed GPG verification

671c089

Raise error if GPG verification unsuccessful

9ead7e9

Add Hbc::Verify::Gpg to list of verifications

e5d8668

Add test helpers for local support path

b74ceba

ndr and others added 8 commits May 18, 2016 12:12

Load gpg signature, key_url as Hbc::URLs

90da219

Add gpg tests

3a14456

and associated support files: test Casks, binaries, signature.

Remove Hbc::Verify::Gpg#successful

181bc96

Don't retrieve signature until #verify is called

009179b

Print warning if gpg is not installed

e034977

Install gpg before running tests in Travis

b2abd8d

Use cask accessor instead of @cask instance variable

ad1b200

fixup! Add gpg tests

4fce884

fanquake added the awaiting maintainer feedback Issue needs response from a maintainer. label Jun 3, 2016

claui reviewed Jun 16, 2016
View reviewed changes

adidalal mentioned this pull request Jun 28, 2016

Documentation: GPG stanza parameters #22458

Closed

fanquake changed the title ~~Activate GPG verification upon install (rebased)~~ Activate GPG verification upon install Aug 10, 2016

jawshooah changed the title ~~Activate GPG verification upon install~~ [WIP] Activate GPG verification upon install Aug 12, 2016

jawshooah mentioned this pull request Oct 20, 2016

cask: activate GPG verification on install Homebrew/brew#1335

Closed

5 tasks

jawshooah removed the awaiting maintainer feedback Issue needs response from a maintainer. label Oct 20, 2016

jawshooah closed this Oct 20, 2016

jawshooah deleted the gpg branch October 20, 2016 05:25

miccal removed core Issue with Homebrew itself rather than with a specific cask. enhancement labels Dec 23, 2016

miccal unassigned jawshooah Dec 23, 2016

Homebrew locked and limited conversation to collaborators May 8, 2018


		def initialize(cask, downloaded_path, command=Hbc::SystemCommand)
		def initialize(cask, downloaded_path, force_fetch=false, command=Hbc::SystemCommand)

Uh oh!

[WIP] Activate GPG verification upon install #16090

[WIP] Activate GPG verification upon install #16090

Uh oh!

Conversation

jawshooah commented Dec 24, 2015

Uh oh!

fanquake commented Dec 24, 2015

Uh oh!

tapeinosyne commented Dec 24, 2015

Uh oh!

adidalal commented Dec 24, 2015

Uh oh!

tapeinosyne commented Dec 24, 2015

Uh oh!

jawshooah commented Dec 24, 2015

Uh oh!

jawshooah commented Dec 24, 2015

Uh oh!

adidalal commented Dec 24, 2015

Uh oh!

tapeinosyne commented Dec 26, 2015

Uh oh!

jawshooah commented Dec 26, 2015

Uh oh!

dconnolly commented Feb 16, 2016

Uh oh!

jawshooah commented Feb 16, 2016

Uh oh!

dconnolly commented Feb 16, 2016

Uh oh!

fanquake commented Mar 5, 2016

Uh oh!

fanquake commented May 10, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jawshooah commented May 18, 2016

Uh oh!

fanquake commented Jun 15, 2016

Uh oh!

vitorgalvao commented Jun 15, 2016

Uh oh!

jawshooah commented Jun 15, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claui Jun 16, 2016

Choose a reason for hiding this comment

Uh oh!

jawshooah Jun 16, 2016

Choose a reason for hiding this comment

Uh oh!

claui Jun 16, 2016

Choose a reason for hiding this comment

Uh oh!

claui commented Jun 16, 2016

Uh oh!

fanquake commented Jul 3, 2016

Uh oh!

fanquake commented Aug 8, 2016

Uh oh!

jawshooah commented Aug 8, 2016

Uh oh!

jawshooah commented Oct 17, 2016

Uh oh!

jawshooah commented Oct 20, 2016

Uh oh!

Uh oh!

fanquake commented May 10, 2016 •

edited

Loading

jawshooah commented Jun 15, 2016 •

edited

Loading