Skip to content

Commit

Permalink
WL#15524 Patch #4 MGM TLS Configuration
Browse files Browse the repository at this point in the history
In the NDB configuration, add boolen options RequireTls and
RequireCertificate to the [MGM] section. Both options default to
false.

Add a new test testMgmd -n MgmdWithoutCertificate

In NdbStdOpt, add the --ndb-mgm-tls command-line option. The allowed
values are "relaxed" and "strict". The default is "relaxed".  This option
will be used for utility programs, allowing the user to specify the
TLS-related behavior of MGM clients.

Change-Id: Id32bb8805ca19a8cf8b52f45c54a7be4d912c5e4
  • Loading branch information
jdduncan committed Jul 21, 2023
1 parent 959e3f6 commit f37c3ae
Show file tree
Hide file tree
Showing 7 changed files with 102 additions and 5 deletions.
1 change: 1 addition & 0 deletions storage/ndb/include/mgmapi/mgmapi_config_parameters.h
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@
#define CFG_MAX_LOGLEVEL 262

#define CFG_MGM_PORT 300
#define CFG_MGM_REQUIRE_TLS 301

#define CFG_DB_MAX_BUFFERED_EPOCH_BYTES 350

Expand Down
12 changes: 12 additions & 0 deletions storage/ndb/include/util/ndb_opts.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "my_alloc.h" // MEM_ROOT
#include "my_sys.h" // loglevel needed by my_getopt.h
#include "my_getopt.h"
#include "typelib.h"
#include "util/BaseString.hpp"
#include "util/require.h"

Expand All @@ -48,6 +49,7 @@ OPT_EXTERN(int, opt_connect_retry_delay,NONE);
OPT_EXTERN(int, opt_connect_retries,NONE);
OPT_EXTERN(const char *,opt_charsets_dir,=0);
OPT_EXTERN(const char *,opt_tls_search_path,=NDB_TLS_SEARCH_PATH);
OPT_EXTERN(unsigned long long,opt_mgm_tls,=0);

#ifndef NDEBUG
OPT_EXTERN(const char *,opt_debug,= 0);
Expand Down Expand Up @@ -76,6 +78,9 @@ enum ndb_std_options {

namespace NdbStdOpt {

static const char * tls_names[] = { "relaxed", "strict", nullptr };
static TYPELIB mgm_tls_typelib = { 2 , "TLS requirement", tls_names, nullptr };

static constexpr struct my_option usage =
{ "usage", '?', "Display this help and exit.",
nullptr, nullptr, nullptr, GET_NO_ARG, NO_ARG,
Expand Down Expand Up @@ -148,6 +153,13 @@ static constexpr struct my_option tls_search_path =
&opt_tls_search_path, nullptr, nullptr, GET_STR, REQUIRED_ARG,
0, 0, 0, nullptr, 0, nullptr};

static constexpr struct my_option mgm_tls =
{ "ndb-mgm-tls", NDB_OPT_NOSHORT,
"MGM client TLS requirement level",
&opt_mgm_tls, nullptr, &mgm_tls_typelib, GET_ENUM, REQUIRED_ARG,
0 /* default=CLIENT_TLS_RELAXED */, 0 /*min*/, 1 /*max*/,
nullptr, 0, nullptr};

#ifndef NDEBUG
static constexpr struct my_option debug =
{ "debug", '#', "Output debug log. Often this is 'd:t:o,filename'.",
Expand Down
30 changes: 28 additions & 2 deletions storage/ndb/src/common/mgmcommon/ConfigInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1919,7 +1919,7 @@ const ConfigInfo::ParamInfo ConfigInfo::m_ParamInfo[] = {
CFG_DB_REQUIRE_TLS,
"RequireTls",
DB_TOKEN,
"Require TLS authenticated secure connections",
"Require TLS-authenticated secure connections",
ConfigInfo::CI_USED,
0,
ConfigInfo::CI_BOOL,
Expand Down Expand Up @@ -3359,6 +3359,32 @@ const ConfigInfo::ParamInfo ConfigInfo::m_ParamInfo[] = {
STR_VALUE(MAX_INT_RNIL)
},

{
CFG_NODE_REQUIRE_CERT,
"RequireCertificate",
MGM_TOKEN,
"Require valid TLS key and certificate at startup time",
ConfigInfo::CI_USED,
false,
ConfigInfo::CI_BOOL,
"false",
"false",
"true"
},

{
CFG_MGM_REQUIRE_TLS,
"RequireTls",
MGM_TOKEN,
"Require TLS-authenticated secure connections",
ConfigInfo::CI_USED,
0,
ConfigInfo::CI_BOOL,
"false",
"false",
"true"
},

/****************************************************************************
* TCP
***************************************************************************/
Expand Down Expand Up @@ -3520,7 +3546,7 @@ const ConfigInfo::ParamInfo ConfigInfo::m_ParamInfo[] = {
CFG_TCP_REQUIRE_TLS,
"RequireLinkTls",
"TCP",
"Use TLS authenticated secure connections for TCP transporter links",
"Use TLS-authenticated secure connections for TCP transporter links",
ConfigInfo::CI_INTERNAL,
0,
ConfigInfo::CI_BOOL,
Expand Down
32 changes: 31 additions & 1 deletion storage/ndb/src/mgmsrv/MgmtSrvr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "util/require.h"
#include <ndb_global.h>
#include <cstring>
#include "openssl/ssl.h"

#include "MgmtSrvr.hpp"
#include "ndb_mgmd_error.h"
Expand Down Expand Up @@ -59,6 +60,7 @@
#include <NdbSleep.h>
#include <portlib/NdbDir.hpp>
#include "portlib/ndb_sockaddr.h"
#include "portlib/ndb_openssl_version.h"
#include <EventLogger.hpp>
#include <logger/FileLogHandler.hpp>
#include <logger/ConsoleLogHandler.hpp>
Expand Down Expand Up @@ -96,6 +98,9 @@ int g_errorInsert = 0;
}\
}

static constexpr bool openssl_version_ok =
(OPENSSL_VERSION_NUMBER >= NDB_TLS_MINIMUM_OPENSSL);

void *
MgmtSrvr::logLevelThread_C(void* m)
{
Expand Down Expand Up @@ -482,6 +487,18 @@ MgmtSrvr::start_mgm_service(const Config* config)
g_eventLogger->error("PortNumber not defined for node %d", _ownNodeId);
DBUG_RETURN(false);
}

// Find the TLS requirement level
Uint32 requireCert = 0;
Uint32 requireTls = 0;

if(openssl_version_ok)
{
require(iter.get(CFG_MGM_REQUIRE_TLS, &requireTls) == 0);
require(iter.get(CFG_NODE_REQUIRE_CERT, &requireCert) == 0);
}
m_require_tls = requireTls;
m_require_cert = requireCert;
}

unsigned short port= m_port;
Expand Down Expand Up @@ -572,7 +589,7 @@ MgmtSrvr::start()
{
DBUG_ENTER("MgmtSrvr::start");

/* Configure TLS */
/* Configure TlsKeyManager */
require(m_tls_search_path);
theFacade->mgm_configure_tls(m_tls_search_path);

Expand All @@ -590,6 +607,19 @@ MgmtSrvr::start()
DBUG_RETURN(false);
}

/* Check for required TLS certificate */
ssl_ctx_st * ctx = theFacade->get_registry()->getTlsKeyManager()->ctx();
if(require_cert() && ! ctx)
{
g_eventLogger->error(
"Shutting down. This node does not have a valid TLS certificate.");
DBUG_RETURN(false);
}

g_eventLogger->info(require_tls() ?
"This server will require all MGM clients to use TLS" :
"Not requiring TLS");

/* Use local MGM port for TransporterRegistry */
if(!connect_to_self())
{
Expand Down
6 changes: 6 additions & 0 deletions storage/ndb/src/mgmsrv/MgmtSrvr.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,9 @@ class MgmtSrvr : private ConfigSubscriber, public trp_client {

const char * m_tls_search_path { nullptr };

bool m_require_tls { false };
bool m_require_cert { false };

bool m_need_restart;

ndb_sockaddr m_connect_address[MAX_NODES];
Expand Down Expand Up @@ -530,6 +533,9 @@ class MgmtSrvr : private ConfigSubscriber, public trp_client {

void show_variables(NdbOut& out = ndbout);

bool require_tls() const { return m_require_tls; }
bool require_cert() const { return m_require_tls || m_require_cert; }

private:
class NodeIdReservations {
struct Reservation {
Expand Down
4 changes: 2 additions & 2 deletions storage/ndb/test/include/ConfigFactory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,9 @@ struct ConfigFactory
return config;
}

static bool
template <typename T> static bool
put(Properties& config, const char* section, Uint32 section_no,
const char* key, Uint32 value)
const char* key, T value)
{
Properties* p;
// Get a copy of the section to modify
Expand Down
22 changes: 22 additions & 0 deletions storage/ndb/test/ndbapi/testMgmd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1935,6 +1935,22 @@ runTestSshKeySigning(NDBT_Context* ctx, NDBT_Step* step)
return NDBT_OK;
}

int
runTestMgmdWithoutCert(NDBT_Context* ctx, NDBT_Step* step)
{
NDBT_Workingdir wd("test_mgmd"); // temporary working directory
BaseString cfg_path = path(wd.path(), "config.ini", nullptr);

Properties config = ConfigFactory::create();
ConfigFactory::put(config, "ndb_mgmd", 1, "RequireCertificate", "true");
CHECK(ConfigFactory::write_config_ini(config, cfg_path.c_str()));

Mgmd mgmd(1);
CHECK(mgmd.start_from_config_ini(wd.path())); // Start management node
CHECK(! mgmd.connect(config)); // Cannot connect
return NDBT_OK;
}

int
runTestNdbdWithoutCert(NDBT_Context* ctx, NDBT_Step* step)
{
Expand Down Expand Up @@ -2197,6 +2213,12 @@ TESTCASE("SshKeySigning",
INITIALIZER(runTestSshKeySigning);
}

TESTCASE("MgmdWithoutCertificate",
"Test MGM server startup with TLS required but no certificate")
{
INITIALIZER(runTestMgmdWithoutCert)
}

TESTCASE("NdbdWithoutCertificate",
"Test data node startup with TLS required but no certificate")
{
Expand Down

0 comments on commit f37c3ae

Please sign in to comment.