From a6e465fba280346e937aa9f0203ea64820c01d78 Mon Sep 17 00:00:00 2001
From: Michiel Visser <opensource@webmichiel.nl>
Date: Wed, 23 Feb 2022 17:27:55 +0100
Subject: [PATCH] LibCrypto: Implement custom BitStringView for ASN.1 decoder

The ASN.1 decoder was originally using AK::BitmapView for decoded
BitStrings, however the specification requires that the bits are stored
in a byte from the most significant to the least significant.

Storing three bits '110' would result in a byte '1100 0000', i.e. 0xC0.
However, AK::BitmapView expects the bits to be stored at the bottom like
'0000 0110', i.e. 0x06. For the current uses the data was always a
multiple of eight bits, resulting in complete bytes, which could
directly be interpreted correctly.

For the implementation of the key usage extension of certificates the
correct implementation of the BitString is required.
---
 Userland/Libraries/LibCrypto/ASN1/DER.cpp |  4 ++--
 Userland/Libraries/LibCrypto/ASN1/DER.h   | 28 ++++++++++++++++++++++-
 Userland/Libraries/LibTLS/Certificate.cpp |  8 +++----
 3 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/Userland/Libraries/LibCrypto/ASN1/DER.cpp b/Userland/Libraries/LibCrypto/ASN1/DER.cpp
index 846327c553a..339759340ce 100644
--- a/Userland/Libraries/LibCrypto/ASN1/DER.cpp
+++ b/Userland/Libraries/LibCrypto/ASN1/DER.cpp
@@ -169,7 +169,7 @@ Result<StringView, DecodeError> Decoder::decode_printable_string(ReadonlyBytes d
     return StringView { data };
 }
 
-Result<const BitmapView, DecodeError> Decoder::decode_bit_string(ReadonlyBytes data)
+Result<BitStringView, DecodeError> Decoder::decode_bit_string(ReadonlyBytes data)
 {
     if (data.size() < 1)
         return DecodeError::InvalidInputFormat;
@@ -180,7 +180,7 @@ Result<const BitmapView, DecodeError> Decoder::decode_bit_string(ReadonlyBytes d
     if (unused_bits > total_size_in_bits)
         return DecodeError::Overflow;
 
-    return BitmapView { const_cast<u8*>(data.offset_pointer(1)), total_size_in_bits - unused_bits };
+    return BitStringView { data.slice(1), unused_bits };
 }
 
 Result<Tag, DecodeError> Decoder::peek()
diff --git a/Userland/Libraries/LibCrypto/ASN1/DER.h b/Userland/Libraries/LibCrypto/ASN1/DER.h
index 023b3e22586..6081c40473d 100644
--- a/Userland/Libraries/LibCrypto/ASN1/DER.h
+++ b/Userland/Libraries/LibCrypto/ASN1/DER.h
@@ -26,6 +26,32 @@ enum class DecodeError {
     UnsupportedFormat,
 };
 
+class BitStringView {
+public:
+    BitStringView(ReadonlyBytes data, size_t unused_bits)
+        : m_data(data)
+        , m_unused_bits(unused_bits)
+    {
+    }
+
+    ReadonlyBytes raw_bytes() const
+    {
+        VERIFY(m_unused_bits == 0);
+        return m_data;
+    }
+
+    bool get(size_t index)
+    {
+        if (index >= 8 * m_data.size() - m_unused_bits)
+            return false;
+        return 0 != (m_data[index / 8] & (1u << (7 - (index % 8))));
+    }
+
+private:
+    ReadonlyBytes m_data;
+    size_t m_unused_bits;
+};
+
 class Decoder {
 public:
     Decoder(ReadonlyBytes data)
@@ -194,7 +220,7 @@ private:
     static Result<std::nullptr_t, DecodeError> decode_null(ReadonlyBytes);
     static Result<Vector<int>, DecodeError> decode_object_identifier(ReadonlyBytes);
     static Result<StringView, DecodeError> decode_printable_string(ReadonlyBytes);
-    static Result<const BitmapView, DecodeError> decode_bit_string(ReadonlyBytes);
+    static Result<BitStringView, DecodeError> decode_bit_string(ReadonlyBytes);
 
     Vector<ReadonlyBytes> m_stack;
     Optional<Tag> m_current_tag;
diff --git a/Userland/Libraries/LibTLS/Certificate.cpp b/Userland/Libraries/LibTLS/Certificate.cpp
index f1c806a36ca..1be8e1e999b 100644
--- a/Userland/Libraries/LibTLS/Certificate.cpp
+++ b/Userland/Libraries/LibTLS/Certificate.cpp
@@ -306,9 +306,9 @@ Optional<Certificate> Certificate::parse_asn1(ReadonlyBytes buffer, bool)
         if (!parse_algorithm_identifier(certificate.key_algorithm).has_value())
             return {};
 
-        READ_OBJECT_OR_FAIL(BitString, const BitmapView, value, "Certificate::TBSCertificate::subject_public_key_info::subject_public_key_info");
+        READ_OBJECT_OR_FAIL(BitString, Crypto::ASN1::BitStringView, value, "Certificate::TBSCertificate::subject_public_key_info::subject_public_key_info");
         // Note: Once we support other kinds of keys, make sure to check the kind here!
-        auto key = Crypto::PK::RSA::parse_rsa_key({ value.data(), value.size_in_bytes() });
+        auto key = Crypto::PK::RSA::parse_rsa_key(value.raw_bytes());
         if (!key.public_key.length()) {
             dbgln_if(TLS_DEBUG, "Certificate::TBSCertificate::subject_public_key_info::subject_public_key_info: Invalid key");
             return {};
@@ -478,8 +478,8 @@ Optional<Certificate> Certificate::parse_asn1(ReadonlyBytes buffer, bool)
 
     // signature_value
     {
-        READ_OBJECT_OR_FAIL(BitString, const BitmapView, value, "Certificate");
-        auto signature_data_result = ByteBuffer::copy(value.data(), value.size_in_bytes());
+        READ_OBJECT_OR_FAIL(BitString, Crypto::ASN1::BitStringView, value, "Certificate");
+        auto signature_data_result = ByteBuffer::copy(value.raw_bytes());
         if (signature_data_result.is_error()) {
             dbgln("Certificate::signature_value: out of memory");
             return {};