LibTLS: Refactor CA loading into central function

cubiclove · Apr 4, 2023 · 459dee1 · 459dee1
1 parent 924758c
commit 459dee1
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 52 deletions.
diff --git a/Tests/LibTLS/TestTLSHandshake.cpp b/Tests/LibTLS/TestTLSHandshake.cpp
@@ -40,46 +40,9 @@ DeprecatedString locate_ca_certs_file()
 
 Vector<Certificate> load_certificates()
 {
- Vector<Certificate> certificates;
-
- auto cacert_result = Core::File::open(locate_ca_certs_file(), Core::File::OpenMode::Read);
- if (cacert_result.is_error()) {
- dbgln("Failed to load CA Certificates: {}", cacert_result.error());
- return certificates;
- }
- 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 certificates;
- }
- auto data = data_result.release_value();
-
- auto decode_result = Crypto::decode_pems(data);
- if (decode_result.is_error()) {
- dbgln("Failed to load CA Certificates: {}", decode_result.error());
- return certificates;
- }
- auto certs = decode_result.release_value();
-
- for (auto& cert : certs) {
- auto certificate_result = Certificate::parse_asn1(cert.bytes());
- // If the certificate does not parse it is likely using elliptic curve keys/signatures, which are not
- // supported right now. It might make sense to cleanup cacert.pem before adding it to the system.
- if (!certificate_result.has_value()) {
- // FIXME: It would be nice to have more informations about the certificate we failed to parse.
- // Like: Issuer, Algorithm, CN, etc
- continue;
- }
- auto certificate = certificate_result.release_value();
- if (certificate.is_certificate_authority && certificate.is_self_signed()) {
- certificates.append(move(certificate));
- } else {
- dbgln("Skipped '{}' because it is not a valid root CA", certificate.subject_identifier_string());
- }
- }
-
- return certificates;
+ auto cacert_file = MUST(Core::File::open(locate_ca_certs_file(), Core::File::OpenMode::Read));
+ auto data = MUST(cacert_file->read_until_eof());
+ return MUST(DefaultRootCACertificates::the().reload_certificates(data));
 }
 
 static Vector<Certificate> s_root_ca_certificates = load_certificates();

diff --git a/Userland/Libraries/LibTLS/Certificate.h b/Userland/Libraries/LibTLS/Certificate.h
@@ -137,7 +137,7 @@ class DefaultRootCACertificates {
 
  Vector<Certificate> const& certificates() const { return m_ca_certificates; }
 
- void reload_certificates(ByteBuffer&);
+ ErrorOr<Vector<Certificate>> reload_certificates(ByteBuffer&);
 
  static DefaultRootCACertificates& the() { return s_the; }
 

diff --git a/Userland/Libraries/LibTLS/TLSv12.cpp b/Userland/Libraries/LibTLS/TLSv12.cpp
@@ -499,18 +499,21 @@ DefaultRootCACertificates::DefaultRootCACertificates()
  return;
  }
  auto data = data_result.release_value();
- reload_certificates(data);
-}
 
-void DefaultRootCACertificates::reload_certificates(ByteBuffer& data)
-{
- auto decode_result = Crypto::decode_pems(data);
- if (decode_result.is_error()) {
- dbgln("Failed to load CA Certificates: {}", decode_result.error());
+ auto reload_result = reload_certificates(data);
+ if (reload_result.is_error()) {
+ dbgln("Failed to load CA Certificates: {}", reload_result.error());
  return;
  }
- m_ca_certificates.clear();
- auto certs = decode_result.release_value();
+
+ m_ca_certificates = reload_result.release_value();
+}
+
+ErrorOr<Vector<Certificate>> DefaultRootCACertificates::reload_certificates(ByteBuffer& data)
+{
+ Vector<Certificate> certificates;
+
+ auto certs = TRY(Crypto::decode_pems(data));
 
  for (auto& cert : certs) {
  auto certificate_result = Certificate::parse_asn1(cert.bytes());
@@ -523,12 +526,14 @@ void DefaultRootCACertificates::reload_certificates(ByteBuffer& data)
  }
  auto certificate = certificate_result.release_value();
  if (certificate.is_certificate_authority && certificate.is_self_signed()) {
- m_ca_certificates.append(move(certificate));
+ TRY(certificates.try_append(move(certificate)));
  } else {
  dbgln("Skipped '{}' because it is not a valid root CA", certificate.subject_identifier_string());
  }
  }
 
- dbgln("Loaded {} of {} ({:.2}%) provided CA Certificates", m_ca_certificates.size(), certs.size(), (m_ca_certificates.size() * 100.0) / certs.size());
+ dbgln("Loaded {} of {} ({:.2}%) provided CA Certificates", certificates.size(), certs.size(), (certificates.size() * 100.0) / certs.size());
+
+ return certificates;
 }
 }