From 6a6f1341504fd61a6799c035445335faedcf95b0 Mon Sep 17 00:00:00 2001 From: Roy Keene Date: Fri, 21 Sep 2018 10:44:00 -0500 Subject: [PATCH 1/4] Abstracted setting file permissions into platform-specific code --- rai/core_test/block_store.cpp | 2 +- rai/lib/plat/posix/perms.cpp | 22 ++++++++++++++++++++++ rai/lib/plat/windows/perms.cpp | 25 +++++++++++++++++++++++++ rai/lib/utility.hpp | 12 ++++++++++++ rai/node/lmdb.cpp | 2 +- rai/node/node.cpp | 4 ++-- rai/node/wallet.cpp | 2 +- rai/rai_node/daemon.cpp | 4 ++-- rai/rai_wallet/entry.cpp | 4 ++-- 9 files changed, 68 insertions(+), 9 deletions(-) diff --git a/rai/core_test/block_store.cpp b/rai/core_test/block_store.cpp index a369cef4a8..05e76a06b1 100644 --- a/rai/core_test/block_store.cpp +++ b/rai/core_test/block_store.cpp @@ -534,7 +534,7 @@ TEST (block_store, DISABLED_already_open) // File can be shared { auto path (rai::unique_path ()); boost::filesystem::create_directories (path.parent_path ()); - boost::filesystem::permissions (path.parent_path (), boost::filesystem::owner_all); + rai::set_secure_perm_directory (path.parent_path ()); std::ofstream file; file.open (path.string ().c_str ()); ASSERT_TRUE (file.is_open ()); diff --git a/rai/lib/plat/posix/perms.cpp b/rai/lib/plat/posix/perms.cpp index 5e993cc156..e34b8aa157 100644 --- a/rai/lib/plat/posix/perms.cpp +++ b/rai/lib/plat/posix/perms.cpp @@ -1,3 +1,5 @@ +#include + #include #include @@ -7,3 +9,23 @@ void rai::set_umask () { umask (077); } + +void rai::set_secure_perm_directory (boost::filesystem::path const & path) +{ + boost::filesystem::permissions (path, boost::filesystem::owner_all); +} + +void rai::set_secure_perm_directory (boost::filesystem::path const & path, boost::system::error_code & ec) +{ + boost::filesystem::permissions (path, boost::filesystem::owner_all, ec); +} + +void rai::set_secure_perm_file (boost::filesystem::path const & path) +{ + boost::filesystem::permissions (path, boost::filesystem::perms::owner_read | boost::filesystem::perms::owner_write); +} + +void rai::set_secure_perm_file (boost::filesystem::path const & path, boost::system::error_code & ec) +{ + boost::filesystem::permissions (path, boost::filesystem::perms::owner_read | boost::filesystem::perms::owner_write, ec); +} diff --git a/rai/lib/plat/windows/perms.cpp b/rai/lib/plat/windows/perms.cpp index a09a260205..dae1a41a43 100644 --- a/rai/lib/plat/windows/perms.cpp +++ b/rai/lib/plat/windows/perms.cpp @@ -1,4 +1,5 @@ #include +#include #include #include @@ -12,3 +13,27 @@ void rai::set_umask () auto result (_umask_s (_S_IWRITE | _S_IREAD, &oldMode)); assert (result == 0); } + +void rai::set_secure_perm_directory (boost::filesystem::path const & path) +{ + boost::filesystem::permissions (path, boost::filesystem::owner_all); +} + +void rai::set_secure_perm_directory (boost::filesystem::path const & path, boost::system::error_code & ec) +{ + boost::filesystem::permissions (path, boost::filesystem::owner_all, ec); +} + +void rai::set_secure_perm_file (boost::filesystem::path const & path) +{ + /* + * XXX:TODO: Set the permissions sanely; For now we rely on directory permissions + */ +} + +void rai::set_secure_perm_file (boost::filesystem::path const & path, boost::system::error_code & ec) +{ + /* + * XXX:TODO: Set the permissions sanely; For now we rely on directory permissions + */ +} diff --git a/rai/lib/utility.hpp b/rai/lib/utility.hpp index 2972a5ddb5..1c7bd3d7ea 100644 --- a/rai/lib/utility.hpp +++ b/rai/lib/utility.hpp @@ -1,5 +1,8 @@ #pragma once +#include +#include + #include #include #include @@ -9,7 +12,16 @@ namespace rai { // Lower priority of calling work generating thread void work_thread_reprioritize (); + +/* + * Functions for managing filesystem permissions, platform specific + */ void set_umask (); +void set_secure_perm_directory (boost::filesystem::path const & path); +void set_secure_perm_directory (boost::filesystem::path const & path, boost::system::error_code & ec); +void set_secure_perm_file (boost::filesystem::path const & path); +void set_secure_perm_file (boost::filesystem::path const & path, boost::system::error_code & ec); + template class observer_set { diff --git a/rai/node/lmdb.cpp b/rai/node/lmdb.cpp index cb44089380..70ef669bd9 100644 --- a/rai/node/lmdb.cpp +++ b/rai/node/lmdb.cpp @@ -14,7 +14,7 @@ rai::mdb_env::mdb_env (bool & error_a, boost::filesystem::path const & path_a, i if (path_a.has_parent_path ()) { boost::filesystem::create_directories (path_a.parent_path (), error_mkdir); - boost::filesystem::permissions (path_a.parent_path (), boost::filesystem::owner_all, error_chmod); + rai::set_secure_perm_directory (path_a.parent_path (), error_chmod); if (!error_mkdir) { auto status1 (mdb_env_create (&environment)); diff --git a/rai/node/node.cpp b/rai/node/node.cpp index 7dbe77603a..c52fcb5899 100644 --- a/rai/node/node.cpp +++ b/rai/node/node.cpp @@ -2498,7 +2498,7 @@ void rai::node::backup_wallet () auto backup_path (application_path / "backup"); boost::filesystem::create_directories (backup_path); - boost::filesystem::permissions (backup_path, boost::filesystem::owner_all, error_chmod); + rai::set_secure_perm_directory (backup_path, error_chmod); i->second->store.write_backup (transaction, backup_path / (i->first.to_string () + ".json")); } auto this_l (shared ()); @@ -4177,7 +4177,7 @@ work (1, nullptr) * @warning May throw a filesystem exception */ boost::filesystem::create_directories (path); - boost::filesystem::permissions (path, boost::filesystem::owner_all, error_chmod); + rai::set_secure_perm_directory (path, error_chmod); logging.max_size = std::numeric_limits::max (); logging.init (path); node = std::make_shared (init, *service, 24000, path, alarm, logging, work); diff --git a/rai/node/wallet.cpp b/rai/node/wallet.cpp index bb6bdda7b3..71373d826f 100644 --- a/rai/node/wallet.cpp +++ b/rai/node/wallet.cpp @@ -533,7 +533,7 @@ void rai::wallet_store::write_backup (rai::transaction const & transaction_a, bo { // Set permissions to 600 boost::system::error_code ec; - boost::filesystem::permissions (path_a, boost::filesystem::perms::owner_read | boost::filesystem::perms::owner_write, ec); + rai::set_secure_perm_file (path_a, ec); std::string json; serialize_json (transaction_a, json); diff --git a/rai/rai_node/daemon.cpp b/rai/rai_node/daemon.cpp index e02c1bb3dd..296624d7c0 100644 --- a/rai/rai_node/daemon.cpp +++ b/rai/rai_node/daemon.cpp @@ -97,13 +97,13 @@ void rai_daemon::daemon::run (boost::filesystem::path const & data_path) { boost::system::error_code error_chmod; boost::filesystem::create_directories (data_path); - boost::filesystem::permissions (data_path, boost::filesystem::owner_all, error_chmod); + rai::set_secure_perm_directory (data_path, error_chmod); rai_daemon::daemon_config config (data_path); auto config_path ((data_path / "config.json")); std::fstream config_file; std::unique_ptr runner; auto error (rai::fetch_object (config, config_path, config_file)); - boost::filesystem::permissions (config_path, boost::filesystem::owner_read | boost::filesystem::owner_write, error_chmod); + rai::set_secure_perm_file (config_path, error_chmod); if (!error) { config.node.logging.init (data_path); diff --git a/rai/rai_wallet/entry.cpp b/rai/rai_wallet/entry.cpp index 695786dcb3..af42269f11 100644 --- a/rai/rai_wallet/entry.cpp +++ b/rai/rai_wallet/entry.cpp @@ -192,7 +192,7 @@ int run_wallet (QApplication & application, int argc, char * const * argv, boost rai_qt::eventloop_processor processor; boost::system::error_code error_chmod; boost::filesystem::create_directories (data_path); - boost::filesystem::permissions (data_path, boost::filesystem::owner_all, error_chmod); + rai::set_secure_perm_directory (data_path, error_chmod); QPixmap pixmap (":/logo.png"); QSplashScreen * splash = new QSplashScreen (pixmap); splash->show (); @@ -205,7 +205,7 @@ int run_wallet (QApplication & application, int argc, char * const * argv, boost std::fstream config_file; auto error (rai::fetch_object (config, config_path, config_file)); config_file.close (); - boost::filesystem::permissions (config_path, boost::filesystem::owner_read | boost::filesystem::owner_write, error_chmod); + rai::set_secure_perm_file (config_path, error_chmod); if (!error) { boost::asio::io_service service; From 701fbe20283ac66bcb4516a0407e009c811e595f Mon Sep 17 00:00:00 2001 From: Roy Keene Date: Fri, 21 Sep 2018 13:06:39 -0500 Subject: [PATCH 2/4] Rely soley on umask() for Windows permission management --- rai/lib/plat/windows/perms.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/rai/lib/plat/windows/perms.cpp b/rai/lib/plat/windows/perms.cpp index dae1a41a43..e5cdacdf5a 100644 --- a/rai/lib/plat/windows/perms.cpp +++ b/rai/lib/plat/windows/perms.cpp @@ -16,24 +16,28 @@ void rai::set_umask () void rai::set_secure_perm_directory (boost::filesystem::path const & path) { - boost::filesystem::permissions (path, boost::filesystem::owner_all); + /* + * XXX:TODO: Set the permissions sanely; For now we rely on umask + */ } void rai::set_secure_perm_directory (boost::filesystem::path const & path, boost::system::error_code & ec) { - boost::filesystem::permissions (path, boost::filesystem::owner_all, ec); + /* + * XXX:TODO: Set the permissions sanely; For now we rely on umask + */ } void rai::set_secure_perm_file (boost::filesystem::path const & path) { /* - * XXX:TODO: Set the permissions sanely; For now we rely on directory permissions + * XXX:TODO: Set the permissions sanely; For now we rely on umask */ } void rai::set_secure_perm_file (boost::filesystem::path const & path, boost::system::error_code & ec) { /* - * XXX:TODO: Set the permissions sanely; For now we rely on directory permissions + * XXX:TODO: Set the permissions sanely; For now we rely on umask */ } From 88b9711e115b17469d360317b1f3cd8354464588 Mon Sep 17 00:00:00 2001 From: Roy Keene Date: Fri, 21 Sep 2018 13:07:09 -0500 Subject: [PATCH 3/4] Ensure the appropriate header is included whenever we use the set_secure_perm_...() functions --- rai/core_test/block_store.cpp | 1 + rai/node/node.cpp | 1 + rai/node/wallet.cpp | 1 + rai/rai_node/daemon.cpp | 1 + rai/rai_wallet/entry.cpp | 1 + 5 files changed, 5 insertions(+) diff --git a/rai/core_test/block_store.cpp b/rai/core_test/block_store.cpp index 05e76a06b1..93b3a71b41 100644 --- a/rai/core_test/block_store.cpp +++ b/rai/core_test/block_store.cpp @@ -1,4 +1,5 @@ #include +#include #include #include #include diff --git a/rai/node/node.cpp b/rai/node/node.cpp index c52fcb5899..d88de02b85 100644 --- a/rai/node/node.cpp +++ b/rai/node/node.cpp @@ -1,6 +1,7 @@ #include #include +#include #include #include diff --git a/rai/node/wallet.cpp b/rai/node/wallet.cpp index 71373d826f..4fb2c7938e 100644 --- a/rai/node/wallet.cpp +++ b/rai/node/wallet.cpp @@ -1,3 +1,4 @@ +#include #include #include diff --git a/rai/rai_node/daemon.cpp b/rai/rai_node/daemon.cpp index 296624d7c0..4f485f11de 100644 --- a/rai/rai_node/daemon.cpp +++ b/rai/rai_node/daemon.cpp @@ -1,3 +1,4 @@ +#include #include #include diff --git a/rai/rai_wallet/entry.cpp b/rai/rai_wallet/entry.cpp index af42269f11..da6f97ca3d 100644 --- a/rai/rai_wallet/entry.cpp +++ b/rai/rai_wallet/entry.cpp @@ -1,3 +1,4 @@ +#include #include #include #include From 54087b5e35048adbc784ef7f0b356dcc9d0c8acf Mon Sep 17 00:00:00 2001 From: Russel Waters Date: Fri, 21 Sep 2018 20:45:12 -0400 Subject: [PATCH 4/4] Update perms.cpp updated to work with windows --- rai/lib/plat/windows/perms.cpp | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/rai/lib/plat/windows/perms.cpp b/rai/lib/plat/windows/perms.cpp index e5cdacdf5a..72c2dfb678 100644 --- a/rai/lib/plat/windows/perms.cpp +++ b/rai/lib/plat/windows/perms.cpp @@ -16,28 +16,20 @@ void rai::set_umask () void rai::set_secure_perm_directory (boost::filesystem::path const & path) { - /* - * XXX:TODO: Set the permissions sanely; For now we rely on umask - */ + boost::filesystem::permissions (path, boost::filesystem::owner_all); } void rai::set_secure_perm_directory (boost::filesystem::path const & path, boost::system::error_code & ec) { - /* - * XXX:TODO: Set the permissions sanely; For now we rely on umask - */ + boost::filesystem::permissions (path, boost::filesystem::owner_all, ec); } void rai::set_secure_perm_file (boost::filesystem::path const & path) { - /* - * XXX:TODO: Set the permissions sanely; For now we rely on umask - */ + boost::filesystem::permissions (path, boost::filesystem::owner_read | boost::filesystem::owner_write); } void rai::set_secure_perm_file (boost::filesystem::path const & path, boost::system::error_code & ec) { - /* - * XXX:TODO: Set the permissions sanely; For now we rely on umask - */ + boost::filesystem::permissions (path, boost::filesystem::owner_read | boost::filesystem::owner_write, ec); }