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

Add option to turn off verification checking on SSL certificate #94

Open
kwkwan opened this issue Nov 9, 2017 · 10 comments
Open

Add option to turn off verification checking on SSL certificate #94

kwkwan opened this issue Nov 9, 2017 · 10 comments
Assignees

Comments

@kwkwan
Copy link
Contributor

kwkwan commented Nov 9, 2017

When we tried to connect to our local development server which is using a self-signed certificate, the connection failed due to an error "certificate verify failed (Faraday::SSLError)". It is better to add an option to turn off verification checking on SSL certificate when connecting to api_host. Thanks.

@skalee skalee self-assigned this Nov 29, 2017
@skalee skalee changed the title Add option to turn off verification checking on SSL certificate Allow passing additional configuration options for Faraday Nov 30, 2017
@skalee skalee changed the title Allow passing additional configuration options for Faraday Add option to turn off verification checking on SSL certificate Nov 30, 2017
@skalee
Copy link
Contributor

skalee commented Nov 30, 2017

Enhanced in #105.

@abunashir
Copy link
Member

abunashir commented Dec 1, 2017

@kwkwan: thanks for opening this issue! I just had a details conversation with @ribose-jeffreylau, let me put together the summary of the conversation so everyone is on the same page and it might be useful in the future.

  • This gem is meant to be used with the actual API host and we are concerned about allowing the user to customize faraday / disable SSL. As the user might even try that do that in production version, which might lead to some security issues

  • We are the one that might need that sort of feature for development, but we should able to get away with a let’s encrypt certificate or so. Ideally, we will have some request space (like we did for digicert) that will do all the actual testing for us, so we were considering if it’s worth spending the time on that, and could it potentially do any harm for exposing that configuration option.

But as Jeffery mentioned, we are using the ribose-ruby client with our acceptance tests (which uses the development host), so it might be good to have the option right now, but in the future, we might consider the above scenario and do something that won't leave any space for security.

cc: @ronaldtse

@skalee
Copy link
Contributor

skalee commented Dec 1, 2017

@abunashir I've just created a PR #106.

@skalee
Copy link
Contributor

skalee commented Dec 1, 2017

How about using a purchased certificate on staging then? That said, #106 may be still useful for proxy setup.

@skalee
Copy link
Contributor

skalee commented Dec 1, 2017

@abunashir @kwkwan If setting Faraday options is not desirable, how about monkey-patching Faraday in development?

module FaradayOverrides
  def initialize *args
    options = args.last
    options[:ssl] = {verify: false}
    super
  end
end

Faraday::Connection.prepend FaradayOverrides

Seems to work for me.

@ronaldtse
Copy link
Contributor

I agree that in release we don't want this sort of option, but in development mode it will be useful. How about just using a ENV["RIBOSE_DISABLE_TLS"] = true to override the Faraday options?

@skalee all deployed environments use valid certificates. @kwkwan 's case is local local (same container to same container).

@abunashir Let's Encrypt won't work for this case because it requires remote domain validation.

The real solution would be to add the locally generated certificate CA into the OS itself. @kwkwan should be straightforward to do like this https://askubuntu.com/questions/73287/how-do-i-install-a-root-certificate .

@abunashir
Copy link
Member

abunashir commented Dec 4, 2017

@ronaldtse: Adding support to pick up SSL configuration from environment variable should work fine. This is less straightforward than configuration but a user might still try to disable the verification mode by setting the environment variable in their env right?

But this also gave another idea like I mentioned in skype, maybe we can do something like, expose the ssl_varify_mode config as usual and then add additional pattern match for our development host and if the host matches then we only pick up the ssl_varify_mode, otherwise this configuration won't do anything.

Ideally, if we can solve this using a ca root certificate like you mentioned in your previous comment then that would be perfect, and that might not require any changes on the client end. If that does not work then please let us know so we can add the necessary changes.

cc: @kwkwan, @ribose-jeffreylau

@skalee
Copy link
Contributor

skalee commented Dec 4, 2017

This is less straightforward than configuration but a user might still try to disable the verification mode by setting the environment variable in their env right?

I suppose there is no way to stop user from disabling the verification mode since he can simply monkey-patch Faraday (as I did in #94 (comment)).

Only two options make sense to me: either make Faraday configuration easy and unrestricted (as in #106), or do not provide any configuration logic at all and rely on user's monkey-patching skills (or make him using trusted certificates).

@ribose-jeffreylau
Copy link
Contributor

Would the methods detailed in this page be useful?

One of the methods, which avoids the need to mess with Faraday, is to simply set an env var:

export SSL_CERT_FILE=/path/to/ca.crt

@ronaldtse
Copy link
Contributor

export SSL_CERT_FILE=/path/to/ca.crt

Agree! Seems easy enough to do.

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

No branches or pull requests

5 participants