Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(658)

Unified Diff: content/browser/loader/resource_loader.cc

Issue 1368863002: Set SSL info when an HTTP auth dialog is triggered by direct navigation. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Respond to creis' comments on the other CL. Created 5 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: content/browser/loader/resource_loader.cc
diff --git a/content/browser/loader/resource_loader.cc b/content/browser/loader/resource_loader.cc
index 64f5e7b341ba6c365ebe3e2d7af950448fb0bd83..ccfa48be14a30f8368e6739363091751f2269427 100644
--- a/content/browser/loader/resource_loader.cc
+++ b/content/browser/loader/resource_loader.cc
@@ -6,6 +6,7 @@
#include "base/command_line.h"
#include "base/location.h"
+#include "base/message_loop/message_loop.h"
#include "base/metrics/histogram.h"
#include "base/profiler/scoped_tracker.h"
#include "base/single_thread_task_runner.h"
@@ -31,6 +32,7 @@
#include "content/public/common/process_type.h"
#include "content/public/common/resource_response.h"
#include "content/public/common/security_style.h"
+#include "content/public/common/ssl_status.h"
#include "net/base/io_buffer.h"
#include "net/base/load_flags.h"
#include "net/http/http_response_headers.h"
@@ -44,6 +46,10 @@ using base::TimeTicks;
namespace content {
namespace {
+// Stores the SignedCertificateTimestamps held in |sct_list| in the
+// SignedCertificateTimestampStore singleton, associated with |process_id|.
+// On return, |sct_ids| contains the assigned ID and verification status of
+// each SignedCertificateTimestamp.
void StoreSignedCertificateTimestamps(
const net::SignedCertificateTimestampAndStatusList& sct_list,
int process_id,
@@ -51,10 +57,10 @@ void StoreSignedCertificateTimestamps(
SignedCertificateTimestampStore* sct_store(
SignedCertificateTimestampStore::GetInstance());
- for (auto iter = sct_list.begin(); iter != sct_list.end(); ++iter) {
- const int sct_id(sct_store->Store(iter->sct.get(), process_id));
+ for (const auto& sct : sct_list) {
+ const int sct_id(sct_store->Store(sct.sct.get(), process_id));
sct_ids->push_back(
- SignedCertificateTimestampIDAndStatus(sct_id, iter->status));
+ SignedCertificateTimestampIDAndStatus(sct_id, sct.status));
}
}
@@ -118,6 +124,21 @@ void PopulateResourceResponse(ResourceRequestInfoImpl* info,
}
}
+std::string StoreAndSerializeSecurityInfo(const GURL& url,
+ const net::SSLInfo& ssl_info,
+ int process_id) {
+ DCHECK(ssl_info.cert.get());
+
+ SignedCertificateTimestampIDStatusList signed_certificate_timestamp_ids;
+ StoreSignedCertificateTimestamps(ssl_info.signed_certificate_timestamps,
+ process_id,
+ &signed_certificate_timestamp_ids);
meacer 2015/09/29 00:17:02 Lines 132-135 aren't needed anymore now that this
palmer 2015/09/29 00:46:36 Done.
+
+ SSLStatus status;
+ GetSSLStatusForRequest(url, ssl_info, process_id, &status);
+ return SerializeSecurityInfo(status);
+}
+
} // namespace
ResourceLoader::ResourceLoader(scoped_ptr<net::URLRequest> request,
@@ -273,9 +294,30 @@ void ResourceLoader::OnAuthRequired(net::URLRequest* unused,
return;
}
+ // Update the SSL state before showing the auth prompt.
+ const net::SSLInfo& ssl_info = request_->response_info().ssl_info;
+ if (ssl_info.cert.get()) {
+ bool is_main_frame = (request_->load_flags() & net::LOAD_MAIN_FRAME) != 0;
+ ResourceRequestInfoImpl* info = GetRequestInfo();
+ int render_process_id;
+ int render_frame_id;
+ if (!info->GetAssociatedRenderFrame(&render_process_id, &render_frame_id))
+ CHECK(false);
+
+ SSLStatus status;
+ GetSSLStatusForRequest(request_->url(), ssl_info, render_process_id,
+ &status);
+
+ SSLManager::OnAuthDialog(render_process_id, render_frame_id, status,
+ is_main_frame);
+ } else {
+ // We should not have any SSL state.
+ DCHECK(!ssl_info.cert_status && ssl_info.security_bits == -1 &&
+ !ssl_info.connection_status);
+ }
+
// Create a login dialog on the UI thread to get authentication data, or pull
// from cache and continue on the IO thread.
-
DCHECK(!login_delegate_.get())
<< "OnAuthRequired called with login_delegate pending";
login_delegate_ = delegate_->CreateLoginDelegate(this, auth_info);
@@ -309,7 +351,7 @@ void ResourceLoader::OnSSLCertificateError(net::URLRequest* request,
int render_process_id;
int render_frame_id;
if (!info->GetAssociatedRenderFrame(&render_process_id, &render_frame_id))
- NOTREACHED();
+ CHECK(false);
SSLManager::OnSSLCertificateError(
weak_ptr_factory_.GetWeakPtr(),
@@ -522,7 +564,7 @@ void ResourceLoader::CancelRequestInternal(int error, bool from_renderer) {
// If the request isn't in flight, then we won't get an asynchronous
// notification from the request, so we have to signal ourselves to finish
// this request.
- base::ThreadTaskRunnerHandle::Get()->PostTask(
+ base::MessageLoop::current()->PostTask(
FROM_HERE, base::Bind(&ResourceLoader::ResponseCompleted,
weak_ptr_factory_.GetWeakPtr()));
}
@@ -533,6 +575,27 @@ void ResourceLoader::CompleteResponseStarted() {
scoped_refptr<ResourceResponse> response(new ResourceResponse());
PopulateResourceResponse(info, request_.get(), response.get());
+ if (request_->ssl_info().cert.get()) {
+ // TODO(vadimt): Remove ScopedTracker below once crbug.com/423948 is fixed.
+ tracked_objects::ScopedTracker tracking_profile3(
+ FROM_HERE_WITH_EXPLICIT_FUNCTION(
+ "423948 ResourceLoader::CompleteResponseStarted3"));
+
+ response->head.security_info = StoreAndSerializeSecurityInfo(
+ request_->url(), request_->ssl_info(), info->GetChildID());
+
+ } else {
+ // We should not have any SSL state.
+ DCHECK(!request_->ssl_info().cert_status &&
+ request_->ssl_info().security_bits == -1 &&
+ !request_->ssl_info().connection_status);
+ }
+
+ // TODO(vadimt): Remove ScopedTracker below once crbug.com/423948 is fixed.
+ tracked_objects::ScopedTracker tracking_profile5(
+ FROM_HERE_WITH_EXPLICIT_FUNCTION(
+ "423948 ResourceLoader::CompleteResponseStarted5"));
+
delegate_->DidReceiveResponse(this);
// TODO(darin): Remove ScopedTracker below once crbug.com/475761 is fixed.
@@ -640,13 +703,9 @@ void ResourceLoader::ResponseCompleted() {
std::string security_info;
const net::SSLInfo& ssl_info = request_->ssl_info();
- if (ssl_info.cert.get() != NULL) {
- SSLStatus ssl_status;
- GetSSLStatusForRequest(request_->url(), ssl_info, info->GetChildID(),
- &ssl_status);
-
- security_info = SerializeSecurityInfo(ssl_status);
- }
+ if (ssl_info.cert.get() != NULL)
+ security_info = StoreAndSerializeSecurityInfo(request_->url(), ssl_info,
meacer 2015/09/29 00:17:02 nit: Braces since this code spans two lines?
palmer 2015/09/29 00:46:35 Done.
+ info->GetChildID());
bool defer = false;
{

Powered by Google App Engine
This is Rietveld 408576698