Introduce configurable pbkdf2 iteration limit

src/workerd/api/crypto-impl-pbkdf2.c++

@@ -38,12 +38,12 @@ private:
         "PBKDF2 requires a positive iteration count (requested ", iterations, ").");
 
     // Note: The user could DoS us by selecting a very high iteration count. Our dead man's switch
-    //   would kick in, resulting in a process restart. We guard against this by limiting the maximum
-    //   iteration count a user can select -- this is an intentional non-conformity. Another approach
-    //   might be to fork OpenSSL's PKCS5_PBKDF2_HMAC() function and insert a check for
-    //   v8::Isolate::IsExecutionTerminating() in the loop, but for now a hard cap seems wisest.
-    JSG_REQUIRE(iterations <= 100000, DOMNotSupportedError,
-        "PBKDF2 iteration counts above 100000 are not supported (requested ", iterations, ").");
+    // would kick in, resulting in a process restart. We guard against this by limiting the
+    // maximum iteration count a user can select -- this is an intentional non-conformity.
+    // Another approach might be to fork OpenSSL's PKCS5_PBKDF2_HMAC() function and insert a
+    // check for v8::Isolate::IsExecutionTerminating() in the loop, but for now a hard cap seems
+    // wisest.
+    checkPbkdfLimits(js, iterations);
 
     auto output = kj::heapArray<kj::byte>(length / 8);
     OSSLCALL(PKCS5_PBKDF2_HMAC(keyData.asPtr().asChars().begin(), keyData.size(),

src/workerd/api/crypto-impl.c++

@@ -10,6 +10,7 @@
 #include <openssl/ec.h>
 #include <openssl/rsa.h>
 #include <openssl/crypto.h>
+#include <workerd/jsg/setup.h>
 
 namespace workerd::api {
 namespace {
@@ -210,4 +211,13 @@ ZeroOnFree::~ZeroOnFree() noexcept(false) {
   OPENSSL_cleanse(inner.begin(), inner.size());
 }
 
+void checkPbkdfLimits(jsg::Lock& js, size_t iterations) {
+  auto& limits = Worker::Isolate::from(js).getLimitEnforcer();
+  KJ_IF_SOME(max, limits.checkPbkdfIterations(js, iterations)) {
+    JSG_FAIL_REQUIRE(DOMNotSupportedError,
+        kj::str("Pbkdf2 failed: iteration counts above ", max ," are not supported (requested ",
+                iterations, ")."));
+  }
+}
+
 }  // namespace workerd::api

src/workerd/api/crypto-impl.h

@@ -339,6 +339,11 @@ class ZeroOnFree {
   kj::Array<kj::byte> inner;
 };
 
+// Check that the requested number of iterations for a key-derivation function
+// is acceptable. If the requested iterations is not acceptable, a JS error will
+// be thrown. Otherwise the method will return normally.
+void checkPbkdfLimits(jsg::Lock& js, size_t iterations);
+
 }  // namespace workerd::api
 
 KJ_DECLARE_NON_POLYMORPHIC(DH);

src/workerd/api/node/crypto.h

@@ -110,8 +110,12 @@ class CryptoImpl final: public jsg::Object {
                               kj::Array<kj::byte> info, uint32_t length);
 
   // Pbkdf2
-  kj::Array<kj::byte> getPbkdf(kj::Array<kj::byte> password, kj::Array<kj::byte> salt,
-                               uint32_t num_iterations, uint32_t keylen, kj::String name);
+  kj::Array<kj::byte> getPbkdf(jsg::Lock& js,
+                               kj::Array<kj::byte> password,
+                               kj::Array<kj::byte> salt,
+                               uint32_t num_iterations,
+                               uint32_t keylen,
+                               kj::String name);
 
   // Keys
   struct KeyExportOptions {

src/workerd/api/node/crypto_pbkdf2.c++

@@ -10,16 +10,19 @@
 
 namespace workerd::api::node {
 
-kj::Array<kj::byte> CryptoImpl::getPbkdf(kj::Array<kj::byte> password,
-kj::Array<kj::byte> salt, uint32_t num_iterations, uint32_t keylen, kj::String name) {
+kj::Array<kj::byte> CryptoImpl::getPbkdf(
+    jsg::Lock& js,
+    kj::Array<kj::byte> password,
+    kj::Array<kj::byte> salt,
+    uint32_t num_iterations,
+    uint32_t keylen,
+    kj::String name) {
   // Should not be needed based on current memory limits, still good to have
   JSG_REQUIRE(password.size() <= INT32_MAX, RangeError, "Pbkdf2 failed: password is too large");
   JSG_REQUIRE(salt.size() <= INT32_MAX, RangeError, "Pbkdf2 failed: salt is too large");
   // Note: The user could DoS us by selecting a very high iteration count. As with the Web Crypto
   // API, intentionally limit the maximum iteration count.
-  JSG_REQUIRE(num_iterations <= 100000, DOMNotSupportedError,
-              "Pbkdf2 failed: iteration counts above 100000 are not supported (requested ",
-              num_iterations, ").");
+  checkPbkdfLimits(js, num_iterations);
 
   const EVP_MD* digest = EVP_get_digestbyname(name.begin());
   JSG_REQUIRE(digest != nullptr, TypeError, "Invalid Pbkdf2 digest: ", name,

src/workerd/io/BUILD.bazel

@@ -105,6 +105,12 @@ wd_cc_library(
     ],
 )
 
+wd_cc_library(
+    name = "limit-enforcer",
+    hdrs = ["limit-enforcer.h"],
+    visibility = ["//visibility:public"]
+)
+
 wd_cc_library(
     name = "worker-interface",
     srcs = ["worker-interface.c++"],

src/workerd/io/limit-enforcer.h

@@ -12,6 +12,8 @@ namespace workerd {
 struct ActorCacheSharedLruOptions;
 class IoContext;
 
+static constexpr size_t DEFAULT_MAX_PBKDF2_ITERATIONS = 100'000;
+
 // Interface for an object that enforces resource limits on an Isolate level.
 //
 // See also LimitEnforcer, which enforces on a per-request level.
@@ -57,6 +59,21 @@ class IsolateLimitEnforcer {
 
   // Report resource usage metrics to the given isolate metrics object.
   virtual void reportMetrics(IsolateObserver& isolateMetrics) const = 0;
+
+  // Called when performing a cypto key derivation function (like pbkdf2) to determine if
+  // if the requested number of iterations is acceptable. If kj::none is returned, the
+  // number of iterations requested is acceptable. If a number is returned, the requested
+  // iterations is unacceptable and the return value specifies the maximum.
+  virtual kj::Maybe<size_t> checkPbkdfIterations(jsg::Lock& js, size_t iterations) const {
+    // By default, historically we've limited this to 100,000 iterations max. We'll set
+    // that as the default for now. To set a default of no-limit, this would be changed
+    // to return kj::none. Note, this current default limit is *WAY* below the recommended
+    // minimum iterations for pbkdf2.
+    // TODO(maybe): We might consider emitting a warning if the number of iterations is
+    // too low to be safe.
+    if (iterations > DEFAULT_MAX_PBKDF2_ITERATIONS) return DEFAULT_MAX_PBKDF2_ITERATIONS;
+    return kj::none;
+  }
 };
 
 // Abstract interface that enforces resource limits on a IoContext.

src/workerd/io/worker-entrypoint.c++

@@ -643,7 +643,8 @@ kj::Promise<WorkerInterface::CustomEvent::Result>
 void requestGc(const Worker& worker) {
   TRACE_EVENT("workerd", "Debug: requestGc()");
   jsg::V8StackScope stackScope;
-  auto lock = worker.getIsolate().getApi().lock(stackScope);
+  auto& isolate = worker.getIsolate();
+  auto lock = isolate.getApi().lock(stackScope);
   lock->requestGcForTesting();
 }
 

src/workerd/io/worker.c++

@@ -978,6 +978,10 @@ Worker::Isolate::Isolate(kj::Own<Api> apiParam,
   auto lock = api->lock(stackScope);
   auto features = api->getFeatureFlags();
 
+  KJ_DASSERT(lock->v8Isolate->GetNumberOfDataSlots() >= 3);
+  KJ_DASSERT(lock->v8Isolate->GetData(3) == nullptr);
+  lock->v8Isolate->SetData(3, this);
+
   lock->setCaptureThrowsAsRejections(features.getCaptureThrowsAsRejections());
   lock->setCommonJsExportDefault(features.getExportCommonJsDefaultNamespace());
 
@@ -1251,6 +1255,12 @@ Worker::Script::~Script() noexcept(false) {
   impl = nullptr;
 }
 
+const Worker::Isolate& Worker::Isolate::from(jsg::Lock& js) {
+  auto ptr = js.v8Isolate->GetData(3);
+  KJ_ASSERT(ptr != nullptr);
+  return *static_cast<const Worker::Isolate*>(ptr);
+}
+
 bool Worker::Isolate::Impl::Lock::checkInWithLimitEnforcer(Worker::Isolate& isolate) {
   shouldReportIsolateMetrics = true;
   return limitEnforcer.exitJs(*lock);

src/workerd/io/worker.h

@@ -264,6 +264,9 @@ class Worker::Isolate: public kj::AtomicRefcounted {
   ~Isolate() noexcept(false);
   KJ_DISALLOW_COPY_AND_MOVE(Isolate);
 
+  // Get the current Worker::Isolate from the current jsg::Lock
+  static const Isolate& from(jsg::Lock& js);
+
   inline const IsolateObserver& getMetrics() const { return *metrics; }
 
   inline kj::StringPtr getId() const { return id; }

src/workerd/server/server.c++

@@ -2524,12 +2524,18 @@ kj::Own<Server::Service> Server::makeWorker(kj::StringPtr name, config::Worker::
     void completedRequest(kj::StringPtr id) const override {}
     bool exitJs(jsg::Lock& lock) const override { return false; }
     void reportMetrics(IsolateObserver& isolateMetrics) const override {}
+    kj::Maybe<size_t> checkPbkdfIterations(jsg::Lock& lock, size_t iterations) const override {
+      // No limit on the number of iterations in workerd
+      return kj::none;
+    }
   };
 
   auto observer = kj::atomicRefcounted<IsolateObserver>();
   auto limitEnforcer = kj::heap<NullIsolateLimitEnforcer>();
   auto api = kj::heap<WorkerdApi>(globalContext->v8System,
-      featureFlags.asReader(), *limitEnforcer, kj::atomicAddRef(*observer));
+                                  featureFlags.asReader(),
+                                  *limitEnforcer,
+                                  kj::atomicAddRef(*observer));
   auto inspectorPolicy = Worker::Isolate::InspectorPolicy::DISALLOW;
   if (inspectorOverride != kj::none) {
     // For workerd, if the inspector is enabled, it is always fully trusted.

src/workerd/tests/test-fixture.c++

@@ -170,7 +170,9 @@ struct MockIsolateLimitEnforcer final: public IsolateLimitEnforcer {
     void completedRequest(kj::StringPtr id) const override {}
     bool exitJs(jsg::Lock& lock) const override { return false; }
     void reportMetrics(IsolateObserver& isolateMetrics) const override {}
-
+    kj::Maybe<size_t> checkPbkdfIterations(jsg::Lock& lock, size_t iterations) const override {
+      return kj::none;
+    }
 };
 
 struct MockErrorReporter final: public Worker::ValidationErrorReporter {