From 909dea0c7ac38f76acb43f9ea9172ddd747f70f5 Mon Sep 17 00:00:00 2001 From: Penny MacNeil Date: Mon, 2 Feb 2015 12:33:30 -0800 Subject: [PATCH] Merge to M41 branch 2272: Allow SW registration only if it's secure AND it's HTTP or HTTPS BUG=453982 TEST=tested manually TEST=content_unittests:ServiceWorkerDispatcherHostTest.Register_FileSystem* Review URL: https://codereview.chromium.org/889323002 Cr-Commit-Position: refs/heads/master@{#314143} (cherry picked from commit 22394d843a6c36eb2e6d7bdf4fb8e7c4b7ae8d68) Review URL: https://codereview.chromium.org/896563002 Cr-Commit-Position: refs/branch-heads/2272@{#186} Cr-Branched-From: 827a380cfdb31aa54c8d56e63ce2c3fd8c3ba4d4-refs/heads/master@{#310958} --- .../service_worker_dispatcher_host.cc | 36 ++++-- ...service_worker_dispatcher_host_unittest.cc | 118 +++++++++++++----- 2 files changed, 114 insertions(+), 40 deletions(-) diff --git a/content/browser/service_worker/service_worker_dispatcher_host.cc b/content/browser/service_worker/service_worker_dispatcher_host.cc index aa271430675bc..b0117a9c96bd1 100644 --- a/content/browser/service_worker/service_worker_dispatcher_host.cc +++ b/content/browser/service_worker/service_worker_dispatcher_host.cc @@ -33,6 +33,8 @@ namespace { const char kNoDocumentURLErrorMessage[] = "No URL is associated with the caller's document."; +const char kDisallowedURLErrorMessage[] = + "The URL is not supported."; const char kShutdownErrorMessage[] = "The Service Worker system has shutdown."; const char kUserDeniedPermissionMessage[] = @@ -52,7 +54,8 @@ bool AllOriginsMatch(const GURL& url_a, const GURL& url_b, const GURL& url_c) { // consistent with Blink's // SecurityOrigin::canAccessFeatureRequiringSecureOrigin. bool OriginCanAccessServiceWorkers(const GURL& url) { - return url.SchemeIsSecure() || net::IsLocalhost(url.host()); + return url.SchemeIsHTTPOrHTTPS() && + (url.SchemeIsSecure() || net::IsLocalhost(url.host())); } bool CanRegisterServiceWorker(const GURL& document_url, @@ -62,7 +65,9 @@ bool CanRegisterServiceWorker(const GURL& document_url, DCHECK(pattern.is_valid()); DCHECK(script_url.is_valid()); return AllOriginsMatch(document_url, pattern, script_url) && - OriginCanAccessServiceWorkers(document_url); + OriginCanAccessServiceWorkers(document_url) && + OriginCanAccessServiceWorkers(pattern) && + OriginCanAccessServiceWorkers(script_url); } bool CanUnregisterServiceWorker(const GURL& document_url, @@ -70,7 +75,8 @@ bool CanUnregisterServiceWorker(const GURL& document_url, DCHECK(document_url.is_valid()); DCHECK(pattern.is_valid()); return document_url.GetOrigin() == pattern.GetOrigin() && - OriginCanAccessServiceWorkers(document_url); + OriginCanAccessServiceWorkers(document_url) && + OriginCanAccessServiceWorkers(pattern); } bool CanGetRegistration(const GURL& document_url, @@ -78,7 +84,8 @@ bool CanGetRegistration(const GURL& document_url, DCHECK(document_url.is_valid()); DCHECK(given_document_url.is_valid()); return document_url.GetOrigin() == given_document_url.GetOrigin() && - OriginCanAccessServiceWorkers(document_url); + OriginCanAccessServiceWorkers(document_url) && + OriginCanAccessServiceWorkers(given_document_url); } } // namespace @@ -296,7 +303,12 @@ void ServiceWorkerDispatcherHost::OnRegisterServiceWorker( if (!CanRegisterServiceWorker( provider_host->document_url(), pattern, script_url)) { - BadMessageReceived(); + // TODO(kinuko): Change this back to BadMessageReceived() once we start + // to check these in the renderer too. (http://crbug.com/453982) + Send(new ServiceWorkerMsg_ServiceWorkerRegistrationError( + thread_id, request_id, WebServiceWorkerError::ErrorTypeSecurity, + base::ASCIIToUTF16(kServiceWorkerRegisterErrorPrefix) + + base::ASCIIToUTF16(kDisallowedURLErrorMessage))); return; } @@ -383,7 +395,12 @@ void ServiceWorkerDispatcherHost::OnUnregisterServiceWorker( } if (!CanUnregisterServiceWorker(provider_host->document_url(), pattern)) { - BadMessageReceived(); + // TODO(kinuko): Change this back to BadMessageReceived() once we start + // to check these in the renderer too. (http://crbug.com/453982) + Send(new ServiceWorkerMsg_ServiceWorkerUnregistrationError( + thread_id, request_id, WebServiceWorkerError::ErrorTypeSecurity, + base::ASCIIToUTF16(kServiceWorkerUnregisterErrorPrefix) + + base::ASCIIToUTF16(kDisallowedURLErrorMessage))); return; } @@ -456,7 +473,12 @@ void ServiceWorkerDispatcherHost::OnGetRegistration( } if (!CanGetRegistration(provider_host->document_url(), document_url)) { - BadMessageReceived(); + // TODO(kinuko): Change this back to BadMessageReceived() once we start + // to check these in the renderer too. (http://crbug.com/453982) + Send(new ServiceWorkerMsg_ServiceWorkerGetRegistrationError( + thread_id, request_id, WebServiceWorkerError::ErrorTypeSecurity, + base::ASCIIToUTF16(kServiceWorkerGetRegistrationErrorPrefix) + + base::ASCIIToUTF16(kDisallowedURLErrorMessage))); return; } diff --git a/content/browser/service_worker/service_worker_dispatcher_host_unittest.cc b/content/browser/service_worker/service_worker_dispatcher_host_unittest.cc index 0d2807f7f65f1..771f196aec298 100644 --- a/content/browser/service_worker/service_worker_dispatcher_host_unittest.cc +++ b/content/browser/service_worker/service_worker_dispatcher_host_unittest.cc @@ -234,10 +234,10 @@ TEST_F(ServiceWorkerDispatcherHostTest, Register_NonSecureOriginShouldFail) { host->SetDocumentUrl(GURL("http://www.example.com/foo")); context()->AddProviderHost(host.Pass()); - SendRegister(kProviderId, - GURL("http://www.example.com/"), - GURL("http://www.example.com/bar")); - EXPECT_EQ(1, dispatcher_host_->bad_messages_received_count_); + Register(kProviderId, + GURL("http://www.example.com/"), + GURL("http://www.example.com/bar"), + ServiceWorkerMsg_ServiceWorkerRegistrationError::ID); } TEST_F(ServiceWorkerDispatcherHostTest, Register_CrossOriginShouldFail) { @@ -248,40 +248,88 @@ TEST_F(ServiceWorkerDispatcherHostTest, Register_CrossOriginShouldFail) { context()->AddProviderHost(host.Pass()); // Script has a different host - SendRegister(kProviderId, - GURL("https://www.example.com/"), - GURL("https://foo.example.com/bar")); - EXPECT_EQ(1, dispatcher_host_->bad_messages_received_count_); + Register(kProviderId, + GURL("https://www.example.com/"), + GURL("https://foo.example.com/bar"), + ServiceWorkerMsg_ServiceWorkerRegistrationError::ID); // Scope has a different host - SendRegister(kProviderId, - GURL("https://foo.example.com/"), - GURL("https://www.example.com/bar")); - EXPECT_EQ(2, dispatcher_host_->bad_messages_received_count_); + Register(kProviderId, + GURL("https://foo.example.com/"), + GURL("https://www.example.com/bar"), + ServiceWorkerMsg_ServiceWorkerRegistrationError::ID); // Script has a different port - SendRegister(kProviderId, - GURL("https://www.example.com/"), - GURL("https://www.example.com:8080/bar")); - EXPECT_EQ(3, dispatcher_host_->bad_messages_received_count_); + Register(kProviderId, + GURL("https://www.example.com/"), + GURL("https://www.example.com:8080/bar"), + ServiceWorkerMsg_ServiceWorkerRegistrationError::ID); // Scope has a different transport - SendRegister(kProviderId, - GURL("wss://www.example.com/"), - GURL("https://www.example.com/bar")); - EXPECT_EQ(4, dispatcher_host_->bad_messages_received_count_); + Register(kProviderId, + GURL("wss://www.example.com/"), + GURL("https://www.example.com/bar"), + ServiceWorkerMsg_ServiceWorkerRegistrationError::ID); // Script and scope have a different host but match each other - SendRegister(kProviderId, - GURL("https://foo.example.com/"), - GURL("https://foo.example.com/bar")); - EXPECT_EQ(5, dispatcher_host_->bad_messages_received_count_); + Register(kProviderId, + GURL("https://foo.example.com/"), + GURL("https://foo.example.com/bar"), + ServiceWorkerMsg_ServiceWorkerRegistrationError::ID); // Script and scope URLs are invalid SendRegister(kProviderId, GURL(), GURL("h@ttps://@")); - EXPECT_EQ(6, dispatcher_host_->bad_messages_received_count_); + EXPECT_EQ(1, dispatcher_host_->bad_messages_received_count_); +} + +TEST_F(ServiceWorkerDispatcherHostTest, + Register_FileSystemDocumentShouldFail) { + const int64 kProviderId = 99; // Dummy value + scoped_ptr host( + CreateServiceWorkerProviderHost(kProviderId)); + host->SetDocumentUrl(GURL("filesystem:https://www.example.com/temporary/a")); + context()->AddProviderHost(host.Pass()); + + Register(kProviderId, + GURL("filesystem:https://www.example.com/temporary/"), + GURL("https://www.example.com/temporary/bar"), + ServiceWorkerMsg_ServiceWorkerRegistrationError::ID); + + Register(kProviderId, + GURL("https://www.example.com/temporary/"), + GURL("filesystem:https://www.example.com/temporary/bar"), + ServiceWorkerMsg_ServiceWorkerRegistrationError::ID); + + Register(kProviderId, + GURL("filesystem:https://www.example.com/temporary/"), + GURL("filesystem:https://www.example.com/temporary/bar"), + ServiceWorkerMsg_ServiceWorkerRegistrationError::ID); +} + +TEST_F(ServiceWorkerDispatcherHostTest, + Register_FileSystemScriptOrScopeShouldFail) { + const int64 kProviderId = 99; // Dummy value + scoped_ptr host( + CreateServiceWorkerProviderHost(kProviderId)); + host->SetDocumentUrl(GURL("https://www.example.com/temporary/")); + context()->AddProviderHost(host.Pass()); + + Register(kProviderId, + GURL("filesystem:https://www.example.com/temporary/"), + GURL("https://www.example.com/temporary/bar"), + ServiceWorkerMsg_ServiceWorkerRegistrationError::ID); + + Register(kProviderId, + GURL("https://www.example.com/temporary/"), + GURL("filesystem:https://www.example.com/temporary/bar"), + ServiceWorkerMsg_ServiceWorkerRegistrationError::ID); + + Register(kProviderId, + GURL("filesystem:https://www.example.com/temporary/"), + GURL("filesystem:https://www.example.com/temporary/bar"), + ServiceWorkerMsg_ServiceWorkerRegistrationError::ID); } TEST_F(ServiceWorkerDispatcherHostTest, Unregister_HTTPS) { @@ -316,8 +364,9 @@ TEST_F(ServiceWorkerDispatcherHostTest, Unregister_CrossOriginShouldFail) { host->SetDocumentUrl(GURL("https://www.example.com/foo")); context()->AddProviderHost(host.Pass()); - SendUnregister(kProviderId, GURL("https://foo.example.com/")); - EXPECT_EQ(1, dispatcher_host_->bad_messages_received_count_); + Unregister(kProviderId, + GURL("https://foo.example.com/"), + ServiceWorkerMsg_ServiceWorkerUnregistrationError::ID); } TEST_F(ServiceWorkerDispatcherHostTest, Unregister_InvalidScopeShouldFail) { @@ -338,8 +387,9 @@ TEST_F(ServiceWorkerDispatcherHostTest, Unregister_NonSecureOriginShouldFail) { host->SetDocumentUrl(GURL("http://www.example.com/foo")); context()->AddProviderHost(host.Pass()); - SendUnregister(kProviderId, GURL("http://www.example.com/")); - EXPECT_EQ(1, dispatcher_host_->bad_messages_received_count_); + Unregister(kProviderId, + GURL("http://www.example.com/"), + ServiceWorkerMsg_ServiceWorkerUnregistrationError::ID); } TEST_F(ServiceWorkerDispatcherHostTest, EarlyContextDeletion) { @@ -404,8 +454,9 @@ TEST_F(ServiceWorkerDispatcherHostTest, GetRegistration_CrossOriginShouldFail) { host->SetDocumentUrl(GURL("https://www.example.com/foo")); context()->AddProviderHost(host.Pass()); - SendGetRegistration(kProviderId, GURL("https://foo.example.com/")); - EXPECT_EQ(1, dispatcher_host_->bad_messages_received_count_); + GetRegistration(kProviderId, + GURL("https://foo.example.com/"), + ServiceWorkerMsg_ServiceWorkerGetRegistrationError::ID); } TEST_F(ServiceWorkerDispatcherHostTest, @@ -428,8 +479,9 @@ TEST_F(ServiceWorkerDispatcherHostTest, host->SetDocumentUrl(GURL("http://www.example.com/foo")); context()->AddProviderHost(host.Pass()); - SendGetRegistration(kProviderId, GURL("http://www.example.com/")); - EXPECT_EQ(1, dispatcher_host_->bad_messages_received_count_); + GetRegistration(kProviderId, + GURL("http://www.example.com/"), + ServiceWorkerMsg_ServiceWorkerGetRegistrationError::ID); } TEST_F(ServiceWorkerDispatcherHostTest, GetRegistration_EarlyContextDeletion) {