LibTLS: Streamline certificate loading

Some refactoring of our root ca loading process: - Remove duplicate code - Remove duplicate calls to `parse_root_ca` - Load user imported certificates in Browser/RequestServer
lanmonster · Apr 12, 2023 · 93232d4 · 93232d4
1 parent e4481ba
commit 93232d4
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 31 deletions.
diff --git a/Tests/LibTLS/TestTLSHandshake.cpp b/Tests/LibTLS/TestTLSHandshake.cpp
@@ -42,7 +42,7 @@ ErrorOr<Vector<Certificate>> load_certificates()
 {
  auto cacert_file = TRY(Core::File::open(locate_ca_certs_file(), Core::File::OpenMode::Read));
  auto data = TRY(cacert_file->read_until_eof());
- return TRY(DefaultRootCACertificates::the().reload_certificates(data));
+ return TRY(DefaultRootCACertificates::parse_pem_root_certificate_authorities(data));
 }
 
 TEST_CASE(test_TLS_hello_handshake)

diff --git a/Userland/Applications/CertificateSettings/CertificateStoreWidget.cpp b/Userland/Applications/CertificateSettings/CertificateStoreWidget.cpp
@@ -29,16 +29,7 @@ void CertificateStoreProxyModel::sort(int column, GUI::SortOrder sort_order)
 
 ErrorOr<void> CertificateStoreModel::load()
 {
- auto cacert_file = TRY(Core::File::open("/etc/cacert.pem"sv, Core::File::OpenMode::Read));
- auto data = TRY(cacert_file->read_until_eof());
-
- auto user_cert_path = TRY(String::formatted("{}/.config/certs.pem", Core::StandardPaths::home_directory()));
- if (FileSystem::exists(user_cert_path)) {
- auto user_cert_file = TRY(Core::File::open(user_cert_path, Core::File::OpenMode::Read));
- TRY(data.try_append(TRY(user_cert_file->read_until_eof())));
- }
-
- m_certificates = TRY(DefaultRootCACertificates::the().reload_certificates(data));
+ m_certificates = TRY(DefaultRootCACertificates::load_certificates());
 
  return {};
 }
@@ -111,7 +102,7 @@ ErrorOr<void> CertificateStoreWidget::import_pem()
  return Error::from_string_view("File is not a .pem or .crt file."sv);
 
  auto data = TRY(fsac_file.release_stream()->read_until_eof());
- auto count = TRY(m_root_ca_model->add(TRY(DefaultRootCACertificates::the().reload_certificates(data))));
+ auto count = TRY(m_root_ca_model->add(TRY(DefaultRootCACertificates::parse_pem_root_certificate_authorities(data))));
 
  if (count == 0) {
  return Error::from_string_view("No valid CA found to import."sv);

diff --git a/Userland/Libraries/LibTLS/CMakeLists.txt b/Userland/Libraries/LibTLS/CMakeLists.txt
@@ -12,6 +12,6 @@ set(SOURCES
 )
 
 serenity_lib(LibTLS tls)
-target_link_libraries(LibTLS PRIVATE LibCore LibCrypto)
+target_link_libraries(LibTLS PRIVATE LibCore LibCrypto LibFileSystem)
 
 include(ca_certificates_data)
diff --git a/Userland/Libraries/LibTLS/Certificate.h b/Userland/Libraries/LibTLS/Certificate.h
@@ -260,7 +260,8 @@ class DefaultRootCACertificates {
 
  Vector<Certificate> const& certificates() const { return m_ca_certificates; }
 
- ErrorOr<Vector<Certificate>> reload_certificates(ByteBuffer&);
+ static ErrorOr<Vector<Certificate>> parse_pem_root_certificate_authorities(ByteBuffer&);
+ static ErrorOr<Vector<Certificate>> load_certificates();
 
  static DefaultRootCACertificates& the() { return s_the; }
 

diff --git a/Userland/Libraries/LibTLS/TLSv12.cpp b/Userland/Libraries/LibTLS/TLSv12.cpp
@@ -10,11 +10,13 @@
 #include <LibCore/ConfigFile.h>
 #include <LibCore/DateTime.h>
 #include <LibCore/File.h>
+#include <LibCore/StandardPaths.h>
 #include <LibCore/Timer.h>
 #include <LibCrypto/ASN1/ASN1.h>
 #include <LibCrypto/ASN1/PEM.h>
 #include <LibCrypto/PK/Code/EMSA_PKCS1_V1_5.h>
 #include <LibCrypto/PK/Code/EMSA_PSS.h>
+#include <LibFileSystem/FileSystem.h>
 #include <LibTLS/Certificate.h>
 #include <LibTLS/TLSv12.h>
 #include <errno.h>
@@ -488,29 +490,30 @@ Vector<Certificate> TLSv12::parse_pem_certificate(ReadonlyBytes certificate_pem_
 Singleton<DefaultRootCACertificates> DefaultRootCACertificates::s_the;
 DefaultRootCACertificates::DefaultRootCACertificates()
 {
- auto cacert_result = Core::File::open("/etc/cacert.pem"sv, Core::File::OpenMode::Read);
- if (cacert_result.is_error()) {
- dbgln("Failed to load CA Certificates: {}", cacert_result.error());
+ auto load_result = load_certificates();
+ if (load_result.is_error()) {
+ dbgln("Failed to load CA Certificates: {}", load_result.error());
  return;
  }
- auto cacert_file = cacert_result.release_value();
- auto data_result = cacert_file->read_until_eof();
- if (data_result.is_error()) {
- dbgln("Failed to load CA Certificates: {}", data_result.error());
- return;
- }
- auto data = data_result.release_value();
 
- auto reload_result = reload_certificates(data);
- if (reload_result.is_error()) {
- dbgln("Failed to load CA Certificates: {}", reload_result.error());
- return;
+ m_ca_certificates = load_result.release_value();
+}
+
+ErrorOr<Vector<Certificate>> DefaultRootCACertificates::load_certificates()
+{
+ auto cacert_file = TRY(Core::File::open("/etc/cacert.pem"sv, Core::File::OpenMode::Read));
+ auto data = TRY(cacert_file->read_until_eof());
+
+ auto user_cert_path = TRY(String::formatted("{}/.config/certs.pem", Core::StandardPaths::home_directory()));
+ if (FileSystem::exists(user_cert_path)) {
+ auto user_cert_file = TRY(Core::File::open(user_cert_path, Core::File::OpenMode::Read));
+ TRY(data.try_append(TRY(user_cert_file->read_until_eof())));
  }
 
- m_ca_certificates = reload_result.release_value();
+ return TRY(parse_pem_root_certificate_authorities(data));
 }
 
-ErrorOr<Vector<Certificate>> DefaultRootCACertificates::reload_certificates(ByteBuffer& data)
+ErrorOr<Vector<Certificate>> DefaultRootCACertificates::parse_pem_root_certificate_authorities(ByteBuffer& data)
 {
  Vector<Certificate> certificates;
 
@@ -530,7 +533,7 @@ ErrorOr<Vector<Certificate>> DefaultRootCACertificates::reload_certificates(Byte
  if (certificate.is_certificate_authority && certificate.is_self_signed()) {
  TRY(certificates.try_append(move(certificate)));
  } else {
- dbgln("Skipped '{}' because it is not a valid root CA", MUST(certificate.subject.to_string()));
+ dbgln("Skipped '{}' because it is not a valid root CA", TRY(certificate.subject.to_string()));
  }
  }