Rate limit printing requests from CUPS Proxy

Bug: chromium:832152
Test: ./libcups_unittests
Change-Id: I556ff53fdb997d9b22d94efd035704242a36b0bc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2359570
Commit-Queue: Pranav Batra <batrapranav@chromium.org>
Reviewed-by: Luum Habtemariam <luum@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809660}

chrome/services/cups_proxy/BUILD.gn

@@ -56,6 +56,7 @@ test("libcups_unittests") {
   sources = [
     "ipp_validator_unittest.cc",
     "printer_installer_unittest.cc",
+    "proxy_manager_unittest.cc",
     "socket_manager_unittest.cc",
   ]
 
@@ -71,6 +72,7 @@ test("libcups_unittests") {
     "//chrome/services/cups_proxy/public/cpp:unit_tests",
     "//chrome/services/cups_proxy/test:test_support",
     "//chromeos",
+    "//content/public/browser",
     "//testing/gmock",
     "//testing/gtest",
   ]

chrome/services/cups_proxy/proxy_manager.cc

@@ -12,6 +12,7 @@
 #include <vector>
 
 #include "base/bind.h"
+#include "base/containers/ring_buffer.h"
 #include "base/feature_list.h"
 #include "base/logging.h"
 #include "base/task/post_task.h"
@@ -45,8 +46,16 @@ class ProxyManagerImpl : public ProxyManager {
                    std::unique_ptr<CupsProxyServiceDelegate> delegate,
                    std::unique_ptr<IppValidator> ipp_validator,
                    std::unique_ptr<PrinterInstaller> printer_installer,
-                   std::unique_ptr<SocketManager> socket_manager);
-  ~ProxyManagerImpl() override;
+                   std::unique_ptr<SocketManager> socket_manager)
+      : delegate_(std::move(delegate)),
+        ipp_validator_(std::move(ipp_validator)),
+        printer_installer_(std::move(printer_installer)),
+        socket_manager_(std::move(socket_manager)),
+        receiver_(this, std::move(request)) {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  }
+
+  ~ProxyManagerImpl() override = default;
 
   void ProxyRequest(const std::string& method,
                     const std::string& url,
@@ -85,6 +94,9 @@ class ProxyManagerImpl : public ProxyManager {
   // Current in-flight request.
   std::unique_ptr<InFlightRequest> in_flight_;
 
+  // Timestamp ring buffer.
+  base::RingBuffer<base::TimeTicks, kRateLimit> timestamp_;
+
   // CupsIppParser Service handle.
   mojo::Remote<ipp_parser::mojom::IppParser> ipp_parser_;
 
@@ -125,22 +137,6 @@ base::Optional<std::vector<uint8_t>> RebuildIppRequest(
   return ret;
 }
 
-ProxyManagerImpl::ProxyManagerImpl(
-    mojo::PendingReceiver<CupsProxier> receiver,
-    std::unique_ptr<CupsProxyServiceDelegate> delegate,
-    std::unique_ptr<IppValidator> ipp_validator,
-    std::unique_ptr<PrinterInstaller> printer_installer,
-    std::unique_ptr<SocketManager> socket_manager)
-    : delegate_(std::move(delegate)),
-      ipp_validator_(std::move(ipp_validator)),
-      printer_installer_(std::move(printer_installer)),
-      socket_manager_(std::move(socket_manager)),
-      receiver_(this, std::move(receiver)) {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-}
-
-ProxyManagerImpl::~ProxyManagerImpl() = default;
-
 void ProxyManagerImpl::ProxyRequest(
     const std::string& method,
     const std::string& url,
@@ -150,6 +146,23 @@ void ProxyManagerImpl::ProxyRequest(
     ProxyRequestCallback cb) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
+  // Limit the rate of requests to kRateLimit per second.
+  // The way the algorithm works is if the number of times this method was
+  // called between a second ago and now, including the current call, exceeds
+  // kRateLimit, the request is blocked and the HTTP 429 Too Many Requests
+  // response status code is returned.
+  base::TimeTicks time = base::TimeTicks::Now();
+  bool block_request =
+      timestamp_.CurrentIndex() >= timestamp_.BufferSize() &&
+      time - timestamp_.ReadBuffer(0) < base::TimeDelta::FromSeconds(1);
+  timestamp_.SaveToBuffer(time);
+  if (block_request) {
+    DVLOG(1) << "CupsPrintService Error: Rate limit (" << kRateLimit
+             << ") exceeded";
+    std::move(cb).Run({}, {}, 429);  // HTTP_STATUS_TOO_MANY_REQUESTS
+    return;
+  }
+
   if (!delegate_->IsPrinterAccessAllowed()) {
     DVLOG(1) << "Printer access not allowed";
     std::move(cb).Run(/*headers=*/{}, /*ipp_message=*/{},
@@ -343,14 +356,12 @@ void ProxyManagerImpl::Fail(const std::string& error_message,
 std::unique_ptr<ProxyManager> ProxyManager::Create(
     mojo::PendingReceiver<mojom::CupsProxier> request,
     std::unique_ptr<CupsProxyServiceDelegate> delegate) {
-  // Setting up injected managers.
-  auto ipp_validator = std::make_unique<IppValidator>(delegate.get());
-  auto printer_installer = std::make_unique<PrinterInstaller>(delegate.get());
-  auto socket_manager = SocketManager::Create(delegate.get());
-
+  auto* delegate_ptr = delegate.get();
   return std::make_unique<ProxyManagerImpl>(
-      std::move(request), std::move(delegate), std::move(ipp_validator),
-      std::move(printer_installer), std::move(socket_manager));
+      std::move(request), std::move(delegate),
+      std::make_unique<IppValidator>(delegate_ptr),
+      std::make_unique<PrinterInstaller>(delegate_ptr),
+      SocketManager::Create(delegate_ptr));
 }
 
 std::unique_ptr<ProxyManager> ProxyManager::CreateForTesting(

chrome/services/cups_proxy/proxy_manager.h

@@ -25,12 +25,15 @@ class SocketManager;
 // This handler's job is vetting incoming arbitrary CUPS IPP requests before
 // they reach the CUPS Daemon. Requests are parsed out-of-process, by the
 // CupsIppParser Service, and validated/rebuilt in-process before being proxied.
-// This handler must be created/accessed from a seqeunced context.
+// This handler must be created/accessed from a sequenced context.
 //
 // Note: This handler only supports processing one request at a time; any
 // concurrent requests will immediately fail with an empty response.
 class ProxyManager : public mojom::CupsProxier {
  public:
+  // Request rate limit per second.
+  static constexpr int kRateLimit = 10;
+
   // Factory function.
   static std::unique_ptr<ProxyManager> Create(
       mojo::PendingReceiver<mojom::CupsProxier> request,

chrome/services/cups_proxy/proxy_manager_unittest.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/services/cups_proxy/proxy_manager.h"
+
+#include <map>
+
+#include "base/test/task_environment.h"
+#include "chrome/services/cups_proxy/fake_cups_proxy_service_delegate.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace cups_proxy {
+
+namespace {
+
+constexpr int kHttpTooManyRequests = 429;
+
+class MyFakeCupsProxyServiceDelegate : public FakeCupsProxyServiceDelegate {
+  bool IsPrinterAccessAllowed() const override { return false; }
+};
+
+class ProxyManagerTest : public testing::Test {
+ public:
+  ProxyManagerTest()
+      : manager_(ProxyManager::Create(
+            {},
+            std::make_unique<MyFakeCupsProxyServiceDelegate>())) {}
+
+  // Proxy a dummy request and add the response code to count_.
+  void ProxyRequest() const {
+    manager_->ProxyRequest({}, {}, {}, {}, {},
+                           base::BindOnce(&ProxyManagerTest::Callback,
+                                          weak_factory_.GetWeakPtr()));
+  }
+
+  // Return the number of times response code status has been received.
+  int NumRequestsByStatusCode(int32_t status) const {
+    const auto it = count_.find(status);
+    return it == count_.end() ? 0 : it->second;
+  }
+
+  // Fast forward the task environment's fake clock.
+  void FastForwardBy(base::TimeDelta delta) {
+    return task_environment_.FastForwardBy(delta);
+  }
+
+ private:
+  // Add the response code status to count_.
+  void Callback(const std::vector<ipp_converter::HttpHeader>&,
+                const std::vector<uint8_t>&,
+                int32_t status) {
+    count_[status]++;
+  }
+
+  base::test::TaskEnvironment task_environment_{
+      base::test::TaskEnvironment::TimeSource::MOCK_TIME};
+
+  std::map<int32_t, int> count_;
+
+  std::unique_ptr<ProxyManager> manager_;
+
+  base::WeakPtrFactory<ProxyManagerTest> weak_factory_{this};
+};
+
+// Test a burst of simultaneous function calls.
+TEST_F(ProxyManagerTest, ProxyRequestRateLimitBurst) {
+  for (int i = 0; i < ProxyManager::kRateLimit + 3; i++)
+    ProxyRequest();
+  EXPECT_EQ(NumRequestsByStatusCode(kHttpTooManyRequests), 3);
+}
+
+// Test a 0.99s gap between two bursts of function calls.
+TEST_F(ProxyManagerTest, ProxyRequestRateLimitShortGap) {
+  for (int i = 0; i < ProxyManager::kRateLimit + 1; i++) {
+    if (i == ProxyManager::kRateLimit / 2)
+      FastForwardBy(base::TimeDelta::FromSecondsD(.99));
+    ProxyRequest();
+  }
+  EXPECT_EQ(NumRequestsByStatusCode(kHttpTooManyRequests), 1);
+}
+
+// Test that the rate limit is reset after 1.01s.
+TEST_F(ProxyManagerTest, ProxyRequestRateLimitLongGap) {
+  for (int i = 0; i < ProxyManager::kRateLimit + 1; i++)
+    ProxyRequest();
+  EXPECT_EQ(NumRequestsByStatusCode(kHttpTooManyRequests), 1);
+  FastForwardBy(base::TimeDelta::FromSecondsD(1.01));
+  for (int i = 0; i < ProxyManager::kRateLimit + 1; i++)
+    ProxyRequest();
+  EXPECT_EQ(NumRequestsByStatusCode(kHttpTooManyRequests), 2);
+}
+
+// Test that calls at a constant rate below the rate limit are allowed.
+TEST_F(ProxyManagerTest, ProxyRequestRateLimitBelow) {
+  for (int i = 0; i < ProxyManager::kRateLimit + 10; i++) {
+    FastForwardBy(
+        base::TimeDelta::FromSecondsD(1.01 / ProxyManager::kRateLimit));
+    ProxyRequest();
+  }
+  EXPECT_EQ(NumRequestsByStatusCode(kHttpTooManyRequests), 0);
+}
+
+// Test that calls at a constant rate above the rate limit are blocked.
+TEST_F(ProxyManagerTest, ProxyRequestRateLimitAbove) {
+  for (int i = 0; i < ProxyManager::kRateLimit + 10; i++) {
+    FastForwardBy(
+        base::TimeDelta::FromSecondsD(.99 / ProxyManager::kRateLimit));
+    ProxyRequest();
+  }
+  EXPECT_EQ(NumRequestsByStatusCode(kHttpTooManyRequests), 10);
+}
+
+}  // namespace
+
+}  // namespace cups_proxy