From e63764a0cef518997a4f4ac40e1a1dfd886ecbc1 Mon Sep 17 00:00:00 2001 From: Danny Halawi Date: Mon, 31 Aug 2020 11:01:48 -0700 Subject: [PATCH] [proto] Fixing Bugs in ASN.1 PDU, ASN.1 Universal Types, and X.509 Certificate (#72) * Fix ASN.1 encoding issues in X.509 & Univ. Types. - GeneralizedTime and UTCTime were flipped for encoding. - Version is EXPLICIT tagged. The encoder was incorrectly using IMPLICIT tagging. * Improve Comment for VersionNumber & Add Val0 back. - Make the coment more clear for encoding the VersionNumber in X.509 cert to der. - Add Val0 back in ASN.1 PDU proto to be able to generate context-specific 0, amongst other things. --- proto/asn1-pdu/asn1_universal_types_to_der.cc | 4 ++-- proto/asn1-pdu/x509_certificate_to_der.cc | 23 ++++++++++++------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/proto/asn1-pdu/asn1_universal_types_to_der.cc b/proto/asn1-pdu/asn1_universal_types_to_der.cc index dda23c3..999b6d2 100644 --- a/proto/asn1-pdu/asn1_universal_types_to_der.cc +++ b/proto/asn1-pdu/asn1_universal_types_to_der.cc @@ -111,7 +111,7 @@ void Encode(const UTCTime& utc_time, std::vector& der) { // after the value is encoded. const size_t tag_len_pos = der.size(); - EncodeTimestamp(utc_time.time_stamp(), false, der); + EncodeTimestamp(utc_time.time_stamp(), true, der); // Check if encoding was successful. if (der.size() != tag_len_pos) { @@ -126,7 +126,7 @@ void Encode(const GeneralizedTime& generalized_time, // after the value is encoded. const size_t tag_len_pos = der.size(); - EncodeTimestamp(generalized_time.time_stamp(), true, der); + EncodeTimestamp(generalized_time.time_stamp(), false, der); // Check if encoding was successful. if (der.size() != tag_len_pos) { diff --git a/proto/asn1-pdu/x509_certificate_to_der.cc b/proto/asn1-pdu/x509_certificate_to_der.cc index b47caf0..3d6c9c7 100644 --- a/proto/asn1-pdu/x509_certificate_to_der.cc +++ b/proto/asn1-pdu/x509_certificate_to_der.cc @@ -307,12 +307,19 @@ DECLARE_ENCODE_FUNCTION(ValiditySequence) { } DECLARE_ENCODE_FUNCTION(VersionNumber) { - // |version| is Context-specific with tag number 0 (RFC 5280, 4.1 & 4.1.2.1). - // Takes on values 0, 1 and 2, so only require length of 1 to - // encode it (RFC 5280, 4.1 & 4.1.2.1). - std::vector der_version = {kAsn1ContextSpecific | 0x00, 0x01, - static_cast(val)}; - der.insert(der.end(), der_version.begin(), der_version.end()); + // RFC 5280, 4.1 & 4.1.2.1: + // version [0] EXPLICIT Version DEFAULT v1, + // Version ::= INTEGER { v1(0), v2(1), v3(2) } + // + // X.690 (2015), 11.5: DEFAULT value in a sequence field is not encoded + if (val != 0) { + // Use a fixed buffer for the EXPLICIT encoding, since the version is always + // a one byte INTEGER. + std::vector der_version = { + kAsn1ContextSpecific | kAsn1Constructed | 0x00, 0x03, kAsn1Integer, + 0x01, static_cast(val)}; + der.insert(der.end(), der_version.begin(), der_version.end()); + } } DECLARE_ENCODE_FUNCTION(TBSCertificateSequence) { @@ -342,8 +349,8 @@ DECLARE_ENCODE_FUNCTION(TBSCertificateSequence) { if (val.has_subject_unique_id()) { size_t pos_of_tag = der.size(); Encode(val.subject_unique_id(), der); - // |subject_unqiue_id| is Context-specific with tag number 2 (RFC 5280, 4.1 - // & 4.1.2.8). + // |subject_unqiue_id| is Context-specific with tag number 2 (RFC + // 5280, 4.1 & 4.1.2.8). ReplaceTag(kAsn1ContextSpecific | 0x02, pos_of_tag, der); } if (val.has_extensions()) {