Skip to content

Add HTTP client options for SSL/TLS methods #530

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

Merged
merged 2 commits into from
May 25, 2015
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
Add HTTP client options for SSL/TLS options/ciphers
  • Loading branch information
Teddy Reed committed May 22, 2015
commit 20316867568f2ab9be574bd46db18ed81636b96a
10 changes: 8 additions & 2 deletions boost/network/protocol/http/client/async_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ struct async_client
optional<string_type> const& certificate_filename,
optional<string_type> const& verify_path,
optional<string_type> const& certificate_file,
optional<string_type> const& private_key_file)
optional<string_type> const& private_key_file,
optional<string_type> const& ciphers, long ssl_options)
: connection_base(cache_resolved, follow_redirect, timeout),
service_ptr(service.get()
? service
Expand All @@ -54,6 +55,8 @@ struct async_client
verify_path_(verify_path),
certificate_file_(certificate_file),
private_key_file_(private_key_file),
ciphers_(ciphers),
ssl_options_(ssl_options),
always_verify_peer_(always_verify_peer) {
connection_base::resolver_strand_.reset(
new boost::asio::io_service::strand(service_));
Expand All @@ -79,7 +82,8 @@ struct async_client
typename connection_base::connection_ptr connection_;
connection_ = connection_base::get_connection(
resolver_, request_, always_verify_peer_, certificate_filename_,
verify_path_, certificate_file_, private_key_file_);
verify_path_, certificate_file_, private_key_file_, ciphers_,
ssl_options_);
return connection_->send_request(method, request_, get_body, callback,
generator);
}
Expand All @@ -93,6 +97,8 @@ struct async_client
optional<string_type> verify_path_;
optional<string_type> certificate_file_;
optional<string_type> private_key_file_;
optional<string_type> ciphers_;
long ssl_options_;
bool always_verify_peer_;
};
} // namespace impl
Expand Down
6 changes: 4 additions & 2 deletions boost/network/protocol/http/client/connection/async_base.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ struct async_connection_base {
optional<string_type> certificate_filename = optional<string_type>(),
optional<string_type> const &verify_path = optional<string_type>(),
optional<string_type> certificate_file = optional<string_type>(),
optional<string_type> private_key_file = optional<string_type>()) {
optional<string_type> private_key_file = optional<string_type>(),
optional<string_type> ciphers = optional<string_type>(),
long ssl_options = 0) {
typedef http_async_connection<Tag, version_major, version_minor>
async_connection;
typedef typename delegate_factory<Tag>::type delegate_factory_type;
Expand All @@ -53,7 +55,7 @@ struct async_connection_base {
delegate_factory_type::new_connection_delegate(
resolver.get_io_service(), https, always_verify_peer,
certificate_filename, verify_path, certificate_file,
private_key_file)));
private_key_file, ciphers, ssl_options)));
BOOST_ASSERT(temp.get() != 0);
return temp;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,14 @@ struct connection_delegate_factory {
asio::io_service& service, bool https, bool always_verify_peer,
optional<string_type> certificate_filename,
optional<string_type> verify_path, optional<string_type> certificate_file,
optional<string_type> private_key_file) {
optional<string_type> private_key_file, optional<string_type> ciphers,
long ssl_options) {
connection_delegate_ptr delegate;
if (https) {
#ifdef BOOST_NETWORK_ENABLE_HTTPS
delegate.reset(new ssl_delegate(service, always_verify_peer,
certificate_filename, verify_path,
certificate_file, private_key_file));
delegate.reset(new ssl_delegate(
service, always_verify_peer, certificate_filename, verify_path,
certificate_file, private_key_file, ciphers, ssl_options));
#else
BOOST_THROW_EXCEPTION(std::runtime_error("HTTPS not supported."));
#endif /* BOOST_NETWORK_ENABLE_HTTPS */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ struct ssl_delegate : connection_delegate,
optional<std::string> certificate_filename,
optional<std::string> verify_path,
optional<std::string> certificate_file,
optional<std::string> private_key_file);
optional<std::string> private_key_file,
optional<std::string> ciphers, long ssl_options);

virtual void connect(asio::ip::tcp::endpoint &endpoint, std::string host,
function<void(system::error_code const &)> handler);
Expand All @@ -45,6 +46,8 @@ struct ssl_delegate : connection_delegate,
optional<std::string> verify_path_;
optional<std::string> certificate_file_;
optional<std::string> private_key_file_;
optional<std::string> ciphers_;
long ssl_options_;
scoped_ptr<asio::ssl::context> context_;
scoped_ptr<asio::ssl::stream<asio::ip::tcp::socket> > socket_;
bool always_verify_peer_;
Expand Down
24 changes: 18 additions & 6 deletions boost/network/protocol/http/client/connection/ssl_delegate.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,31 @@
boost::network::http::impl::ssl_delegate::ssl_delegate(
asio::io_service &service, bool always_verify_peer,
optional<std::string> certificate_filename,
optional<std::string> verify_path, optional<std::string> certificate_file,
optional<std::string> private_key_file)
optional<std::string> verify_path,
optional<std::string> certificate_file,
optional<std::string> private_key_file,
optional<std::string> ciphers,
long ssl_options)
: service_(service),
certificate_filename_(certificate_filename),
verify_path_(verify_path),
certificate_file_(certificate_file),
private_key_file_(private_key_file),
ciphers_(ciphers),
ssl_options_(ssl_options),
always_verify_peer_(always_verify_peer) {}

void boost::network::http::impl::ssl_delegate::connect(
asio::ip::tcp::endpoint &endpoint, std::string host,
function<void(system::error_code const &)> handler) {
context_.reset(
new asio::ssl::context(service_, asio::ssl::context::sslv23_client));
new asio::ssl::context(service_, asio::ssl::context::tlsv1_client));
Copy link
Member

Choose a reason for hiding this comment

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

Please do not change the default from SSLv23 to TLSv1. This will be a silent breaking change and I'm not convinced this is the best default for users of the library anyway.

Copy link
Author

Choose a reason for hiding this comment

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

Testing with Apple's provided OpenSSL 0.9.8 the ::SSLv23_method() should use the options to decide what type of protocol it negotiates. To disable SSL3 you could use the sslv23_client method and: options.openssl_options(SSL_OP_NO_SSLv3 | SSL_OP_NO_SSLv2 | SSL_OP_ALL);. However, I'm still seeing an SSLv3 client hello. On newer versions of openssl this works as expected and the sslv23_client with the SSL_OP_NO_SSLv3 option forces the client to send a TLSv1 client hello.

I'll give this more thought, but using tlsv1_client forces Apple's OpenSSL to TLSv1 client hello (and negotiate it's best supported TLS1.0) while other newer OpenSSLs will do the same and negotiate TLS1.1/TLS1.2 if supported by the server. This is the best possible scenario without considering supporting older clients.

Thoughts on a less-silent but still-default breaking change?

if (ciphers_) {
::SSL_CTX_set_cipher_list(context_->native_handle(), ciphers_->c_str());
}
if (ssl_options_ != 0) {
context_->set_options(ssl_options_);
}
if (certificate_filename_ || verify_path_) {
context_->set_verify_mode(asio::ssl::context::verify_peer);
if (certificate_filename_)
Expand All @@ -36,9 +47,10 @@ void boost::network::http::impl::ssl_delegate::connect(
} else {
if (always_verify_peer_) {
context_->set_verify_mode(asio::ssl::context::verify_peer);
context_->set_default_verify_paths(); // use openssl default verify paths. uses openssl environment variables SSL_CERT_DIR, SSL_CERT_FILE
}
else
// use openssl default verify paths. uses openssl environment variables
// SSL_CERT_DIR, SSL_CERT_FILE
context_->set_default_verify_paths();
} else
Copy link
Member

Choose a reason for hiding this comment

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

This formatting looks weird to me. Please make consistent with the surrounding code, and don't use tab characters... if you can run clang-format on it using the .clang-format configuration in the root of the package.

Copy link
Author

Choose a reason for hiding this comment

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

Interesting. I used the cpp-netlib's .clang-format and clang-format version 3.5.0 (tags/RELEASE_350/final), it should have picked up "UseTab: false".

context_->set_verify_mode(asio::ssl::context::verify_none);
}
if (certificate_file_)
Expand Down
22 changes: 13 additions & 9 deletions boost/network/protocol/http/client/connection/sync_base.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -245,22 +245,26 @@ struct sync_connection_base {
// optional
// ranges
static sync_connection_base<Tag, version_major, version_minor>*
new_connection(
resolver_type& resolver, resolver_function_type resolve, bool https,
bool always_verify_peer, int timeout,
optional<string_type> const& certificate_filename =
optional<string_type>(),
optional<string_type> const& verify_path = optional<string_type>(),
optional<string_type> const& certificate_file = optional<string_type>(),
optional<string_type> const& private_key_file = optional<string_type>()) {
new_connection(
resolver_type& resolver, resolver_function_type resolve, bool https,
bool always_verify_peer, int timeout,
optional<string_type> const& certificate_filename =
optional<string_type>(),
optional<string_type> const& verify_path = optional<string_type>(),
optional<string_type> const& certificate_file =
optional<string_type>(),
optional<string_type> const& private_key_file =
optional<string_type>(),
optional<string_type> const& ciphers = optional<string_type>(),
long ssl_options = 0) {
if (https) {
#ifdef BOOST_NETWORK_ENABLE_HTTPS
return dynamic_cast<
sync_connection_base<Tag, version_major, version_minor>*>(
new https_sync_connection<Tag, version_major, version_minor>(
resolver, resolve, always_verify_peer, timeout,
certificate_filename, verify_path, certificate_file,
private_key_file));
private_key_file, ciphers, ssl_options));
#else
throw std::runtime_error("HTTPS not supported.");
#endif
Expand Down
10 changes: 9 additions & 1 deletion boost/network/protocol/http/client/connection/sync_ssl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@ struct https_sync_connection
optional<string_type>(),
optional<string_type> const& verify_path = optional<string_type>(),
optional<string_type> const& certificate_file = optional<string_type>(),
optional<string_type> const& private_key_file = optional<string_type>())
optional<string_type> const& private_key_file = optional<string_type>(),
optional<string_type> const& ciphers = optional<string_type>(),
long ssl_options = 0)
: connection_base(),
timeout_(timeout),
timer_(resolver.get_io_service()),
Expand All @@ -65,6 +67,12 @@ struct https_sync_connection
context_(resolver.get_io_service(),
boost::asio::ssl::context::sslv23_client),
socket_(resolver.get_io_service(), context_) {
if (ciphers) {
::SSL_CTX_set_cipher_list(context_.native_handle(), ciphers->c_str());
}
if (ssl_options != 0) {
context_.set_options(ssl_options);
}
if (certificate_filename || verify_path) {
context_.set_verify_mode(boost::asio::ssl::context::verify_peer);
// FIXME make the certificate filename and verify path parameters
Expand Down
4 changes: 2 additions & 2 deletions boost/network/protocol/http/client/facade.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,8 @@ struct basic_client_facade {
options.cache_resolved(), options.follow_redirects(),
options.always_verify_peer(), options.openssl_certificate(),
options.openssl_verify_path(), options.openssl_certificate_file(),
options.openssl_private_key_file(), options.io_service(),
options.timeout()));
options.openssl_private_key_file(), options.openssl_ciphers(),
options.openssl_options(), options.io_service(), options.timeout()));
}
};

Expand Down
24 changes: 24 additions & 0 deletions boost/network/protocol/http/client/options.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ struct client_options {
openssl_verify_path_(),
openssl_certificate_file_(),
openssl_private_key_file_(),
openssl_ciphers_(),
openssl_options_(0),
io_service_(),
always_verify_peer_(false),
timeout_(0) {}
Expand All @@ -38,6 +40,8 @@ struct client_options {
openssl_verify_path_(other.openssl_verify_path_),
openssl_certificate_file_(other.openssl_certificate_file_),
openssl_private_key_file_(other.openssl_private_key_file_),
openssl_ciphers_(other.openssl_ciphers_),
openssl_options_(other.openssl_options_),
io_service_(other.io_service_),
always_verify_peer_(other.always_verify_peer_),
timeout_(other.timeout_) {}
Expand All @@ -55,6 +59,8 @@ struct client_options {
swap(openssl_verify_path_, other.openssl_verify_path_);
swap(openssl_certificate_file_, other.openssl_certificate_file_);
swap(openssl_private_key_file_, other.openssl_private_key_file_);
swap(openssl_ciphers_, other.openssl_ciphers_);
swap(openssl_options_, other.openssl_options_);
swap(io_service_, other.io_service_);
swap(always_verify_peer_, other.always_verify_peer_);
swap(timeout_, other.timeout_);
Expand Down Expand Up @@ -90,6 +96,16 @@ struct client_options {
return *this;
}

client_options& openssl_ciphers(string_type const& v) {
openssl_ciphers_ = v;
return *this;
}

client_options& openssl_options(long o) {
openssl_options_ ^= o;
Copy link
Member

Choose a reason for hiding this comment

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

I don't get this. Why aren't you just setting the options_ and letting users explicitly OR the options they want as input?

Copy link
Author

Choose a reason for hiding this comment

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

Good catch, thanks!

return *this;
}

client_options& io_service(boost::shared_ptr<boost::asio::io_service> v) {
io_service_ = v;
return *this;
Expand Down Expand Up @@ -125,6 +141,12 @@ struct client_options {
return openssl_private_key_file_;
}

boost::optional<string_type> openssl_ciphers() const {
return openssl_ciphers_;
}

long openssl_options() const { return openssl_options_; }

boost::shared_ptr<boost::asio::io_service> io_service() const {
return io_service_;
}
Expand All @@ -140,6 +162,8 @@ struct client_options {
boost::optional<string_type> openssl_verify_path_;
boost::optional<string_type> openssl_certificate_file_;
boost::optional<string_type> openssl_private_key_file_;
boost::optional<string_type> openssl_ciphers_;
long openssl_options_;
boost::shared_ptr<boost::asio::io_service> io_service_;
bool always_verify_peer_;
int timeout_;
Expand Down
3 changes: 2 additions & 1 deletion boost/network/protocol/http/client/pimpl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,12 @@ struct basic_client_impl
optional<string_type> const& verify_path,
optional<string_type> const& certificate_file,
optional<string_type> const& private_key_file,
optional<string_type> const& ciphers, long ssl_options,
boost::shared_ptr<boost::asio::io_service> service,
int timeout)
: base_type(cache_resolved, follow_redirect, always_verify_peer, timeout,
service, certificate_filename, verify_path, certificate_file,
private_key_file) {}
private_key_file, ciphers, ssl_options) {}

~basic_client_impl() {}
};
Expand Down
10 changes: 8 additions & 2 deletions boost/network/protocol/http/client/sync_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ struct sync_client
optional<string_type> verify_path_;
optional<string_type> certificate_file_;
optional<string_type> private_key_file_;
optional<string_type> ciphers_;
long ssl_options_;
bool always_verify_peer_;

sync_client(
Expand All @@ -53,7 +55,9 @@ struct sync_client
optional<string_type>(),
optional<string_type> const& verify_path = optional<string_type>(),
optional<string_type> const& certificate_file = optional<string_type>(),
optional<string_type> const& private_key_file = optional<string_type>())
optional<string_type> const& private_key_file = optional<string_type>(),
optional<string_type> const& ciphers = optional<string_type>(),
long ssl_options = 0)
: connection_base(cache_resolved, follow_redirect, timeout),
service_ptr(service.get() ? service
: make_shared<boost::asio::io_service>()),
Expand All @@ -63,6 +67,8 @@ struct sync_client
verify_path_(verify_path),
certificate_file_(certificate_file),
private_key_file_(private_key_file),
ciphers_(ciphers),
ssl_options_(ssl_options),
always_verify_peer_(always_verify_peer) {}

~sync_client() {
Expand All @@ -79,7 +85,7 @@ struct sync_client
typename connection_base::connection_ptr connection_;
connection_ = connection_base::get_connection(
resolver_, request_, always_verify_peer_, certificate_filename_,
verify_path_, certificate_file_, private_key_file_);
verify_path_, certificate_file_, private_key_file_, ciphers_);
return connection_->send_request(method, request_, get_body, callback,
generator);
}
Expand Down
13 changes: 9 additions & 4 deletions boost/network/protocol/http/policies/async_connection.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,15 @@ struct async_connection_policy : resolver_policy<Tag>::type {
optional<string_type> const& certificate_filename,
optional<string_type> const& verify_path,
optional<string_type> const& certificate_file,
optional<string_type> const& private_key_file) {
optional<string_type> const& private_key_file,
optional<string_type> const& ciphers, long ssl_options) {
pimpl = impl::async_connection_base<
Tag, version_major,
version_minor>::new_connection(resolve, resolver, follow_redirect,
always_verify_peer, https, timeout,
certificate_filename, verify_path,
certificate_file, private_key_file);
certificate_file, private_key_file,
ciphers, ssl_options);
}

basic_response<Tag> send_request(string_type const& method,
Expand All @@ -72,7 +74,9 @@ struct async_connection_policy : resolver_policy<Tag>::type {
optional<string_type>(),
optional<string_type> const& verify_path = optional<string_type>(),
optional<string_type> const& certificate_file = optional<string_type>(),
optional<string_type> const& private_key_file = optional<string_type>()) {
optional<string_type> const& private_key_file = optional<string_type>(),
optional<string_type> const& ciphers = optional<string_type>(),
long ssl_options = 0) {
string_type protocol_ = protocol(request_);
connection_ptr connection_(new connection_impl(
follow_redirect_, always_verify_peer,
Expand All @@ -81,7 +85,8 @@ struct async_connection_policy : resolver_policy<Tag>::type {
this, boost::arg<1>(), boost::arg<2>(), boost::arg<3>(),
boost::arg<4>()),
resolver, boost::iequals(protocol_, string_type("https")), timeout_,
certificate_filename, verify_path, certificate_file, private_key_file));
certificate_filename, verify_path, certificate_file, private_key_file,
ciphers, ssl_options));
return connection_;
}

Expand Down
Loading