Skip to content

Commit

Permalink
Bug#35983164 NdbProcess treats space, backslash, double quote differe…
Browse files Browse the repository at this point in the history
…nt 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
  • Loading branch information
zmur committed Dec 7, 2023
1 parent 6cc06d7 commit c56eb1d
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 12 deletions.
30 changes: 30 additions & 0 deletions storage/ndb/include/portlib/NdbProcess.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,10 @@ class NdbProcess {
return proc;
}

static std::unique_ptr<NdbProcess> create_via_ssh(
const BaseString &name, const BaseString &host, const BaseString &path,
const BaseString &cwd, const Args &args, Pipes *const fds = nullptr);

private:
#ifdef _WIN32
process_handle_t m_proc{InvalidHandle, InvalidHandle, 0, 0};
Expand Down Expand Up @@ -241,6 +245,32 @@ inline void NdbProcess::Args::add(const Args &args) {
for (unsigned i = 0; i < args.m_args.size(); i++) add(args.m_args[i].c_str());
}

std::unique_ptr<NdbProcess> NdbProcess::create_via_ssh(
const BaseString &name, const BaseString &host, const BaseString &path,
const BaseString &cwd, const Args &args, Pipes *const fds) {
BaseString ssh_name = "ssh";
Args ssh_args;
ssh_args.add(host.c_str());
/*
* Arguments need to be quoted. What kind of quoting depends on what shell is
* used on remote host when ssh executes the command.
*
* And for Windows also what quoting the command itself requires on the
* command line.
*
* Currently caller of this function need to provide proper quoting.
*
* This function do quote the arguments such that ssh sees them as is, as
* done by start_process called via create.
*
* On Windows it is assumed that ssh follows the quoting rules for Microsoft
* C/C++ runtime.
*/
ssh_args.add(path.c_str());
ssh_args.add(args);
return create(name, ssh_name, cwd, ssh_args, fds);
}

std::optional<BaseString> NdbProcess::quote_for_windows_crt(
const char *str) noexcept {
/*
Expand Down
35 changes: 23 additions & 12 deletions storage/ndb/tools/sign_keys.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,18 @@ enum {
SIGN_CO_PROCESS = 3
} signing_method = SIGN_LOCAL;

static inline bool signing_over_ssh() {
switch (signing_method) {
case SIGN_SSH_SIGN_KEYS:
case SIGN_SSH_OPENSSL:
return true;
case SIGN_LOCAL:
case SIGN_CO_PROCESS:
return false;
}
return false;
}

short exp_schedule[6] = {0, 0, 0, 0, 0, 0};

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

switch (signing_method) {
case SIGN_SSH_SIGN_KEYS: // 1: Run ndb_sign_keys on remote CA host via ssh
cmd.assign("ssh");
args.add(opt_ca_host);
args.add(opt_remote_path ? opt_remote_path : "ndb_sign_keys");
cmd.assign(opt_remote_path ? opt_remote_path : "ndb_sign_keys");
args.add("--stdio");
args.add("--duration=", csr->duration());
if (opt_ca_cert != ClusterCertAuthority::CertFile)
Expand All @@ -1125,9 +1135,7 @@ int remote_signing_method(BaseString &cmd, NdbProcess::Args &args,
if (remote_ca_path) args.add("--ndb-tls-search-path=", remote_ca_path);
return 1;
case SIGN_SSH_OPENSSL: // 2: Run openssl on remote CA host over ssh
cmd.assign("ssh");
args.add(opt_ca_host);
args.add(opt_remote_path ? opt_remote_path : "openssl");
cmd.assign(opt_remote_path ? opt_remote_path : "openssl");
args.add("x509");
args.add("-req");
args.add2("-CA", opt_ca_cert); // full pathname on remote server
Expand Down Expand Up @@ -1162,13 +1170,12 @@ int fetch_CA_cert_from_remote_openssl(stack_st_X509 *CA_certs) {
NdbProcess::Args args;
NdbProcess::Pipes pipes;

BaseString cmd("ssh");
args.add(opt_ca_host);
args.add(opt_remote_path ? opt_remote_path : "openssl");
BaseString cmd(opt_remote_path ? opt_remote_path : "openssl");
args.add("x509");
args.add2("-in", opt_ca_cert);

auto proc = NdbProcess::create("OpensslFetchCA", cmd, nullptr, args, &pipes);
auto proc = NdbProcess::create_via_ssh("OpensslFetchCA", opt_ca_host, cmd,
nullptr, args, &pipes);
if (!proc) return 133;

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

/* Create process */
std::unique_ptr<NdbProcess> proc =
NdbProcess::create("RemoteKeySigning", cmd, nullptr, args, &pipes);
std::unique_ptr<NdbProcess> proc;
if (signing_over_ssh())
proc = NdbProcess::create_via_ssh("RemoteKeySigning", opt_ca_host, cmd,
nullptr, args, &pipes);
else
proc = NdbProcess::create("RemoteKeySigning", cmd, nullptr, args, &pipes);
if (!proc) return fatal_error(133, "Failed to create process.\n");

/* Write CSR and passphrase to coprocess */
Expand Down

0 comments on commit c56eb1d

Please sign in to comment.