Do not force a verify_mode with a our stronger ssl settings

We have discovered that in some cases and some plaftorms
configuring a default `verify_mode` when creating a SSL/TCPServer
could make the certificate verification fail. Ruby default behavior is
to use `NIL` when creating a new ssl context, this revert that change.

keep in mind that all TCP clients using SSL **must** use `VERIFY_PEER`
as their verify mode to prevent man in the middle attack.

Fix: https://github.com/elastic/logstash/issues/3657
This commit is contained in:
Pier-Hugues Pellerin 2015-07-29 00:12:02 -04:00
parent 4e8bd0f15a
commit cbfd1eebf5
4 changed files with 76 additions and 6 deletions

View file

@ -15,4 +15,4 @@ gem "stud", "~> 0.0.19", :group => :build
gem "fpm", "~> 1.3.3", :group => :build
gem "rubyzip", "~> 1.1.7", :group => :build
gem "gems", "~> 0.8.3", :group => :build
gem "flores", "~> 0.0.4", :group => :development
gem "flores", "~> 0.0.6", :group => :development

View file

@ -49,7 +49,7 @@ GEM
file-dependencies (0.1.6)
minitar
filesize (0.0.4)
flores (0.0.4)
flores (0.0.6)
fpm (1.3.3)
arr-pm (~> 0.0.9)
backports (>= 2.6.2)
@ -133,7 +133,7 @@ DEPENDENCIES
ci_reporter_rspec (= 1.0.0)
coveralls
file-dependencies (= 0.1.6)
flores (~> 0.0.4)
flores (~> 0.0.6)
fpm (~> 1.3.3)
gems (~> 0.8.3)
logstash-core (= 2.0.0.dev)!

View file

@ -1,4 +1,3 @@
require "openssl"
# :nodoc:
@ -51,12 +50,18 @@ class OpenSSL::SSL::SSLContext
end
# Overwriting the DEFAULT_PARAMS const idea from here: https://www.ruby-lang.org/en/news/2014/10/27/changing-default-settings-of-ext-openssl/
#
# This monkeypatch doesn't enforce a `VERIFY_MODE` on the SSLContext,
# SSLContext are both used for the client and the server implementation,
# If set the `verify_mode` to peer the server wont accept any connection,
# because it will try to verify the client certificate, this is a protocol
# details implemented at the plugin level.
#
# For more details see: https://github.com/elastic/logstash/issues/3657
remove_const(:DEFAULT_PARAMS) if const_defined?(:DEFAULT_PARAMS)
DEFAULT_PARAMS = {
:ssl_version => "SSLv23",
:verify_mode => OpenSSL::SSL::VERIFY_PEER,
:ciphers => MOZILLA_INTERMEDIATE_CIPHERS,
:options => __default_options # Not a constant because it's computed at start-time.
}
end

View file

@ -1,4 +1,7 @@
# encoding: utf-8
require "socket"
require "logstash/patches"
require "flores/pki"
describe "OpenSSL defaults" do
subject { OpenSSL::SSL::SSLContext.new }
@ -22,4 +25,66 @@ describe "OpenSSL defaults" do
# SSLContext#ciphers returns an array of [ciphername, tlsversion, key_bits, alg_bits]
expect(encryption_bits).not_to be_any { |bits| bits < 128 }
end
it "should not include a default `verify_mode`" do
expect(OpenSSL::SSL::SSLContext::DEFAULT_PARAMS[:verify_mode]).to eq(nil)
end
context "SSLSocket" do
# Code taken from the flores library by @jordansissels,
# https://github.com/jordansissel/ruby-flores/blob/master/spec/flores/pki_integration_spec.rb
# since these helpers were created to fix this particular issue
let(:csr) { Flores::PKI::CertificateSigningRequest.new }
# Here, I use a 1024-bit key for faster tests.
# Please do not use such small keys in production.
let(:key_bits) { 1024 }
let(:key) { OpenSSL::PKey::RSA.generate(key_bits, 65537) }
let(:certificate_duration) { Flores::Random.number(1..86400) }
context "with self-signed client/server certificate" do
let(:certificate_subject) { "CN=server.example.com" }
let(:certificate) { csr.create }
# Returns [socket, address, port]
let(:listener) { Flores::Random.tcp_listener }
let(:server) { listener[0] }
let(:server_address) { listener[1] }
let(:server_port) { listener[2] }
let(:server_context) { OpenSSL::SSL::SSLContext.new }
let(:client_context) { OpenSSL::SSL::SSLContext.new }
before do
csr.subject = certificate_subject
csr.public_key = key.public_key
csr.start_time = Time.now
csr.expire_time = csr.start_time + certificate_duration
csr.signing_key = key
csr.want_signature_ability = true
server_context.cert = certificate
server_context.key = key
client_store = OpenSSL::X509::Store.new
client_store.add_cert(certificate)
client_context.cert_store = client_store
client_context.verify_mode = OpenSSL::SSL::VERIFY_PEER
ssl_server = OpenSSL::SSL::SSLServer.new(server, server_context)
Thread.new do
begin
ssl_server.accept
rescue => e
puts "Server accept failed: #{e}"
end
end
end
it "should successfully connect as a client" do
socket = TCPSocket.new(server_address, server_port)
ssl_client = OpenSSL::SSL::SSLSocket.new(socket, client_context)
expect { ssl_client.connect }.not_to raise_error
end
end
end
end