summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMichiel Visser <opensource@webmichiel.nl>2022-02-21 22:25:34 +0100
committerAli Mohammad Pur <Ali.mpfard@gmail.com>2022-04-17 10:10:19 +0430
commitfea5aeda0bc087e75aa938efbca1bd998a947bdb (patch)
treef9b680304cb37154f76cd8e28a744cd8ddddbb52
parentd5cef41bb6f525384506ae2a9399914bf4df4a6e (diff)
downloadserenity-fea5aeda0bc087e75aa938efbca1bd998a947bdb.zip
LibTLS: Verify the certificate chain sent by the server
With this change the certificate chain sent by the server will actually be verified, instead of just checking the names of the certificates. To determine if a certificate is signed by a root certificate, the list of root certificates is now a HashMap mapping from the unique identifier string to the certificate. This allows us to take the issuer of a certificate and easily check if it is a root certificate. If a certificate is not signed by a root certificate, we will check that it is signed by the next certificate in the chain. This also removes the ad-hoc checking of certificate validity from multiple places, and moves all checking to the verify_chain.
-rw-r--r--Userland/Libraries/LibTLS/Certificate.cpp5
-rw-r--r--Userland/Libraries/LibTLS/Certificate.h61
-rw-r--r--Userland/Libraries/LibTLS/Handshake.cpp15
-rw-r--r--Userland/Libraries/LibTLS/HandshakeCertificate.cpp6
-rw-r--r--Userland/Libraries/LibTLS/TLSv12.cpp119
-rw-r--r--Userland/Libraries/LibTLS/TLSv12.h3
6 files changed, 162 insertions, 47 deletions
diff --git a/Userland/Libraries/LibTLS/Certificate.cpp b/Userland/Libraries/LibTLS/Certificate.cpp
index d08891ee04..f1c806a36c 100644
--- a/Userland/Libraries/LibTLS/Certificate.cpp
+++ b/Userland/Libraries/LibTLS/Certificate.cpp
@@ -91,6 +91,11 @@ Optional<Certificate> Certificate::parse_asn1(ReadonlyBytes buffer, bool)
} while (0)
Certificate certificate;
+ auto copy_buffer_result = ByteBuffer::copy(buffer.data(), buffer.size());
+ if (copy_buffer_result.is_error())
+ return {};
+ certificate.original_asn1 = copy_buffer_result.release_value();
+
Crypto::ASN1::Decoder decoder { buffer };
// Certificate ::= Sequence {
// certificate TBSCertificate,
diff --git a/Userland/Libraries/LibTLS/Certificate.h b/Userland/Libraries/LibTLS/Certificate.h
index eda9dba194..aef856406a 100644
--- a/Userland/Libraries/LibTLS/Certificate.h
+++ b/Userland/Libraries/LibTLS/Certificate.h
@@ -55,10 +55,71 @@ public:
ByteBuffer data {};
CertificateKeyAlgorithm signature_algorithm { CertificateKeyAlgorithm::Unsupported };
ByteBuffer signature_value {};
+ ByteBuffer original_asn1 {};
static Optional<Certificate> parse_asn1(ReadonlyBytes, bool client_cert = false);
bool is_valid() const;
+
+ String subject_identifier_string() const
+ {
+ StringBuilder cert_name;
+ if (!subject.country.is_empty()) {
+ cert_name.append("/C=");
+ cert_name.append(subject.country);
+ }
+ if (!subject.state.is_empty()) {
+ cert_name.append("/ST=");
+ cert_name.append(subject.state);
+ }
+ if (!subject.location.is_empty()) {
+ cert_name.append("/L=");
+ cert_name.append(subject.location);
+ }
+ if (!subject.entity.is_empty()) {
+ cert_name.append("/O=");
+ cert_name.append(subject.entity);
+ }
+ if (!subject.unit.is_empty()) {
+ cert_name.append("/OU=");
+ cert_name.append(subject.unit);
+ }
+ if (!subject.subject.is_empty()) {
+ cert_name.append("/CN=");
+ cert_name.append(subject.subject);
+ }
+ return cert_name.build();
+ }
+
+ String issuer_identifier_string() const
+ {
+ StringBuilder cert_name;
+ if (!issuer.country.is_empty()) {
+ cert_name.append("/C=");
+ cert_name.append(issuer.country);
+ }
+ if (!issuer.state.is_empty()) {
+ cert_name.append("/ST=");
+ cert_name.append(issuer.state);
+ }
+ if (!issuer.location.is_empty()) {
+ cert_name.append("/L=");
+ cert_name.append(issuer.location);
+ }
+ if (!issuer.entity.is_empty()) {
+ cert_name.append("/O=");
+ cert_name.append(issuer.entity);
+ }
+ if (!issuer.unit.is_empty()) {
+ cert_name.append("/OU=");
+ cert_name.append(issuer.unit);
+ }
+ if (!issuer.subject.is_empty()) {
+ cert_name.append("/CN=");
+ cert_name.append(issuer.subject);
+ }
+ return cert_name.build();
+ }
};
class DefaultRootCACertificates {
diff --git a/Userland/Libraries/LibTLS/Handshake.cpp b/Userland/Libraries/LibTLS/Handshake.cpp
index 5720674be7..1cb0aa420c 100644
--- a/Userland/Libraries/LibTLS/Handshake.cpp
+++ b/Userland/Libraries/LibTLS/Handshake.cpp
@@ -313,21 +313,6 @@ ssize_t TLSv12::handle_handshake_payload(ReadonlyBytes vbuffer)
VERIFY_NOT_REACHED();
}
payload_res = handle_certificate(buffer.slice(1, payload_size));
- if (m_context.certificates.size()) {
- auto it = m_context.certificates.find_if([](auto const& cert) { return cert.is_valid(); });
-
- if (it.is_end()) {
- // no valid certificates
- dbgln("No valid certificates found");
- payload_res = (i8)Error::BadCertificate;
- m_context.critical_error = payload_res;
- break;
- }
-
- // swap the first certificate with the valid one
- if (it.index() != 0)
- swap(m_context.certificates[0], m_context.certificates[it.index()]);
- }
} else {
payload_res = (i8)Error::UnexpectedMessage;
}
diff --git a/Userland/Libraries/LibTLS/HandshakeCertificate.cpp b/Userland/Libraries/LibTLS/HandshakeCertificate.cpp
index ed7cc5b508..dd1bcfbad2 100644
--- a/Userland/Libraries/LibTLS/HandshakeCertificate.cpp
+++ b/Userland/Libraries/LibTLS/HandshakeCertificate.cpp
@@ -82,10 +82,8 @@ ssize_t TLSv12::handle_certificate(ReadonlyBytes buffer)
auto certificate = Certificate::parse_asn1(buffer.slice(res_cert, certificate_size_specific), false);
if (certificate.has_value()) {
- if (certificate.value().is_valid()) {
- m_context.certificates.append(certificate.value());
- valid_certificate = true;
- }
+ m_context.certificates.append(certificate.value());
+ valid_certificate = true;
}
res_cert += certificate_size_specific;
} while (remaining > 0);
diff --git a/Userland/Libraries/LibTLS/TLSv12.cpp b/Userland/Libraries/LibTLS/TLSv12.cpp
index 282ebc4033..572c778e52 100644
--- a/Userland/Libraries/LibTLS/TLSv12.cpp
+++ b/Userland/Libraries/LibTLS/TLSv12.cpp
@@ -13,6 +13,7 @@
#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 <LibTLS/TLSv12.h>
#include <errno.h>
@@ -175,15 +176,18 @@ void TLSv12::try_disambiguate_error() const
void TLSv12::set_root_certificates(Vector<Certificate> certificates)
{
- if (!m_context.root_certificates.is_empty())
+ if (!m_context.root_certificates.is_empty()) {
dbgln("TLS warn: resetting root certificates!");
+ m_context.root_certificates.clear();
+ }
for (auto& cert : certificates) {
if (!cert.is_valid())
dbgln("Certificate for {} by {} is invalid, things may or may not work!", cert.subject.subject, cert.issuer.subject);
// FIXME: Figure out what we should do when our root certs are invalid.
+
+ m_context.root_certificates.set(cert.subject_identifier_string(), cert);
}
- m_context.root_certificates = move(certificates);
dbgln_if(TLS_DEBUG, "{}: Set {} root certificates", this, m_context.root_certificates.size());
}
@@ -200,40 +204,101 @@ bool Context::verify_chain() const
local_chain = &certificates;
}
- // FIXME: Actually verify the signature, instead of just checking the name.
- HashMap<String, String> chain;
- HashTable<String> roots;
- // First, walk the root certs.
- for (auto& cert : root_certificates) {
- roots.set(cert.subject.subject);
- chain.set(cert.subject.subject, cert.issuer.subject);
+ if (local_chain->is_empty()) {
+ dbgln("verify_chain: Attempting to verify an empty chain");
+ return false;
}
- // Then, walk the local certs.
- for (auto& cert : *local_chain) {
- auto& issuer_unique_name = cert.issuer.unit.is_empty() ? cert.issuer.subject : cert.issuer.unit;
- chain.set(cert.subject.subject, issuer_unique_name);
- }
+ for (size_t cert_index = 0; cert_index < local_chain->size(); ++cert_index) {
+ auto cert = local_chain->at(cert_index);
- // Then verify the chain.
- for (auto& it : chain) {
- if (it.key == it.value) { // Allow self-signed certificates.
- if (!roots.contains(it.key))
- dbgln("Self-signed warning: Certificate for {} is self-signed", it.key);
- continue;
- }
+ auto subject_string = cert.subject_identifier_string();
+ auto issuer_string = cert.issuer_identifier_string();
- auto ref = chain.get(it.value);
- if (!ref.has_value()) {
- dbgln("{}: Certificate for {} is not signed by anyone we trust ({})", this, it.key, it.value);
+ if (!cert.is_valid()) {
+ dbgln("verify_chain: Certificate is not valid {}", subject_string);
return false;
}
- if (ref.value() == it.key) // Allow (but warn about) mutually recursively signed cert A <-> B.
- dbgln("Co-dependency warning: Certificate for {} is issued by {}, which itself is issued by {}", ref.value(), it.key, ref.value());
+ auto maybe_root_certificate = root_certificates.get(issuer_string);
+ if (maybe_root_certificate.has_value()) {
+ auto root_certificate = maybe_root_certificate.release_value();
+ auto verification_correct = verify_certificate_pair(cert, root_certificate);
+
+ if (!verification_correct) {
+ dbgln("verify_chain: Signature inconsistent, {} was not signed by {} (root certificate)", subject_string, issuer_string);
+ return false;
+ }
+
+ // Root certificate reached, and correctly verified, so we can stop now
+ return true;
+ } else {
+ if (subject_string == issuer_string) {
+ dbgln("verify_chain: Non-root self-signed certificate");
+ return false;
+ }
+ if ((cert_index + 1) >= local_chain->size()) {
+ dbgln("verify_chain: No trusted root certificate found before end of certificate chain");
+ dbgln("verify_chain: Last certificate in chain was signed by {}", issuer_string);
+ return false;
+ }
+
+ auto parent_certificate = local_chain->at(cert_index + 1);
+ if (issuer_string != parent_certificate.subject_identifier_string()) {
+ dbgln("verify_chain: Next certificate in the chain is not the issuer of this certificate");
+ return false;
+ }
+
+ bool verification_correct = verify_certificate_pair(cert, parent_certificate);
+ if (!verification_correct) {
+ dbgln("verify_chain: Signature inconsistent, {} was not signed by {}", subject_string, issuer_string);
+ return false;
+ }
+ }
}
- return true;
+ // Either a root certificate is reached, or parent validation fails as the end of the local chain is reached
+ VERIFY_NOT_REACHED();
+}
+
+bool Context::verify_certificate_pair(Certificate& subject, Certificate& issuer) const
+{
+ Crypto::Hash::HashKind kind;
+ switch (subject.signature_algorithm) {
+ case CertificateKeyAlgorithm::RSA_SHA1:
+ kind = Crypto::Hash::HashKind::SHA1;
+ break;
+ case CertificateKeyAlgorithm::RSA_SHA256:
+ kind = Crypto::Hash::HashKind::SHA256;
+ break;
+ case CertificateKeyAlgorithm::RSA_SHA384:
+ kind = Crypto::Hash::HashKind::SHA384;
+ break;
+ case CertificateKeyAlgorithm::RSA_SHA512:
+ kind = Crypto::Hash::HashKind::SHA512;
+ break;
+ default:
+ dbgln("verify_certificate_pair: Unknown signature algorithm, expected RSA with SHA1/256/384/512, got {}", (u8)subject.signature_algorithm);
+ return false;
+ }
+
+ Crypto::PK::RSAPrivateKey dummy_private_key;
+ auto rsa = Crypto::PK::RSA(issuer.public_key, dummy_private_key);
+ auto verification_buffer_result = ByteBuffer::create_uninitialized(subject.signature_value.size());
+ if (verification_buffer_result.is_error()) {
+ dbgln("verify_certificate_pair: Unable to allocate buffer for verification");
+ return false;
+ }
+ auto verification_buffer = verification_buffer_result.release_value();
+ auto verification_buffer_bytes = verification_buffer.bytes();
+ rsa.verify(subject.signature_value, verification_buffer_bytes);
+
+ // FIXME: This slice is subject hack, this will work for most certificates, but you actually have to parse
+ // the ASN.1 data to correctly extract the signed part of the certificate.
+ ReadonlyBytes message = subject.original_asn1.bytes().slice(4, subject.original_asn1.size() - 4 - (5 + subject.signature_value.size()) - 15);
+ auto pkcs1 = Crypto::PK::EMSA_PKCS1_V1_5<Crypto::Hash::Manager>(kind);
+ auto verification = pkcs1.verify(message, verification_buffer_bytes, subject.signature_value.size() * 8);
+ return verification == Crypto::VerificationConsistency::Consistent;
}
template<typename HMACType>
diff --git a/Userland/Libraries/LibTLS/TLSv12.h b/Userland/Libraries/LibTLS/TLSv12.h
index 4eda14d91b..20a8478388 100644
--- a/Userland/Libraries/LibTLS/TLSv12.h
+++ b/Userland/Libraries/LibTLS/TLSv12.h
@@ -262,6 +262,7 @@ struct Options {
struct Context {
bool verify_chain() const;
+ bool verify_certificate_pair(Certificate& subject, Certificate& issuer) const;
Options options;
@@ -321,7 +322,7 @@ struct Context {
// message flags
u8 handshake_messages[11] { 0 };
ByteBuffer user_data;
- Vector<Certificate> root_certificates;
+ HashMap<String, Certificate> root_certificates;
Vector<String> alpn;
StringView negotiated_alpn;