From c8494fc4dcb566749cb922b5884eeae847bb711b Mon Sep 17 00:00:00 2001 From: RafaelGSS Date: Wed, 27 Jul 2022 21:49:26 -0300 Subject: [PATCH] src: use policy per environment --- src/env-inl.h | 4 +++ src/env.cc | 4 +++ src/env.h | 3 ++ src/node.cc | 8 ------ src/node_options.cc | 9 +++--- src/node_options.h | 3 +- src/policy/policy.cc | 53 +++++++++++++++++++++--------------- src/policy/policy.h | 18 ++++++------ src/policy/policy_deny.h | 3 +- src/policy/policy_deny_fs.cc | 23 ++++++++-------- src/policy/policy_deny_fs.h | 27 +++++++++--------- 11 files changed, 83 insertions(+), 72 deletions(-) diff --git a/src/env-inl.h b/src/env-inl.h index e2da40141d7740..cc13ec8cb7b462 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -335,6 +335,10 @@ inline TickInfo* Environment::tick_info() { return &tick_info_; } +inline policy::Policy* Environment::policy() { + return &policy_; +} + inline uint64_t Environment::timer_base() const { return timer_base_; } diff --git a/src/env.cc b/src/env.cc index e775474306b746..4c7f96e870abec 100644 --- a/src/env.cc +++ b/src/env.cc @@ -884,6 +884,10 @@ Environment::Environment(IsolateData* isolate_data, "args", std::move(traced_value)); } + + policy()->Apply( + options_->policy_deny_fs, + policy::Permission::kFileSystem); } Environment::Environment(IsolateData* isolate_data, diff --git a/src/env.h b/src/env.h index 5f624baa42fc50..5834f26bbd2efd 100644 --- a/src/env.h +++ b/src/env.h @@ -43,6 +43,7 @@ #include "util.h" #include "uv.h" #include "v8.h" +#include "policy/policy.h" #include #include @@ -1160,6 +1161,7 @@ class Environment : public MemoryRetainer { inline ImmediateInfo* immediate_info(); inline TickInfo* tick_info(); inline uint64_t timer_base() const; + inline policy::Policy* policy(); inline std::shared_ptr env_vars(); inline void set_env_vars(std::shared_ptr env_vars); @@ -1526,6 +1528,7 @@ class Environment : public MemoryRetainer { AsyncHooks async_hooks_; ImmediateInfo immediate_info_; TickInfo tick_info_; + policy::Policy policy_; const uint64_t timer_base_; std::shared_ptr env_vars_; bool printed_error_ = false; diff --git a/src/node.cc b/src/node.cc index 4212bd8b7025a5..f3d28208cc8c81 100644 --- a/src/node.cc +++ b/src/node.cc @@ -858,14 +858,6 @@ int ProcessGlobalArgs(std::vector* args, if (v8_args_as_char_ptr.size() > 1) return 9; - if (policy::root_policy.Apply( - per_process::cli_options->policy_deny_fs, - policy::Permission::kFileSystem).IsNothing()) { - errors->emplace_back( - "invalid permissions passed to --policy-deny-fs"); - return 12; - } - return 0; } diff --git a/src/node_options.cc b/src/node_options.cc index fcf57e277715f5..810ad754253842 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -380,6 +380,11 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() { &EnvironmentOptions::experimental_policy_integrity, kAllowedInEnvironment); Implies("--policy-integrity", "[has_policy_integrity_string]"); + + AddOption("--policy-deny-fs", + "denied permissions to the filesystem", + &EnvironmentOptions::policy_deny_fs, + kAllowedInEnvironment); AddOption("--experimental-repl-await", "experimental await keyword support in REPL", &EnvironmentOptions::experimental_repl_await, @@ -859,10 +864,6 @@ PerProcessOptionsParser::PerProcessOptionsParser( "force FIPS crypto (cannot be disabled)", &PerProcessOptions::force_fips_crypto, kAllowedInEnvironment); - AddOption("--policy-deny-fs", - "denied permissions to the filesystem", - &PerProcessOptions::policy_deny_fs, - kAllowedInEnvironment); AddOption("--secure-heap", "total size of the OpenSSL secure heap", &PerProcessOptions::secure_heap, diff --git a/src/node_options.h b/src/node_options.h index e268cab57107d0..c42596c6a103b0 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -118,6 +118,7 @@ class EnvironmentOptions : public Options { std::string experimental_policy; std::string experimental_policy_integrity; bool has_policy_integrity_string = false; + std::string policy_deny_fs; bool experimental_repl_await = true; bool experimental_vm_modules = false; bool expose_internals = false; @@ -244,8 +245,6 @@ class PerProcessOptions : public Options { bool print_v8_help = false; bool print_version = false; - std::string policy_deny_fs; - #ifdef NODE_HAVE_I18N_SUPPORT std::string icu_data_dir; #endif diff --git a/src/policy/policy.cc b/src/policy/policy.cc index 5ebcf638e73d3b..55b63e33c90122 100644 --- a/src/policy/policy.cc +++ b/src/policy/policy.cc @@ -8,9 +8,10 @@ #include "v8.h" +#include +#include #include #include -#include namespace node { @@ -28,10 +29,6 @@ using v8::Value; namespace policy { -// The root policy is establish at process start using -// the --policy-deny-* command line arguments. -Policy root_policy; - namespace { // policy.deny('fs.in', ['/tmp/']) @@ -43,26 +40,30 @@ static void Deny(const FunctionCallbackInfo& args) { CHECK(args[0]->IsString()); CHECK(args.Length() >= 2 || args[1]->IsArray()); - std::string denyScope = *String::Utf8Value(isolate, args[0]); - Permission scope = Policy::StringToPermission(denyScope); + std::string deny_scope = *String::Utf8Value(isolate, args[0]); + Permission scope = Policy::StringToPermission(deny_scope); if (scope == Permission::kPermissionsRoot) { return args.GetReturnValue().Set(false); } - Local jsParams = Local::Cast(args[1]); - std::vector params; - for (uint32_t i = 0; i < jsParams->Length(); ++i) { - Local arg( - jsParams - ->Get(isolate->GetCurrentContext(), Integer::New(isolate, i)) - .ToLocalChecked()); + Local js_params = Local::Cast(args[1]); + Local context = isolate->GetCurrentContext(); + std::vector params; + for (uint32_t i = 0; i < js_params->Length(); ++i) { + Local arg; + if (!js_params->Get(context, Integer::New(isolate, i)).ToLocal(&arg)) { + return; + } String::Utf8Value utf8_arg(isolate, arg); + if (*utf8_arg == nullptr) { + return; + } params.push_back(*utf8_arg); } return args.GetReturnValue() - .Set(root_policy.Deny(scope, params)); + .Set(env->policy()->Deny(scope, params)); } // policy.check('fs.in', '/tmp/') @@ -73,15 +74,23 @@ static void Check(const FunctionCallbackInfo& args) { CHECK(args[0]->IsString()); CHECK(args.Length() >= 2 || args[1]->IsString()); - std::string denyScope = *String::Utf8Value(isolate, args[0]); - Permission scope = Policy::StringToPermission(denyScope); + String::Utf8Value utf8_deny_scope(isolate, args[0]); + if (*utf8_deny_scope == nullptr) { + return; + } + + const std::string deny_scope = *utf8_deny_scope; + Permission scope = Policy::StringToPermission(deny_scope); if (scope == Permission::kPermissionsRoot) { - // TODO(rafaelgss): throw? return args.GetReturnValue().Set(false); } - return args.GetReturnValue() - .Set(root_policy.is_granted(scope, *String::Utf8Value(isolate, args[1]))); + String::Utf8Value utf8_arg(isolate, args[1]); + if (*utf8_arg == nullptr) { + return; + } + + return args.GetReturnValue().Set(env->policy()->is_granted(scope, *utf8_arg)); } } // namespace @@ -96,7 +105,7 @@ const char* Policy::PermissionToString(const Permission perm) { #define V(Name, label, _) \ if (perm == label) return Permission::k##Name; -Permission Policy::StringToPermission(std::string perm) { +Permission Policy::StringToPermission(const std::string& perm) { PERMISSIONS(V) return Permission::kPermissionsRoot; } @@ -123,7 +132,7 @@ Maybe Policy::Apply(const std::string& deny, Permission scope) { return Just(false); } -bool Policy::Deny(Permission scope, std::vector params) { +bool Policy::Deny(Permission scope, const std::vector& params) { auto policy = deny_policies.find(scope); if (policy != deny_policies.end()) { return policy->second->Deny(scope, params); diff --git a/src/policy/policy.h b/src/policy/policy.h index c02bc3eaa76aa8..31650f1a8514ec 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -18,18 +18,17 @@ class Environment; namespace policy { #define THROW_IF_INSUFFICIENT_PERMISSIONS(env, perm_, resource_, ...) \ - if (!node::policy::root_policy.is_granted(perm_, resource_)) { \ + if (!env->policy()->is_granted(perm_, resource_)) { \ return node::policy::Policy::ThrowAccessDenied((env), perm_); \ } class Policy { public: - // TODO(rafaelgss): release pointers Policy() { - auto denyFs = new PolicyDenyFs(); -#define V(Name, _, __) \ - deny_policies.insert(std::make_pair(Permission::k##Name, denyFs)); - FILESYSTEM_PERMISSIONS(V) + std::shared_ptr deny_fs = std::make_shared(); +#define V(Name, _, __) \ + deny_policies.insert(std::make_pair(Permission::k##Name, deny_fs)); + FILESYSTEM_PERMISSIONS(V) #undef V } @@ -45,20 +44,19 @@ class Policy { return is_granted(permission, res.c_str()); } - static Permission StringToPermission(std::string perm); + static Permission StringToPermission(const std::string& perm); static const char* PermissionToString(Permission perm); static void ThrowAccessDenied(Environment* env, Permission perm); // CLI Call v8::Maybe Apply(const std::string& deny, Permission scope); // Policy.Deny API - bool Deny(Permission scope, std::vector params); + bool Deny(Permission scope, const std::vector& params); private: - std::map deny_policies; + std::map> deny_policies; }; -extern policy::Policy root_policy; } // namespace policy } // namespace node diff --git a/src/policy/policy_deny.h b/src/policy/policy_deny.h index 0e1907c9fc1749..2a19e24ff38dd5 100644 --- a/src/policy/policy_deny.h +++ b/src/policy/policy_deny.h @@ -30,7 +30,8 @@ enum class Permission { class PolicyDeny { public: virtual v8::Maybe Apply(const std::string& deny) = 0; - virtual bool Deny(Permission scope, std::vector params) = 0; + virtual bool Deny(Permission scope, + const std::vector& params) = 0; virtual bool is_granted(Permission perm, const std::string& param = "") = 0; }; diff --git a/src/policy/policy_deny_fs.cc b/src/policy/policy_deny_fs.cc index b320e60176a92c..6e1600fb7a7bfb 100644 --- a/src/policy/policy_deny_fs.cc +++ b/src/policy/policy_deny_fs.cc @@ -48,7 +48,8 @@ Maybe PolicyDenyFs::Apply(const std::string& deny) { return Just(true); } -bool PolicyDenyFs::Deny(Permission perm, std::vector params) { +bool PolicyDenyFs::Deny(Permission perm, + const std::vector& params) { if (perm == Permission::kFileSystem) { deny_all_in_ = true; deny_all_out_ = true; @@ -78,25 +79,25 @@ bool PolicyDenyFs::Deny(Permission perm, std::vector params) { } void PolicyDenyFs::RestrictAccess(Permission perm, const std::string& res) { - char resolvedPath[PATH_MAX]; + char resolved_path[PATH_MAX]; // check the result - realpath(res.c_str(), resolvedPath); + realpath(res.c_str(), resolved_path); - std::filesystem::path path(resolvedPath); + std::filesystem::path path(resolved_path); bool isDir = std::filesystem::is_directory(path); // when there are parameters deny_params_ is automatically // set to false if (perm == Permission::kFileSystemIn) { deny_all_in_ = false; - deny_in_params_.push_back(std::make_pair(resolvedPath, isDir)); + deny_in_params_.push_back(std::make_pair(resolved_path, isDir)); } else if (perm == Permission::kFileSystemOut) { deny_all_out_ = false; - deny_out_params_.push_back(std::make_pair(resolvedPath, isDir)); + deny_out_params_.push_back(std::make_pair(resolved_path, isDir)); } } void PolicyDenyFs::RestrictAccess(Permission perm, - std::vector params) { + const std::vector& params) { for (auto& param : params) { RestrictAccess(perm, param); } @@ -118,15 +119,15 @@ bool PolicyDenyFs::is_granted(Permission perm, const std::string& param = "") { } bool PolicyDenyFs::is_granted(DenyFsParams params, const std::string& opt) { - char resolvedPath[PATH_MAX]; - realpath(opt.c_str(), resolvedPath); + char resolved_path[PATH_MAX]; + realpath(opt.c_str(), resolved_path); for (auto& param : params) { // is folder if (param.second) { - if (strstr(resolvedPath, param.first.c_str()) == resolvedPath) { + if (strstr(resolved_path, param.first.c_str()) == resolved_path) { return false; } - } else if (param.first == resolvedPath) { + } else if (param.first == resolved_path) { return false; } } diff --git a/src/policy/policy_deny_fs.h b/src/policy/policy_deny_fs.h index 4c3d1d557d0241..06bd9e67de03d4 100644 --- a/src/policy/policy_deny_fs.h +++ b/src/policy/policy_deny_fs.h @@ -8,8 +8,6 @@ #include "policy/policy_deny.h" #include -using v8::Maybe; - namespace node { namespace policy { @@ -17,20 +15,21 @@ namespace policy { using DenyFsParams = std::vector>; // TODO(rafaelgss): implement radix-tree algorithm -class PolicyDenyFs : public PolicyDeny { +class PolicyDenyFs final : public PolicyDeny { public: - Maybe Apply(const std::string& deny); - bool Deny(Permission scope, std::vector params); - bool is_granted(Permission perm, const std::string& param); + v8::Maybe Apply(const std::string& deny) override; + bool Deny(Permission scope, const std::vector& params) override; + bool is_granted(Permission perm, const std::string& param) override; + private: - static bool is_granted(DenyFsParams params, const std::string& opt); - void RestrictAccess(Permission scope, const std::string& param); - void RestrictAccess(Permission scope, std::vector params); - - DenyFsParams deny_in_params_; - DenyFsParams deny_out_params_; - bool deny_all_in_; - bool deny_all_out_; + static bool is_granted(DenyFsParams params, const std::string& opt); + void RestrictAccess(Permission scope, const std::string& param); + void RestrictAccess(Permission scope, const std::vector& params); + + DenyFsParams deny_in_params_; + DenyFsParams deny_out_params_; + bool deny_all_in_; + bool deny_all_out_; }; } // namespace policy