Introduce configurable pbkdf2 iteration limit

Introduces the ability for workerd embedders to configure the
max iteration limit for KDFs. Removes the limit for workerd by
default. Keeps the current limit of 100,000 when not configured.

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,25 @@ ZeroOnFree::~ZeroOnFree() noexcept(false) {
   OPENSSL_cleanse(inner.begin(), inner.size());
 }
 
+void checkPbkdfLimits(jsg::Lock& js, size_t iterations) {
+  // If the jsg::Lock embedder data has not been set then we're running in an
+  // environment that does not yet pass along the necessary information for the
+  // Worker::Isolate::from(...) below to work. Let's fall back on the old default
+  // limit here.
+  if (js.getEmbedderData() == nullptr) {
+    JSG_REQUIRE(iterations <= DEFAULT_MAX_PBKDF2_ITERATIONS, DOMNotSupportedError,
+                kj::str(
+                  "Pbkdf2 failed: iteration counts above 100000 are not supported (requested ",
+                  iterations, ")."));
+    return;
+  }
+
+  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, &isolate);
   lock->requestGcForTesting();
 }
 

src/workerd/io/worker.c++

@@ -503,7 +503,7 @@ struct Worker::Isolate::Impl {
           oldCurrentApi(currentApi),
           limitEnforcer(isolate.getLimitEnforcer()),
           consoleMode(isolate.consoleMode),
-          lock(isolate.api->lock(stackScope)) {
+          lock(isolate.api->lock(stackScope, &isolate)) {
       WarnAboutIsolateLockScope::maybeWarn();
 
       // Increment the success count to expose forward progress to all threads.
@@ -975,7 +975,7 @@ Worker::Isolate::Isolate(kj::Own<Api> apiParam,
   metrics->created();
   // We just created our isolate, so we don't need to use Isolate::Impl::Lock (nor an async lock).
   jsg::V8StackScope stackScope;
-  auto lock = api->lock(stackScope);
+  auto lock = api->lock(stackScope, this);
   auto features = api->getFeatureFlags();
 
   lock->setCaptureThrowsAsRejections(features.getCaptureThrowsAsRejections());
@@ -1251,6 +1251,14 @@ Worker::Script::~Script() noexcept(false) {
   impl = nullptr;
 }
 
+const Worker::Isolate& Worker::Isolate::from(jsg::Lock& js) {
+  auto embedderData = js.getEmbedderData();
+  KJ_ASSERT(embedderData != nullptr,
+      "unable to get the Worker::Isolate from the jsg::Lock. "
+      "is the embedder data set correctly when the jsg::Lock is created?");
+  return *static_cast<const Worker::Isolate*>(embedderData);
+}
+
 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; }
@@ -422,6 +425,11 @@ class Worker::Api {
   //   that this doesn't have to allocate and so it's not possible to hold a lock while returning
   //   to the event loop.
 
+  virtual kj::Own<jsg::Lock> lock(jsg::V8StackScope& stackScope, const void* embedderData) const {
+    // In the default impl we do not pass along any embedder data to the jsg::Lock
+    return lock(stackScope);
+  }
+
   // Get the FeatureFlags this isolate is configured with. Returns a Reader that is owned by the
   // Api.
   virtual CompatibilityFlags::Reader getFeatureFlags() const = 0;

src/workerd/jsg/jsg.c++

@@ -115,9 +115,10 @@ void Data::moveFromTraced(Data& other, v8::TracedReference<v8::Data>& otherTrace
   other.tracedHandle = kj::none;
 }
 
-Lock::Lock(v8::Isolate* v8Isolate)
+Lock::Lock(v8::Isolate* v8Isolate, const void* embedderData)
     : v8Isolate(v8Isolate), locker(v8Isolate),
       previousData(v8Isolate->GetData(2)),
+      embedderData(embedderData),
       warningsLogged(IsolateBase::from(v8Isolate).areWarningsLogged()) {
   if (previousData != nullptr) {
     // Hmm, there's already a current lock. It must be a recursive lock (i.e. a second lock taken

src/workerd/jsg/jsg.h

@@ -2231,18 +2231,25 @@ class Lock {
   void runMicrotasks();
   void terminateExecution();
 
+  inline const void* getEmbedderData() const { return embedderData; }
+
 private:
   friend class IsolateBase;
   template <typename TypeWrapper>
   friend class Isolate;
 
-  Lock(v8::Isolate* v8Isolate);
+  Lock(v8::Isolate* v8Isolate, const void* embedderData = nullptr);
   ~Lock() noexcept(false);
 
   v8::Locker locker;
 
   void* previousData;
 
+  // The embedderData is an additional void* that the embedder may use
+  // to associate an additional bit of context with the Lock. It is set
+  // via the constructor and accessed via getEmbedderData()
+  const void* embedderData = nullptr;
+
   bool warningsLogged;
 };
 

src/workerd/jsg/setup.c++

@@ -271,7 +271,9 @@ namespace {
   }
 }
 
-IsolateBase::IsolateBase(const V8System& system, v8::Isolate::CreateParams&& createParams, kj::Own<IsolateObserver> observer)
+IsolateBase::IsolateBase(const V8System& system,
+                         v8::Isolate::CreateParams&& createParams,
+                         kj::Own<IsolateObserver> observer)
     : system(system),
       ptr(newIsolate(kj::mv(createParams))),
       heapTracer(ptr),

src/workerd/jsg/setup.h

@@ -122,7 +122,7 @@ class IsolateBase {
   // Returns a random UUID for this isolate instance.
   kj::StringPtr getUuid();
 
-  IsolateObserver& getObserver() { return *observer; }
+  inline IsolateObserver& getObserver() { return *observer; }
 
 private:
   template <typename TypeWrapper>
@@ -205,7 +205,8 @@ class IsolateBase {
   kj::TreeMap<uintptr_t, CodeBlockInfo> codeMap;
 
   explicit IsolateBase(const V8System& system, v8::Isolate::CreateParams&& createParams,
-    kj::Own<IsolateObserver> observer);
+                       kj::Own<IsolateObserver> observer);
+
   ~IsolateBase() noexcept(false);
   KJ_DISALLOW_COPY_AND_MOVE(IsolateBase);
 
@@ -343,8 +344,8 @@ class Isolate: public IsolateBase {
     // `V8StackScope` must be provided to prove that one has been created on the stack before
     // taking a lock. Any GC'd pointers stored on the stack must be kept within this scope in
     // order for V8's stack-scanning GC to find them.
-    Lock(const Isolate& isolate, V8StackScope&)
-        : jsg::Lock(isolate.ptr), jsgIsolate(const_cast<Isolate&>(isolate)) {
+    Lock(const Isolate& isolate, V8StackScope&, const void* embedderData = nullptr)
+        : jsg::Lock(isolate.ptr, embedderData), jsgIsolate(const_cast<Isolate&>(isolate)) {
       jsgIsolate.clearDestructionQueue();
     }
 

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/server/workerd-api.c++

@@ -124,7 +124,10 @@ struct WorkerdApi::Impl {
        IsolateLimitEnforcer& limitEnforcer,
        kj::Own<jsg::IsolateObserver> observer)
       : features(capnp::clone(featuresParam)),
-        jsgIsolate(v8System, Configuration(*this), kj::mv(observer), limitEnforcer.getCreateParams()) {}
+        jsgIsolate(v8System,
+                   Configuration(*this),
+                   kj::mv(observer),
+                   limitEnforcer.getCreateParams()) {}
 
   static v8::Local<v8::String> compileTextGlobal(JsgWorkerdIsolate::Lock& lock,
       capnp::Text::Reader reader) {
@@ -168,7 +171,10 @@ WorkerdApi::WorkerdApi(jsg::V8System& v8System,
 WorkerdApi::~WorkerdApi() noexcept(false) {}
 
 kj::Own<jsg::Lock> WorkerdApi::lock(jsg::V8StackScope& stackScope) const {
-  return kj::heap<JsgWorkerdIsolate::Lock>(impl->jsgIsolate, stackScope);
+  return lock(stackScope, nullptr);
+}
+kj::Own<jsg::Lock> WorkerdApi::lock(jsg::V8StackScope& stackScope, const void* embedderData) const {
+  return kj::heap<JsgWorkerdIsolate::Lock>(impl->jsgIsolate, stackScope, embedderData);
 }
 CompatibilityFlags::Reader WorkerdApi::getFeatureFlags() const {
   return *impl->features;

src/workerd/server/workerd-api.h

@@ -22,6 +22,7 @@ class WorkerdApi final: public Worker::Api {
   static const WorkerdApi& from(const Worker::Api&);
 
   kj::Own<jsg::Lock> lock(jsg::V8StackScope& stackScope) const override;
+  kj::Own<jsg::Lock> lock(jsg::V8StackScope& stackScope, const void* embedderData) const override;
   CompatibilityFlags::Reader getFeatureFlags() const override;
   jsg::JsContext<api::ServiceWorkerGlobalScope> newContext(jsg::Lock& lock) const override;
   jsg::Dict<NamedExport> unwrapExports(

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 {