Skip to content

Commit

Permalink
Cleanup unreachable cert adding code
Browse files Browse the repository at this point in the history
The ability to use Chrome to import certs directly into the OS was
removed in M49. This cleans up the related code, which is now
unreachable.

Most notably, this removes OnCertAdded/OnCertRemoved from the
CertDatabase, which were intended to relate to direct user additions
of certs, and instead folds the notifications (which only exist
for ChromeOS/Linux when using chrome://certificates) into a generic
OnCertDBChanged, which better reflects the cross-platform knowledge
we have available.

BUG=514767

Review-Url: https://codereview.chromium.org/2363653002
Cr-Commit-Position: refs/heads/master@{#424638}
  • Loading branch information
sleevi authored and Commit bot committed Oct 12, 2016
1 parent 5e8b08e commit 1778469
Show file tree
Hide file tree
Showing 44 changed files with 74 additions and 854 deletions.
9 changes: 0 additions & 9 deletions android_webview/browser/aw_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -193,15 +193,6 @@ AwBrowserContext* AwContentBrowserClient::InitBrowserContext() {
return browser_context_.get();
}

void AwContentBrowserClient::AddCertificate(net::CertificateMimeType cert_type,
const void* cert_data,
size_t cert_size,
int render_process_id,
int render_frame_id) {
if (cert_size > 0)
net::android::StoreCertificate(cert_type, cert_data, cert_size);
}

content::BrowserMainParts* AwContentBrowserClient::CreateBrowserMainParts(
const content::MainFunctionParams& parameters) {
return new AwBrowserMainParts(this);
Expand Down
6 changes: 0 additions & 6 deletions android_webview/browser/aw_content_browser_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,6 @@ class AwContentBrowserClient : public content::ContentBrowserClient {
// moment during startup. AwContentBrowserClient owns the result.
AwBrowserContext* InitBrowserContext();

// Overriden methods from ContentBrowserClient.
void AddCertificate(net::CertificateMimeType cert_type,
const void* cert_data,
size_t cert_size,
int render_process_id,
int render_frame_id) override;
content::BrowserMainParts* CreateBrowserMainParts(
const content::MainFunctionParams& parameters) override;
content::WebContentsViewDelegate* GetWebContentsViewDelegate(
Expand Down
16 changes: 0 additions & 16 deletions chrome/app/generated_resources.grd
Original file line number Diff line number Diff line change
Expand Up @@ -3261,22 +3261,6 @@ From <ph name="DOWNLOAD_DOMAIN">$3<ex>example.com</ex></ph>
</message>
</if>

<!-- Client certificate enrollment failure infobar -->
<message name="IDS_ADD_CERT_ERR_INVALID_CERT" desc="Error message when the server returns an invalid client certificate after key generation">
The server returned an invalid client certificate. Error <ph name="ERROR_number">$1<ex>207</ex></ph> (<ph name="ERROR_NAME">$2<ex>net::ERR_CERT_INVALID</ex></ph>).
</message>
<message name="IDS_ADD_CERT_ERR_FAILED" desc="Generic error message for a failure to add the generated certificate to the cert store/keychain">
There was an error while trying to store the client certificate. Error <ph name="ERROR_number">$1<ex>207</ex></ph> (<ph name="ERROR_NAME">$2<ex>net::ERR_CERT_INVALID</ex></ph>).
</message>

<!-- Client certificate enrollment infobar -->
<message name="IDS_ADD_CERT_SUCCESS_INFOBAR_LABEL" desc="Label displayed in an infobar when the browser successfully imports a certificate">
Successfully stored client certificate issued by <ph name="ISSUER">$1<ex>VeriSign</ex></ph>.
</message>
<message name="IDS_ADD_CERT_SUCCESS_INFOBAR_BUTTON" desc="The label of the 'view' button on the infobar after a client certificate import; clicking opens a certificate viewer for the new certificate">
View
</message>

<!-- DevTools attached infobar -->
<message name="IDS_DEV_TOOLS_INFOBAR_LABEL" desc="Label displayed in an infobar when external debugger is attached to the browser">
"<ph name="CLIENT_NAME">$1<ex>Extension Foo</ex></ph>" is debugging this browser.
Expand Down
3 changes: 0 additions & 3 deletions chrome/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2817,8 +2817,6 @@ split_static_library("browser") {
"signin/signin_ui_util.h",
"speech/extension_api/tts_extension_api_constants.cc", # Should be moved to extensions section?
"speech/extension_api/tts_extension_api_constants.h",
"ssl/ssl_add_certificate.cc",
"ssl/ssl_add_certificate.h",
"ssl/ssl_client_auth_observer.cc",
"ssl/ssl_client_auth_observer.h",
"status_icons/desktop_notification_balloon.cc",
Expand Down Expand Up @@ -3319,7 +3317,6 @@ split_static_library("browser") {
"signin/oauth2_token_service_delegate_android.h",
"ssl/security_state_model_android.cc",
"ssl/security_state_model_android.h",
"ssl/ssl_add_certificate_android.cc",
"sync/glue/synced_tab_delegate_android.cc",
"sync/glue/synced_tab_delegate_android.h",
"sync/glue/synced_window_delegate_android.cc",
Expand Down
11 changes: 0 additions & 11 deletions chrome/browser/chrome_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@
#include "chrome/browser/speech/chrome_speech_recognition_manager_delegate.h"
#include "chrome/browser/speech/tts_controller.h"
#include "chrome/browser/speech/tts_message_filter.h"
#include "chrome/browser/ssl/ssl_add_certificate.h"
#include "chrome/browser/ssl/ssl_blocking_page.h"
#include "chrome/browser/ssl/ssl_cert_reporter.h"
#include "chrome/browser/ssl/ssl_client_certificate_selector.h"
Expand Down Expand Up @@ -2219,16 +2218,6 @@ void ChromeContentBrowserClient::SelectClientCertificate(
std::move(delegate));
}

void ChromeContentBrowserClient::AddCertificate(
net::CertificateMimeType cert_type,
const void* cert_data,
size_t cert_size,
int render_process_id,
int render_frame_id) {
chrome::SSLAddCertificate(cert_type, cert_data, cert_size,
render_process_id, render_frame_id);
}

content::MediaObserver* ChromeContentBrowserClient::GetMediaObserver() {
return MediaCaptureDevicesDispatcher::GetInstance();
}
Expand Down
5 changes: 0 additions & 5 deletions chrome/browser/chrome_content_browser_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -184,11 +184,6 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
content::WebContents* web_contents,
net::SSLCertRequestInfo* cert_request_info,
std::unique_ptr<content::ClientCertificateDelegate> delegate) override;
void AddCertificate(net::CertificateMimeType cert_type,
const void* cert_data,
size_t cert_size,
int render_process_id,
int render_frame_id) override;
content::MediaObserver* GetMediaObserver() override;
content::PlatformNotificationService* GetPlatformNotificationService()
override;
Expand Down
28 changes: 12 additions & 16 deletions chrome/browser/chromeos/platform_keys/platform_keys_nss.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
#include "net/cert/nss_cert_database.h"
#include "net/cert/x509_util_nss.h"
#include "net/ssl/ssl_cert_request_info.h"
#include "net/third_party/mozilla_security_manager/nsNSSCertificateDB.h"

using content::BrowserContext;
using content::BrowserThread;
Expand Down Expand Up @@ -619,31 +620,26 @@ void GetCertificatesWithDB(std::unique_ptr<GetCertificatesState> state,
void ImportCertificateWithDB(std::unique_ptr<ImportCertificateState> state,
net::NSSCertDatabase* cert_db) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
// TODO(pneubeck): Use |state->slot_| to verify that we're really importing to
// the correct token.
// |cert_db| is not required, ignore it.
net::CertDatabase* db = net::CertDatabase::GetInstance();

const net::Error cert_status =
static_cast<net::Error>(db->CheckUserCert(state->certificate_.get()));
if (cert_status == net::ERR_NO_PRIVATE_KEY_FOR_CERT) {
state->OnError(FROM_HERE, kErrorKeyNotFound);

if (!state->certificate_) {
state->OnError(FROM_HERE, net::ErrorToString(net::ERR_CERT_INVALID));
return;
} else if (cert_status != net::OK) {
state->OnError(FROM_HERE, net::ErrorToString(cert_status));
}
if (state->certificate_->HasExpired()) {
state->OnError(FROM_HERE, net::ErrorToString(net::ERR_CERT_DATE_INVALID));
return;
}

// Check that the private key is in the correct slot.
PK11SlotInfo* slot =
PK11_KeyForCertExists(state->certificate_->os_cert_handle(), NULL, NULL);
if (slot != state->slot_.get()) {
crypto::ScopedPK11Slot slot(
PK11_KeyForCertExists(state->certificate_->os_cert_handle(), NULL, NULL));
if (slot.get() != state->slot_.get()) {
state->OnError(FROM_HERE, kErrorKeyNotFound);
return;
}

const net::Error import_status =
static_cast<net::Error>(db->AddUserCert(state->certificate_.get()));
const net::Error import_status = static_cast<net::Error>(
cert_db->ImportUserCert(state->certificate_.get()));
if (import_status != net::OK) {
LOG(ERROR) << "Could not import certificate.";
state->OnError(FROM_HERE, net::ErrorToString(import_status));
Expand Down
206 changes: 0 additions & 206 deletions chrome/browser/ssl/ssl_add_certificate.cc

This file was deleted.

29 changes: 0 additions & 29 deletions chrome/browser/ssl/ssl_add_certificate.h

This file was deleted.

Loading

0 comments on commit 1778469

Please sign in to comment.