Cert Management UI V2: Enable delete functionality for added-by-you certs
Bug: 40928765
Change-Id: I9d927e95fae1bc26d32daa0693df4271f2fb5155
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5925341
Reviewed-by: Matt Mueller <[email protected]>
Commit-Queue: Carlos IL <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1373464}
diff --git a/chrome/app/certificate_manager.grdp b/chrome/app/certificate_manager.grdp
index 6b871db..6c949860 100644
--- a/chrome/app/certificate_manager.grdp
+++ b/chrome/app/certificate_manager.grdp
@@ -159,6 +159,9 @@
If you delete one of your own certificates, you can no longer use it to identify yourself.
</message>
</if>
+ <message name="IDS_SETTINGS_CERTIFICATE_MANAGER_V2_DELETE_SERVER_CERT_DESCRIPTION" desc="Contents of certificate manager dialog confirming deletion of a server certificate.">
+ If you delete a server certificate, Chrome will no longer use it to identify servers.
+ </message>
<message name="IDS_SETTINGS_CERTIFICATE_MANAGER_V2_READ_FILE_ERROR" desc="Eror message displayed in certificate manager when the user tried to import a certificate but the file could not be read.">
Error reading file
</message>
diff --git a/chrome/app/certificate_manager_grdp/IDS_SETTINGS_CERTIFICATE_MANAGER_V2_DELETE_SERVER_CERT_DESCRIPTION.png.sha1 b/chrome/app/certificate_manager_grdp/IDS_SETTINGS_CERTIFICATE_MANAGER_V2_DELETE_SERVER_CERT_DESCRIPTION.png.sha1
new file mode 100644
index 0000000..f462371
--- /dev/null
+++ b/chrome/app/certificate_manager_grdp/IDS_SETTINGS_CERTIFICATE_MANAGER_V2_DELETE_SERVER_CERT_DESCRIPTION.png.sha1
@@ -0,0 +1 @@
+25ebb7f9f9ee09cf5f7e31676b55bde9953ef3f6
\ No newline at end of file
diff --git a/chrome/browser/net/server_certificate_database.cc b/chrome/browser/net/server_certificate_database.cc
index 655c3007..dc1e634 100644
--- a/chrome/browser/net/server_certificate_database.cc
+++ b/chrome/browser/net/server_certificate_database.cc
@@ -158,6 +158,16 @@
return statement.ColumnInt(0);
}
+bool ServerCertificateDatabase::DeleteCertificate(
+ const std::string& sha256hash_hex) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+ sql::Statement delete_statement(db_.GetCachedStatement(
+ SQL_FROM_HERE, "DELETE FROM certificates WHERE sha256hash_hex=?"));
+ DCHECK(delete_statement.is_valid());
+ delete_statement.BindString(0, sha256hash_hex);
+ return delete_statement.Run();
+}
+
ServerCertificateDatabase::CertInformation::CertInformation() = default;
ServerCertificateDatabase::CertInformation::~CertInformation() = default;
ServerCertificateDatabase::CertInformation::CertInformation(
diff --git a/chrome/browser/net/server_certificate_database.h b/chrome/browser/net/server_certificate_database.h
index 696ef01..6262adde 100644
--- a/chrome/browser/net/server_certificate_database.h
+++ b/chrome/browser/net/server_certificate_database.h
@@ -56,6 +56,9 @@
uint32_t RetrieveCertificatesCount();
+ // Delete the certificate with a matching hash from the database.
+ bool DeleteCertificate(const std::string& sha256hash_hex);
+
private:
sql::InitStatus InitInternal(const base::FilePath& storage_dir);
diff --git a/chrome/browser/net/server_certificate_database_service.cc b/chrome/browser/net/server_certificate_database_service.cc
index bb1b221..85475de3 100644
--- a/chrome/browser/net/server_certificate_database_service.cc
+++ b/chrome/browser/net/server_certificate_database_service.cc
@@ -139,4 +139,13 @@
.Then(std::move(callback));
}
+void ServerCertificateDatabaseService::DeleteCertificate(
+ const std::string& sha256hash_hex,
+ base::OnceCallback<void(bool)> callback) {
+ server_cert_database_
+ .AsyncCall(&net::ServerCertificateDatabase::DeleteCertificate)
+ .WithArgs(sha256hash_hex)
+ .Then(std::move(callback));
+}
+
} // namespace net
diff --git a/chrome/browser/net/server_certificate_database_service.h b/chrome/browser/net/server_certificate_database_service.h
index 0ad8145..16d88f0 100644
--- a/chrome/browser/net/server_certificate_database_service.h
+++ b/chrome/browser/net/server_certificate_database_service.h
@@ -86,6 +86,9 @@
void GetCertificatesCount(base::OnceCallback<void(uint32_t)> callback);
+ void DeleteCertificate(const std::string& sha256hash_hex,
+ base::OnceCallback<void(bool)> callback);
+
private:
#if BUILDFLAG(IS_CHROMEOS)
void NSSMigrationComplete(
diff --git a/chrome/browser/net/server_certificate_database_unittest.cc b/chrome/browser/net/server_certificate_database_unittest.cc
index a2e8a83..69fb827 100644
--- a/chrome/browser/net/server_certificate_database_unittest.cc
+++ b/chrome/browser/net/server_certificate_database_unittest.cc
@@ -103,6 +103,32 @@
CertInfoEquals(std::ref(intermediate_cert_info))));
}
+TEST_F(ServerCertificateDatabaseTest, Delete) {
+ EXPECT_TRUE(database_->RetrieveAllCertificates().empty());
+
+ auto [leaf, intermediate, root] = CertBuilder::CreateSimpleChain3();
+
+ ServerCertificateDatabase::CertInformation root_cert_info = MakeCertInfo(
+ root->GetDER(), CertificateTrust::CERTIFICATE_TRUST_TYPE_TRUSTED);
+ ServerCertificateDatabase::CertInformation intermediate_cert_info =
+ MakeCertInfo(intermediate->GetDER(),
+ CertificateTrust::CERTIFICATE_TRUST_TYPE_UNSPECIFIED);
+
+ EXPECT_TRUE(database_->InsertOrUpdateCert(root_cert_info));
+ EXPECT_TRUE(database_->InsertOrUpdateCert(intermediate_cert_info));
+
+ EXPECT_THAT(
+ database_->RetrieveAllCertificates(),
+ UnorderedElementsAre(CertInfoEquals(std::ref(root_cert_info)),
+ CertInfoEquals(std::ref(intermediate_cert_info))));
+
+ EXPECT_TRUE(
+ database_->DeleteCertificate(intermediate_cert_info.sha256hash_hex));
+
+ EXPECT_THAT(database_->RetrieveAllCertificates(),
+ UnorderedElementsAre(CertInfoEquals(std::ref(root_cert_info))));
+}
+
TEST(ServerCertificateDatabaseTrustTest, TestTrustMappings) {
auto [leaf, intermediate, root] = CertBuilder::CreateSimpleChain3();
diff --git a/chrome/browser/ui/webui/certificate_manager/certificate_manager_handler.cc b/chrome/browser/ui/webui/certificate_manager/certificate_manager_handler.cc
index 730c809..4b319e8f 100644
--- a/chrome/browser/ui/webui/certificate_manager/certificate_manager_handler.cc
+++ b/chrome/browser/ui/webui/certificate_manager/certificate_manager_handler.cc
@@ -197,7 +197,7 @@
"trusted_certs",
chrome_browser_server_certificate_database::CertificateTrust::
CERTIFICATE_TRUST_TYPE_TRUSTED,
- profile_);
+ profile_, &remote_client_);
break;
case certificate_manager_v2::mojom::CertificateSource::
kUserIntermediateCerts:
@@ -205,7 +205,7 @@
"intermediate_certs",
chrome_browser_server_certificate_database::CertificateTrust::
CERTIFICATE_TRUST_TYPE_UNSPECIFIED,
- profile_);
+ profile_, &remote_client_);
break;
case certificate_manager_v2::mojom::CertificateSource::
kUserDistrustedCerts:
@@ -213,7 +213,7 @@
"distrusted_certs",
chrome_browser_server_certificate_database::CertificateTrust::
CERTIFICATE_TRUST_TYPE_DISTRUSTED,
- profile_);
+ profile_, &remote_client_);
break;
#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
case certificate_manager_v2::mojom::CertificateSource::
diff --git a/chrome/browser/ui/webui/certificate_manager/user_cert_sources.cc b/chrome/browser/ui/webui/certificate_manager/user_cert_sources.cc
index 2759cfc..9cf98a0 100644
--- a/chrome/browser/ui/webui/certificate_manager/user_cert_sources.cc
+++ b/chrome/browser/ui/webui/certificate_manager/user_cert_sources.cc
@@ -7,6 +7,7 @@
#include <vector>
#include "base/memory/weak_ptr.h"
+#include "base/strings/utf_string_conversions.h"
#include "chrome/browser/net/server_certificate_database.h"
#include "chrome/browser/net/server_certificate_database.pb.h"
#include "chrome/browser/net/server_certificate_database_service.h"
@@ -18,11 +19,13 @@
#include "chrome/browser/ui/webui/certificate_viewer_webui.h"
#include "chrome/common/chrome_features.h"
#include "chrome/common/net/x509_certificate_model.h"
+#include "chrome/grit/generated_resources.h"
#include "content/public/browser/network_service_instance.h"
#include "content/public/browser/web_contents.h"
#include "crypto/sha2.h"
#include "net/cert/x509_util.h"
#include "services/cert_verifier/public/mojom/cert_verifier_service_factory.mojom.h"
+#include "ui/base/l10n/l10n_util.h"
namespace {
@@ -39,9 +42,11 @@
}
x509_certificate_model::X509CertificateModel model(
net::x509_util::CreateCryptoBuffer(cert_info.der_cert), "");
+ // TODO(crbug.com/40928765): is_deletable should be set to false if
+ // CACertificateManagementAllowed is set to None.
cert_infos.push_back(certificate_manager_v2::mojom::SummaryCertInfo::New(
- model.HashCertSHA256(), model.GetTitle(),
- /*is_deletable=*/false));
+ cert_info.sha256hash_hex, model.GetTitle(),
+ /*is_deletable=*/true));
}
std::move(callback).Run(std::move(cert_infos));
}
@@ -106,15 +111,49 @@
std::move(export_certs), file_name);
}
+void DeleteCertificateResultAsync(
+ CertificateManagerPageHandler::DeleteCertificateCallback callback,
+ bool result) {
+ if (result) {
+ std::move(callback).Run(
+ certificate_manager_v2::mojom::ActionResult::NewSuccess(
+ certificate_manager_v2::mojom::SuccessResult::kSuccess));
+ return;
+ }
+ std::move(callback).Run(certificate_manager_v2::mojom::ActionResult::NewError(
+ "Error deleting certificate"));
+}
+
+void GotDeleteConfirmation(
+ const std::string& sha256hash_hex,
+ CertificateManagerPageHandler::DeleteCertificateCallback callback,
+ base::WeakPtr<Profile> profile,
+ bool confirmed) {
+ if (confirmed && profile) {
+ net::ServerCertificateDatabaseService* server_cert_service =
+ net::ServerCertificateDatabaseServiceFactory::GetForBrowserContext(
+ profile.get());
+ server_cert_service->DeleteCertificate(
+ sha256hash_hex,
+ base::BindOnce(&DeleteCertificateResultAsync, std::move(callback)));
+ return;
+ }
+ std::move(callback).Run(nullptr);
+}
+
} // namespace
-UserCertSource::UserCertSource(std::string export_file_name,
- chrome_browser_server_certificate_database::
- CertificateTrust::CertificateTrustType trust,
- raw_ptr<Profile> profile)
+UserCertSource::UserCertSource(
+ std::string export_file_name,
+ chrome_browser_server_certificate_database::CertificateTrust::
+ CertificateTrustType trust,
+ raw_ptr<Profile> profile,
+ mojo::Remote<certificate_manager_v2::mojom::CertificateManagerPage>*
+ remote_client)
: export_file_name_(std::move(export_file_name)),
trust_(trust),
- profile_(profile) {}
+ profile_(profile),
+ remote_client_(remote_client) {}
void UserCertSource::GetCertificateInfos(
CertificateManagerPageHandler::GetCertificatesCallback callback) {
@@ -149,3 +188,19 @@
server_cert_service->GetAllCertificates(base::BindOnce(
&ExportCertificatesAsync, web_contents, trust_, export_file_name_));
}
+
+void UserCertSource::DeleteCertificate(
+ const std::string& sha256hash_hex,
+ CertificateManagerPageHandler::DeleteCertificateCallback callback) {
+ // TODO(crbug.com/40928765): This should early return if
+ // CACertificateManagementAllowed is set to None.
+ (*remote_client_)
+ ->AskForConfirmation(
+ l10n_util::GetStringFUTF8(
+ IDS_SETTINGS_CERTIFICATE_MANAGER_V2_DELETE_CERT_TITLE,
+ base::UTF8ToUTF16(sha256hash_hex)),
+ l10n_util::GetStringUTF8(
+ IDS_SETTINGS_CERTIFICATE_MANAGER_V2_DELETE_SERVER_CERT_DESCRIPTION),
+ base::BindOnce(&GotDeleteConfirmation, sha256hash_hex,
+ std::move(callback), profile_->GetWeakPtr()));
+}
diff --git a/chrome/browser/ui/webui/certificate_manager/user_cert_sources.h b/chrome/browser/ui/webui/certificate_manager/user_cert_sources.h
index eb275cb..c473b87 100644
--- a/chrome/browser/ui/webui/certificate_manager/user_cert_sources.h
+++ b/chrome/browser/ui/webui/certificate_manager/user_cert_sources.h
@@ -15,10 +15,13 @@
class UserCertSource : public CertificateManagerPageHandler::CertSource {
public:
- UserCertSource(std::string export_file_name,
- chrome_browser_server_certificate_database::CertificateTrust::
- CertificateTrustType trust,
- raw_ptr<Profile> profile);
+ UserCertSource(
+ std::string export_file_name,
+ chrome_browser_server_certificate_database::CertificateTrust::
+ CertificateTrustType trust,
+ raw_ptr<Profile> profile,
+ mojo::Remote<certificate_manager_v2::mojom::CertificateManagerPage>*
+ remote_client);
void GetCertificateInfos(
CertificateManagerPageHandler::GetCertificatesCallback callback) override;
@@ -30,11 +33,18 @@
void ExportCertificates(
base::WeakPtr<content::WebContents> web_contents) override;
+ void DeleteCertificate(
+ const std::string& sha256hash_hex,
+ CertificateManagerPageHandler::DeleteCertificateCallback callback)
+ override;
+
private:
std::string export_file_name_;
chrome_browser_server_certificate_database::CertificateTrust::
CertificateTrustType trust_;
raw_ptr<Profile> profile_;
+ raw_ptr<mojo::Remote<certificate_manager_v2::mojom::CertificateManagerPage>>
+ remote_client_;
};
#endif // CHROME_BROWSER_UI_WEBUI_CERTIFICATE_MANAGER_USER_CERT_SOURCES_H_
