hcq/MaxScale
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MXS-3345: Defer path permission checks

Browse Source 
Doing the directory permission checks after all the values have been set
helps avoid problems with intermediate values that aren't valid. This
happens when --basedir generates invalid derived paths and the correct path
is provided as an argument right after it.

The path parameter is read from the configuration file only if it hasn't
been modified by a command line option. The case where an invalid command
line option is given but a valid configuration parameter would override it
is still treated as an error.

Also added a clarifying comment into set_dirs to make sure the handling
for the two path parameters is not moved inside set_runtime_dirs.

Fixed the testing scripts for the REST API and MaxCtrl now that the
directory permission checks are done correctly. Previously some paths seem
to have been ignored.
This commit is contained in:
Markus Mäkelä
2021-02-05 16:39:14 +02:00
parent 16a15e02be
commit 47bcb6ad02
4 changed files with 74 additions and 64 deletions
Download Patch File Download Diff File Expand all files Collapse all files

129
server/core/gateway.cc
View File

@ -182,23 +182,24 @@ static bool resolve_maxscale_conf_fname(char** cnf_full_path,
const char* home_dir, const char* home_dir,
char* cnf_file_arg); char* cnf_file_arg);
static char* check_dir_access(char* dirname, bool, bool); static bool check_dir_access(const char* dirname, bool wr, bool rd);
static int set_user(const char* user); bool check_paths();
bool pid_file_exists(); static int set_user(const char* user);
void write_child_exit_code(int fd, int code); bool pid_file_exists();
static bool change_cwd(); void write_child_exit_code(int fd, int code);
static void log_exit_status(); static bool change_cwd();
static bool daemonize(); static void log_exit_status();
static bool sniff_configuration(const char* filepath); static bool daemonize();
static bool modules_process_init(); static bool sniff_configuration(const char* filepath);
static void modules_process_finish(); static bool modules_process_init();
static void disable_module_unloading(const char* arg); static void modules_process_finish();
static void enable_module_unloading(const char* arg); static void disable_module_unloading(const char* arg);
static void enable_statement_logging(const char* arg); static void enable_module_unloading(const char* arg);
static void disable_statement_logging(const char* arg); static void enable_statement_logging(const char* arg);
static void redirect_output_to_file(const char* arg); static void disable_statement_logging(const char* arg);
static bool user_is_acceptable(const char* specified_user); static void redirect_output_to_file(const char* arg);
static bool init_sqlite3(); static bool user_is_acceptable(const char* specified_user);
static bool init_sqlite3();
struct DEBUG_ARGUMENT struct DEBUG_ARGUMENT
{ {
@ -714,51 +715,32 @@ static bool resolve_maxscale_conf_fname(char** cnf_full_path,
* @return NULL if directory can be read and written, an error message if either * @return NULL if directory can be read and written, an error message if either
* read or write is not permitted. * read or write is not permitted.
*/ */
static char* check_dir_access(char* dirname, bool rd, bool wr) static bool check_dir_access(const char* dirname, bool rd, bool wr)
{ {
char errbuf[PATH_MAX * 2]; mxb_assert(dirname);
char* errstr = NULL; std::ostringstream ss;
if (dirname == NULL)
{
errstr = MXS_STRDUP_A("Directory argument is NULL");
goto retblock;
}
if (access(dirname, F_OK) != 0) if (access(dirname, F_OK) != 0)
{ {
snprintf(errbuf, PATH_MAX * 2 - 1, "Can't access '%s'.", dirname); ss << "Can't access '" << dirname << "'.";
errbuf[PATH_MAX * 2 - 1] = '\0';
errstr = MXS_STRDUP_A(errbuf);
goto retblock;
} }
else if (rd && access(dirname, R_OK) != 0)
if (rd && !path_is_readable(dirname))
{ {
snprintf(errbuf, ss << "MaxScale doesn't have read permission to '" << dirname << "'.";
PATH_MAX * 2 - 1,
"MaxScale doesn't have read permission "
"to '%s'.",
dirname);
errbuf[PATH_MAX * 2 - 1] = '\0';
errstr = MXS_STRDUP_A(errbuf);
goto retblock;
} }
else if (wr && access(dirname, W_OK) != 0)
if (wr && !path_is_writable(dirname))
{ {
snprintf(errbuf, ss << "MaxScale doesn't have write permission to '" << dirname << "'.";
PATH_MAX * 2 - 1,
"MaxScale doesn't have write permission "
"to '%s'.",
dirname);
errbuf[PATH_MAX * 2 - 1] = '\0';
errstr = MXS_STRDUP_A(errbuf);
goto retblock;
} }
retblock: auto err = ss.str();
return errstr;
if (!err.empty())
{
print_log_n_stderr(true, true, err.c_str(), err.c_str(), errno);
}
return err.empty();
} }
static bool init_log() static bool init_log()
@ -1427,6 +1409,10 @@ bool set_dirs(const char* basedir)
rv = set_runtime_dirs(basedir); rv = set_runtime_dirs(basedir);
// The two paths here are not inside set_runtime_dirs on purpose: they are set by --basedir but not by
// --runtimedir. The former is used with tarball installations and the latter is used to run multiple
// MaxScale instances on the same server.
if (rv && (rv = handle_path_arg(&path, basedir, MXS_DEFAULT_LIB_SUBPATH, true, false))) if (rv && (rv = handle_path_arg(&path, basedir, MXS_DEFAULT_LIB_SUBPATH, true, false)))
{ {
set_libdir(path); set_libdir(path);
@ -1824,7 +1810,10 @@ int main(int argc, char** argv)
break; break;
case '?': case '?':
usage(); if (check_paths())
{
usage();
}
rc = EXIT_SUCCESS; rc = EXIT_SUCCESS;
goto return_main; goto return_main;
@ -2058,6 +2047,12 @@ int main(int argc, char** argv)
goto return_main; goto return_main;
} }
if (!check_paths())
{
rc = MAXSCALE_BADCONFIG;
goto return_main;
}
if (daemon_mode) if (daemon_mode)
{ {
if (!change_cwd()) if (!change_cwd())
@ -2690,22 +2685,28 @@ bool handle_path_arg(char** dest, const char* path, const char* arg, bool rd, bo
strcat(pathbuffer, arg); strcat(pathbuffer, arg);
} }
if ((errstr = check_dir_access(pathbuffer, rd, wr)) == NULL) *dest = MXS_STRDUP_A(pathbuffer);
{ rval = true;
*dest = MXS_STRDUP_A(pathbuffer);
rval = true;
}
else
{
print_log_n_stderr(true, true, errstr, errstr, 0);
MXS_FREE(errstr);
errstr = NULL;
}
} }
return rval; return rval;
} }
bool check_paths()
{
return check_dir_access(get_logdir(), true, false)
&& check_dir_access(get_cachedir(), true, true)
&& check_dir_access(get_configdir(), true, false)
&& check_dir_access(get_module_configdir(), true, false)
&& check_dir_access(get_datadir(), true, false)
&& check_dir_access(get_langdir(), true, false)
&& check_dir_access(get_piddir(), true, true)
&& check_dir_access(get_config_persistdir(), true, true)
&& check_dir_access(get_connector_plugindir(), true, false)
&& check_dir_access(get_libdir(), true, false)
&& check_dir_access(get_execdir(), true, false);
}
void set_log_augmentation(const char* value) void set_log_augmentation(const char* value)
{ {
// Command line arguments are handled first, thus command line argument // Command line arguments are handled first, thus command line argument

4
test/maxscale_test.cnf
View File

@ -6,6 +6,10 @@ datadir=@CMAKE_INSTALL_PREFIX@/lib/maxscale
cachedir=@CMAKE_INSTALL_PREFIX@/cache/maxscale cachedir=@CMAKE_INSTALL_PREFIX@/cache/maxscale
language=@CMAKE_INSTALL_PREFIX@/lib/maxscale/ language=@CMAKE_INSTALL_PREFIX@/lib/maxscale/
piddir=@CMAKE_INSTALL_PREFIX@/run/maxscale/ piddir=@CMAKE_INSTALL_PREFIX@/run/maxscale/
persistdir=@CMAKE_INSTALL_PREFIX@/lib/maxscale/maxscale.cnf.d/
module_configdir=@CMAKE_INSTALL_PREFIX@/etc/maxscale.modules.d/
# This isn't really correct but it's OK as long as it's an empty directory
connector_plugindir=@CMAKE_INSTALL_PREFIX@/etc/maxscale.modules.d/
admin_auth=false admin_auth=false
[server1] [server1]

3
test/maxscale_test_secondary.cnf
View File

@ -7,6 +7,9 @@ cachedir=@CMAKE_INSTALL_PREFIX@/secondary/cache/maxscale
language=@CMAKE_INSTALL_PREFIX@/secondary/lib/maxscale/ language=@CMAKE_INSTALL_PREFIX@/secondary/lib/maxscale/
piddir=@CMAKE_INSTALL_PREFIX@/secondary/run/maxscale piddir=@CMAKE_INSTALL_PREFIX@/secondary/run/maxscale
persistdir=@CMAKE_INSTALL_PREFIX@/secondary/lib/maxscale/maxscale.cnf.d/ persistdir=@CMAKE_INSTALL_PREFIX@/secondary/lib/maxscale/maxscale.cnf.d/
module_configdir=@CMAKE_INSTALL_PREFIX@/secondary/etc/maxscale.modules.d/
# This isn't really correct but it's OK as long as it's an empty directory
connector_plugindir=@CMAKE_INSTALL_PREFIX@/secondary/etc/maxscale.modules.d/
admin_auth=false admin_auth=false
admin_port=8990 admin_port=8990

2
test/start_double_maxscale.sh
View File

@ -20,10 +20,12 @@ rm -r $maxscaledir/secondary/run/maxscale
test -f /tmp/maxadmin.sock && rm /tmp/maxadmin.sock test -f /tmp/maxadmin.sock && rm /tmp/maxadmin.sock
test -f /tmp/maxadmin2.sock && rm /tmp/maxadmin2.sock test -f /tmp/maxadmin2.sock && rm /tmp/maxadmin2.sock
mkdir -m 0755 -p $maxscaledir/etc/maxscale.modules.d/
mkdir -m 0755 -p $maxscaledir/lib/maxscale/maxscale.cnf.d mkdir -m 0755 -p $maxscaledir/lib/maxscale/maxscale.cnf.d
mkdir -m 0755 -p $maxscaledir/cache/maxscale mkdir -m 0755 -p $maxscaledir/cache/maxscale
mkdir -m 0755 -p $maxscaledir/run/maxscale mkdir -m 0755 -p $maxscaledir/run/maxscale
mkdir -m 0755 -p $maxscaledir/log/maxscale mkdir -m 0755 -p $maxscaledir/log/maxscale
mkdir -m 0755 -p $maxscaledir/secondary/etc/maxscale.modules.d/
mkdir -m 0755 -p $maxscaledir/secondary/lib/maxscale/maxscale.cnf.d mkdir -m 0755 -p $maxscaledir/secondary/lib/maxscale/maxscale.cnf.d
mkdir -m 0755 -p $maxscaledir/secondary/cache/maxscale mkdir -m 0755 -p $maxscaledir/secondary/cache/maxscale
mkdir -m 0755 -p $maxscaledir/secondary/run/maxscale mkdir -m 0755 -p $maxscaledir/secondary/run/maxscale