Skip to content

Commit

Permalink
WL#15524 Patch #6 Command authorization in MGM client
Browse files Browse the repository at this point in the history
This completes the authorization framework introduced in the previous
patch, adding the client side and a test case.

It adds three new error codes to the MGM API: the generic code
NDB_MGM_NOT_AUTHORIZED, and two more specific codes,
NDB_MGM_AUTH_REQUIRES_TLS and NDB_MGM_AUTH_REQUIRES_CLIENT_CERT.

A new class RewindInputStream helps the MGM client to reset its
protocol parsing context whenever it recieves an authorization failure
from the server.

The test case is testMgmd -n RequireTls

Change-Id: I9cc8bbbad5c1131f6de2fb4b3a6f4b11e82df7e3
  • Loading branch information
jdduncan committed Jul 21, 2023
1 parent 8e63be1 commit 52fcb43
Show file tree
Hide file tree
Showing 7 changed files with 153 additions and 2 deletions.
2 changes: 1 addition & 1 deletion mysql-test/suite/ndb/r/ndbinfo_plans.result
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ ndb$disk_write_speed_base 488 48
ndb$diskpagebuffer 10 64
ndb$diskstat 10 48
ndb$diskstats_1sec 200 52
ndb$error_messages 792 52
ndb$error_messages 795 52
ndb$frag_locks 344 96
ndb$frag_mem_use 344 100
ndb$frag_operations 344 192
Expand Down
8 changes: 8 additions & 0 deletions storage/ndb/include/mgmapi/mgmapi_error.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,14 @@ extern "C" {
Connecting node should retry. */
NDB_MGM_ALLOCID_CONFIG_RETRY = 1103,

/* Authorization failures. Server did not allow command. */
/** Generic authorization failure. */
NDB_MGM_NOT_AUTHORIZED = 1501,
/** Command requires TLS */
NDB_MGM_AUTH_REQUIRES_TLS = 1502,
/** Command requires TLS client certificate */
NDB_MGM_AUTH_REQUIRES_CLIENT_CERT = 1503,

/* Service errors - Start/Stop Node or System */
/** Start failed */
NDB_MGM_START_FAILED = 2001,
Expand Down
15 changes: 15 additions & 0 deletions storage/ndb/include/util/InputStream.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,19 @@ class SocketInputStream : public SecureSocketInputStream {
~SocketInputStream() override {}
};

/* RewindInputStream takes an existing stream plus one line of null-terminated
* buffered data read using gets(). When reading from the RewindInputStream,
* the first line requested will come from the buffered line. Subsequent
* read requests will come from the source stream.
*/
class RewindInputStream : public InputStream {
InputStream & m_stream;
const char * m_buf;
bool m_first;
public:
RewindInputStream(InputStream & s, const char * buf) :
m_stream(s), m_buf(buf), m_first(true) {}
char* gets(char * buf, int bufLen) override;
};

#endif
16 changes: 16 additions & 0 deletions storage/ndb/src/common/util/InputStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "ndb_global.h"

#include "InputStream.hpp"
#include "util/cstrbuf.h"

FileInputStream Stdin(stdin);

Expand Down Expand Up @@ -87,3 +88,18 @@ SecureSocketInputStream::gets(char * buf, int bufLen) {

return buf;
}

char *
RewindInputStream::gets(char * buf, int bufLen) {

if(m_first) {
m_first = false;
cstrbuf buffer({buf, buf + bufLen});
buffer.append(m_buf);
buffer.append("\n");
require(! buffer.is_truncated());
return buf;
}

return m_stream.gets(buf, bufLen);
}
53 changes: 53 additions & 0 deletions storage/ndb/src/mgmapi/mgmapi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,37 @@ ndb_mgm_get_latest_error_msg(const NdbMgmHandle h)
}


static const Properties *
handle_authorization_failure(NdbMgmHandle handle, InputStream & in)
{
/* Read the failure details and set the internal error conditions. */
handle->last_error = NDB_MGM_NOT_AUTHORIZED;

const ParserRow<ParserDummy> details[] = {
MGM_CMD("Authorization failed", nullptr, ""),
MGM_ARG("Error", String, Mandatory, "Error message"),
MGM_END()
};

Parser_t::Context ctx;
ParserDummy session(handle->socket);
Parser_t parser(details, in);
const Properties * reply = parser.parse(ctx, session);

const char * reason = nullptr;
if(reply && reply->get("Error", &reason))
{
if(strcmp(reason, "Requires TLS Client Certificate") == 0)
handle->last_error = NDB_MGM_AUTH_REQUIRES_CLIENT_CERT;
else if(strcmp(reason, "Requires TLS") == 0)
handle->last_error = NDB_MGM_AUTH_REQUIRES_TLS;
}

delete reply;
return nullptr;
}


/*
ndb_mgm_call
Expand Down Expand Up @@ -605,6 +636,14 @@ ndb_mgm_call(NdbMgmHandle handle,
CHECK_TIMEDOUT_RET(handle, in, out, NULL, cmd);
DBUG_RETURN(NULL);
}

/* Check for Authorization failure */
if(strcmp(ctx.m_tokenBuffer, "Authorization failed") == 0)
{
RewindInputStream str(in, ctx.m_tokenBuffer);
DBUG_RETURN(handle_authorization_failure(handle, str));
}

/**
* Print some info about why the parser returns NULL
*/
Expand Down Expand Up @@ -1326,6 +1365,12 @@ ndb_mgm_get_status2(NdbMgmHandle handle, const enum ndb_mgm_node_type types[])
SET_ERROR(handle, NDB_MGM_ILLEGAL_SERVER_REPLY, "Probably disconnected");
DBUG_RETURN(NULL);
}
if(strcmp("Authorization failed", buf) == 0)
{
RewindInputStream str(in, buf);
(void) handle_authorization_failure(handle, str);
DBUG_RETURN(nullptr);
}
if(strcmp("node status\n", buf) != 0) {
CHECK_TIMEDOUT_RET(handle, in, out, NULL, get_status_str);
ndbout << in.timedout() << " " << out.timedout() << buf << endl;
Expand Down Expand Up @@ -1569,6 +1614,12 @@ ndb_mgm_get_status3(NdbMgmHandle handle, const enum ndb_mgm_node_type types[])
SET_ERROR(handle, NDB_MGM_ILLEGAL_SERVER_REPLY, "Probably disconnected");
DBUG_RETURN(NULL);
}
if(strcmp("Authorization failed", buf) == 0)
{
RewindInputStream str(in, buf);
(void) handle_authorization_failure(handle, str);
DBUG_RETURN(nullptr);
}
if(strcmp("node status\n", buf) != 0) {
CHECK_TIMEDOUT_RET(handle, in, out, NULL, get_status_str);
ndbout << in.timedout() << " " << out.timedout() << buf << endl;
Expand Down Expand Up @@ -3627,6 +3678,7 @@ ndb_mgm_check_connection(NdbMgmHandle handle)
DBUG_ENTER("ndb_mgm_check_connection");
CHECK_HANDLE(handle, -1);
CHECK_CONNECTED(handle, -1);
/* Treated as bootstrap command; cannot result in authorization failure */
SecureSocketOutputStream out(handle->socket, handle->timeout);
SecureSocketInputStream in(handle->socket, handle->timeout);
char buf[32];
Expand Down Expand Up @@ -3844,6 +3896,7 @@ int ndb_mgm_end_session(NdbMgmHandle handle)
s_output.println("%s", end_session_str);
s_output.println("%s", "");

/* Treated as bootstrap command; cannot result in authorization failure */
SecureSocketInputStream in(handle->socket, handle->timeout);
char buf[32];
in.gets(buf, sizeof(buf));
Expand Down
8 changes: 7 additions & 1 deletion storage/ndb/src/mgmapi/mgmapi_error.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,13 @@ const struct Ndb_Mgm_Error_Msg ndb_mgm_error_msgs[] = {
{ NDB_MGM_SERVER_NOT_CONNECTED, "Management server not connected" },
{ NDB_MGM_COULD_NOT_CONNECT_TO_SOCKET, "Could not connect to socket" },
{ NDB_MGM_ILLEGAL_BIND_ADDRESS, "Illegal bind address" },


/* Authorization Failure */
{ NDB_MGM_NOT_AUTHORIZED, "Not Authorized" },
{ NDB_MGM_AUTH_REQUIRES_TLS, "Not Authorized: TLS Required" },
{ NDB_MGM_AUTH_REQUIRES_CLIENT_CERT,
"Not Authorized: Valid TLS Certificate Required" },

/* Service errors - Start/Stop Node or System */
{ NDB_MGM_START_FAILED, "Start failed" },
{ NDB_MGM_STOP_FAILED, "Stop failed" },
Expand Down
53 changes: 53 additions & 0 deletions storage/ndb/test/ndbapi/testMgmd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2111,6 +2111,54 @@ int runTestStartTls(NDBT_Context* ctx, NDBT_Step* step)
return NDBT_OK;
}

int runTestRequireTls(NDBT_Context* ctx, NDBT_Step* step)
{
/* Create a configuration file in the working directory */
NDBT_Workingdir wd("test_tls");
BaseString cfg_path = path(wd.path(), "config.ini", nullptr);
Properties config = ConfigFactory::create();
ConfigFactory::put(config, "ndb_mgmd", 1, "RequireTls", "true");
CHECK(ConfigFactory::write_config_ini(config, cfg_path.c_str()));

/* Create keys in test_tls, and initialize our own TLS context */
TlsKeyManager tls_km;
bool k = sign_tls_keys(wd);
CHECK(k);
tls_km.init(wd.path(), 0, NODE_TYPE_API, true);
CHECK(tls_km.ctx());

/* Start a management server that will require TLS */
Mgmd mgmd(1);
NdbProcess::Args mgmdArgs;
mgmd.common_args(mgmdArgs, wd.path());
mgmdArgs.add("--ndb-tls-search-path=", wd.path());
CHECK(mgmd.start(wd.path(), mgmdArgs)); // Start management node
sleep(1); // Wait for confirmed config

/* Our management client */
NdbMgmHandle handle = ndb_mgm_create_handle();
ndb_mgm_set_connectstring(handle, mgmd.connectstring(config).c_str());
ndb_mgm_set_ssl_ctx(handle, tls_km.ctx());

int r = ndb_mgm_connect(handle, 3, 5, 1); // Connect to management node
CHECK(r == 0);

ndb_mgm_severity sev = { NDB_MGM_EVENT_SEVERITY_ON, 1 };
r = ndb_mgm_get_clusterlog_severity_filter(handle, &sev, 1);
CHECK(r < 1); // COMMAND IS NOT YET ALLOWED
int err = ndb_mgm_get_latest_error(handle);
CHECK(err == NDB_MGM_AUTH_REQUIRES_TLS);

r = ndb_mgm_start_tls(handle);
printf("ndb_mgm_start_tls(): %d\n", r); // START TLS
CHECK(r == 0);

r = ndb_mgm_get_clusterlog_severity_filter(handle, &sev, 1);
CHECK(r == 1); // NOW COMMAND IS ALLOWED

return NDBT_OK;
}

NDBT_TESTSUITE(testMgmd);
DRIVER(DummyDriver); /* turn off use of NdbApi */

Expand Down Expand Up @@ -2241,6 +2289,11 @@ TESTCASE("StartTls", "Test START TLS in MGM protocol")
INITIALIZER(runTestStartTls);
}

TESTCASE("RequireTls", "Test MGM server that requires TLS")
{
INITIALIZER(runTestRequireTls);
}

#endif

NDBT_TESTSUITE_END(testMgmd)
Expand Down

0 comments on commit 52fcb43

Please sign in to comment.