Clarify the INSTALL instructions by mattcaswell · Pull Request #9268 · openssl/openssl

mattcaswell · 2019-06-28T11:11:31Z

Ensure users understand that they need to have appropriate permissions
to write to the install location.

The following Stack Overflow post was pointed out to me which motivated this change:

https://stackoverflow.com/questions/41938564/problems-during-last-step-of-installing-openssl-on-mac-os-sierra

Ensure users understand that they need to have appropriate permissions to write to the install location.

mspncp · 2019-06-28T11:49:20Z

INSTALL

+ appropriate permissions to write to the installation directory. For example,
+ in many cases on Unix, you may need to instead run the command like this:
+
+    $ sudo make install


It might sensible be to add a warning here that it is not recommended to install to the default location on a linux/unix system, overwriting the preinstalled openssl version from the distributor. (You made such a recommendation just recently on a GitHub issue, but I don't recall where it was)

Also, you could add the recommendation to install to a writable location without sudo (e.g. below the home directory) using the --prefix configure option, if it's for testing purposes only.

romen · 2019-06-28T11:50:14Z

On the other hand, suggesting sudo make install to the users that are not able to figure permission problems on their own would probably rise a few more serious problems that can have serious repercussions on the perception of the project.

I have assisted many users that don't have a lot of familiarity with UNIX/Linux and followed the instructions to compile openssl from source, somehow reach the sudo make install and end up with a custom compiled OpenSSL in /usr/local. Thus messing many many distribution provided applications that had problems linking at run time with a different version of OpenSSL than the one they were compiled against!
Not knowing any better they all blamed OpenSSL for messing up their system (that they generally solved by formatting and reinstalling)!

Edit: @mspncp beat me to it, his comment gives suggestions rather than just raising an issue!

mspncp · 2019-06-28T12:12:14Z

Edit: @mspncp beat me to it

Yeah, by less than a minute ;-)

mattcaswell · 2019-06-28T12:30:35Z

On the other hand, suggesting sudo make install to the users that are not able to figure permission problems on their own would probably rise a few more serious problems that can have serious repercussions on the perception of the project.

So would you suggest to drop the "sudo make install" bit and just leave it as saying that you should have the right permissions. Or drop this PR altogether?

mspncp · 2019-06-28T12:34:17Z

I think your clarification is useful, if you also mention the caveats and alternatives, like suggested in #9268 (comment).

mspncp · 2019-06-28T12:34:58Z

(Note that Nicola edited his comment)

paulidale · 2019-06-28T12:42:11Z

I'd drop the sudo make install lest somebody blame the project for destroying their system.

I do wonder if this PR is mollycoddling, I don't remember seeing any source distributions noting that permissions are required to install (I'm sure I have, I just don't remember any).

romen · 2019-06-28T13:17:09Z

On the other hand, suggesting sudo make install to the users that are not able to figure permission problems on their own would probably rise a few more serious problems that can have serious repercussions on the perception of the project.

So would you suggest to drop the "sudo make install" bit and just leave it as saying that you should have the right permissions. Or drop this PR altogether?

As my use case illustrated, inexperienced users will hit stack overflow and discover that sudo make install is the solution, and won't have any caveat whatsoever about the risk of installing in the system default location.

I believe we should provide the useful part of the answer, highlighting the permission problem as you did, and in the same place a note about the risks of installing in a system-wide location, recommending using --prefix to install it somewhere else for testing (where probably make install is sufficient).

mspncp · 2019-06-28T13:42:41Z

I do wonder if this PR is mollycoddling

@paulidale @richsalz please don't forget that times have changed since the good ol' days when only the tough UNIX guys on their mainframes would compile software from scratch. ;-) The StackOverflow question shows IMHO that things which used to be obvious to people aren't so obvious anymore.

richsalz · 2019-06-28T14:13:53Z

https://www.urbandictionary.com/define.php?term=get%20off%20my%20lawn

levitte

I would only say that they normally need admin rights to install, and that how to get admin rights is system dependent. No mention of sudo...

mattcaswell · 2019-07-26T11:26:42Z

I dropped the "sudo" suggestion from this PR. I also added a warning about ensuring you install to a suitably protected location (see #9460). I also added a warning about not overwriting a system installed version of OpenSSL.

Ping @mspncp.

mattcaswell · 2019-08-07T14:32:50Z

Ping?

mspncp · 2019-08-07T15:44:07Z

Sorry, I forgot about this one. I'll take a look this evening.

mspncp

Looks good to me now. Approved with or without my suggested change.

mspncp · 2019-08-07T23:00:27Z

INSTALL


+ The installation directory should be appropriately protected to ensure
+ untrusted users cannot make changes to OpenSSL binaries or files, or install
+ engines. If you already have a pre-installed version of OpenSSL as part of


Just a cosmetical change. Feel free to adopt my suggestion or leave everything as it is.

The installation directory should be write protected to ensure that unprivileged users cannot make changes to OpenSSL binaries or files, or install

(Personally, I find the term untrusted too negative. Also, I replaced "appropriately protected" by "write protected`.)

mspncp · 2019-08-08T05:48:32Z

@mattcaswell I posted a short note on the original Stack Overflow thread about this pull request.

paulidale

LGTM too.

Ensure users understand that they need to have appropriate permissions to write to the install location. Reviewed-by: Matthias St. Pierre <[email protected]> Reviewed-by: Paul Dale <[email protected]> (Merged from #9268) (cherry picked from commit 7c03bb9)

mattcaswell · 2019-08-08T09:23:13Z

Pushed to master and 1.1.1 - thanks. I took one half of @mspncp's suggestion and changed "untrusted" to "unprivileged".

Ensure users understand that they need to have appropriate permissions to write to the install location. Reviewed-by: Matthias St. Pierre <[email protected]> Reviewed-by: Paul Dale <[email protected]> (Merged from #9268)

Clarify the INSTALL instructions

042ebba

Ensure users understand that they need to have appropriate permissions to write to the install location.

mattcaswell added branch: master Applies to master branch branch: 1.1.1 Applies to OpenSSL_1_1_1-stable branch (EOL) labels Jun 28, 2019

mspncp reviewed Jun 28, 2019

View reviewed changes

levitte reviewed Jun 28, 2019

View reviewed changes

fixup! Clarify the INSTALL instructions

93174ad

mattcaswell force-pushed the clarify-install-v2 branch from c2ea98f to 93174ad Compare July 26, 2019 11:24

mspncp approved these changes Aug 7, 2019

View reviewed changes

mspncp added the approval: done This pull request has the required number of approvals label Aug 7, 2019

paulidale approved these changes Aug 8, 2019

View reviewed changes

mattcaswell closed this Aug 8, 2019

claucece mentioned this pull request Dec 21, 2020

Refactor golang tests using openssl cloudflare/go#49

Closed

Uh oh!

Comments

Conversation

mattcaswell commented Jun 28, 2019

Uh oh!

mspncp Jun 28, 2019

Choose a reason for hiding this comment

Uh oh!

romen commented Jun 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mspncp commented Jun 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mattcaswell commented Jun 28, 2019

Uh oh!

mspncp commented Jun 28, 2019

Uh oh!

mspncp commented Jun 28, 2019

Uh oh!

paulidale commented Jun 28, 2019

Uh oh!

romen commented Jun 28, 2019

Uh oh!

mspncp commented Jun 28, 2019

Uh oh!

richsalz commented Jun 28, 2019

Uh oh!

levitte left a comment

Choose a reason for hiding this comment

Uh oh!

mattcaswell commented Jul 26, 2019

Uh oh!

mattcaswell commented Aug 7, 2019

Uh oh!

mspncp commented Aug 7, 2019

Uh oh!

mspncp left a comment

Choose a reason for hiding this comment

Uh oh!

mspncp Aug 7, 2019

Choose a reason for hiding this comment

Uh oh!

mspncp commented Aug 8, 2019

Uh oh!

paulidale left a comment

Choose a reason for hiding this comment

Uh oh!

mattcaswell commented Aug 8, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

romen commented Jun 28, 2019 •

edited

Loading

mspncp commented Jun 28, 2019 •

edited

Loading