Skip to content

Commit 52fcb43

Browse files
committed
WL#15524 Patch #6 Command authorization in MGM client
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
1 parent 8e63be1 commit 52fcb43

File tree

7 files changed

+153
-2
lines changed

7 files changed

+153
-2
lines changed

mysql-test/suite/ndb/r/ndbinfo_plans.result

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ ndb$disk_write_speed_base 488 48
7575
ndb$diskpagebuffer 10 64
7676
ndb$diskstat 10 48
7777
ndb$diskstats_1sec 200 52
78-
ndb$error_messages 792 52
78+
ndb$error_messages 795 52
7979
ndb$frag_locks 344 96
8080
ndb$frag_mem_use 344 100
8181
ndb$frag_operations 344 192

storage/ndb/include/mgmapi/mgmapi_error.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,14 @@ extern "C" {
7272
Connecting node should retry. */
7373
NDB_MGM_ALLOCID_CONFIG_RETRY = 1103,
7474

75+
/* Authorization failures. Server did not allow command. */
76+
/** Generic authorization failure. */
77+
NDB_MGM_NOT_AUTHORIZED = 1501,
78+
/** Command requires TLS */
79+
NDB_MGM_AUTH_REQUIRES_TLS = 1502,
80+
/** Command requires TLS client certificate */
81+
NDB_MGM_AUTH_REQUIRES_CLIENT_CERT = 1503,
82+
7583
/* Service errors - Start/Stop Node or System */
7684
/** Start failed */
7785
NDB_MGM_START_FAILED = 2001,

storage/ndb/include/util/InputStream.hpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,4 +80,19 @@ class SocketInputStream : public SecureSocketInputStream {
8080
~SocketInputStream() override {}
8181
};
8282

83+
/* RewindInputStream takes an existing stream plus one line of null-terminated
84+
* buffered data read using gets(). When reading from the RewindInputStream,
85+
* the first line requested will come from the buffered line. Subsequent
86+
* read requests will come from the source stream.
87+
*/
88+
class RewindInputStream : public InputStream {
89+
InputStream & m_stream;
90+
const char * m_buf;
91+
bool m_first;
92+
public:
93+
RewindInputStream(InputStream & s, const char * buf) :
94+
m_stream(s), m_buf(buf), m_first(true) {}
95+
char* gets(char * buf, int bufLen) override;
96+
};
97+
8398
#endif

storage/ndb/src/common/util/InputStream.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include "ndb_global.h"
2828

2929
#include "InputStream.hpp"
30+
#include "util/cstrbuf.h"
3031

3132
FileInputStream Stdin(stdin);
3233

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

8889
return buf;
8990
}
91+
92+
char *
93+
RewindInputStream::gets(char * buf, int bufLen) {
94+
95+
if(m_first) {
96+
m_first = false;
97+
cstrbuf buffer({buf, buf + bufLen});
98+
buffer.append(m_buf);
99+
buffer.append("\n");
100+
require(! buffer.is_truncated());
101+
return buf;
102+
}
103+
104+
return m_stream.gets(buf, bufLen);
105+
}

storage/ndb/src/mgmapi/mgmapi.cpp

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,37 @@ ndb_mgm_get_latest_error_msg(const NdbMgmHandle h)
469469
}
470470

471471

472+
static const Properties *
473+
handle_authorization_failure(NdbMgmHandle handle, InputStream & in)
474+
{
475+
/* Read the failure details and set the internal error conditions. */
476+
handle->last_error = NDB_MGM_NOT_AUTHORIZED;
477+
478+
const ParserRow<ParserDummy> details[] = {
479+
MGM_CMD("Authorization failed", nullptr, ""),
480+
MGM_ARG("Error", String, Mandatory, "Error message"),
481+
MGM_END()
482+
};
483+
484+
Parser_t::Context ctx;
485+
ParserDummy session(handle->socket);
486+
Parser_t parser(details, in);
487+
const Properties * reply = parser.parse(ctx, session);
488+
489+
const char * reason = nullptr;
490+
if(reply && reply->get("Error", &reason))
491+
{
492+
if(strcmp(reason, "Requires TLS Client Certificate") == 0)
493+
handle->last_error = NDB_MGM_AUTH_REQUIRES_CLIENT_CERT;
494+
else if(strcmp(reason, "Requires TLS") == 0)
495+
handle->last_error = NDB_MGM_AUTH_REQUIRES_TLS;
496+
}
497+
498+
delete reply;
499+
return nullptr;
500+
}
501+
502+
472503
/*
473504
ndb_mgm_call
474505
@@ -605,6 +636,14 @@ ndb_mgm_call(NdbMgmHandle handle,
605636
CHECK_TIMEDOUT_RET(handle, in, out, NULL, cmd);
606637
DBUG_RETURN(NULL);
607638
}
639+
640+
/* Check for Authorization failure */
641+
if(strcmp(ctx.m_tokenBuffer, "Authorization failed") == 0)
642+
{
643+
RewindInputStream str(in, ctx.m_tokenBuffer);
644+
DBUG_RETURN(handle_authorization_failure(handle, str));
645+
}
646+
608647
/**
609648
* Print some info about why the parser returns NULL
610649
*/
@@ -1326,6 +1365,12 @@ ndb_mgm_get_status2(NdbMgmHandle handle, const enum ndb_mgm_node_type types[])
13261365
SET_ERROR(handle, NDB_MGM_ILLEGAL_SERVER_REPLY, "Probably disconnected");
13271366
DBUG_RETURN(NULL);
13281367
}
1368+
if(strcmp("Authorization failed", buf) == 0)
1369+
{
1370+
RewindInputStream str(in, buf);
1371+
(void) handle_authorization_failure(handle, str);
1372+
DBUG_RETURN(nullptr);
1373+
}
13291374
if(strcmp("node status\n", buf) != 0) {
13301375
CHECK_TIMEDOUT_RET(handle, in, out, NULL, get_status_str);
13311376
ndbout << in.timedout() << " " << out.timedout() << buf << endl;
@@ -1569,6 +1614,12 @@ ndb_mgm_get_status3(NdbMgmHandle handle, const enum ndb_mgm_node_type types[])
15691614
SET_ERROR(handle, NDB_MGM_ILLEGAL_SERVER_REPLY, "Probably disconnected");
15701615
DBUG_RETURN(NULL);
15711616
}
1617+
if(strcmp("Authorization failed", buf) == 0)
1618+
{
1619+
RewindInputStream str(in, buf);
1620+
(void) handle_authorization_failure(handle, str);
1621+
DBUG_RETURN(nullptr);
1622+
}
15721623
if(strcmp("node status\n", buf) != 0) {
15731624
CHECK_TIMEDOUT_RET(handle, in, out, NULL, get_status_str);
15741625
ndbout << in.timedout() << " " << out.timedout() << buf << endl;
@@ -3627,6 +3678,7 @@ ndb_mgm_check_connection(NdbMgmHandle handle)
36273678
DBUG_ENTER("ndb_mgm_check_connection");
36283679
CHECK_HANDLE(handle, -1);
36293680
CHECK_CONNECTED(handle, -1);
3681+
/* Treated as bootstrap command; cannot result in authorization failure */
36303682
SecureSocketOutputStream out(handle->socket, handle->timeout);
36313683
SecureSocketInputStream in(handle->socket, handle->timeout);
36323684
char buf[32];
@@ -3844,6 +3896,7 @@ int ndb_mgm_end_session(NdbMgmHandle handle)
38443896
s_output.println("%s", end_session_str);
38453897
s_output.println("%s", "");
38463898

3899+
/* Treated as bootstrap command; cannot result in authorization failure */
38473900
SecureSocketInputStream in(handle->socket, handle->timeout);
38483901
char buf[32];
38493902
in.gets(buf, sizeof(buf));

storage/ndb/src/mgmapi/mgmapi_error.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,13 @@ const struct Ndb_Mgm_Error_Msg ndb_mgm_error_msgs[] = {
3737
{ NDB_MGM_SERVER_NOT_CONNECTED, "Management server not connected" },
3838
{ NDB_MGM_COULD_NOT_CONNECT_TO_SOCKET, "Could not connect to socket" },
3939
{ NDB_MGM_ILLEGAL_BIND_ADDRESS, "Illegal bind address" },
40-
40+
41+
/* Authorization Failure */
42+
{ NDB_MGM_NOT_AUTHORIZED, "Not Authorized" },
43+
{ NDB_MGM_AUTH_REQUIRES_TLS, "Not Authorized: TLS Required" },
44+
{ NDB_MGM_AUTH_REQUIRES_CLIENT_CERT,
45+
"Not Authorized: Valid TLS Certificate Required" },
46+
4147
/* Service errors - Start/Stop Node or System */
4248
{ NDB_MGM_START_FAILED, "Start failed" },
4349
{ NDB_MGM_STOP_FAILED, "Stop failed" },

storage/ndb/test/ndbapi/testMgmd.cpp

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2111,6 +2111,54 @@ int runTestStartTls(NDBT_Context* ctx, NDBT_Step* step)
21112111
return NDBT_OK;
21122112
}
21132113

2114+
int runTestRequireTls(NDBT_Context* ctx, NDBT_Step* step)
2115+
{
2116+
/* Create a configuration file in the working directory */
2117+
NDBT_Workingdir wd("test_tls");
2118+
BaseString cfg_path = path(wd.path(), "config.ini", nullptr);
2119+
Properties config = ConfigFactory::create();
2120+
ConfigFactory::put(config, "ndb_mgmd", 1, "RequireTls", "true");
2121+
CHECK(ConfigFactory::write_config_ini(config, cfg_path.c_str()));
2122+
2123+
/* Create keys in test_tls, and initialize our own TLS context */
2124+
TlsKeyManager tls_km;
2125+
bool k = sign_tls_keys(wd);
2126+
CHECK(k);
2127+
tls_km.init(wd.path(), 0, NODE_TYPE_API, true);
2128+
CHECK(tls_km.ctx());
2129+
2130+
/* Start a management server that will require TLS */
2131+
Mgmd mgmd(1);
2132+
NdbProcess::Args mgmdArgs;
2133+
mgmd.common_args(mgmdArgs, wd.path());
2134+
mgmdArgs.add("--ndb-tls-search-path=", wd.path());
2135+
CHECK(mgmd.start(wd.path(), mgmdArgs)); // Start management node
2136+
sleep(1); // Wait for confirmed config
2137+
2138+
/* Our management client */
2139+
NdbMgmHandle handle = ndb_mgm_create_handle();
2140+
ndb_mgm_set_connectstring(handle, mgmd.connectstring(config).c_str());
2141+
ndb_mgm_set_ssl_ctx(handle, tls_km.ctx());
2142+
2143+
int r = ndb_mgm_connect(handle, 3, 5, 1); // Connect to management node
2144+
CHECK(r == 0);
2145+
2146+
ndb_mgm_severity sev = { NDB_MGM_EVENT_SEVERITY_ON, 1 };
2147+
r = ndb_mgm_get_clusterlog_severity_filter(handle, &sev, 1);
2148+
CHECK(r < 1); // COMMAND IS NOT YET ALLOWED
2149+
int err = ndb_mgm_get_latest_error(handle);
2150+
CHECK(err == NDB_MGM_AUTH_REQUIRES_TLS);
2151+
2152+
r = ndb_mgm_start_tls(handle);
2153+
printf("ndb_mgm_start_tls(): %d\n", r); // START TLS
2154+
CHECK(r == 0);
2155+
2156+
r = ndb_mgm_get_clusterlog_severity_filter(handle, &sev, 1);
2157+
CHECK(r == 1); // NOW COMMAND IS ALLOWED
2158+
2159+
return NDBT_OK;
2160+
}
2161+
21142162
NDBT_TESTSUITE(testMgmd);
21152163
DRIVER(DummyDriver); /* turn off use of NdbApi */
21162164

@@ -2241,6 +2289,11 @@ TESTCASE("StartTls", "Test START TLS in MGM protocol")
22412289
INITIALIZER(runTestStartTls);
22422290
}
22432291

2292+
TESTCASE("RequireTls", "Test MGM server that requires TLS")
2293+
{
2294+
INITIALIZER(runTestRequireTls);
2295+
}
2296+
22442297
#endif
22452298

22462299
NDBT_TESTSUITE_END(testMgmd)

0 commit comments

Comments
 (0)