Skip to content
Closed
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
3 changes: 3 additions & 0 deletions api/envoy/api/v2/auth/cert.proto
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,9 @@ message UpstreamTlsContext {
//
// TLS renegotiation is considered insecure and shouldn't be used unless absolutely necessary.
bool allow_renegotiation = 3;

// If true, clients will resume TLS sessions
bool allow_session_resumption = 4;
}

message DownstreamTlsContext {
Expand Down
4 changes: 4 additions & 0 deletions include/envoy/ssl/context_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@ class ClientContextConfig : public virtual ContextConfig {
* @return true if server-initiated TLS renegotiation will be allowed.
*/
virtual bool allowRenegotiation() const PURE;
/**
* @return true if client should resume TLS sessions
*/
virtual bool allowSessionResumption() const PURE;
};

typedef std::unique_ptr<ClientContextConfig> ClientContextConfigPtr;
Expand Down
3 changes: 2 additions & 1 deletion source/common/ssl/context_config_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,8 @@ ClientContextConfigImpl::ClientContextConfigImpl(
const envoy::api::v2::auth::UpstreamTlsContext& config,
Server::Configuration::TransportSocketFactoryContext& factory_context)
: ContextConfigImpl(config.common_tls_context(), factory_context),
server_name_indication_(config.sni()), allow_renegotiation_(config.allow_renegotiation()) {
server_name_indication_(config.sni()), allow_renegotiation_(config.allow_renegotiation()),
allow_session_resumption_(config.allow_session_resumption()) {
// BoringSSL treats this as a C string, so embedded NULL characters will not
// be handled correctly.
if (server_name_indication_.find('\0') != std::string::npos) {
Expand Down
2 changes: 2 additions & 0 deletions source/common/ssl/context_config_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,12 @@ class ClientContextConfigImpl : public ContextConfigImpl, public ClientContextCo
// Ssl::ClientContextConfig
const std::string& serverNameIndication() const override { return server_name_indication_; }
bool allowRenegotiation() const override { return allow_renegotiation_; }
bool allowSessionResumption() const override { return allow_session_resumption_; }

private:
const std::string server_name_indication_;
const bool allow_renegotiation_;
const bool allow_session_resumption_;
};

class ServerContextConfigImpl : public ContextConfigImpl, public ServerContextConfig {
Expand Down
27 changes: 26 additions & 1 deletion source/common/ssl/context_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "openssl/hmac.h"
#include "openssl/rand.h"
#include "openssl/x509v3.h"
#include <openssl/ssl.h>

namespace Envoy {
namespace Ssl {
Expand Down Expand Up @@ -483,17 +484,41 @@ CertificateDetailsPtr ContextImpl::certificateDetails(X509* cert, const std::str

ClientContextImpl::ClientContextImpl(Stats::Scope& scope, const ClientContextConfig& config)
: ContextImpl(scope, config), server_name_indication_(config.serverNameIndication()),
allow_renegotiation_(config.allowRenegotiation()) {
allow_renegotiation_(config.allowRenegotiation()), allow_session_resumption_(config.allowSessionResumption()) {
if (!parsed_alpn_protocols_.empty()) {
int rc = SSL_CTX_set_alpn_protos(ctx_.get(), &parsed_alpn_protocols_[0],
parsed_alpn_protocols_.size());
RELEASE_ASSERT(rc == 0, "");
}

if (allow_session_resumption_) {
SSL_CTX_set_session_cache_mode(ctx_.get(), SSL_SESS_CACHE_CLIENT|SSL_SESS_CACHE_NO_INTERNAL);
SSL_CTX_sess_set_new_cb(
ctx_.get(),
[](SSL * ssl, SSL_SESSION *session) {
ContextImpl* context_impl = static_cast<ContextImpl*>(
SSL_CTX_get_ex_data(SSL_get_SSL_CTX(ssl), sslContextIndex()));
ClientContextImpl* client_context_impl = dynamic_cast<ClientContextImpl*>(context_impl);
int len = i2d_SSL_SESSION(session, NULL);
if (len <= ENVOY_MAX_SESSION_SIZE) {
client_context_impl->session_.resize(len);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we only have one resumption per client context? What if the same context is used to speak to multiple endpoints, each of which who negotiates their own resumption ticket?

unsigned char* temp = client_context_impl->session_.data();
i2d_SSL_SESSION(session, &temp);
}
return 0;
});
}
}

bssl::UniquePtr<SSL> ClientContextImpl::newSsl() const {
bssl::UniquePtr<SSL> ssl_con(ContextImpl::newSsl());

if (allow_session_resumption_ && !session_.empty()) {
const unsigned char* temp = session_.data();
SSL_SESSION *session = d2i_SSL_SESSION(NULL, &temp, session_.size());
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Isn't session leaked? d2i_SSL_SESSION returns with ref count 1, SSL_set_session will increment 1 and decrement once it is no longer used. You need to use bssl::UniquePtr here.

SSL_set_session(ssl_con.get(), session);
}

if (!server_name_indication_.empty()) {
int rc = SSL_set_tlsext_host_name(ssl_con.get(), server_name_indication_.c_str());
RELEASE_ASSERT(rc, "");
Expand Down
4 changes: 4 additions & 0 deletions source/common/ssl/context_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ namespace Envoy {
#error Envoy requires BoringSSL
#endif

#define ENVOY_MAX_SESSION_SIZE 4096
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

make this a constexpr int inside unnamed namespace in .cc file?


namespace Ssl {

// clang-format off
Expand Down Expand Up @@ -146,6 +148,8 @@ class ClientContextImpl : public ContextImpl, public ClientContext {
private:
const std::string server_name_indication_;
const bool allow_renegotiation_;
const bool allow_session_resumption_;
std::vector<uint8_t> session_;
};

class ServerContextImpl : public ContextImpl, public ServerContext {
Expand Down