From 0ddee9613b442a173eb1042b62ab4b5c9dca9b89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Wed, 6 May 2020 11:39:36 +0300 Subject: [PATCH 1/4] MXS-2981: Treat missing TLS files as an error Also treats partially defined TLS files as an error. --- server/core/admin.cc | 44 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/server/core/admin.cc b/server/core/admin.cc index 01f0bbe42..2d9bdae1a 100644 --- a/server/core/admin.cc +++ b/server/core/admin.cc @@ -39,6 +39,13 @@ using std::string; using std::ifstream; +enum class LoadResult +{ + OK, + IGNORE, + ERROR +}; + static struct MHD_Daemon* http_daemon = NULL; /** In-memory certificates in PEM format */ @@ -336,34 +343,48 @@ static char* load_cert(const char* file) if (!infile.good()) { - MXS_ERROR("Failed to load certificate file: %s", file); delete rval; rval = NULL; } } + if (!rval) + { + MXS_ERROR("Failed to load certificate file '%s': %d, %s", file, errno, mxb_strerror(errno)); + } + return rval; } -static bool load_ssl_certificates() +static LoadResult load_ssl_certificates() { - bool rval = false; + LoadResult rval = LoadResult::IGNORE; const char* key = config_get_global_options()->admin_ssl_key; const char* cert = config_get_global_options()->admin_ssl_cert; const char* ca = config_get_global_options()->admin_ssl_ca_cert; - if (*key && *cert) + if (!*key != !*cert) + { + MXS_ERROR("Both the admin TLS certificate and private key must be defined."); + rval = LoadResult::ERROR; + } + else if (*key && *cert) { admin_ssl_key = load_cert(key); admin_ssl_cert = load_cert(cert); - admin_ssl_ca_cert = load_cert(ca); - if (admin_ssl_key && admin_ssl_cert) + if (*ca) { - rval = true; + admin_ssl_ca_cert = load_cert(ca); + } + + if (admin_ssl_key && admin_ssl_cert && (!*ca || admin_ssl_ca_cert)) + { + rval = LoadResult::OK; } else { + rval = LoadResult::ERROR; delete admin_ssl_key; delete admin_ssl_cert; delete admin_ssl_ca_cert; @@ -404,10 +425,17 @@ bool mxs_admin_init() options |= MHD_USE_DUAL_STACK; } - if (load_ssl_certificates()) + auto ssl_res = load_ssl_certificates(); + + if (ssl_res == LoadResult::OK) { using_ssl = true; options |= MHD_USE_SSL; + MXS_NOTICE("The REST API will be encrypted, all requests must use HTTPS."); + } + else if (ssl_res == LoadResult::ERROR) + { + return false; } // The port argument is ignored and the port in the struct sockaddr is used instead From a2b5a1aba3d2db1c4cf5de3a76d5444d527ae30f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Wed, 6 May 2020 12:24:21 +0300 Subject: [PATCH 2/4] MXS-2980: Forward options to the interactive mode The --quiet option does not make sense in the interactive mode so it isn't forwarded. Added code that reports TLS certificate loading errors. The errors themselves aren't very exact but at least they give a hint as to why it failed. --- maxctrl/lib/common.js | 7 ++++++- maxctrl/lib/core.js | 16 ++++++++++++++-- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/maxctrl/lib/common.js b/maxctrl/lib/common.js index 238afdd5b..89627f9fc 100644 --- a/maxctrl/lib/common.js +++ b/maxctrl/lib/common.js @@ -369,7 +369,12 @@ module.exports = function() { args.auth = {user: argv.u, pass: argv.p} args.json = true args.timeout = this.argv.timeout - setTlsCerts(args) + + try { + setTlsCerts(args) + } catch (err) { + return error('Failed to set TLS certificates: ' + JSON.stringify(err, null, 4)) + } return request(args) .then(function(res) { diff --git a/maxctrl/lib/core.js b/maxctrl/lib/core.js index d08d94482..d6ba6c958 100644 --- a/maxctrl/lib/core.js +++ b/maxctrl/lib/core.js @@ -56,7 +56,7 @@ program }) .option('q', { alias: 'quiet', - describe: 'Silence all output', + describe: 'Silence all output. This option is not used in the interactive mode.', default: false, type: 'boolean' }) @@ -123,7 +123,19 @@ program base_opts = ['--user=' + argv.user, '--password=' + argv.password, '--hosts=' + argv.hosts, - '--timeout=' + argv.timeout] + '--timeout=' + argv.timeout, + '--tsv=' + argv.tsv, + '--secure=' + argv.secure, + '--tls-verify-server-cert=' + argv['tls-verify-server-cert']] + + // Only set the string options if they are defined, otherwise we'll end up with the value as + // the string 'undefined' + for (i of ['tls-key', 'tls-cert', 'tls-passphrase', 'tls-ca-cert']) { + if (argv[i]) { + base_opts.push('--' + i + '=' + argv[i]) + } + } + return askQuestion() } else { maxctrl(argv, function() { From b4108270dcb5b4ac5e98826e33a1e41d56ee222a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 7 May 2020 12:30:49 +0300 Subject: [PATCH 3/4] MXS-2982: Fix documetation link in --help output --- server/core/gateway.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/core/gateway.cc b/server/core/gateway.cc index 5e3a283ed..9f002a95a 100644 --- a/server/core/gateway.cc +++ b/server/core/gateway.cc @@ -1085,7 +1085,7 @@ static void usage(void) "dir will be '/path/maxscale/var/log/maxscale', the config dir will be\n" "'/path/maxscale/etc' and the default config file will be\n" "'/path/maxscale/etc/maxscale.cnf'.\n\n" - "MaxScale documentation: https://mariadb.com/kb/en/mariadb-enterprise/mariadb-maxscale-21/ \n", + "MaxScale documentation: https://mariadb.com/kb/en/maxscale/ \n", get_configdir(), default_cnf_fname, get_configdir(), From 73eba01ce97ed683576e7f8d7125e209c6e25e09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 7 May 2020 16:48:59 +0300 Subject: [PATCH 4/4] Fix `cluster sync` documentation The MaxScale instance isn't stopped if the synchronization fails. Added missing documentation for the command argument. --- maxctrl/lib/cluster.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/maxctrl/lib/cluster.js b/maxctrl/lib/cluster.js index 4a2eeb9ca..6ac12b250 100644 --- a/maxctrl/lib/cluster.js +++ b/maxctrl/lib/cluster.js @@ -178,9 +178,8 @@ exports.builder = function(yargs) { }) .command('sync ', 'Synchronize the cluster with target MaxScale server.', function(yargs) { return yargs.epilog('This command will alter all MaxScale instances given in the --hosts ' + - 'option to represent the MaxScale. If the synchronization of ' + - 'a MaxScale instance fails, it will be disabled by executing the `stop maxscale` ' + - 'command on that instance. Synchronization can be attempted again if a previous ' + + 'option to represent the MaxScale. Value of ' + + 'must be in HOST:PORT format. Synchronization can be attempted again if a previous ' + 'attempt failed due to a network failure or some other ephemeral error. Any other ' + 'errors require manual synchronization of the MaxScale configuration files and a ' + 'restart of the failed Maxscale.\n\n' +