src: use policy per environment

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_;
 }

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,

src/env.h

@@ -43,6 +43,7 @@
 #include "util.h"
 #include "uv.h"
 #include "v8.h"
+#include "policy/policy.h"
 
 #include <array>
 #include <atomic>
@@ -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<KVStore> env_vars();
   inline void set_env_vars(std::shared_ptr<KVStore> 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<KVStore> env_vars_;
   bool printed_error_ = false;

src/node.cc

@@ -858,14 +858,6 @@ int ProcessGlobalArgs(std::vector<std::string>* 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;
 }
 

src/node_file.cc

@@ -1622,8 +1622,6 @@ static void RealPath(const FunctionCallbackInfo<Value>& args) {
 
   BufferValue path(isolate, args[0]);
   CHECK_NOT_NULL(*path);
-  THROW_IF_INSUFFICIENT_PERMISSIONS(
-      env, policy::Permission::kFileSystemIn, *path);
 
   const enum encoding encoding = ParseEncoding(isolate, args[1], UTF8);
 

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,

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

src/policy/policy.cc

@@ -8,30 +8,25 @@
 
 #include "v8.h"
 
+#include <iostream>
+#include <memory>
 #include <string>
 #include <vector>
-#include <iostream>
 
 namespace node {
 
 using v8::Array;
 using v8::Context;
 using v8::FunctionCallbackInfo;
 using v8::Integer;
-using v8::Just;
 using v8::Local;
-using v8::Maybe;
 using v8::Nothing;
 using v8::Object;
 using v8::String;
 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 +38,30 @@ static void Deny(const FunctionCallbackInfo<Value>& 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<Array> jsParams = Local<Array>::Cast(args[1]);
-  std::vector<std::string> params;
-  for (uint32_t i = 0; i < jsParams->Length(); ++i) {
-    Local<Value> arg(
-        jsParams
-          ->Get(isolate->GetCurrentContext(), Integer::New(isolate, i))
-          .ToLocalChecked());
+  Local<Array> js_params = Local<Array>::Cast(args[1]);
+  Local<Context> context = isolate->GetCurrentContext();
 
+  std::vector<std::string> params;
+  for (uint32_t i = 0; i < js_params->Length(); ++i) {
+    Local<Value> 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 +72,23 @@ static void Check(const FunctionCallbackInfo<Value>& 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 +103,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;
 }
@@ -115,15 +122,14 @@ void Policy::ThrowAccessDenied(Environment* env, Permission perm) {
   env->isolate()->ThrowException(err);
 }
 
-Maybe<bool> Policy::Apply(const std::string& deny, Permission scope) {
+void Policy::Apply(const std::string& deny, Permission scope) {
   auto policy = deny_policies.find(scope);
   if (policy != deny_policies.end()) {
-    return policy->second->Apply(deny);
+    policy->second->Apply(deny);
   }
-  return Just(false);
 }
 
-bool Policy::Deny(Permission scope, std::vector<std::string> params) {
+bool Policy::Deny(Permission scope, const std::vector<std::string>& params) {
   auto policy = deny_policies.find(scope);
   if (policy != deny_policies.end()) {
     return policy->second->Deny(scope, params);

src/policy/policy.h

@@ -18,18 +18,20 @@ class Environment;
 namespace policy {
 
 #define THROW_IF_INSUFFICIENT_PERMISSIONS(env, perm_, resource_, ...)          \
-  if (!node::policy::root_policy.is_granted(perm_, resource_)) {               \
-    return node::policy::Policy::ThrowAccessDenied((env), perm_);              \
-  }
+  do {                                                                         \
+    if (UNLIKELY(!(env)->policy()->is_granted(perm_, resource_))) {            \
+      node::policy::Policy::ThrowAccessDenied((env), perm_);                   \
+      return __VA_ARGS__;                                                      \
+    }                                                                          \
+  } while (0)
 
 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<PolicyDeny> deny_fs = std::make_shared<PolicyDenyFs>();
+#define V(Name, _, __)                                                         \
+  deny_policies.insert(std::make_pair(Permission::k##Name, deny_fs));
+    FILESYSTEM_PERMISSIONS(V)
 #undef V
   }
 
@@ -45,20 +47,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<bool> Apply(const std::string& deny, Permission scope);
+    void Apply(const std::string& deny, Permission scope);
     // Policy.Deny API
-    bool Deny(Permission scope, std::vector<std::string> params);
+    bool Deny(Permission scope, const std::vector<std::string>& params);
 
  private:
-    std::map<Permission, PolicyDeny*> deny_policies;
+    std::map<Permission, std::shared_ptr<PolicyDeny>> deny_policies;
 };
 
-extern policy::Policy root_policy;
 }  // namespace policy
 
 }  // namespace node

src/policy/policy_deny.h

@@ -29,8 +29,9 @@ enum class Permission {
 
 class PolicyDeny {
  public:
-  virtual v8::Maybe<bool> Apply(const std::string& deny) = 0;
-  virtual bool Deny(Permission scope, std::vector<std::string> params) = 0;
+  virtual void Apply(const std::string& deny) = 0;
+  virtual bool Deny(Permission scope,
+                    const std::vector<std::string>& params) = 0;
   virtual bool is_granted(Permission perm, const std::string& param = "") = 0;
 };
 

src/policy/policy_deny_fs.cc

@@ -10,25 +10,22 @@
 #include <string>
 #include <vector>
 
-using v8::Just;
-using v8::Maybe;
-
 namespace node {
 
 namespace policy {
 
 // deny = 'fs'
 // deny = 'in:/tmp/'
 // deny = 'in:/tmp/,out:./example.js'
-Maybe<bool> PolicyDenyFs::Apply(const std::string& deny) {
+void PolicyDenyFs::Apply(const std::string& deny) {
   for (const auto& name : SplitString(deny, ',')) {
     Permission perm = Permission::kPermissionsRoot;
     for (std::string& opt : SplitString(name, ':')) {
       if (perm == Permission::kPermissionsRoot) {
         if (opt == "fs") {
           deny_all_in_ = true;
           deny_all_out_ = true;
-          return Just(true);
+          return;
         }
         if (opt == "in") {
           perm = Permission::kFileSystemIn;
@@ -37,18 +34,17 @@ Maybe<bool> PolicyDenyFs::Apply(const std::string& deny) {
           perm = Permission::kFileSystemOut;
           deny_all_out_ = true;
         } else {
-          return Just(false);
+          return;
         }
       } else {
         RestrictAccess(perm, opt);
       }
     }
   }
-
-  return Just(true);
 }
 
-bool PolicyDenyFs::Deny(Permission perm, std::vector<std::string> params) {
+bool PolicyDenyFs::Deny(Permission perm,
+                        const std::vector<std::string>& params) {
   if (perm == Permission::kFileSystem) {
     deny_all_in_ = true;
     deny_all_out_ = true;
@@ -78,25 +74,35 @@ bool PolicyDenyFs::Deny(Permission perm, std::vector<std::string> params) {
 }
 
 void PolicyDenyFs::RestrictAccess(Permission perm, const std::string& res) {
-  char resolvedPath[PATH_MAX];
-  // check the result
-  realpath(res.c_str(), resolvedPath);
+  uv_fs_t req;
+  req.ptr = nullptr;
+  // This function has certain platform-specific caveats that were discovered
+  // when used in Node.
+  // https://docs.libuv.org/en/latest/fs.html?highlight=uv_fs_realpath#c.uv_fs_realpath
+  uv_fs_realpath(nullptr, &req, res.c_str(), nullptr);
+
+  if (req.ptr == nullptr) {
+    // TODO(rafaelgss): req.ptr is null when the path doesn't exist.
+    // This behavior is different from realpath(1)
+    return;
+  }
 
-  std::filesystem::path path(resolvedPath);
+  std::string resolved_path = std::string(static_cast<char*>(req.ptr));
+  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<std::string> params) {
+                                  const std::vector<std::string>& params) {
   for (auto& param : params) {
     RestrictAccess(perm, param);
   }
@@ -118,15 +124,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;
     }
   }

src/policy/policy_deny_fs.h

@@ -8,29 +8,28 @@
 #include "policy/policy_deny.h"
 #include <vector>
 
-using v8::Maybe;
-
 namespace node {
 
 namespace policy {
 
 using DenyFsParams = std::vector<std::pair<std::string, bool /* is_folder */>>;
 
 // TODO(rafaelgss): implement radix-tree algorithm
-class PolicyDenyFs : public PolicyDeny {
+class PolicyDenyFs final : public PolicyDeny {
  public:
-    Maybe<bool> Apply(const std::string& deny);
-    bool Deny(Permission scope, std::vector<std::string> params);
-    bool is_granted(Permission perm, const std::string& param);
+  void Apply(const std::string& deny) override;
+  bool Deny(Permission scope, const std::vector<std::string>& 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<std::string> 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<std::string>& params);
+
+  DenyFsParams deny_in_params_;
+  DenyFsParams deny_out_params_;
+  bool deny_all_in_;
+  bool deny_all_out_;
 };
 
 }  // namespace policy