wbtiger
/
foundationdb
mirror of https://github.com/apple/foundationdb.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #8102 from sfc-gh-jshim/fdbcli-tls-connect 

Make FDBCLI check coordinator :tls suffix
This commit is contained in:
Junhyun Shim 2022-09-06 16:17:49 +02:00 committed by GitHub
parent f60a1e488f 738a101a58
commit f2e68d0566
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 124 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

64
bindings/python/tests/fdbcli_tests.py
View File

@ -7,6 +7,7 @@ import subprocess
import logging
import functools
import json
import tempfile
import time
import random
from argparse import ArgumentParser, RawDescriptionHelpFormatter
@ -770,6 +771,68 @@ def integer_options():
assert lines[1].startswith('Committed')
assert error_output == b''
def tls_address_suffix():
# fdbcli shall prevent a non-TLS fdbcli run from connecting to an all-TLS cluster, and vice versa
preamble = 'eNW1yf1M:eNW1yf1M@'
def make_addr(port: int, tls: bool = False):
return "127.0.0.1:{}{}".format(port, ":tls" if tls else "")
testcases = [
# IsServerTLS, NumServerAddrs
(True, 1),
(False, 1),
(True, 3),
(False, 3),
]
err_output_server_no_tls = "ERROR: fdbcli is configured with TLS, but none of the coordinators have TLS addresses."
err_output_server_tls = "ERROR: fdbcli is not configured with TLS, but all of the coordinators have TLS addresses."
# technically the contents of the certs and key files are not evaluated
# before tls-suffix check against tls configuration takes place,
# but we generate the certs and keys anyway to avoid
# imposing nuanced TLSConfig evaluation ordering requirement on the testcase
with tempfile.TemporaryDirectory() as tmpdir:
cert_file = tmpdir + "/client-cert.pem"
key_file = tmpdir + "/client-key.pem"
ca_file = tmpdir + "/server-ca.pem"
mkcert_process = subprocess.run([
args.build_dir + "/bin/mkcert",
"--server-chain-length", "1",
"--client-chain-length", "1",
"--server-cert-file", tmpdir + "/server-cert.pem",
"--client-cert-file", tmpdir + "/client-cert.pem",
"--server-key-file", tmpdir + "/server-key.pem",
"--client-key-file", tmpdir + "/client-key.pem",
"--server-ca-file", tmpdir + "/server-ca.pem",
"--client-ca-file", tmpdir + "/client-ca.pem",
],
capture_output=True)
if mkcert_process.returncode != 0:
print("mkcert returned with code {}".format(mkcert_process.returncode))
print("Output:\n{}{}\n".format(
mkcert_process.stdout.decode("utf8").strip(),
mkcert_process.stderr.decode("utf8").strip()))
assert False
cluster_fn = tmpdir + "/fdb.cluster"
for testcase in testcases:
is_server_tls, num_server_addrs = testcase
with open(cluster_fn, "w") as fp:
fp.write(preamble + ",".join(
[make_addr(port=4000 + addr_idx, tls=is_server_tls) for addr_idx in range(num_server_addrs)]))
fp.close()
tls_args = ["--tls-certificate-file",
cert_file,
"--tls-key-file",
key_file,
"--tls-ca-file",
ca_file] if not is_server_tls else []
fdbcli_process = subprocess.run(command_template[:2] + [cluster_fn] + tls_args, capture_output=True)
assert fdbcli_process.returncode != 0
err_out = fdbcli_process.stderr.decode("utf8").strip()
if is_server_tls:
assert err_out == err_output_server_tls, f"unexpected output: {err_out}"
else:
assert err_out == err_output_server_no_tls, f"unexpected output: {err_out}"
if __name__ == '__main__':
parser = ArgumentParser(formatter_class=RawDescriptionHelpFormatter,
description="""
@ -816,6 +879,7 @@ if __name__ == '__main__':
tenants()
versionepoch()
integer_options()
tls_address_suffix()
else:
assert args.process_number > 1, "Process number should be positive"
coordinators()

68
fdbcli/fdbcli.actor.cpp
View File

@ -1050,7 +1050,7 @@ Future<T> stopNetworkAfter(Future<T> what) {
}
}
ACTOR Future<int> cli(CLIOptions opt, LineNoise* plinenoise) {
ACTOR Future<int> cli(CLIOptions opt, LineNoise* plinenoise, Reference<ClusterConnectionFile> ccf) {
state LineNoise& linenoise = *plinenoise;
state bool intrans = false;
@ -1075,20 +1075,6 @@ ACTOR Future<int> cli(CLIOptions opt, LineNoise* plinenoise) {
state FdbOptions* options = &globalOptions;
state Reference<ClusterConnectionFile> ccf;
state std::pair<std::string, bool> resolvedClusterFile =
ClusterConnectionFile::lookupClusterFileName(opt.clusterFile);
try {
ccf = makeReference<ClusterConnectionFile>(resolvedClusterFile.first);
} catch (Error& e) {
if (e.code() == error_code_operation_cancelled) {
throw;
}
fprintf(stderr, "%s\n", ClusterConnectionFile::getErrorString(resolvedClusterFile, e).c_str());
return 1;
}
// Ordinarily, this is done when the network is run. However, network thread should be set before TraceEvents are
// logged. This thread will eventually run the network, so call it now.
TraceEvent::setNetworkThread();
@ -1987,7 +1973,7 @@ ACTOR Future<int> cli(CLIOptions opt, LineNoise* plinenoise) {
}
}
ACTOR Future<int> runCli(CLIOptions opt) {
ACTOR Future<int> runCli(CLIOptions opt, Reference<ClusterConnectionFile> ccf) {
state LineNoise linenoise(
[](std::string const& line, std::vector<std::string>& completions) { fdbcliCompCmd(line, completions); },
[enabled = opt.cliHints](std::string const& line) -> LineNoise::Hint {
@ -2051,7 +2037,7 @@ ACTOR Future<int> runCli(CLIOptions opt) {
.GetLastError();
}
state int result = wait(cli(opt, &linenoise));
state int result = wait(cli(opt, &linenoise, ccf));
if (!historyFilename.empty()) {
try {
@ -2073,6 +2059,33 @@ ACTOR Future<Void> timeExit(double duration) {
return Void();
}
const char* checkTlsConfigAgainstCoordAddrs(const ClusterConnectionString& ccs) {
// Resolve TLS config and inspect whether any of the certificate, key, ca bytes has been set
extern TLSConfig tlsConfig;
auto const loaded = tlsConfig.loadSync();
const bool tlsConfigured =
!loaded.getCertificateBytes().empty() || !loaded.getKeyBytes().empty() || !loaded.getCABytes().empty();
int tlsAddrs = 0;
int totalAddrs = 0;
for (const auto& addr : ccs.coords) {
if (addr.isTLS())
tlsAddrs++;
totalAddrs++;
}
for (const auto& host : ccs.hostnames) {
if (host.isTLS)
tlsAddrs++;
totalAddrs++;
}
if (tlsConfigured && tlsAddrs == 0) {
return "fdbcli is configured with TLS, but none of the coordinators have TLS addresses.";
} else if (!tlsConfigured && tlsAddrs == totalAddrs) {
return "fdbcli is not configured with TLS, but all of the coordinators have TLS addresses.";
} else {
return nullptr;
}
}
int main(int argc, char** argv) {
platformInit();
Error::init();
@ -2177,6 +2190,25 @@ int main(int argc, char** argv) {
return 0;
}
Reference<ClusterConnectionFile> ccf;
std::pair<std::string, bool> resolvedClusterFile = ClusterConnectionFile::lookupClusterFileName(opt.clusterFile);
try {
ccf = makeReference<ClusterConnectionFile>(resolvedClusterFile.first);
} catch (Error& e) {
if (e.code() == error_code_operation_cancelled) {
throw;
}
fprintf(stderr, "%s\n", ClusterConnectionFile::getErrorString(resolvedClusterFile, e).c_str());
return 1;
}
// Make sure that TLS configuration lines up with ":tls" prefix on coordinator addresses
if (auto errorMsg = checkTlsConfigAgainstCoordAddrs(ccf->getConnectionString())) {
fprintf(stderr, "ERROR: %s\n", errorMsg);
return 1;
}
try {
API->selectApiVersion(opt.apiVersion);
if (opt.useFutureProtocolVersion) {
@ -2188,7 +2220,7 @@ int main(int argc, char** argv) {
return opt.exit_code;
}
Future<Void> memoryUsageMonitor = startMemoryUsageMonitor(opt.memLimit);
Future<int> cliFuture = runCli(opt);
Future<int> cliFuture = runCli(opt, ccf);
Future<Void> timeoutFuture = opt.exit_timeout ? timeExit(opt.exit_timeout) : Never();
auto f = stopNetworkAfter(success(cliFuture) || timeoutFuture);
API->runNetwork();

13
flow/MkCertCli.cpp
View File

@ -49,6 +49,7 @@ enum EMkCertOpt : int {
OPT_PRINT_SERVER_CERT,
OPT_PRINT_CLIENT_CERT,
OPT_PRINT_ARGUMENTS,
OPT_ENABLE_TRACE,
};
CSimpleOpt::SOption gOptions[] = { { OPT_HELP, "--help", SO_NONE },
@ -68,6 +69,7 @@ CSimpleOpt::SOption gOptions[] = { { OPT_HELP, "--help", SO_NONE },
{ OPT_PRINT_SERVER_CERT, "--print-server-cert", SO_NONE },
{ OPT_PRINT_CLIENT_CERT, "--print-client-cert", SO_NONE },
{ OPT_PRINT_ARGUMENTS, "--print-args", SO_NONE },
{ OPT_ENABLE_TRACE, "--trace", SO_NONE },
SO_END_OF_OPTIONS };
template <size_t Len>
@ -191,6 +193,7 @@ int main(int argc, char** argv) {
auto printServerCert = false;
auto printClientCert = false;
auto printArguments = false;
auto enableTrace = false;
auto args = CSimpleOpt(argc, argv, gOptions, SO_O_EXACT | SO_O_HYPHEN_TO_UNDERSCORE);
while (args.Next()) {
if (auto err = args.LastError()) {
@ -266,6 +269,8 @@ int main(int argc, char** argv) {
case OPT_PRINT_ARGUMENTS:
printArguments = true;
break;
case OPT_ENABLE_TRACE:
enableTrace = true;
default:
fmt::print(stderr, "ERROR: Unknown option {}\n", args.OptionText());
return FDB_EXIT_ERROR;
@ -277,12 +282,14 @@ int main(int argc, char** argv) {
platformInit();
Error::init();
g_network = newNet2(TLSConfig());
openTraceFile(NetworkAddress(), 10 << 20, 10 << 20, ".", "mkcert");
if (enableTrace)
openTraceFile(NetworkAddress(), 10 << 20, 10 << 20, ".", "mkcert");
auto thread = std::thread([]() { g_network->run(); });
auto cleanUpGuard = ScopeExit([&thread]() {
auto cleanUpGuard = ScopeExit([&thread, enableTrace]() {
g_network->stop();
thread.join();
closeTraceFile();
if (enableTrace)
closeTraceFile();
});
serverArgs.transformPathToAbs();