ncr / rack-proxy

A request/response rewriting HTTP proxy. A Rack app.
MIT License
269 stars 94 forks source link

Please let's never default to OpenSSL::SSL::VERIFY_NONE #113

Open oskarpearson opened 1 year ago

oskarpearson commented 1 year ago

Hi folks

The logic about ssl certificate verification in https://github.com/ncr/rack-proxy/blob/ce04ba5a15dd0c32d3f1b223fc980e3210f8008e/lib/rack/proxy.rb is pretty confusing.

There are two variables interacting - ssl_verify_none and verify_mode. imho we should only have one. Or are they doing different things entirely?

https://github.com/ncr/rack-proxy#using-ssltls-certificates-with-http-connection doesn't specifically make it clear that unless you supply verify_mode: OpenSSL::SSL::VERIFY_PEER it'll default to OpenSSL::SSL::VERIFY_NONE which is a really bad default. At least, that's my reading of the code!

Context: http://www.rubyinside.com/how-to-cure-nethttps-risky-default-https-behavior-4010.html

oskarpearson commented 1 year ago

At the very least I'd propose changing OpenSSL::SSL::VERIFY_NONE in the two places it's used in proxy.rb to refer to OpenSSL::SSL::VERIFY_NONE

oskarpearson commented 1 year ago

@ncr - not sure if you've had a look at this?