Remove the Python plugin import blocker code.

The sudo.conf file is considered a trusted source of information
and these checks suffer from TOCTOU issues anyway.
This commit is contained in:
Todd C. Miller
2022-12-26 07:43:55 -07:00
parent 73abff2d05
commit fa1b86fca6
10 changed files with 8 additions and 277 deletions

View File

@@ -465,7 +465,6 @@ plugins/python/pyhelpers.h
plugins/python/pyhelpers_cpychecker.h plugins/python/pyhelpers_cpychecker.h
plugins/python/python_baseplugin.c plugins/python/python_baseplugin.c
plugins/python/python_convmessage.c plugins/python/python_convmessage.c
plugins/python/python_importblocker.c
plugins/python/python_loghandler.c plugins/python/python_loghandler.c
plugins/python/python_plugin.exp plugins/python/python_plugin.exp
plugins/python/python_plugin_approval.c plugins/python/python_plugin_approval.c
@@ -553,8 +552,6 @@ plugins/python/regress/testdata/check_loading_succeeds_with_missing_classname.st
plugins/python/regress/testdata/check_multiple_approval_plugin_and_arguments.stderr plugins/python/regress/testdata/check_multiple_approval_plugin_and_arguments.stderr
plugins/python/regress/testdata/check_multiple_approval_plugin_and_arguments.stdout plugins/python/regress/testdata/check_multiple_approval_plugin_and_arguments.stdout
plugins/python/regress/testdata/check_python_plugins_do_not_affect_each_other.stdout plugins/python/regress/testdata/check_python_plugins_do_not_affect_each_other.stdout
plugins/python/regress/testdata/sudo.conf.developer_mode
plugins/python/regress/testdata/sudo.conf.normal_mode
plugins/python/regress/testhelpers.c plugins/python/regress/testhelpers.c
plugins/python/regress/testhelpers.h plugins/python/regress/testhelpers.h
plugins/python/sudo_python_debug.c plugins/python/sudo_python_debug.c

View File

@@ -115,13 +115,14 @@ install_gid = 0
SHELL = @SHELL@ SHELL = @SHELL@
EXAMPLES = example_conversation.py example_debugging.py example_group_plugin.py example_io_plugin.py example_policy_plugin.py \ EXAMPLES = example_approval_plugin.py example_audit_plugin.py \
example_audit_plugin.py example_approval_plugin.py example_conversation.py example_debugging.py \
example_group_plugin.py example_io_plugin.py example_policy_plugin.py
OBJS = python_plugin_common.lo python_plugin_policy.lo python_plugin_io.lo python_plugin_group.lo pyhelpers.lo \ OBJS = python_plugin_common.lo python_plugin_policy.lo python_plugin_io.lo \
python_loghandler.lo \ python_plugin_group.lo pyhelpers.lo python_loghandler.lo \
python_importblocker.lo python_convmessage.lo sudo_python_module.lo sudo_python_debug.lo \ python_convmessage.lo sudo_python_module.lo sudo_python_debug.lo \
python_baseplugin.lo python_plugin_audit.lo python_plugin_approval.lo python_baseplugin.lo python_plugin_audit.lo python_plugin_approval.lo
IOBJS = $(OBJS:.lo=.i) IOBJS = $(OBJS:.lo=.i)
@@ -342,26 +343,6 @@ python_convmessage.i: $(srcdir)/python_convmessage.c \
$(CC) -E -o $@ $(CPPFLAGS) $< $(CC) -E -o $@ $(CPPFLAGS) $<
python_convmessage.plog: python_convmessage.i python_convmessage.plog: python_convmessage.i
rm -f $@; pvs-studio --cfg $(PVS_CFG) --sourcetree-root $(top_srcdir) --skip-cl-exe yes --source-file $(srcdir)/python_convmessage.c --i-file $< --output-file $@ rm -f $@; pvs-studio --cfg $(PVS_CFG) --sourcetree-root $(top_srcdir) --skip-cl-exe yes --source-file $(srcdir)/python_convmessage.c --i-file $< --output-file $@
python_importblocker.lo: $(srcdir)/python_importblocker.c \
$(incdir)/compat/stdbool.h $(incdir)/sudo_compat.h \
$(incdir)/sudo_debug.h $(incdir)/sudo_plugin.h \
$(incdir)/sudo_queue.h $(incdir)/sudo_util.h \
$(srcdir)/pyhelpers.h \
$(srcdir)/pyhelpers_cpychecker.h \
$(srcdir)/sudo_python_debug.h \
$(srcdir)/sudo_python_module.h $(top_builddir)/config.h
$(LIBTOOL) $(LTFLAGS) --mode=compile $(CC) -c $(CPPFLAGS) $(CFLAGS) $(ASAN_CFLAGS) $(PIE_CFLAGS) $(HARDENING_CFLAGS) $(srcdir)/python_importblocker.c
python_importblocker.i: $(srcdir)/python_importblocker.c \
$(incdir)/compat/stdbool.h $(incdir)/sudo_compat.h \
$(incdir)/sudo_debug.h $(incdir)/sudo_plugin.h \
$(incdir)/sudo_queue.h $(incdir)/sudo_util.h \
$(srcdir)/pyhelpers.h \
$(srcdir)/pyhelpers_cpychecker.h \
$(srcdir)/sudo_python_debug.h \
$(srcdir)/sudo_python_module.h $(top_builddir)/config.h
$(CC) -E -o $@ $(CPPFLAGS) $<
python_importblocker.plog: python_importblocker.i
rm -f $@; pvs-studio --cfg $(PVS_CFG) --sourcetree-root $(top_srcdir) --skip-cl-exe yes --source-file $(srcdir)/python_importblocker.c --i-file $< --output-file $@
python_loghandler.lo: $(srcdir)/python_loghandler.c $(incdir)/compat/stdbool.h \ python_loghandler.lo: $(srcdir)/python_loghandler.c $(incdir)/compat/stdbool.h \
$(incdir)/sudo_compat.h $(incdir)/sudo_debug.h \ $(incdir)/sudo_compat.h $(incdir)/sudo_debug.h \
$(incdir)/sudo_plugin.h $(incdir)/sudo_queue.h \ $(incdir)/sudo_plugin.h $(incdir)/sudo_queue.h \

View File

@@ -1,217 +0,0 @@
/*
* SPDX-License-Identifier: ISC
*
* Copyright (c) 2019-2020 Robert Manner <robert.manner@oneidentity.com>
*
* Permission to use, copy, modify, and distribute this software for any
* purpose with or without fee is hereby granted, provided that the above
* copyright notice and this permission notice appear in all copies.
*
* THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
* WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
* MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
* ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
* WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
* ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
* OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
/*
* This is an open source non-commercial project. Dear PVS-Studio, please check it.
* PVS-Studio Static Code Analyzer for C, C++ and C#: http://www.viva64.com
*/
#include "sudo_python_module.h"
#include "sudo_util.h"
#include <sys/types.h>
#include <sys/stat.h>
#include <unistd.h>
static int
_verify_import(const char *file_path)
{
debug_decl(_verify_import, PYTHON_DEBUG_INTERNAL);
// Check mode and owner similar to what we do in open_sudoers().
// This is to help avoid loading a potentially insecure module.
struct stat sb;
if (stat(file_path, &sb) != 0) {
PyErr_Format(PyExc_ImportError, "Failed to stat file '%s'", file_path);
debug_return_int(SUDO_RC_ERROR);
}
if (sb.st_uid != ROOT_UID) {
PyErr_Format(PyExc_ImportError, "File '%s' must be owned by uid %d", file_path, ROOT_UID);
debug_return_int(SUDO_RC_ERROR);
}
if ((sb.st_mode & (S_IWGRP|S_IWOTH)) != 0) {
PyErr_Format(PyExc_ImportError, "File '%s' must be only be writable by owner", file_path);
debug_return_int(SUDO_RC_ERROR);
}
debug_return_int(SUDO_RC_OK);
}
static PyObject *
_sudo_ImportBlocker__Init(PyObject *py_self, PyObject *py_args)
{
debug_decl(_sudo_ImportBlocker__Init, PYTHON_DEBUG_C_CALLS);
py_debug_python_call("ImportBlocker", "__init__", py_args, NULL, PYTHON_DEBUG_C_CALLS);
PyObject *py_meta_path = NULL;
if (!PyArg_UnpackTuple(py_args, "sudo.ImportBlocker.__init__", 2, 2, &py_self, &py_meta_path))
goto cleanup;
if (PyObject_SetAttrString(py_self, "meta_path", py_meta_path) != 0)
goto cleanup;
cleanup:
if (PyErr_Occurred())
debug_return_ptr(NULL);
debug_return_ptr_pynone;
}
static PyObject *
_sudo_ImportBlocker__find_spec(PyObject *py_self, PyObject *py_args)
{
debug_decl(_sudo_ImportBlocker__find_spec, PYTHON_DEBUG_C_CALLS);
PyObject *py_fullname = NULL, *py_path = NULL, *py_target = NULL,
*py_meta_path = NULL, *py_meta_path_iterator = NULL,
*py_finder = NULL, *py_spec = NULL, *py_loader = NULL,
*py_import_path = NULL;
py_debug_python_call("ImportBlocker", "find_spec", py_args, NULL, PYTHON_DEBUG_C_CALLS);
if (!PyArg_UnpackTuple(py_args, "sudo.ImportBlocker.find_spec", 2, 4, &py_self, &py_fullname, &py_path, &py_target))
goto cleanup;
py_meta_path = PyObject_GetAttrString(py_self, "meta_path");
if (py_meta_path == NULL)
goto cleanup;
py_meta_path_iterator = PyObject_GetIter(py_meta_path);
if (py_meta_path_iterator == NULL)
goto cleanup;
while ((py_finder = PyIter_Next(py_meta_path_iterator)) != NULL) {
py_spec = PyObject_CallMethod(py_finder, "find_spec", "(OO)",
py_fullname, py_path, py_target);
if (py_spec == NULL) {
goto cleanup;
}
if (py_spec != Py_None && PyObject_HasAttrString(py_spec, "loader")) {
// the finder could be resolved and contains a loader
py_loader = PyObject_GetAttrString(py_spec, "loader");
if (py_loader != NULL && PyObject_HasAttrString(py_loader, "get_filename")) {
// there is a file associated with the import (.py, .so, etc)
py_import_path = PyObject_CallMethod(py_loader, "get_filename", "");
const char *import_path = PyUnicode_AsUTF8(py_import_path);
sudo_debug_printf(SUDO_DEBUG_DIAG, "ImportBlocker: verifying permissions "
"on file '%s'\n", import_path);
if (_verify_import(import_path) != SUDO_RC_OK)
goto cleanup;
Py_CLEAR(py_import_path);
} else {
sudo_debug_printf(SUDO_DEBUG_DIAG, "ImportBlocker: internal module import '%s'\n",
PyUnicode_AsUTF8(py_fullname));
}
goto cleanup;
}
Py_CLEAR(py_spec);
Py_CLEAR(py_finder);
}
Py_CLEAR(py_spec);
py_spec = Py_None;
Py_INCREF(py_spec);
cleanup:
Py_CLEAR(py_meta_path_iterator);
Py_CLEAR(py_meta_path);
Py_CLEAR(py_finder);
Py_CLEAR(py_import_path);
Py_CLEAR(py_loader);
if (PyErr_Occurred()) {
Py_CLEAR(py_spec);
debug_return_ptr(NULL);
}
debug_return_ptr(py_spec);
}
static PyMethodDef _sudo_ImportBlocker_class_methods[] =
{
{"__init__", _sudo_ImportBlocker__Init, METH_VARARGS, ""},
{"find_spec", _sudo_ImportBlocker__find_spec, METH_VARARGS, ""},
{NULL, NULL, 0, NULL}
};
// This possibly can be replaced with PySys_AddAuditHook for python >= 3.8
//
// This function is equivalent of the python call:
// sys.meta_path = [sudo.ImportBlocker(sys.meta_path)]
int
sudo_module_register_importblocker(void)
{
debug_decl(sudo_module_register_importblocker, PYTHON_DEBUG_C_CALLS);
int rc = SUDO_RC_ERROR;
PyObject *py_meta_path = NULL, *py_import_blocker_cls = NULL,
*py_import_blocker = NULL;
py_meta_path = PySys_GetObject("meta_path"); // note: borrowed reference
if (py_meta_path == NULL) {
PyErr_Format(sudo_exc_SudoException, "'sys.meta_path' is not available. "
"Unable to register import blocker hook which is meant to "
"verify that no such module get loaded by the sudo python plugins"
"which are writable by others than root.");
goto cleanup;
}
Py_INCREF(py_meta_path);
py_import_blocker_cls = sudo_module_create_class("sudo.ImportBlocker", _sudo_ImportBlocker_class_methods, NULL);
if (py_import_blocker_cls == NULL)
goto cleanup;
// call the constructor
py_import_blocker = PyObject_CallFunctionObjArgs(py_import_blocker_cls, py_meta_path, NULL);
if (py_import_blocker == NULL)
goto cleanup;
Py_DECREF(py_meta_path);
py_meta_path = PyList_New(1);
if (py_meta_path == NULL)
goto cleanup;
if (PyList_SetItem(py_meta_path, 0, py_import_blocker) != 0)
goto cleanup;
py_import_blocker = NULL; // list has stolen it
if (PySys_SetObject("meta_path", py_meta_path) != 0) {
goto cleanup;
}
rc = SUDO_RC_OK;
cleanup:
Py_CLEAR(py_meta_path);
Py_CLEAR(py_import_blocker);
Py_CLEAR(py_import_blocker_cls);
debug_return_int(rc);
}

View File

@@ -518,10 +518,6 @@ python_plugin_init(struct PluginContext *plugin_ctx, char * const plugin_options
} }
PyThreadState_Swap(plugin_ctx->py_interpreter); PyThreadState_Swap(plugin_ctx->py_interpreter);
if (!sudo_conf_developer_mode() && sudo_module_register_importblocker() < 0) {
goto cleanup;
}
if (sudo_module_set_default_loghandler() < 0) if (sudo_module_set_default_loghandler() < 0)
goto cleanup; goto cleanup;

View File

@@ -118,10 +118,7 @@ init(void)
VERIFY_TRUE(asprintf(&data.tmp_dir, TEMP_PATH_TEMPLATE) >= 0); VERIFY_TRUE(asprintf(&data.tmp_dir, TEMP_PATH_TEMPLATE) >= 0);
VERIFY_NOT_NULL(mkdtemp(data.tmp_dir)); VERIFY_NOT_NULL(mkdtemp(data.tmp_dir));
// by default we test in developer mode, so the python plugin can be loaded
sudo_conf_clear_paths(); sudo_conf_clear_paths();
VERIFY_INT(sudo_conf_read(sudo_conf_developer_mode, SUDO_CONF_ALL), true);
VERIFY_TRUE(sudo_conf_developer_mode());
// some default values for the plugin open: // some default values for the plugin open:
data.settings = create_str_array(1, NULL); data.settings = create_str_array(1, NULL);
@@ -473,8 +470,7 @@ create_debug_config(const char *debug_spec)
snprintf(config_path, sizeof(config_path), "%s/sudo.conf", data.tmp_dir); snprintf(config_path, sizeof(config_path), "%s/sudo.conf", data.tmp_dir);
char *content = NULL; char *content = NULL;
if (asprintf(&content, "Set developer_mode true\n" if (asprintf(&content, "Debug %s %s/debug.log %s\n",
"Debug %s %s/debug.log %s\n",
"python_plugin.so", data.tmp_dir, debug_spec) < 0) "python_plugin.so", data.tmp_dir, debug_spec) < 0)
{ {
printf("Failed to allocate string\n"); printf("Failed to allocate string\n");
@@ -629,16 +625,6 @@ check_loading_fails_with_wrong_path(void)
return check_loading_fails("wrong_path"); return check_loading_fails("wrong_path");
} }
static int
check_loading_fails_plugin_is_not_owned_by_root(void)
{
sudo_conf_clear_paths();
VERIFY_INT(sudo_conf_read(sudo_conf_normal_mode, SUDO_CONF_ALL), true);
create_debugging_plugin_options();
return check_loading_fails("not_owned_by_root");
}
static int static int
check_example_conversation_plugin_reason_log(int simulate_suspend, const char *description) check_example_conversation_plugin_reason_log(int simulate_suspend, const char *description)
{ {
@@ -1553,7 +1539,6 @@ main(int argc, char *argv[])
RUN_TEST(check_loading_fails_with_missing_classname()); RUN_TEST(check_loading_fails_with_missing_classname());
RUN_TEST(check_loading_fails_with_wrong_classname()); RUN_TEST(check_loading_fails_with_wrong_classname());
RUN_TEST(check_loading_fails_with_wrong_path()); RUN_TEST(check_loading_fails_with_wrong_path());
RUN_TEST(check_loading_fails_plugin_is_not_owned_by_root());
RUN_TEST(check_plugin_unload()); RUN_TEST(check_plugin_unload());
RUN_TEST(check_example_conversation_plugin_reason_log(false, "without_suspend")); RUN_TEST(check_example_conversation_plugin_reason_log(false, "without_suspend"));

View File

@@ -1 +0,0 @@
Set developer_mode true

View File

@@ -1 +0,0 @@
Set developer_mode false

View File

@@ -23,9 +23,6 @@
#include "testhelpers.h" #include "testhelpers.h"
const char *sudo_conf_developer_mode = TESTDATA_DIR "sudo.conf.developer_mode";
const char *sudo_conf_normal_mode = TESTDATA_DIR "sudo.conf.normal_mode";
struct TestData data; struct TestData data;
/* /*

View File

@@ -31,9 +31,6 @@
#endif #endif
#define TESTDATA_DIR SRC_DIR "/regress/testdata/" #define TESTDATA_DIR SRC_DIR "/regress/testdata/"
extern const char *sudo_conf_developer_mode;
extern const char *sudo_conf_normal_mode;
#define TEMP_PATH_TEMPLATE "/tmp/sudo_check_python_exampleXXXXXX" #define TEMP_PATH_TEMPLATE "/tmp/sudo_check_python_exampleXXXXXX"
extern struct TestData { extern struct TestData {

View File

@@ -36,9 +36,6 @@ extern PyObject *sudo_type_LogHandler;
PyObject *sudo_module_create_class(const char *class_name, PyMethodDef *class_methods, PyObject *sudo_module_create_class(const char *class_name, PyMethodDef *class_methods,
PyObject *base_class); PyObject *base_class);
CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION
int sudo_module_register_importblocker(void);
CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION
int sudo_module_register_conv_message(PyObject *py_module); int sudo_module_register_conv_message(PyObject *py_module);