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

Enable certificate verification on downloads #19

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

joehorsnell
Copy link

Disabling TLS server certificate verification undermines the security of HTTPS
(ie, one might as well use HTTP) and is considered extremely bad practice:

Looks like it's been there since the initial release, so probably an
oversight?

Disabling TLS server certificate verification undermines the security of HTTPS
(ie, one might as well use HTTP) and is considered extremely bad practice:

* https://mislav.net/2013/07/ruby-openssl/
* https://brakemanscanner.org/docs/warning_types/ssl_verification_bypass/

Looks like it's been there since the initial release, so probably an
oversight?
@joehorsnell
Copy link
Author

Any thoughts on this @tr4n2uil / @AnkurGel?

Any idea on getting the specs fixed? Unrelated to these changes, I think?

@joehorsnell
Copy link
Author

Linking to comment on #27.

@bipul21
Copy link
Collaborator

bipul21 commented Nov 4, 2020

Hi,

Thanks for pointing out this issue, this indeed undermines the security of HTTPS.
The right fix here would be to pass the root CA certificate used by the MITM Proxy but
to keep things simple user should be able to pass VERIFY::NONE here if knowingly
based on the network parameters.
So, here default value should be VERIFY::PEER which overrides to VERIFY::NONE based on a parameter

An optional argument can be passed during the initialization as.
bs_local = BrowserStack::Local.new verify="none"

Also with appropriate messaging on the initial exception which says to add
verify=”none” as arguments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants