OpenSSL verify should not be set to NONE

[RolphH]

httpsnippet/src/targets/ruby/native.js

Line 32 in 1855a74

.push('http.verify_mode = OpenSSL::SSL::VERIFY_NONE')

This disable the check of the SSL certificate and should be prevented. Can this be changed to have SSL verified?

[darrenjennings]

This was likely added so that a certificate was not needed in order to make an SSL request. I'm not a ruby developer, wouldn't you also need http.cert param and a way to pass in a cert/key? Can you elaborate on desired behavior?

[trev]

This is indeed to make integration easier at the expense of security. The integrator could be following these instructions blindly and end up with a MITM vulnerability.

The alternatives are:

1. Change to OpenSSL::SSL::VERIFY_PEER and let the integrating party deal with the possible certificate verify failed. error. Solutions can be found here: https://www.engineyard.com/blog/ruby-ssl-error-certificate-verify-failed#updatecacertificates
2. Add a Ruby comment above http.verify_mode = OpenSSL::SSL::VERIFY_NONE stating: # This is insecure, ensure you update verify_mode to OpenSSL::SSL::VERIFY_PEER in production
3. Leave as is and inadvertently create security holes in production systems.

I suggest option 1 with the addition of a comment above it referencing the EngineYard article.

Happy to create a PR if it'll get merged.

[trev]

WDYT @darrenjennings ?

[darrenjennings]

@trev your suggestion to change it to VERIFY_PEER LGTM. Users can change to VERIFY_NONE if desired, while it keeps the snippets secure by default. Important to note also that this code doesn't even work currently so would also hope to get #123 in as well. Can treat as two separate PRs.

[darrenjennings]

I was talking with @gschier who mentioned it could be good to be secure by default but since clients are not limited in the number of options they can have we can always make it an option with sane defaults. Open to bikeshedding the name of the options for VERIFY_PEER vs VERIFY_NONE

[gschier]

Here are some examples from various places:

• NodeJS: rejectUnauthorized
• Curl CLI: --insecure
• Go: InsecureSkipVerify
• Python: verify

My vote is for the most descriptive: insecureSkipVerify

[darrenjennings]

Good find @gschier. Let's go with insecureSkipVerify then. @trev or @RolphH do you want to submit a pull request? If not @gschier or I can do it, but glad to take contributions from the community.

[sandstrom]

The line .push('http.verify_mode = OpenSSL::SSL::VERIFY_NONE') should be removed from the ruby target.

It's a huge security problem to not verify HTTPS certificates, since any MITM can swap out the certificate without any warning.

https://brakemanscanner.org/docs/warning_types/ssl_verification_bypass/