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

Do not recommend adding --insecure to solve certificate issues #1810

Closed
wants to merge 1 commit into from
Closed

Do not recommend adding --insecure to solve certificate issues #1810

wants to merge 1 commit into from

Conversation

jedisct1
Copy link

There is a massive amount of scripts, examples and tutorials unconditionally adding the --insecure option to the curl command in order to access public servers with perfectly valid certificates.

A possible culprit is the curl command itself. As soon as a certificate error is returned, it suggests simply retrying the same command with --insecure added to it. So, people follow the advice, and since it "fixes things", end up using it unconditionally.

This is terrible.

The main causes of certificates that don't validate are:

  • A host name that doesn't match the one in the certificate
  • A client with a clock not being properly set
  • The absence of a CA cert bundle, since this has to be explicitly installed on some Linux distributions
  • A totally outdated CA cert bundle.
  • Self-signed certificates.

The following diff displays some hints about what the root cause of a certificate error is likely to be, instead of suggesting --insecure.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 73.233% when pulling 19bbd72 on jedisct1:jedisct1/dont-recommend-insecure into 16c71fa on curl:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 73.233% when pulling d5b83ba on jedisct1:jedisct1/dont-recommend-insecure into 16c71fa on curl:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.09%) to 73.248% when pulling d5b83ba on jedisct1:jedisct1/dont-recommend-insecure into 16c71fa on curl:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 73.233% when pulling d5b83ba on jedisct1:jedisct1/dont-recommend-insecure into 16c71fa on curl:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.004%) to 73.335% when pulling d5b83ba on jedisct1:jedisct1/dont-recommend-insecure into 16c71fa on curl:master.

@bagder
Copy link
Member

bagder commented Aug 20, 2017

Thanks.

Yes, recommending switching off the certificate checks is a rather terrible thing to do. That's also why the option is called --insecure? in its long form: to underscore it makes the transfer insecure.

This said, I'm pretty sure this error message is already a wall of text that is way too big. I think that it due to its size mostly scare people away and I doubt very many actually read the whole thing and weigh their options.

Since this already refers to a URL with most of this described in detail (and it makes sense to make that the single home for those instructions and recommendations), I think we should instead strive toward shortening this error message to a few lines.

Perhaps something like this:

More details here: https://curl.haxx.se/docs/sslcerts.html

curl failed to verify the legitimacy of the server and therefore could not establish a secure connection to it. To learn more about this situation and how to fix it, please visit the web page mentioned above.

@jedisct1
Copy link
Author

Indeed, this message is already way too long.

Your short version is great!

That web page still lists --insecure as the first thing to try :) But unlike a static error message, the page content can be easily kept up to date.

@bagder
Copy link
Member

bagder commented Aug 20, 2017

That web page is simply the docs/SSLCERTS.md file rendered on the web site. It can certainly be improved!

bagder added a commit that referenced this pull request Aug 22, 2017
The previuous message was just too long for ordinary people and it was
encouraging users to use `--insecure` a little too easy.

Based-on-work-by: Frank Denis in #1810
@vszakats
Copy link
Member

vszakats commented Aug 22, 2017

Here's some rough numbers about turning off certificate verification:
https://github.com/search?l=&q=--insecure+OR+--no-check-certificate+OR+--proxy-insecure&type=Code
(without -k hits, which is likely also common.)

Ref: https://twitter.com/vszakats/status/839812059814588416

@bagder bagder closed this in f412a5a Aug 22, 2017
@bagder
Copy link
Member

bagder commented Aug 22, 2017

thanks @jedisct1!

@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

None yet

4 participants