diff options
Diffstat (limited to 'chromium/chrome/browser/extensions/api/certificate_provider')
3 files changed, 125 insertions, 37 deletions
diff --git a/chromium/chrome/browser/extensions/api/certificate_provider/certificate_provider_api.cc b/chromium/chrome/browser/extensions/api/certificate_provider/certificate_provider_api.cc index 533bdee1e87..4530f54ea05 100644 --- a/chromium/chrome/browser/extensions/api/certificate_provider/certificate_provider_api.cc +++ b/chromium/chrome/browser/extensions/api/certificate_provider/certificate_provider_api.cc @@ -12,6 +12,8 @@ #include "base/bind.h" #include "base/logging.h" +#include "base/macros.h" +#include "base/values.h" #include "chrome/browser/chromeos/certificate_provider/certificate_provider_service.h" #include "chrome/browser/chromeos/certificate_provider/certificate_provider_service_factory.h" #include "chrome/browser/chromeos/certificate_provider/pin_dialog_manager.h" @@ -19,6 +21,7 @@ #include "chrome/common/extensions/api/certificate_provider.h" #include "chrome/common/extensions/api/certificate_provider_internal.h" #include "chromeos/constants/security_token_pin_types.h" +#include "extensions/browser/quota_service.h" #include "net/cert/x509_certificate.h" #include "net/ssl/ssl_private_key.h" #include "third_party/blink/public/mojom/devtools/console_message.mojom.h" @@ -75,9 +78,55 @@ const char kCertificateProviderPreviousDialogActive[] = "Previous request not finished"; const char kCertificateProviderNoUserInput[] = "No user input received"; +// The BucketMapper implementation for the requestPin API that avoids using the +// quota when the current request uses the requestId that is strictly greater +// than all previous ones. +class RequestPinExceptFirstQuotaBucketMapper final + : public QuotaLimitHeuristic::BucketMapper { + public: + RequestPinExceptFirstQuotaBucketMapper() = default; + ~RequestPinExceptFirstQuotaBucketMapper() override = default; + + void GetBucketsForArgs(const base::ListValue* args, + QuotaLimitHeuristic::BucketList* buckets) override { + if (args->GetList().empty()) + return; + const base::Value& details = args->GetList()[0]; + if (!details.is_dict()) + return; + const base::Value* sign_request_id = + details.FindKeyOfType("signRequestId", base::Value::Type::INTEGER); + if (!sign_request_id) + return; + if (sign_request_id->GetInt() > biggest_request_id_) { + // Either it's the first request with the newly issued requestId, or it's + // an invalid requestId (bigger than the real one). Return a new bucket in + // order to apply no quota for the former case; for the latter case the + // quota doesn't matter much, except that we're maybe making it stricter + // for future requests (which is bearable). + biggest_request_id_ = sign_request_id->GetInt(); + new_request_bucket_ = std::make_unique<QuotaLimitHeuristic::Bucket>(); + buckets->push_back(new_request_bucket_.get()); + return; + } + // Either it's a repeatitive request for the given requestId, or the + // extension reordered the requests. Fall back to the default bucket (shared + // between all requests) in that case. + buckets->push_back(&default_bucket_); + } + + private: + int biggest_request_id_ = -1; + QuotaLimitHeuristic::Bucket default_bucket_; + std::unique_ptr<QuotaLimitHeuristic::Bucket> new_request_bucket_; + + DISALLOW_COPY_AND_ASSIGN(RequestPinExceptFirstQuotaBucketMapper); +}; + } // namespace -const int api::certificate_provider::kMaxClosedDialogsPer10Mins = 2; +const int api::certificate_provider::kMaxClosedDialogsPerMinute = 10; +const int api::certificate_provider::kMaxClosedDialogsPer10Minutes = 30; CertificateProviderInternalReportCertificatesFunction:: ~CertificateProviderInternalReportCertificatesFunction() {} @@ -256,12 +305,25 @@ bool CertificateProviderRequestPinFunction::ShouldSkipQuotaLimiting() const { } void CertificateProviderRequestPinFunction::GetQuotaLimitHeuristics( - extensions::QuotaLimitHeuristics* heuristics) const { + QuotaLimitHeuristics* heuristics) const { + // Apply a 1-minute and a 10-minute quotas. A special bucket mapper is used in + // order to, approximately, skip applying quotas to the first request for each + // requestId (such logic cannot be done in ShouldSkipQuotaLimiting(), since + // it's not called with the request's parameters). The limitation constants + // are decremented below to account the first request. + QuotaLimitHeuristic::Config short_limit_config = { - api::certificate_provider::kMaxClosedDialogsPer10Mins, + api::certificate_provider::kMaxClosedDialogsPerMinute - 1, + base::TimeDelta::FromMinutes(1)}; + heuristics->push_back(std::make_unique<QuotaService::TimedLimit>( + short_limit_config, new RequestPinExceptFirstQuotaBucketMapper, + "MAX_PIN_DIALOGS_CLOSED_PER_MINUTE")); + + QuotaLimitHeuristic::Config long_limit_config = { + api::certificate_provider::kMaxClosedDialogsPer10Minutes - 1, base::TimeDelta::FromMinutes(10)}; heuristics->push_back(std::make_unique<QuotaService::TimedLimit>( - short_limit_config, new QuotaLimitHeuristic::SingletonBucketMapper(), + long_limit_config, new RequestPinExceptFirstQuotaBucketMapper, "MAX_PIN_DIALOGS_CLOSED_PER_10_MINUTES")); } diff --git a/chromium/chrome/browser/extensions/api/certificate_provider/certificate_provider_api.h b/chromium/chrome/browser/extensions/api/certificate_provider/certificate_provider_api.h index b61e947336d..623b666b35d 100644 --- a/chromium/chrome/browser/extensions/api/certificate_provider/certificate_provider_api.h +++ b/chromium/chrome/browser/extensions/api/certificate_provider/certificate_provider_api.h @@ -19,9 +19,10 @@ namespace extensions { namespace api { namespace certificate_provider { -// The maximum number of times per 10 minutes, extension is allowed to show PIN -// dialog again after user closed the previous one. -extern const int kMaxClosedDialogsPer10Mins; +// The maximum number of times in the given interval the extension is allowed to +// show the PIN dialog again after user closed the previous one. +extern const int kMaxClosedDialogsPerMinute; +extern const int kMaxClosedDialogsPer10Minutes; struct CertificateInfo; } diff --git a/chromium/chrome/browser/extensions/api/certificate_provider/certificate_provider_apitest.cc b/chromium/chrome/browser/extensions/api/certificate_provider/certificate_provider_apitest.cc index 1611eb4a730..1de2320a536 100644 --- a/chromium/chrome/browser/extensions/api/certificate_provider/certificate_provider_apitest.cc +++ b/chromium/chrome/browser/extensions/api/certificate_provider/certificate_provider_apitest.cc @@ -171,9 +171,9 @@ class CertificateProviderRequestPinTest : public CertificateProviderApiTest { CertificateProviderApiTest::TearDownOnMainThread(); } - void AddFakeSignRequest() { + void AddFakeSignRequest(int sign_request_id) { cert_provider_service_->pin_dialog_manager()->AddSignRequestId( - extension_->id(), kFakeSignRequestId, {}); + extension_->id(), sign_request_id, {}); } void NavigateTo(const std::string& test_page_file_name) { @@ -372,7 +372,7 @@ IN_PROC_BROWSER_TEST_F(CertificateProviderApiTest, Basic) { // User enters the correct PIN. IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, ShowPinDialogAccept) { - AddFakeSignRequest(); + AddFakeSignRequest(kFakeSignRequestId); NavigateTo("basic.html"); // Enter the valid PIN. @@ -382,14 +382,14 @@ IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, ShowPinDialogAccept) { EXPECT_FALSE(GetActivePinDialogView()); } -// User closes the dialog kMaxClosedDialogsPer10Mins times, and the extension +// User closes the dialog kMaxClosedDialogsPerMinute times, and the extension // should be blocked from showing it again. IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, ShowPinDialogClose) { - AddFakeSignRequest(); + AddFakeSignRequest(kFakeSignRequestId); NavigateTo("basic.html"); for (int i = 0; - i < extensions::api::certificate_provider::kMaxClosedDialogsPer10Mins; + i < extensions::api::certificate_provider::kMaxClosedDialogsPerMinute; i++) { ExtensionTestMessageListener listener("User closed the dialog", false); GetActivePinDialogWindow()->Close(); @@ -401,7 +401,7 @@ IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, ShowPinDialogClose) { ASSERT_TRUE(close_listener.WaitUntilSatisfied()); close_listener.Reply("GetLastError"); ExtensionTestMessageListener last_error_listener( - "This request exceeds the MAX_PIN_DIALOGS_CLOSED_PER_10_MINUTES quota.", + "This request exceeds the MAX_PIN_DIALOGS_CLOSED_PER_MINUTE quota.", false); ASSERT_TRUE(last_error_listener.WaitUntilSatisfied()); EXPECT_FALSE(GetActivePinDialogView()); @@ -410,7 +410,7 @@ IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, ShowPinDialogClose) { // User enters a wrong PIN first and a correct PIN on the second try. IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, ShowPinDialogWrongPin) { - AddFakeSignRequest(); + AddFakeSignRequest(kFakeSignRequestId); NavigateTo("basic.html"); EnterWrongPinAndWaitForMessage(); @@ -428,7 +428,7 @@ IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, // User enters wrong PIN three times. IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, ShowPinDialogWrongPinThreeTimes) { - AddFakeSignRequest(); + AddFakeSignRequest(kFakeSignRequestId); NavigateTo("basic.html"); for (int i = 0; i < kWrongPinAttemptsLimit; i++) EnterWrongPinAndWaitForMessage(); @@ -446,7 +446,7 @@ IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, // User closes the dialog while the extension is processing the request. IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, ShowPinDialogCloseWhileProcessing) { - AddFakeSignRequest(); + AddFakeSignRequest(kFakeSignRequestId); NavigateTo("operated.html"); EXPECT_TRUE(SendCommandAndWaitForMessage("Request", "request1:begun")); @@ -462,7 +462,7 @@ IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, EXPECT_FALSE(GetActivePinDialogView()); } -// Extension closes the dialog kMaxClosedDialogsPer10Mins times after the user +// Extension closes the dialog kMaxClosedDialogsPerMinute times after the user // inputs some value, and it should be blocked from showing it again. IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, RepeatedProgrammaticCloseAfterInput) { @@ -470,9 +470,9 @@ IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, for (int i = 0; i < - extensions::api::certificate_provider::kMaxClosedDialogsPer10Mins + 1; + extensions::api::certificate_provider::kMaxClosedDialogsPerMinute + 1; i++) { - AddFakeSignRequest(); + AddFakeSignRequest(kFakeSignRequestId); EXPECT_TRUE(SendCommandAndWaitForMessage( "Request", base::StringPrintf("request%d:begun", i + 1))); @@ -482,20 +482,20 @@ IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, EXPECT_FALSE(GetActivePinDialogView()); } - AddFakeSignRequest(); + AddFakeSignRequest(kFakeSignRequestId); EXPECT_TRUE(SendCommandAndWaitForMessage( "Request", base::StringPrintf( "request%d:error:This request exceeds the " - "MAX_PIN_DIALOGS_CLOSED_PER_10_MINUTES quota.", - extensions::api::certificate_provider::kMaxClosedDialogsPer10Mins + + "MAX_PIN_DIALOGS_CLOSED_PER_MINUTE quota.", + extensions::api::certificate_provider::kMaxClosedDialogsPerMinute + 2))); EXPECT_FALSE(GetActivePinDialogView()); } // Extension erroneously attempts to close the PIN dialog twice. IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, DoubleClose) { - AddFakeSignRequest(); + AddFakeSignRequest(kFakeSignRequestId); NavigateTo("operated.html"); EXPECT_TRUE(SendCommand("Request")); @@ -505,7 +505,7 @@ IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, DoubleClose) { EXPECT_FALSE(GetActivePinDialogView()); } -// Extension closes the dialog kMaxClosedDialogsPer10Mins times before the user +// Extension closes the dialog kMaxClosedDialogsPerMinute times before the user // inputs anything, and it should be blocked from showing it again. IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, RepeatedProgrammaticCloseBeforeInput) { @@ -513,22 +513,22 @@ IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, for (int i = 0; i < - extensions::api::certificate_provider::kMaxClosedDialogsPer10Mins + 1; + extensions::api::certificate_provider::kMaxClosedDialogsPerMinute + 1; i++) { - AddFakeSignRequest(); + AddFakeSignRequest(kFakeSignRequestId); EXPECT_TRUE(SendCommand("Request")); EXPECT_TRUE(SendCommandAndWaitForMessage( "Stop", base::StringPrintf("stop%d:success", i + 1))); EXPECT_FALSE(GetActivePinDialogView()); } - AddFakeSignRequest(); + AddFakeSignRequest(kFakeSignRequestId); EXPECT_TRUE(SendCommandAndWaitForMessage( "Request", base::StringPrintf( "request%d:error:This request exceeds the " - "MAX_PIN_DIALOGS_CLOSED_PER_10_MINUTES quota.", - extensions::api::certificate_provider::kMaxClosedDialogsPer10Mins + + "MAX_PIN_DIALOGS_CLOSED_PER_MINUTE quota.", + extensions::api::certificate_provider::kMaxClosedDialogsPerMinute + 2))); EXPECT_FALSE(GetActivePinDialogView()); } @@ -537,7 +537,7 @@ IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, // the user provided any input. IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, StopWithErrorBeforeInput) { - AddFakeSignRequest(); + AddFakeSignRequest(kFakeSignRequestId); NavigateTo("operated.html"); EXPECT_TRUE(SendCommand("Request")); @@ -557,7 +557,7 @@ IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, InvalidRequestId) { // Extension specifies zero left attempts in the very first PIN request. IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, ZeroAttemptsAtStart) { - AddFakeSignRequest(); + AddFakeSignRequest(kFakeSignRequestId); NavigateTo("operated.html"); EXPECT_TRUE(SendCommandAndWaitForMessage("RequestWithZeroAttempts", @@ -573,7 +573,7 @@ IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, ZeroAttemptsAtStart) { // Extension erroneously passes a negative attempts left count. IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, NegativeAttempts) { - AddFakeSignRequest(); + AddFakeSignRequest(kFakeSignRequestId); NavigateTo("operated.html"); EXPECT_TRUE(SendCommandAndWaitForMessage( @@ -583,7 +583,7 @@ IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, NegativeAttempts) { // Extension erroneously attempts to close a non-existing dialog. IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, CloseNonExisting) { - AddFakeSignRequest(); + AddFakeSignRequest(kFakeSignRequestId); NavigateTo("operated.html"); EXPECT_TRUE(SendCommandAndWaitForMessage( @@ -593,7 +593,7 @@ IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, CloseNonExisting) { // Extension erroneously attempts to stop a non-existing dialog with an error. IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, StopNonExisting) { - AddFakeSignRequest(); + AddFakeSignRequest(kFakeSignRequestId); NavigateTo("operated.html"); EXPECT_TRUE(SendCommandAndWaitForMessage( @@ -605,7 +605,7 @@ IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, StopNonExisting) { // user closed the previously stopped with an error PIN request. IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, UpdateAlreadyStopped) { - AddFakeSignRequest(); + AddFakeSignRequest(kFakeSignRequestId); NavigateTo("operated.html"); EXPECT_TRUE(SendCommandAndWaitForMessage("Request", "request1:begun")); @@ -622,7 +622,7 @@ IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, // Extension starts a new PIN request after it stopped the previous one with an // error. IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, StartAfterStop) { - AddFakeSignRequest(); + AddFakeSignRequest(kFakeSignRequestId); NavigateTo("operated.html"); EXPECT_TRUE(SendCommandAndWaitForMessage("Request", "request1:begun")); @@ -637,3 +637,28 @@ IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, StartAfterStop) { EXPECT_TRUE(listener.WaitUntilSatisfied()); EXPECT_FALSE(GetActivePinDialogView()->textfield_for_testing()->GetEnabled()); } + +// Test that no quota is applied to the first PIN requests for each requestId. +IN_PROC_BROWSER_TEST_F(CertificateProviderRequestPinTest, + RepeatedCloseWithDifferentIds) { + NavigateTo("operated.html"); + + for (int i = 0; + i < + extensions::api::certificate_provider::kMaxClosedDialogsPer10Minutes + 2; + i++) { + AddFakeSignRequest(kFakeSignRequestId + i); + EXPECT_TRUE(SendCommandAndWaitForMessage( + "Request", base::StringPrintf("request%d:begun", i + 1))); + + ExtensionTestMessageListener listener( + base::StringPrintf("request%d:empty", i + 1), false); + ASSERT_TRUE(GetActivePinDialogView()); + GetActivePinDialogView()->GetWidget()->CloseWithReason( + views::Widget::ClosedReason::kCloseButtonClicked); + EXPECT_TRUE(listener.WaitUntilSatisfied()); + EXPECT_FALSE(GetActivePinDialogView()); + + EXPECT_TRUE(SendCommand("IncrementRequestId")); + } +} |