Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Match the process' effective user id against the data owner at the server startup. #3785

Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 49 additions & 15 deletions dbms/programs/server/Server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@

#include <memory>
#include <sys/resource.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <errno.h>
#include <pwd.h>
#include <unistd.h>
#include <Poco/Version.h>
#include <Poco/DirectoryIterator.h>
#include <Poco/Net/HTTPServer.h>
Expand Down Expand Up @@ -70,6 +74,9 @@ namespace ErrorCodes
extern const int EXCESSIVE_ELEMENT_IN_CONFIG;
extern const int INVALID_CONFIG_PARAMETER;
extern const int SYSTEM_ERROR;
extern const int FAILED_TO_STAT_DATA;
sergey-v-galtsev marked this conversation as resolved.
Show resolved Hide resolved
extern const int FAILED_TO_GETPWUID;
extern const int MISMATCHING_USERS_FOR_PROCESS_AND_DATA;
}


Expand All @@ -83,6 +90,25 @@ static std::string getCanonicalPath(std::string && path)
return std::move(path);
}

static std::string getUserName(uid_t user_id) {
/// Try to convert user id into user name.
auto buffer_size = sysconf(_SC_GETPW_R_SIZE_MAX);
if (buffer_size <= 0)
buffer_size = 32;
std::string buffer;
buffer.reserve(buffer_size);

struct passwd passwd_entry;
struct passwd * result = nullptr;
const auto error = getpwuid_r(user_id, &passwd_entry, buffer.data(), buffer_size, &result);

if (error)
throwFromErrno("Failed to find user name for " + toString(user_id), ErrorCodes::FAILED_TO_GETPWUID, error);
else if (result)
return result->pw_name;
return toString(user_id);
}

void Server::uninitialize()
{
logger().information("shutting down");
Expand Down Expand Up @@ -166,6 +192,22 @@ int Server::main(const std::vector<std::string> & /*args*/)
std::string path = getCanonicalPath(config().getString("path", DBMS_DEFAULT_PATH));
std::string default_database = config().getString("default_database", "default");

/// Check that the process' user id matches the owner of the data.
sergey-v-galtsev marked this conversation as resolved.
Show resolved Hide resolved
const auto effective_user_id = geteuid();
struct stat statbuf;
const auto effective_user = getUserName(effective_user_id);
LOG_INFO(log, "effective_user = " + effective_user);
if (stat(path.c_str(), &statbuf) == 0 && effective_user_id != statbuf.st_uid)
{
const auto effective_user = getUserName(effective_user_id);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate.

const auto data_owner = getUserName(statbuf.st_uid);
std::string message = "Effective user of the process (" + effective_user +
") does not match the owner of the data (" + data_owner + ").";
if (effective_user_id == 0)
message += " Run under 'sudo -u " + data_owner + "'.";
throw Exception(message, ErrorCodes::MISMATCHING_USERS_FOR_PROCESS_AND_DATA);
}

global_context->setPath(path);

/// Create directories for 'path' and for default database, if not exist.
Expand Down Expand Up @@ -376,21 +418,13 @@ int Server::main(const std::vector<std::string> & /*args*/)
format_schema_path.createDirectories();

LOG_INFO(log, "Loading metadata.");
try
{
loadMetadataSystem(*global_context);
/// After attaching system databases we can initialize system log.
global_context->initializeSystemLogs();
/// After the system database is created, attach virtual system tables (in addition to query_log and part_log)
attachSystemTablesServer(*global_context->getDatabase("system"), has_zookeeper);
/// Then, load remaining databases
loadMetadata(*global_context);
}
catch (...)
{
tryLogCurrentException(log, "Caught exception while loading metadata");
sergey-v-galtsev marked this conversation as resolved.
Show resolved Hide resolved
throw;
}
loadMetadataSystem(*global_context);
/// After attaching system databases we can initialize system log.
global_context->initializeSystemLogs();
/// After the system database is created, attach virtual system tables (in addition to query_log and part_log)
attachSystemTablesServer(*global_context->getDatabase("system"), has_zookeeper);
/// Then, load remaining databases
loadMetadata(*global_context);
LOG_DEBUG(log, "Loaded metadata.");

global_context->setCurrentDatabase(default_database);
Expand Down
3 changes: 3 additions & 0 deletions dbms/src/Common/ErrorCodes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,9 @@ namespace ErrorCodes
extern const int SYSTEM_ERROR = 425;
extern const int NULL_POINTER_DEREFERENCE = 426;
extern const int CANNOT_COMPILE_REGEXP = 427;
extern const int FAILED_TO_STAT_DATA = 428;
extern const int FAILED_TO_GETPWUID = 429;
extern const int MISMATCHING_USERS_FOR_PROCESS_AND_DATA = 430;

extern const int KEEPER_EXCEPTION = 999;
extern const int POCO_EXCEPTION = 1000;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?xml version="1.0"?>
<yandex>
<logger>
<log>/var/log/clickhouse-server/log.log</log>
<errorlog>/var/log/clickhouse-server/clickhouse-server.err.log</errorlog>
<stderr>/var/log/clickhouse-server/stderr.log</stderr>
<stdout>/var/log/clickhouse-server/stdout.log</stdout>
</logger>

<users_config>users.xml</users_config>
</yandex>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<yandex>
<path>/no_such_directory/data/</path>
</yandex>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<yandex>
<path>/root/data/</path>
</yandex>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<?xml version="1.0"?>
<yandex>
</yandex>
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import os
import pwd
import pytest
import re

from helpers.cluster import ClickHouseCluster


def expect_failure_with_message(config, expected_message):
cluster = ClickHouseCluster(__file__)
node = cluster.add_instance('node', main_configs=[config], with_zookeeper=False)

with pytest.raises(Exception):
cluster.start()

cluster.shutdown() # cleanup

with open(os.path.join(node.path, 'logs/stderr.log')) as log:
last_message = log.readlines()[-1].strip()
if re.search(expected_message, last_message) is None:
pytest.fail('Expected the server to fail with a message "{}", but the last message is "{}"'.format(expected_message, last_message))


def test_no_such_directory():
expect_failure_with_message('configs/no_such_directory.xml', 'Failed to stat.*no_such_directory')


def test_different_user():
current_user_id = os.getuid()

if current_user_id != 0:
current_user_name = pwd.getpwuid(current_user_id).pw_name

expect_failure_with_message(
'configs/owner_mismatch.xml',
'Effective user of the process \(({}|{})\) does not match the owner of the data \((0|root)\)'.format(current_user_id, current_user_name),
)