LibTLS: Refactor CA loading into central function

3 files changed, 20 insertions, 52 deletions

diff --git a/Tests/LibTLS/TestTLSHandshake.cpp b/Tests/LibTLS/TestTLSHandshake.cpp
index c7b0d35834..37531645dd 100644
--- 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
index 265cc4b789..97f84de0a4 100644
--- a/Userland/Libraries/LibTLS/Certificate.h
+++ b/Userland/Libraries/LibTLS/Certificate.h
@@ -137,7 +137,7 @@ public:
 
     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
index d2ff01791b..7dc10f0c7f 100644
--- 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;
 }
 }