Skip to content

Commit c56eb1d

Browse files
committed
Bug#35983164 NdbProcess treats space, backslash, double quote different on Windows and posix [#3]
Adding a new method NdbProcess::create_via_ssh to be used when invoking programs over ssh. Currently no extra quoting of arguments will be done due to the ssh indirection. To properly provide that one would need to know about what shell will ssh use to execute command on remote host, and on Windows one also need to know what quoting the program expects. Change-Id: Iff588581c2afa6f599e6055f916dafb5d3cff602
1 parent 6cc06d7 commit c56eb1d

File tree

2 files changed

+53
-12
lines changed

2 files changed

+53
-12
lines changed

storage/ndb/include/portlib/NdbProcess.hpp

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,10 @@ class NdbProcess {
139139
return proc;
140140
}
141141

142+
static std::unique_ptr<NdbProcess> create_via_ssh(
143+
const BaseString &name, const BaseString &host, const BaseString &path,
144+
const BaseString &cwd, const Args &args, Pipes *const fds = nullptr);
145+
142146
private:
143147
#ifdef _WIN32
144148
process_handle_t m_proc{InvalidHandle, InvalidHandle, 0, 0};
@@ -241,6 +245,32 @@ inline void NdbProcess::Args::add(const Args &args) {
241245
for (unsigned i = 0; i < args.m_args.size(); i++) add(args.m_args[i].c_str());
242246
}
243247

248+
std::unique_ptr<NdbProcess> NdbProcess::create_via_ssh(
249+
const BaseString &name, const BaseString &host, const BaseString &path,
250+
const BaseString &cwd, const Args &args, Pipes *const fds) {
251+
BaseString ssh_name = "ssh";
252+
Args ssh_args;
253+
ssh_args.add(host.c_str());
254+
/*
255+
* Arguments need to be quoted. What kind of quoting depends on what shell is
256+
* used on remote host when ssh executes the command.
257+
*
258+
* And for Windows also what quoting the command itself requires on the
259+
* command line.
260+
*
261+
* Currently caller of this function need to provide proper quoting.
262+
*
263+
* This function do quote the arguments such that ssh sees them as is, as
264+
* done by start_process called via create.
265+
*
266+
* On Windows it is assumed that ssh follows the quoting rules for Microsoft
267+
* C/C++ runtime.
268+
*/
269+
ssh_args.add(path.c_str());
270+
ssh_args.add(args);
271+
return create(name, ssh_name, cwd, ssh_args, fds);
272+
}
273+
244274
std::optional<BaseString> NdbProcess::quote_for_windows_crt(
245275
const char *str) noexcept {
246276
/*

storage/ndb/tools/sign_keys.cpp

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,18 @@ enum {
9898
SIGN_CO_PROCESS = 3
9999
} signing_method = SIGN_LOCAL;
100100

101+
static inline bool signing_over_ssh() {
102+
switch (signing_method) {
103+
case SIGN_SSH_SIGN_KEYS:
104+
case SIGN_SSH_OPENSSL:
105+
return true;
106+
case SIGN_LOCAL:
107+
case SIGN_CO_PROCESS:
108+
return false;
109+
}
110+
return false;
111+
}
112+
101113
short exp_schedule[6] = {0, 0, 0, 0, 0, 0};
102114

103115
const char *remote_ca_path = nullptr;
@@ -1113,9 +1125,7 @@ int remote_signing_method(BaseString &cmd, NdbProcess::Args &args,
11131125

11141126
switch (signing_method) {
11151127
case SIGN_SSH_SIGN_KEYS: // 1: Run ndb_sign_keys on remote CA host via ssh
1116-
cmd.assign("ssh");
1117-
args.add(opt_ca_host);
1118-
args.add(opt_remote_path ? opt_remote_path : "ndb_sign_keys");
1128+
cmd.assign(opt_remote_path ? opt_remote_path : "ndb_sign_keys");
11191129
args.add("--stdio");
11201130
args.add("--duration=", csr->duration());
11211131
if (opt_ca_cert != ClusterCertAuthority::CertFile)
@@ -1125,9 +1135,7 @@ int remote_signing_method(BaseString &cmd, NdbProcess::Args &args,
11251135
if (remote_ca_path) args.add("--ndb-tls-search-path=", remote_ca_path);
11261136
return 1;
11271137
case SIGN_SSH_OPENSSL: // 2: Run openssl on remote CA host over ssh
1128-
cmd.assign("ssh");
1129-
args.add(opt_ca_host);
1130-
args.add(opt_remote_path ? opt_remote_path : "openssl");
1138+
cmd.assign(opt_remote_path ? opt_remote_path : "openssl");
11311139
args.add("x509");
11321140
args.add("-req");
11331141
args.add2("-CA", opt_ca_cert); // full pathname on remote server
@@ -1162,13 +1170,12 @@ int fetch_CA_cert_from_remote_openssl(stack_st_X509 *CA_certs) {
11621170
NdbProcess::Args args;
11631171
NdbProcess::Pipes pipes;
11641172

1165-
BaseString cmd("ssh");
1166-
args.add(opt_ca_host);
1167-
args.add(opt_remote_path ? opt_remote_path : "openssl");
1173+
BaseString cmd(opt_remote_path ? opt_remote_path : "openssl");
11681174
args.add("x509");
11691175
args.add2("-in", opt_ca_cert);
11701176

1171-
auto proc = NdbProcess::create("OpensslFetchCA", cmd, nullptr, args, &pipes);
1177+
auto proc = NdbProcess::create_via_ssh("OpensslFetchCA", opt_ca_host, cmd,
1178+
nullptr, args, &pipes);
11721179
if (!proc) return 133;
11731180

11741181
FILE *rfp = pipes.open(pipes.parentRead(), "r");
@@ -1216,8 +1223,12 @@ int remote_key_signing(const SigningRequest *csr, EVP_PKEY *key,
12161223
}
12171224

12181225
/* Create process */
1219-
std::unique_ptr<NdbProcess> proc =
1220-
NdbProcess::create("RemoteKeySigning", cmd, nullptr, args, &pipes);
1226+
std::unique_ptr<NdbProcess> proc;
1227+
if (signing_over_ssh())
1228+
proc = NdbProcess::create_via_ssh("RemoteKeySigning", opt_ca_host, cmd,
1229+
nullptr, args, &pipes);
1230+
else
1231+
proc = NdbProcess::create("RemoteKeySigning", cmd, nullptr, args, &pipes);
12211232
if (!proc) return fatal_error(133, "Failed to create process.\n");
12221233

12231234
/* Write CSR and passphrase to coprocess */

0 commit comments

Comments
 (0)