diff options
author | pkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-09-22 18:46:15 +0000 |
---|---|---|
committer | pkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-09-22 18:46:15 +0000 |
commit | 653dc46f668da5aed227aafe39ec66fada3df230 (patch) | |
tree | 8e727f8d7cfc758b8ca32a06947c161aea05d8fd | |
parent | 59f994ca94ee8bcb4e87467eed3d3f50b358fd5a (diff) | |
download | chromium_src-653dc46f668da5aed227aafe39ec66fada3df230.zip chromium_src-653dc46f668da5aed227aafe39ec66fada3df230.tar.gz chromium_src-653dc46f668da5aed227aafe39ec66fada3df230.tar.bz2 |
For the SSL cert status, convert anonymous enum that gives bit values into a typedefed uint32. This allows code all over Chromium to use an explicit type instead of "int". (This isn't possible by simply naming the enum as technically the enum doesn't define all of the possible combinations of bits.) This also means the individual named bit constants themselves have the same explicit type. I find the resulting code to be noticeably clearer. This also exposed a bug in SSLErrorInfo::GetErrorsForCertStatus() where not having an explicit type allowed a function argument ordering bug to creep in, so I claim this is safer too.
I also added CERT_STATUS_NO_ERROR in place of "0" as a magic number.
Normally this makes things like DCHECK_EQ() unhappy, but when I'd originally tested this I didn't seem to need to make any changes due to that. Will be watching the trybots...
The original motiviation for this change was to find a way to eliminate some cases of passing anonymous-typed values as template arguments (which happens when you use a value from the enum in e.g. EXPECT_EQ()), which is technically illegal in C++03, though we don't warn about it. Simply naming the enum would have done this, but this would have encouraged readers to actually use the enum name as a type, which for a bitfield is inappropriate for the reason given in the first paragraph.
BUG=92247
TEST=Compiles
Review URL: http://codereview.chromium.org/7819009
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@102322 0039d316-1c4b-4281-b951-d872f2087c98
41 files changed, 152 insertions, 121 deletions
diff --git a/chrome/browser/automation/testing_automation_provider.cc b/chrome/browser/automation/testing_automation_provider.cc index 71f1eac..5f12eef 100644 --- a/chrome/browser/automation/testing_automation_provider.cc +++ b/chrome/browser/automation/testing_automation_provider.cc @@ -1480,11 +1480,12 @@ void TestingAutomationProvider::WaitForTabToBeRestored( } } -void TestingAutomationProvider::GetSecurityState(int handle, - bool* success, - SecurityStyle* security_style, - int* ssl_cert_status, - int* insecure_content_status) { +void TestingAutomationProvider::GetSecurityState( + int handle, + bool* success, + SecurityStyle* security_style, + net::CertStatus* ssl_cert_status, + int* insecure_content_status) { if (tab_tracker_->ContainsHandle(handle)) { NavigationController* tab = tab_tracker_->GetResource(handle); NavigationEntry* entry = tab->GetActiveEntry(); diff --git a/chrome/browser/automation/testing_automation_provider.h b/chrome/browser/automation/testing_automation_provider.h index 06a2b1a..61b8a55 100644 --- a/chrome/browser/automation/testing_automation_provider.h +++ b/chrome/browser/automation/testing_automation_provider.h @@ -19,6 +19,7 @@ #include "chrome/browser/ui/browser_list.h" #include "content/common/notification_registrar.h" #include "content/common/page_type.h" +#include "net/base/cert_status_flags.h" #include "third_party/WebKit/Source/WebKit/chromium/public/WebInputEvent.h" class ImporterList; @@ -196,8 +197,10 @@ class TestingAutomationProvider : public AutomationProvider, void WaitForTabToBeRestored(int tab_handle, IPC::Message* reply_message); // Gets the security state for the tab associated to the specified |handle|. - void GetSecurityState(int handle, bool* success, - SecurityStyle* security_style, int* ssl_cert_status, + void GetSecurityState(int handle, + bool* success, + SecurityStyle* security_style, + net::CertStatus* ssl_cert_status, int* insecure_content_status); // Gets the page type for the tab associated to the specified |handle|. diff --git a/chrome/browser/page_info_model.cc b/chrome/browser/page_info_model.cc index 4395e50..9e4394d 100644 --- a/chrome/browser/page_info_model.cc +++ b/chrome/browser/page_info_model.cc @@ -68,7 +68,7 @@ PageInfoModel::PageInfoModel(Profile* profile, CertStore::GetInstance()->RetrieveCert(ssl.cert_id(), &cert) && !net::IsCertStatusError(status_with_warnings_removed)) { // No error found so far, check cert_status warnings. - int cert_status = ssl.cert_status(); + net::CertStatus cert_status = ssl.cert_status(); if (cert_status & cert_warnings) { string16 issuer_name(UTF8ToUTF16(cert->issuer().GetDisplayName())); if (issuer_name.empty()) { @@ -89,7 +89,7 @@ PageInfoModel::PageInfoModel(Profile* profile, NOTREACHED() << "Need to specify string for this warning"; } icon_id = ICON_STATE_WARNING_MINOR; - } else if ((ssl.cert_status() & net::CERT_STATUS_IS_EV) != 0) { + } else if (ssl.cert_status() & net::CERT_STATUS_IS_EV) { // EV HTTPS page. DCHECK(!cert->subject().organization_names.empty()); headline = @@ -119,7 +119,7 @@ PageInfoModel::PageInfoModel(Profile* profile, UTF8ToUTF16(cert->subject().organization_names[0]), locality, UTF8ToUTF16(cert->issuer().GetDisplayName()))); - } else if ((ssl.cert_status() & net::CERT_STATUS_IS_DNSSEC) != 0) { + } else if (ssl.cert_status() & net::CERT_STATUS_IS_DNSSEC) { // DNSSEC authenticated page. if (empty_subject_name) headline.clear(); // Don't display any title. diff --git a/chrome/browser/ssl/ssl_browser_tests.cc b/chrome/browser/ssl/ssl_browser_tests.cc index 59b3712..cf21e5f 100644 --- a/chrome/browser/ssl/ssl_browser_tests.cc +++ b/chrome/browser/ssl/ssl_browser_tests.cc @@ -66,7 +66,7 @@ class SSLUITest : public InProcessBrowserTest { } void CheckAuthenticationBrokenState(TabContents* tab, - int error, + net::CertStatus error, bool ran_insecure_content, bool interstitial) { NavigationEntry* entry = tab->controller().GetActiveEntry(); diff --git a/chrome/browser/ssl/ssl_error_info.cc b/chrome/browser/ssl/ssl_error_info.cc index 63555d4..5eb1927 100644 --- a/chrome/browser/ssl/ssl_error_info.cc +++ b/chrome/browser/ssl/ssl_error_info.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -231,10 +231,10 @@ SSLErrorInfo::ErrorType SSLErrorInfo::NetErrorToErrorType(int net_error) { // static int SSLErrorInfo::GetErrorsForCertStatus(int cert_id, - int cert_status, + net::CertStatus cert_status, const GURL& url, std::vector<SSLErrorInfo>* errors) { - const int kErrorFlags[] = { + const net::CertStatus kErrorFlags[] = { net::CERT_STATUS_COMMON_NAME_INVALID, net::CERT_STATUS_DATE_INVALID, net::CERT_STATUS_AUTHORITY_INVALID, diff --git a/chrome/browser/ssl/ssl_error_info.h b/chrome/browser/ssl/ssl_error_info.h index ca87a0b4..5ef19f1 100644 --- a/chrome/browser/ssl/ssl_error_info.h +++ b/chrome/browser/ssl/ssl_error_info.h @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -10,6 +10,7 @@ #include <vector> #include "base/string16.h" +#include "net/base/cert_status_flags.h" #include "net/base/x509_certificate.h" class GURL; @@ -46,9 +47,10 @@ class SSLErrorInfo { // Populates the specified |errors| vector with the errors contained in // |cert_status|. Returns the number of errors found. // Callers only interested in the error count can pass NULL for |errors|. - static int GetErrorsForCertStatus(int cert_status, - int cert_id, - const GURL& request_url, + // TODO(wtc): Document |cert_id| and |url| arguments. + static int GetErrorsForCertStatus(int cert_id, + net::CertStatus cert_status, + const GURL& url, std::vector<SSLErrorInfo>* errors); // A title describing the error, usually to be used with the details below. diff --git a/chrome/browser/tab_contents/render_view_context_menu.cc b/chrome/browser/tab_contents/render_view_context_menu.cc index 06f3155..25142d3 100644 --- a/chrome/browser/tab_contents/render_view_context_menu.cc +++ b/chrome/browser/tab_contents/render_view_context_menu.cc @@ -1665,7 +1665,10 @@ void RenderViewContextMenu::ExecuteCommand(int id, int event_flags) { // Deserialize the SSL info. NavigationEntry::SSLStatus ssl; if (!params_.security_info.empty()) { - int cert_id, cert_status, security_bits, connection_status; + int cert_id; + net::CertStatus cert_status; + int security_bits; + int connection_status; SSLManager::DeserializeSecurityInfo(params_.security_info, &cert_id, &cert_status, diff --git a/chrome/browser/ui/toolbar/toolbar_model.cc b/chrome/browser/ui/toolbar/toolbar_model.cc index a7284d2..6716e73 100644 --- a/chrome/browser/ui/toolbar/toolbar_model.cc +++ b/chrome/browser/ui/toolbar/toolbar_model.cc @@ -86,8 +86,8 @@ ToolbarModel::SecurityLevel ToolbarModel::GetSecurityLevel() const { if (ssl.displayed_insecure_content()) return SECURITY_WARNING; if (net::IsCertStatusError(ssl.cert_status())) { - DCHECK_EQ(ssl.cert_status() & net::CERT_STATUS_ALL_ERRORS, - net::CERT_STATUS_UNABLE_TO_CHECK_REVOCATION); + DCHECK_EQ(net::CERT_STATUS_UNABLE_TO_CHECK_REVOCATION, + ssl.cert_status() & net::CERT_STATUS_ALL_ERRORS); return SECURITY_WARNING; } if ((ssl.cert_status() & net::CERT_STATUS_IS_EV) && diff --git a/chrome/common/automation_messages_internal.h b/chrome/common/automation_messages_internal.h index 55020c6..17e1def 100644 --- a/chrome/common/automation_messages_internal.h +++ b/chrome/common/automation_messages_internal.h @@ -12,6 +12,7 @@ #include "content/common/navigation_types.h" #include "googleurl/src/gurl.h" #include "ipc/ipc_message_macros.h" +#include "net/base/cert_status_flags.h" #include "net/url_request/url_request_status.h" #include "ui/gfx/rect.h" #include "webkit/glue/window_open_disposition.h" @@ -552,15 +553,15 @@ IPC_MESSAGE_ROUTED1(AutomationMsg_DidNavigate, // Response: // - bool: whether the operation was successful. // - SecurityStyle: the security style of the tab. -// - int: the status of the server's ssl cert (0 means no errors or no ssl -// was used). +// - net::CertStatus: the status of the server's ssl cert (0 means no errors or +// no ssl was used). // - int: the insecure content state, 0 means no insecure contents. IPC_SYNC_MESSAGE_CONTROL1_4(AutomationMsg_GetSecurityState, int, bool, SecurityStyle, - int, + net::CertStatus, int) // This message requests the page type of the page displayed in the specified diff --git a/chrome/test/automation/tab_proxy.cc b/chrome/test/automation/tab_proxy.cc index d45fe9a..0a2a5c1 100644 --- a/chrome/test/automation/tab_proxy.cc +++ b/chrome/test/automation/tab_proxy.cc @@ -571,7 +571,7 @@ bool TabProxy::WaitForTabToBeRestored(uint32 timeout_ms) { } bool TabProxy::GetSecurityState(SecurityStyle* security_style, - int* ssl_cert_status, + net::CertStatus* ssl_cert_status, int* insecure_content_status) { DCHECK(security_style && ssl_cert_status && insecure_content_status); diff --git a/chrome/test/automation/tab_proxy.h b/chrome/test/automation/tab_proxy.h index 26d3ea3..a92f422 100644 --- a/chrome/test/automation/tab_proxy.h +++ b/chrome/test/automation/tab_proxy.h @@ -282,7 +282,7 @@ class TabProxy : public AutomationResourceProxy, // Retrieves the different security states for the current tab. bool GetSecurityState(SecurityStyle* security_style, - int* ssl_cert_status, + net::CertStatus* ssl_cert_status, int* insecure_content_status) WARN_UNUSED_RESULT; // Returns the type of the page currently showing (normal, interstitial, diff --git a/content/browser/load_from_memory_cache_details.cc b/content/browser/load_from_memory_cache_details.cc index 8574112..ed80587 100644 --- a/content/browser/load_from_memory_cache_details.cc +++ b/content/browser/load_from_memory_cache_details.cc @@ -8,7 +8,7 @@ LoadFromMemoryCacheDetails::LoadFromMemoryCacheDetails( const GURL& url, int pid, int cert_id, - int cert_status) + net::CertStatus cert_status) : url_(url), pid_(pid), cert_id_(cert_id), diff --git a/content/browser/load_from_memory_cache_details.h b/content/browser/load_from_memory_cache_details.h index 0689792..4524014 100644 --- a/content/browser/load_from_memory_cache_details.h +++ b/content/browser/load_from_memory_cache_details.h @@ -8,6 +8,7 @@ #include "base/basictypes.h" #include "googleurl/src/gurl.h" +#include "net/base/cert_status_flags.h" class LoadFromMemoryCacheDetails { public: @@ -15,19 +16,19 @@ class LoadFromMemoryCacheDetails { const GURL& url, int pid, int cert_id, - int cert_status); + net::CertStatus cert_status); ~LoadFromMemoryCacheDetails(); const GURL& url() const { return url_; } int pid() const { return pid_; } int ssl_cert_id() const { return cert_id_; } - int ssl_cert_status() const { return cert_status_; } + net::CertStatus ssl_cert_status() const { return cert_status_; } private: GURL url_; int pid_; int cert_id_; - int cert_status_; + net::CertStatus cert_status_; DISALLOW_COPY_AND_ASSIGN(LoadFromMemoryCacheDetails); }; diff --git a/content/browser/renderer_host/resource_request_details.h b/content/browser/renderer_host/resource_request_details.h index f76b326..8bc070b 100644 --- a/content/browser/renderer_host/resource_request_details.h +++ b/content/browser/renderer_host/resource_request_details.h @@ -13,6 +13,7 @@ #include <string> #include "googleurl/src/gurl.h" +#include "net/base/cert_status_flags.h" #include "net/base/host_port_pair.h" #include "net/url_request/url_request_status.h" #include "webkit/glue/resource_type.h" @@ -37,7 +38,7 @@ class ResourceRequestDetails { int origin_child_id() const { return origin_child_id_; } const net::URLRequestStatus& status() const { return status_; } int ssl_cert_id() const { return ssl_cert_id_; } - int ssl_cert_status() const { return ssl_cert_status_; } + net::CertStatus ssl_cert_status() const { return ssl_cert_status_; } ResourceType::Type resource_type() const { return resource_type_; } net::HostPortPair socket_address() const { return socket_address_; } @@ -51,7 +52,7 @@ class ResourceRequestDetails { int origin_child_id_; net::URLRequestStatus status_; int ssl_cert_id_; - int ssl_cert_status_; + net::CertStatus ssl_cert_status_; ResourceType::Type resource_type_; net::HostPortPair socket_address_; }; diff --git a/content/browser/ssl/ssl_manager.cc b/content/browser/ssl/ssl_manager.cc index 2f88040..6ae6fb9 100644 --- a/content/browser/ssl/ssl_manager.cc +++ b/content/browser/ssl/ssl_manager.cc @@ -55,12 +55,12 @@ void SSLManager::NotifySSLInternalStateChanged( // static std::string SSLManager::SerializeSecurityInfo(int cert_id, - int cert_status, + net::CertStatus cert_status, int security_bits, int ssl_connection_status) { Pickle pickle; pickle.WriteInt(cert_id); - pickle.WriteInt(cert_status); + pickle.WriteUInt32(cert_status); pickle.WriteInt(security_bits); pickle.WriteInt(ssl_connection_status); return std::string(static_cast<const char*>(pickle.data()), pickle.size()); @@ -69,7 +69,7 @@ std::string SSLManager::SerializeSecurityInfo(int cert_id, // static bool SSLManager::DeserializeSecurityInfo(const std::string& state, int* cert_id, - int* cert_status, + net::CertStatus* cert_status, int* security_bits, int* ssl_connection_status) { DCHECK(cert_id && cert_status && security_bits && ssl_connection_status); @@ -86,7 +86,7 @@ bool SSLManager::DeserializeSecurityInfo(const std::string& state, Pickle pickle(state.data(), static_cast<int>(state.size())); void * iter = NULL; return pickle.ReadInt(&iter, cert_id) && - pickle.ReadInt(&iter, cert_status) && + pickle.ReadUInt32(&iter, cert_status) && pickle.ReadInt(&iter, security_bits) && pickle.ReadInt(&iter, ssl_connection_status); } @@ -124,8 +124,10 @@ void SSLManager::DidCommitProvisionalLoad( if (details->is_main_frame) { if (entry) { // Decode the security details. - int ssl_cert_id, ssl_cert_status, ssl_security_bits, - ssl_connection_status; + int ssl_cert_id; + net::CertStatus ssl_cert_status; + int ssl_security_bits; + int ssl_connection_status; DeserializeSecurityInfo(details->serialized_security_info, &ssl_cert_id, &ssl_cert_status, diff --git a/content/browser/ssl/ssl_manager.h b/content/browser/ssl/ssl_manager.h index 1e59e7b..24892bf 100644 --- a/content/browser/ssl/ssl_manager.h +++ b/content/browser/ssl/ssl_manager.h @@ -15,6 +15,7 @@ #include "content/common/notification_observer.h" #include "content/common/notification_registrar.h" #include "googleurl/src/gurl.h" +#include "net/base/cert_status_flags.h" #include "net/base/net_errors.h" class LoadFromMemoryCacheDetails; @@ -57,14 +58,15 @@ class SSLManager : public NotificationObserver { // Convenience methods for serializing/deserializing the security info. static std::string SerializeSecurityInfo(int cert_id, - int cert_status, + net::CertStatus cert_status, int security_bits, int connection_status); - CONTENT_EXPORT static bool DeserializeSecurityInfo(const std::string& state, - int* cert_id, - int* cert_status, - int* security_bits, - int* connection_status); + CONTENT_EXPORT static bool DeserializeSecurityInfo( + const std::string& state, + int* cert_id, + net::CertStatus* cert_status, + int* security_bits, + int* connection_status); // Construct an SSLManager for the specified tab. // If |delegate| is NULL, SSLPolicy::GetDefaultPolicy() is used. diff --git a/content/browser/ssl/ssl_policy.cc b/content/browser/ssl/ssl_policy.cc index 1d7c981..1d9f3f6 100644 --- a/content/browser/ssl/ssl_policy.cc +++ b/content/browser/ssl/ssl_policy.cc @@ -131,7 +131,8 @@ void SSLPolicy::UpdateEntry(NavigationEntry* entry, TabContents* tab_contents) { // If CERT_STATUS_UNABLE_TO_CHECK_REVOCATION is the only certificate error, // don't lower the security style to SECURITY_STYLE_AUTHENTICATION_BROKEN. - int cert_errors = entry->ssl().cert_status() & net::CERT_STATUS_ALL_ERRORS; + net::CertStatus cert_errors = + entry->ssl().cert_status() & net::CERT_STATUS_ALL_ERRORS; if (cert_errors) { if (cert_errors != net::CERT_STATUS_UNABLE_TO_CHECK_REVOCATION) entry->ssl().set_security_style(SECURITY_STYLE_AUTHENTICATION_BROKEN); diff --git a/content/browser/ssl/ssl_request_info.cc b/content/browser/ssl/ssl_request_info.cc index 19d4e4d..b999799 100644 --- a/content/browser/ssl/ssl_request_info.cc +++ b/content/browser/ssl/ssl_request_info.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -8,7 +8,7 @@ SSLRequestInfo::SSLRequestInfo(const GURL& url, ResourceType::Type resource_type, int child_id, int ssl_cert_id, - int ssl_cert_status) + net::CertStatus ssl_cert_status) : url_(url), resource_type_(resource_type), child_id_(child_id), diff --git a/content/browser/ssl/ssl_request_info.h b/content/browser/ssl/ssl_request_info.h index 1ab433d..a7f9fbe 100644 --- a/content/browser/ssl/ssl_request_info.h +++ b/content/browser/ssl/ssl_request_info.h @@ -10,6 +10,7 @@ #include "base/memory/ref_counted.h" #include "googleurl/src/gurl.h" +#include "net/base/cert_status_flags.h" #include "webkit/glue/resource_type.h" // SSLRequestInfo wraps up the information SSLPolicy needs about a request in @@ -21,13 +22,13 @@ class SSLRequestInfo : public base::RefCounted<SSLRequestInfo> { ResourceType::Type resource_type, int child_id, int ssl_cert_id, - int ssl_cert_status); + net::CertStatus ssl_cert_status); const GURL& url() const { return url_; } ResourceType::Type resource_type() const { return resource_type_; } int child_id() const { return child_id_; } int ssl_cert_id() const { return ssl_cert_id_; } - int ssl_cert_status() const { return ssl_cert_status_; } + net::CertStatus ssl_cert_status() const { return ssl_cert_status_; } private: friend class base::RefCounted<SSLRequestInfo>; @@ -38,7 +39,7 @@ class SSLRequestInfo : public base::RefCounted<SSLRequestInfo> { ResourceType::Type resource_type_; int child_id_; int ssl_cert_id_; - int ssl_cert_status_; + net::CertStatus ssl_cert_status_; DISALLOW_COPY_AND_ASSIGN(SSLRequestInfo); }; diff --git a/content/browser/tab_contents/navigation_entry.h b/content/browser/tab_contents/navigation_entry.h index 2ed55b8..40bc8a8 100644 --- a/content/browser/tab_contents/navigation_entry.h +++ b/content/browser/tab_contents/navigation_entry.h @@ -15,6 +15,7 @@ #include "content/common/page_type.h" #include "content/common/security_style.h" #include "googleurl/src/gurl.h" +#include "net/base/cert_status_flags.h" #include "third_party/skia/include/core/SkBitmap.h" class SiteInstance; @@ -75,10 +76,10 @@ class CONTENT_EXPORT NavigationEntry { return cert_id_; } - void set_cert_status(int ssl_cert_status) { + void set_cert_status(net::CertStatus ssl_cert_status) { cert_status_ = ssl_cert_status; } - int cert_status() const { + net::CertStatus cert_status() const { return cert_status_; } @@ -125,7 +126,7 @@ class CONTENT_EXPORT NavigationEntry { // See the accessors above for descriptions. SecurityStyle security_style_; int cert_id_; - int cert_status_; + net::CertStatus cert_status_; int security_bits_; int connection_status_; int content_status_; diff --git a/content/browser/tab_contents/navigation_entry_unittest.cc b/content/browser/tab_contents/navigation_entry_unittest.cc index ee7d083..a7b5ff9 100644 --- a/content/browser/tab_contents/navigation_entry_unittest.cc +++ b/content/browser/tab_contents/navigation_entry_unittest.cc @@ -113,13 +113,14 @@ TEST_F(NavigationEntryTest, NavigationEntrySSLStatus) { // Change from the defaults entry2_.get()->ssl().set_security_style(SECURITY_STYLE_AUTHENTICATED); entry2_.get()->ssl().set_cert_id(4); - entry2_.get()->ssl().set_cert_status(1); + entry2_.get()->ssl().set_cert_status(net::CERT_STATUS_COMMON_NAME_INVALID); entry2_.get()->ssl().set_security_bits(0); entry2_.get()->ssl().set_displayed_insecure_content(); EXPECT_EQ(SECURITY_STYLE_AUTHENTICATED, entry2_.get()->ssl().security_style()); EXPECT_EQ(4, entry2_.get()->ssl().cert_id()); - EXPECT_EQ(1, entry2_.get()->ssl().cert_status()); + EXPECT_EQ(net::CERT_STATUS_COMMON_NAME_INVALID, + entry2_.get()->ssl().cert_status()); EXPECT_EQ(0, entry2_.get()->ssl().security_bits()); EXPECT_TRUE(entry2_.get()->ssl().displayed_insecure_content()); diff --git a/content/browser/tab_contents/provisional_load_details.h b/content/browser/tab_contents/provisional_load_details.h index 0e8331f..87d6569 100644 --- a/content/browser/tab_contents/provisional_load_details.h +++ b/content/browser/tab_contents/provisional_load_details.h @@ -11,6 +11,7 @@ #include "base/basictypes.h" #include "content/common/page_transition_types.h" #include "googleurl/src/gurl.h" +#include "net/base/cert_status_flags.h" // This class captures some of the information associated to the provisional // load of a frame. It is provided as Details with the @@ -48,7 +49,7 @@ class ProvisionalLoadDetails { int ssl_cert_id() const { return ssl_cert_id_; } - int ssl_cert_status() const { return ssl_cert_status_; } + net::CertStatus ssl_cert_status() const { return ssl_cert_status_; } int ssl_security_bits() const { return ssl_security_bits_; } @@ -65,7 +66,7 @@ class ProvisionalLoadDetails { bool is_main_frame_; bool is_in_page_navigation_; int ssl_cert_id_; - int ssl_cert_status_; + net::CertStatus ssl_cert_status_; int ssl_security_bits_; int ssl_connection_status_; bool is_error_page_; diff --git a/content/browser/tab_contents/tab_contents.cc b/content/browser/tab_contents/tab_contents.cc index 287e661..9e80c93 100644 --- a/content/browser/tab_contents/tab_contents.cc +++ b/content/browser/tab_contents/tab_contents.cc @@ -1030,7 +1030,10 @@ void TabContents::OnDidLoadResourceFromMemoryCache( cache.Increment(); // Send out a notification that we loaded a resource from our memory cache. - int cert_id = 0, cert_status = 0, security_bits = -1, connection_status = 0; + int cert_id = 0; + net::CertStatus cert_status = 0; + int security_bits = -1; + int connection_status = 0; SSLManager::DeserializeSecurityInfo(security_info, &cert_id, &cert_status, &security_bits, diff --git a/net/base/cert_status_flags.cc b/net/base/cert_status_flags.cc index a6bdce4..c34eb1b 100644 --- a/net/base/cert_status_flags.cc +++ b/net/base/cert_status_flags.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -9,7 +9,7 @@ namespace net { -int MapNetErrorToCertStatus(int error) { +CertStatus MapNetErrorToCertStatus(int error) { switch (error) { case ERR_CERT_COMMON_NAME_INVALID: return CERT_STATUS_COMMON_NAME_INVALID; @@ -40,7 +40,7 @@ int MapNetErrorToCertStatus(int error) { } } -int MapCertStatusToNetError(int cert_status) { +int MapCertStatusToNetError(CertStatus cert_status) { // A certificate may have multiple errors. We report the most // serious error. diff --git a/net/base/cert_status_flags.h b/net/base/cert_status_flags.h index 5303af1..7ad90be 100644 --- a/net/base/cert_status_flags.h +++ b/net/base/cert_status_flags.h @@ -1,4 +1,4 @@ -// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -6,45 +6,49 @@ #define NET_BASE_CERT_STATUS_FLAGS_H_ #pragma once +#include "base/basictypes.h" + namespace net { -// Status flags, such as errors and extended validation. -enum { - // Bits 0 to 15 are for errors. - CERT_STATUS_ALL_ERRORS = 0xFFFF, - CERT_STATUS_COMMON_NAME_INVALID = 1 << 0, - CERT_STATUS_DATE_INVALID = 1 << 1, - CERT_STATUS_AUTHORITY_INVALID = 1 << 2, - // 1 << 3 is reserved for ERR_CERT_CONTAINS_ERRORS (not useful with WinHTTP). - CERT_STATUS_NO_REVOCATION_MECHANISM = 1 << 4, - CERT_STATUS_UNABLE_TO_CHECK_REVOCATION = 1 << 5, - CERT_STATUS_REVOKED = 1 << 6, - CERT_STATUS_INVALID = 1 << 7, - CERT_STATUS_WEAK_SIGNATURE_ALGORITHM = 1 << 8, - CERT_STATUS_NOT_IN_DNS = 1 << 9, - CERT_STATUS_NON_UNIQUE_NAME = 1 << 10, - - // Bits 16 to 30 are for non-error statuses. - CERT_STATUS_IS_EV = 1 << 16, - CERT_STATUS_REV_CHECKING_ENABLED = 1 << 17, - CERT_STATUS_IS_DNSSEC = 1 << 18, - - // 1 << 31 (the sign bit) is reserved so that the cert status will never be - // negative. -}; +// Bitmask of status flags of a certificate, representing any errors, as well as +// other non-error status information such as whether the certificate is EV. +typedef uint32 CertStatus; + +// The possible status bits for CertStatus. +// NOTE: Because these names have appeared in bug reports, we preserve them as +// MACRO_STYLE for continuity, instead of renaming them to kConstantStyle as +// befits most static consts. +// Bits 0 to 15 are for errors. +static const CertStatus CERT_STATUS_ALL_ERRORS = 0xFFFF; +static const CertStatus CERT_STATUS_COMMON_NAME_INVALID = 1 << 0; +static const CertStatus CERT_STATUS_DATE_INVALID = 1 << 1; +static const CertStatus CERT_STATUS_AUTHORITY_INVALID = 1 << 2; +// 1 << 3 is reserved for ERR_CERT_CONTAINS_ERRORS (not useful with WinHTTP). +static const CertStatus CERT_STATUS_NO_REVOCATION_MECHANISM = 1 << 4; +static const CertStatus CERT_STATUS_UNABLE_TO_CHECK_REVOCATION = 1 << 5; +static const CertStatus CERT_STATUS_REVOKED = 1 << 6; +static const CertStatus CERT_STATUS_INVALID = 1 << 7; +static const CertStatus CERT_STATUS_WEAK_SIGNATURE_ALGORITHM = 1 << 8; +static const CertStatus CERT_STATUS_NOT_IN_DNS = 1 << 9; +static const CertStatus CERT_STATUS_NON_UNIQUE_NAME = 1 << 10; + +// Bits 16 to 31 are for non-error statuses. +static const CertStatus CERT_STATUS_IS_EV = 1 << 16; +static const CertStatus CERT_STATUS_REV_CHECKING_ENABLED = 1 << 17; +static const CertStatus CERT_STATUS_IS_DNSSEC = 1 << 18; // Returns true if the specified cert status has an error set. -static inline bool IsCertStatusError(int status) { +static inline bool IsCertStatusError(CertStatus status) { return (CERT_STATUS_ALL_ERRORS & status) != 0; } // Maps a network error code to the equivalent certificate status flag. If // the error code is not a certificate error, it is mapped to 0. -int MapNetErrorToCertStatus(int error); +CertStatus MapNetErrorToCertStatus(int error); // Maps the most serious certificate error in the certificate status flags // to the equivalent network error code. -int MapCertStatusToNetError(int cert_status); +int MapCertStatusToNetError(CertStatus cert_status); } // namespace net diff --git a/net/base/cert_verify_result.h b/net/base/cert_verify_result.h index aa65500..e038b57 100644 --- a/net/base/cert_verify_result.h +++ b/net/base/cert_verify_result.h @@ -8,6 +8,7 @@ #include <vector> +#include "net/base/cert_status_flags.h" #include "net/base/net_export.h" #include "base/memory/ref_counted.h" #include "net/base/x509_cert_types.h" @@ -36,7 +37,7 @@ class NET_EXPORT CertVerifyResult { // these status flags apply to the certificate chain returned in // |verified_cert|, rather than the originally supplied certificate // chain. - int cert_status; + CertStatus cert_status; // Properties of the certificate chain. bool has_md5; diff --git a/net/base/ssl_config_service.cc b/net/base/ssl_config_service.cc index 29e1b79..8631bc9 100644 --- a/net/base/ssl_config_service.cc +++ b/net/base/ssl_config_service.cc @@ -26,7 +26,7 @@ SSLConfig::~SSLConfig() { } bool SSLConfig::IsAllowedBadCert(X509Certificate* cert, - int* cert_status) const { + CertStatus* cert_status) const { std::string der_cert; if (!cert->GetDEREncoded(&der_cert)) return false; @@ -34,7 +34,7 @@ bool SSLConfig::IsAllowedBadCert(X509Certificate* cert, } bool SSLConfig::IsAllowedBadCert(const base::StringPiece& der_cert, - int* cert_status) const { + CertStatus* cert_status) const { for (size_t i = 0; i < allowed_bad_certs.size(); ++i) { if (der_cert == allowed_bad_certs[i].der_cert) { if (cert_status) diff --git a/net/base/ssl_config_service.h b/net/base/ssl_config_service.h index b5c4a54..3e32587 100644 --- a/net/base/ssl_config_service.h +++ b/net/base/ssl_config_service.h @@ -12,6 +12,7 @@ #include "base/memory/ref_counted.h" #include "base/observer_list.h" #include "base/string_piece.h" +#include "net/base/cert_status_flags.h" #include "net/base/net_export.h" #include "net/base/x509_certificate.h" @@ -27,12 +28,12 @@ struct NET_EXPORT SSLConfig { // Returns true if |cert| is one of the certs in |allowed_bad_certs|. // The expected cert status is written to |cert_status|. |*cert_status| can // be NULL if user doesn't care about the cert status. - bool IsAllowedBadCert(X509Certificate* cert, int* cert_status) const; + bool IsAllowedBadCert(X509Certificate* cert, CertStatus* cert_status) const; // Same as above except works with DER encoded certificates instead // of X509Certificate. bool IsAllowedBadCert(const base::StringPiece& der_cert, - int* cert_status) const; + CertStatus* cert_status) const; bool rev_checking_enabled; // True if server certificate revocation // checking is enabled. @@ -77,7 +78,7 @@ struct NET_EXPORT SSLConfig { ~CertAndStatus(); std::string der_cert; - int cert_status; + CertStatus cert_status; }; // Add any known-bad SSL certificate (with its cert status) to diff --git a/net/base/ssl_info.h b/net/base/ssl_info.h index 9adc76d..369784f 100644 --- a/net/base/ssl_info.h +++ b/net/base/ssl_info.h @@ -9,6 +9,7 @@ #include <vector> #include "base/memory/ref_counted.h" +#include "net/base/cert_status_flags.h" #include "net/base/net_export.h" #include "net/base/x509_cert_types.h" @@ -46,7 +47,7 @@ class NET_EXPORT SSLInfo { // Bitmask of status info of |cert|, representing, for example, known errors // and extended validation (EV) status. // See cert_status_flags.h for values. - int cert_status; + CertStatus cert_status; // The security strength, in bits, of the SSL cipher suite. // 0 means the connection is not encrypted. diff --git a/net/base/x509_certificate_mac.cc b/net/base/x509_certificate_mac.cc index a83e22a..2c95981 100644 --- a/net/base/x509_certificate_mac.cc +++ b/net/base/x509_certificate_mac.cc @@ -56,7 +56,7 @@ int NetErrorFromOSStatus(OSStatus status) { } } -int CertStatusFromOSStatus(OSStatus status) { +CertStatus CertStatusFromOSStatus(OSStatus status) { switch (status) { case noErr: return 0; diff --git a/net/base/x509_certificate_nss.cc b/net/base/x509_certificate_nss.cc index 3b23f93..c226132 100644 --- a/net/base/x509_certificate_nss.cc +++ b/net/base/x509_certificate_nss.cc @@ -172,7 +172,7 @@ int MapSecurityError(int err) { } // Map PORT_GetError() return values to our cert status flags. -int MapCertErrorToCertStatus(int err) { +CertStatus MapCertErrorToCertStatus(int err) { switch (err) { case SSL_ERROR_BAD_CERT_DOMAIN: return CERT_STATUS_COMMON_NAME_INVALID; @@ -952,9 +952,9 @@ int X509Certificate::VerifyInternal(const std::string& hostname, // CERT_PKIXVerifyCert rerports the wrong error code for // expired certificates (NSS bug 491174) if (err == SEC_ERROR_CERT_NOT_VALID && - (verify_result->cert_status & CERT_STATUS_DATE_INVALID) != 0) + (verify_result->cert_status & CERT_STATUS_DATE_INVALID)) err = SEC_ERROR_EXPIRED_CERTIFICATE; - int cert_status = MapCertErrorToCertStatus(err); + CertStatus cert_status = MapCertErrorToCertStatus(err); if (cert_status) { verify_result->cert_status |= cert_status; return MapCertStatusToNetError(verify_result->cert_status); diff --git a/net/base/x509_certificate_openssl.cc b/net/base/x509_certificate_openssl.cc index c824dc3..0092577 100644 --- a/net/base/x509_certificate_openssl.cc +++ b/net/base/x509_certificate_openssl.cc @@ -134,7 +134,7 @@ void ParseSubjectAltName(X509Certificate::OSCertHandle cert, } // Maps X509_STORE_CTX_get_error() return values to our cert status flags. -int MapCertErrorToCertStatus(int err) { +CertStatus MapCertErrorToCertStatus(int err) { switch (err) { case X509_V_ERR_SUBJECT_ISSUER_MISMATCH: return CERT_STATUS_COMMON_NAME_INVALID; @@ -463,7 +463,7 @@ int X509Certificate::VerifyInternal(const std::string& hostname, if (X509_verify_cert(ctx.get()) != 1) { int x509_error = X509_STORE_CTX_get_error(ctx.get()); - int cert_status = MapCertErrorToCertStatus(x509_error); + CertStatus cert_status = MapCertErrorToCertStatus(x509_error); LOG(ERROR) << "X509 Verification error " << X509_verify_cert_error_string(x509_error) << " : " << x509_error diff --git a/net/base/x509_certificate_unittest.cc b/net/base/x509_certificate_unittest.cc index ea71dab..de15a9c 100644 --- a/net/base/x509_certificate_unittest.cc +++ b/net/base/x509_certificate_unittest.cc @@ -236,7 +236,7 @@ void CheckGoogleCert(const scoped_refptr<X509Certificate>& google_cert, int flags = X509Certificate::VERIFY_REV_CHECKING_ENABLED | X509Certificate::VERIFY_EV_CERT; EXPECT_EQ(OK, google_cert->Verify("www.google.com", flags, &verify_result)); - EXPECT_EQ(0, verify_result.cert_status & CERT_STATUS_IS_EV); + EXPECT_FALSE(verify_result.cert_status & CERT_STATUS_IS_EV); #endif } @@ -302,7 +302,7 @@ TEST(X509CertificateTest, WebkitCertParsing) { X509Certificate::VERIFY_EV_CERT; CertVerifyResult verify_result; EXPECT_EQ(OK, webkit_cert->Verify("webkit.org", flags, &verify_result)); - EXPECT_EQ(0, verify_result.cert_status & CERT_STATUS_IS_EV); + EXPECT_FALSE(verify_result.cert_status & CERT_STATUS_IS_EV); #endif // Test that the wildcard cert matches properly. @@ -365,12 +365,12 @@ TEST(X509CertificateTest, ThawteCertParsing) { CertVerifyResult verify_result; // EV cert verification requires revocation checking. EXPECT_EQ(OK, thawte_cert->Verify("www.thawte.com", flags, &verify_result)); - EXPECT_NE(0, verify_result.cert_status & CERT_STATUS_IS_EV); + EXPECT_TRUE(verify_result.cert_status & CERT_STATUS_IS_EV); // Consequently, if we don't have revocation checking enabled, we can't claim // any cert is EV. flags = X509Certificate::VERIFY_EV_CERT; EXPECT_EQ(OK, thawte_cert->Verify("www.thawte.com", flags, &verify_result)); - EXPECT_EQ(0, verify_result.cert_status & CERT_STATUS_IS_EV); + EXPECT_FALSE(verify_result.cert_status & CERT_STATUS_IS_EV); #endif } @@ -402,8 +402,8 @@ TEST(X509CertificateTest, PaypalNullCertParsing) { // name mismatch, or our certificate blacklist should cause us to report an // invalid certificate. #if !defined(OS_MACOSX) && !defined(USE_OPENSSL) - EXPECT_NE(0, verify_result.cert_status & - (CERT_STATUS_COMMON_NAME_INVALID | CERT_STATUS_INVALID)); + EXPECT_TRUE(verify_result.cert_status & + (CERT_STATUS_COMMON_NAME_INVALID | CERT_STATUS_INVALID)); #endif } @@ -499,7 +499,7 @@ TEST(X509CertificateTest, DISABLED_GlobalSignR3EVTest) { X509Certificate::VERIFY_EV_CERT; int error = cert_chain->Verify("2029.globalsign.com", flags, &verify_result); if (error == OK) - EXPECT_NE(0, verify_result.cert_status & CERT_STATUS_IS_EV); + EXPECT_TRUE(verify_result.cert_status & CERT_STATUS_IS_EV); else EXPECT_EQ(ERR_CERT_DATE_INVALID, error); } @@ -696,13 +696,13 @@ TEST(X509CertificateTest, InvalidKeyUsage) { EXPECT_EQ(ERR_CERT_AUTHORITY_INVALID, error); #else EXPECT_EQ(ERR_CERT_INVALID, error); - EXPECT_NE(0, verify_result.cert_status & CERT_STATUS_INVALID); + EXPECT_TRUE(verify_result.cert_status & CERT_STATUS_INVALID); #endif // TODO(wtc): fix http://crbug.com/75520 to get all the certificate errors // from NSS. #if !defined(USE_NSS) // The certificate is issued by an unknown CA. - EXPECT_NE(0, verify_result.cert_status & CERT_STATUS_AUTHORITY_INVALID); + EXPECT_TRUE(verify_result.cert_status & CERT_STATUS_AUTHORITY_INVALID); #endif } diff --git a/net/base/x509_certificate_win.cc b/net/base/x509_certificate_win.cc index 0432d79..1336f8c 100644 --- a/net/base/x509_certificate_win.cc +++ b/net/base/x509_certificate_win.cc @@ -95,7 +95,7 @@ int MapSecurityError(SECURITY_STATUS err) { // Map the errors in the chain_context->TrustStatus.dwErrorStatus returned by // CertGetCertificateChain to our certificate status flags. int MapCertChainErrorStatusToCertStatus(DWORD error_status) { - int cert_status = 0; + CertStatus cert_status = 0; // We don't include CERT_TRUST_IS_NOT_TIME_NESTED because it's obsolete and // we wouldn't consider it an error anyway diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc index b66c269..81b8718 100644 --- a/net/http/http_network_transaction_unittest.cc +++ b/net/http/http_network_transaction_unittest.cc @@ -4227,7 +4227,7 @@ TEST_F(HttpNetworkTransactionTest, ResetStateForRestart) { // Setup state in response_ HttpResponseInfo* response = &trans->response_; response->auth_challenge = new AuthChallengeInfo(); - response->ssl_info.cert_status = -15; + response->ssl_info.cert_status = static_cast<CertStatus>(-1); // Nonsensical. response->response_time = base::Time::Now(); response->was_cached = true; // (Wouldn't ever actually be true...) diff --git a/net/http/http_response_info.cc b/net/http/http_response_info.cc index a99990b..2496731 100644 --- a/net/http/http_response_info.cc +++ b/net/http/http_response_info.cc @@ -151,8 +151,8 @@ bool HttpResponseInfo::InitFromPickle(const Pickle& pickle, return false; } if (flags & RESPONSE_INFO_HAS_CERT_STATUS) { - int cert_status; - if (!pickle.ReadInt(&iter, &cert_status)) + CertStatus cert_status; + if (!pickle.ReadUInt32(&iter, &cert_status)) return false; ssl_info.cert_status = cert_status; } @@ -244,7 +244,7 @@ void HttpResponseInfo::Persist(Pickle* pickle, if (ssl_info.is_valid()) { ssl_info.cert->Persist(pickle); - pickle->WriteInt(ssl_info.cert_status); + pickle->WriteUInt32(ssl_info.cert_status); if (ssl_info.security_bits != -1) pickle->WriteInt(ssl_info.security_bits); if (ssl_info.connection_status != 0) diff --git a/net/http/http_transaction_unittest.h b/net/http/http_transaction_unittest.h index 714f263..fa6572a 100644 --- a/net/http/http_transaction_unittest.h +++ b/net/http/http_transaction_unittest.h @@ -62,7 +62,7 @@ struct MockTransaction { const char* data; int test_mode; MockTransactionHandler handler; - int cert_status; + net::CertStatus cert_status; }; extern const MockTransaction kSimpleGET_Transaction; diff --git a/net/socket/ssl_client_socket_mac.cc b/net/socket/ssl_client_socket_mac.cc index 96bae2d..bd13772 100644 --- a/net/socket/ssl_client_socket_mac.cc +++ b/net/socket/ssl_client_socket_mac.cc @@ -1141,7 +1141,7 @@ int SSLClientSocketMac::DoVerifyCert() { DCHECK(server_cert_); VLOG(1) << "DoVerifyCert..."; - int cert_status; + CertStatus cert_status; if (ssl_config_.IsAllowedBadCert(server_cert_, &cert_status)) { VLOG(1) << "Received an expected bad cert with status: " << cert_status; server_cert_verify_result_.Reset(); diff --git a/net/socket/ssl_client_socket_nss.cc b/net/socket/ssl_client_socket_nss.cc index 879689a..cca0591 100644 --- a/net/socket/ssl_client_socket_nss.cc +++ b/net/socket/ssl_client_socket_nss.cc @@ -1653,7 +1653,7 @@ int SSLClientSocketNSS::DoVerifyCert(int result) { base::StringPiece der_cert( reinterpret_cast<char*>(server_cert_nss_->derCert.data), server_cert_nss_->derCert.len); - int cert_status; + CertStatus cert_status; if (ssl_config_.IsAllowedBadCert(der_cert, &cert_status)) { DCHECK(start_cert_verification_time_.is_null()); VLOG(1) << "Received an expected bad cert with status: " << cert_status; diff --git a/net/socket/ssl_client_socket_openssl.cc b/net/socket/ssl_client_socket_openssl.cc index c0efab5..52014ea 100644 --- a/net/socket/ssl_client_socket_openssl.cc +++ b/net/socket/ssl_client_socket_openssl.cc @@ -821,7 +821,7 @@ int SSLClientSocketOpenSSL::DoVerifyCert(int result) { DCHECK(server_cert_); GotoState(STATE_VERIFY_CERT_COMPLETE); - int cert_status; + CertStatus cert_status; if (ssl_config_.IsAllowedBadCert(server_cert_, &cert_status)) { VLOG(1) << "Received an expected bad cert with status: " << cert_status; server_cert_verify_result_.Reset(); diff --git a/net/socket/ssl_client_socket_win.cc b/net/socket/ssl_client_socket_win.cc index f970068..f1f6ec5 100644 --- a/net/socket/ssl_client_socket_win.cc +++ b/net/socket/ssl_client_socket_win.cc @@ -1157,7 +1157,7 @@ int SSLClientSocketWin::DoVerifyCert() { next_state_ = STATE_VERIFY_CERT_COMPLETE; DCHECK(server_cert_); - int cert_status; + CertStatus cert_status; if (ssl_config_.IsAllowedBadCert(server_cert_, &cert_status)) { VLOG(1) << "Received an expected bad cert with status: " << cert_status; server_cert_verify_result_.Reset(); |