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

Chain of exec's instead of curl | bash #133

Closed
wants to merge 1 commit into from

Conversation

Hoodoo
Copy link

@Hoodoo Hoodoo commented Dec 1, 2016

If I understand correctly, the module currently relies on a script it downloads to call for verification.

I propose to download the installer and the detached key, verify and install as described here: https://rvm.io/rvm/security instead.

It is a bit procedural and maybe not the best style but it beats piping unverified code to bash IMO.

@Hoodoo
Copy link
Author

Hoodoo commented Dec 1, 2016

All checks have failed with this:

Gem::InstallError: beaker requires Ruby version >= 2.2.5.
An error occurred while installing beaker (3.5.0), and Bundler cannot continue.
Make sure that `gem install beaker -v '3.5.0'` succeeds before bundling.
The command "eval bundle install --jobs=3 --retry=3" failed 3 times.
The command "bundle install --jobs=3 --retry=3" failed and exited with 5 during .
Your build has been stopped.

@TJM
Copy link

TJM commented Dec 1, 2016

@Hoodoo - Creating predictable temporary files (as root especially) is a bad idea. Also, many "semi-secure" environments have "noexec" on /tmp. Maybe we could use the "puppet-staging" module to do the downloads, which places them in a more protected path (/opt/staging by default), rather than running curl directly. In addition to that, it feels like maybe the "staging" module could have an improvement to "verify" downloads, rather than having the rvm module doing gpg verification? I know that puppet-archive has "checksum" validation, which is better than nothing. At a minimum, I suggest creating a "temporary directory" (somewhere that is not in /tmp) to work in.

@Hoodoo
Copy link
Author

Hoodoo commented Dec 1, 2016

Well the file doesn't have to be executable. I also wouldn't like to add another dependency.

The thing is, it currently feeds curl output to bash without verifying, and gpg would run only if the script which would happen to be at get.rvm.io calls it, and that's what I wanted to prevent.

As far as I can see, they work on that $install_from option in master, so in the future one might download the installer, verify it and serve with puppet instead of curl, and in that case this whole pull request makes no sense.

The staging module looks interesting and adding verification to it looks doable.

@TJM
Copy link

TJM commented Dec 1, 2016

Hoodoo - You are right, sorry about that. I did notice (after I submitted) that you were running the installer as an argument to bash.

I like where this is going, though. I am glad someone has taken initiative to fix it. :)

@Hoodoo
Copy link
Author

Hoodoo commented Dec 1, 2016

It has user variable, maybe user's home is a better place than tmp.

@vox-pupuli-tasks
Copy link

Dear @Hoodoo, thanks for the PR!

This is Vox Pupuli Tasks, your friendly Vox Pupuli Github Bot. I noticed that your pull request has CI failures. Can you please have a look at the failing CI jobs?
If you need any help, you can reach out to us on our IRC channel voxpupuli on Freenode or our Slack channel voxpupuli at slack.puppet.com.
You can find my sourcecode at voxpupuli/vox-pupuli-tasks

@ekohl
Copy link
Member

ekohl commented Jul 6, 2022

I'm going to close this PR for inactivity and its age, but a fresh PR is welcome.

@ekohl ekohl closed this Jul 6, 2022
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.

3 participants