Skip to content

Commit 3ad8757

Browse files
trondndaverigby
authored andcommitted
Don't accept cipherlist with no usable ciphers
Fail if we cannot use any of the ciphers provided in the list of ciphers (to avoid ending up in a situation where memcached only listens on an SSL socket, but that cannot be used as there is no cipher to use) Change-Id: I48a671f66b87887f4d2e244b27990eac0ed83b98 Reviewed-on: http://review.couchbase.org/113701 Tested-by: Trond Norbye <[email protected]> Reviewed-by: Dave Rigby <[email protected]>
1 parent ae6deff commit 3ad8757

File tree

3 files changed

+33
-1
lines changed

3 files changed

+33
-1
lines changed

daemon/settings.cc

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
#include "ssl_utils.h"
3434

3535
#include <mcbp/mcbp.h>
36+
#include <memcached/openssl.h>
3637
#include <utilities/json_utilities.h>
3738
#include <utilities/logtags.h>
3839

@@ -389,6 +390,21 @@ static void handle_root(Settings& s, const nlohmann::json& obj) {
389390
* @param obj the object in the configuration
390391
*/
391392
static void handle_ssl_cipher_list(Settings& s, const nlohmann::json& obj) {
393+
const auto value = obj.get<std::string>();
394+
cb::openssl::unique_ssl_ctx_ptr ctx;
395+
ctx.reset(SSL_CTX_new(SSLv23_server_method()));
396+
if (!ctx) {
397+
throw std::bad_alloc{};
398+
}
399+
400+
if (!value.empty() &&
401+
SSL_CTX_set_cipher_list(ctx.get(), value.c_str()) == 0) {
402+
std::string msg = "Failed to select any of the requested ciphers (";
403+
msg.append(value);
404+
msg.append(")");
405+
throw std::runtime_error(msg);
406+
}
407+
392408
s.setSslCipherList(obj.get<std::string>());
393409
}
394410

include/memcached/openssl.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,5 +37,13 @@ struct X509deletor {
3737
};
3838

3939
using unique_x509_ptr = std::unique_ptr<X509, X509deletor>;
40+
41+
struct SSL_CTX_Deletor {
42+
void operator()(SSL_CTX* ctx) {
43+
SSL_CTX_free(ctx);
44+
}
45+
};
46+
47+
using unique_ssl_ctx_ptr = std::unique_ptr<SSL_CTX, SSL_CTX_Deletor>;
4048
}
4149
}

tests/config_parse_test/config_parse_test.cc

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -731,7 +731,7 @@ TEST_F(SettingsTest, SslCipherList) {
731731
FAIL() << exception.what();
732732
}
733733

734-
// An empty string is also allowed
734+
// An empty string is also allowed (giving default ciphers)
735735
obj["ssl_cipher_list"] = "";
736736
try {
737737
Settings settings(obj);
@@ -740,6 +740,14 @@ TEST_F(SettingsTest, SslCipherList) {
740740
} catch (std::exception& exception) {
741741
FAIL() << exception.what();
742742
}
743+
744+
// Detect that we need at least 1 cipher defined
745+
obj["ssl_cipher_list"] = "foobar";
746+
try {
747+
Settings settings(obj);
748+
FAIL() << "We should not be allowed to disable all ciphers";
749+
} catch (std::exception&) {
750+
}
743751
}
744752

745753
TEST_F(SettingsTest, SslCipherOrder) {

0 commit comments

Comments
 (0)