Skip to content

[HOLD] Enable hostnames for SNI but leave certificate verification off. #2124

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
24 changes: 16 additions & 8 deletions libstuff/SSSLState.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ SSSLState::~SSSLState() {
}

// --------------------------------------------------------------------------
SSSLState* SSSLOpen(int s) {
SSSLState* SSSLOpen(int s, const string& hostname) {
// Initialize the SSL state
SASSERT(s >= 0);
SSSLState* state = new SSSLState;
Expand All @@ -32,7 +32,15 @@ SSSLState* SSSLOpen(int s) {
mbedtls_ssl_init(&state->ssl);
mbedtls_ssl_config_init(&state->conf);
mbedtls_net_init(&state->net_ctx);
state->net_ctx.fd = s;

/* This block doesn't work, we'd like it to, but we need a real certificate chain to use.
mbedtls_x509_crt cacert;
mbedtls_x509_crt_init(&cacert);
if (mbedtls_x509_crt_parse_file(&cacert, "/etc/ssl/certs/ca-certificates.crt") != 0) {
STHROW("Failed to load CA chain");
}
mbedtls_ssl_conf_ca_chain(&state->conf, &cacert, nullptr);
*/

mbedtls_entropy_init(&state->ec);
mbedtls_ctr_drbg_init(&state->ctr_drbg);
Expand All @@ -42,19 +50,19 @@ SSSLState* SSSLOpen(int s) {
STHROW("ssl config defaults failed");
}

mbedtls_ssl_conf_authmode(&state->conf, MBEDTLS_SSL_VERIFY_OPTIONAL);
mbedtls_ssl_conf_rng(&state->conf, mbedtls_ctr_drbg_random, &state->ctr_drbg);

if (mbedtls_ssl_setup(&state->ssl, &state->conf)) {
STHROW("ssl setup failed");
}

/* We don't verify hostnames, and it's also why above we set MBEDTLS_SSL_VERIFY_OPTIONAL instead of MBEDTLS_SSL_VERIFY_REQUIRED.
* This could be a possible securiy improvement.
if (mbedtls_ssl_set_hostname(&state->ssl, "your.server.hostname")) {
STHROW("ssl set hostname failed");
// We'd like to set MBEDTLS_SSL_VERIFY_REQUIRED, but we need a certificate chain to verify against.
mbedtls_ssl_conf_authmode(&state->conf, MBEDTLS_SSL_VERIFY_NONE);
if (hostname.size()) {
if (mbedtls_ssl_set_hostname(&state->ssl, hostname.c_str())) {
STHROW("ssl set hostname failed");
}
}
*/

mbedtls_net_init(&state->net_ctx);
state->net_ctx.fd = state->s;
Expand Down
2 changes: 1 addition & 1 deletion libstuff/SSSLState.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ struct SSSLState {
};

// SSL helpers
extern SSSLState* SSSLOpen(int s);
extern SSSLState* SSSLOpen(int s, const string& hostname = "");
extern int SSSLSend(SSSLState* ssl, const char* buffer, int length);
extern int SSSLSend(SSSLState* ssl, const SFastBuffer& buffer);
extern bool SSSLSendConsume(SSSLState* ssl, SFastBuffer& sendBuffer);
Expand Down
6 changes: 5 additions & 1 deletion libstuff/STCPManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,11 @@ STCPManager::Socket::Socket(const string& host, bool useSSL)
if (s < 0) {
STHROW("Couldn't open socket to " + host);
}
ssl = useSSL ? SSSLOpen(s) : nullptr;

string domain;
uint16_t port;
SParseHost(host, domain, port);
ssl = useSSL ? SSSLOpen(s, domain) : nullptr;
SASSERT(!useSSL || ssl);
}

Expand Down