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

ssl[:verify]: Add :noop option to turn off only the hostname verification #111

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions lib/manticore/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,10 @@ def newThread(runnable)
# @option options [Symbol] ssl[:verify] (:default) Hostname verification setting. Set to `:none` to turn off hostname verification. Setting to `:browser` will
# cause Manticore to accept a certificate for *.foo.com for all subdomains and sub-subdomains (eg a.b.foo.com).
# The default `:default` is like `:browser` but more strict - only accepts a single level of subdomains for wildcards,
# eg `b.foo.com` will be accepted for a `*.foo.com` certificate, but `a.b.foo.com` will not be.
# @option options [Client::TrustStrategiesInterface] ssl[:trust_strategy] (nil) A trust strategy to use in addition to any built by `ssl[:verify]`.
# @see Client::TrustStrategiesInterface#coerce
# eg `b.foo.com` will be accepted for a `*.foo.com` certificate, but `a.b.foo.com` will not be. Set to `:noop` to
# turn off hostname verification, unlike :none, this option still validates the certificate.
# @option options [Client::TrustStrategies] ssl[:trust_strategy] (nil) A trust strategy to use in addition to any built by `ssl[:verify]`.
# @see Client::TrustStrategies#coerce
# @option options [String] ssl[:truststore] (nil) Path to a custom trust store to use the verifying SSL connections
# @option options [String] ssl[:truststore_password] (nil) Password used for decrypting the server trust store
# @option options [String] ssl[:truststore_type] (nil) Format of the trust store, ie "JKS" or "PKCS12". If left nil, the type will be inferred from the truststore filename.
Expand Down Expand Up @@ -680,8 +681,10 @@ def ssl_socket_factory_from_options(ssl_options)
verifier = DefaultHostnameVerifier.new
when :strict # compatibility
verifier = SSLConnectionSocketFactory::STRICT_HOSTNAME_VERIFIER
when :noop
verifier = NoopHostnameVerifier::INSTANCE
else
raise "Invalid value for :verify. Valid values are (:default, :browser, :none)"
raise "Invalid value for :verify. Valid values are (:default, :browser, :noop, :none)"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is simply confusing, I think the other values were trying to help the user understand.

but now having :noop, :none just adds confusion to the mix, guess we could accept :noop internally but maybe as undocumented or at least not shown here...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reviewing, I agree,:noop is indeed confusing, I ended up using it to keep it similar to the Apache's verifier. Other names that came to my mind are:certificate and :ca. Do you have any suggestion?

end

if ssl_options.include?(:trust_strategy)
Expand Down
18 changes: 18 additions & 0 deletions spec/manticore/client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,24 @@
end
end
end

context "when noop" do
let(:client) { Manticore::Client.new :ssl => {verify: :noop, ca_file: File.expand_path("../../ssl/root-ca.crt", __FILE__)} }
let(:hostname_verifier) { Proc.new { |hostname, session| true } }

context "and hostname mismatch the certificate CN" do
it "request and succeed" do
# 127.0.0.1 was used on purpose here so it's mismatch the certificate's CN localhost
expect { client.get("https://127.0.0.1:55444/").body }.to_not raise_exception
end

context "and the server SSL certificate is expired" do
it "request and fail" do
expect { client.get("https://127.0.0.1:55446/").body }.to raise_exception(Manticore::ClientProtocolException)
end
end
end
end
end

describe ":cipher_suites" do
Expand Down