Skip to content
This repository was archived by the owner on Feb 26, 2024. It is now read-only.

Validate file is a gem on signature command#56

Merged
aellispierce merged 5 commits into
mainfrom
validate_file_gemminess
Jan 18, 2022
Merged

Validate file is a gem on signature command#56
aellispierce merged 5 commits into
mainfrom
validate_file_gemminess

Conversation

@aellispierce
Copy link
Copy Markdown

Previously, we could sign any random file. However, when the verification retrieved and verified the signature, it would blow up. This makes it so that the gemminess of a file is verified before we sign it, so that only legit gems can be signed.

Closes #37


installer.say "Verifying #{gem_path}"

raise Gem::CommandLineError, "#{gem_path} is not a file" unless File.file?(gem_path)
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

From what @jchestershopify and I could tell, when these pre-install hooks are called, Rubygems has already validated that the given package is a valid gem, both locally and remotely. If the file does not exist or is not a valid gemfile, no package exists on the installer at line 32. Plus, Rubygems raises an error:

gem install --verify-signatures blahh

[snip of irrelevant warning]
         
ERROR:  Could not find a valid gem 'blahh' (>= 0) in any repository
ERROR:  Possible alternatives: blah

We're confident this is unreachable, but if there's something we've overlooked please let us know.


private

def is_a_gem?(file)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think this method can be a bit simplified, because verify returns true if nothing is raised.

  def is_a_gem?(file)
    begin
      Gem::Package.new(file).verify
    rescue Gem::Package::FormatError
      false
    end
  end

The verify method also raises a Gem::Security::Exception exception. Have you had a look if it makes sense to rescue the exception as well?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@doodzik Hmmm looks like that exception will return something like root cert /CN=you/DC=example is not trusted if you have specified a gem security policy on build. Since we're creating a new package and verifying it in memory, the security policy would always be nil and Gem::Security::Exception should never raise.

So, I don't believe we need to rescue that and my preference would be to not rescue, as i think if it did happen that would be truly exceptional and we'd want to know about it.

aellispierce and others added 2 commits January 18, 2022 11:41
Co-authored-by: Jacques Chester <jacques.chester@shopify.com>
When these pre-install hooks are called, Rubygems has already validated
that the given package is a valid gem, both locally and remotely. If the
file does not exist or is not a valid gemfile, no package exists on the
installer at line 32. Plus, Rubygems raises an error.

Co-authored-by: Jacques Chester <jacques.chester@shopify.com>
@aellispierce aellispierce force-pushed the validate_file_gemminess branch from be9b963 to 37460c4 Compare January 18, 2022 16:42
Copy link
Copy Markdown

@rochlefebvre rochlefebvre left a comment

Choose a reason for hiding this comment

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

Nice!

Ruby 3.1 adds net/smtp to default standard library gems. Since we don't
have a mailer in this project we need to explicitly not include it.

Ref:
https://stackoverflow.com/questions/70500220/rails-7-ruby-3-1-loaderror-cannot-load-such-file-net-smtp
If numbers are not quoted, the YAML parser will treat 3.0 as '3' and so
the latest version minor version of 3, 3.1 will run instead of sticking
with the 3.0.x patch version.

Also adds quotes around the other ruby versions for consistency
@aellispierce aellispierce merged commit d157c51 into main Jan 18, 2022
@aellispierce aellispierce deleted the validate_file_gemminess branch January 18, 2022 22:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Only accept gem files gem signatures --sign and gem signatures --verify

3 participants