From 1af358b30b7c7d94aca109c83c8ffab66c00a360 Mon Sep 17 00:00:00 2001 From: Danny Halawi Date: Mon, 31 Aug 2020 16:07:26 +0000 Subject: [PATCH 1/4] 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. --- proto/asn1-pdu/asn1_pdu.proto | 1 - proto/asn1-pdu/asn1_universal_types_to_der.cc | 4 ++-- proto/asn1-pdu/x509_certificate_to_der.cc | 22 ++++++++++++------- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/proto/asn1-pdu/asn1_pdu.proto b/proto/asn1-pdu/asn1_pdu.proto index 551e285..524ed8c 100644 --- a/proto/asn1-pdu/asn1_pdu.proto +++ b/proto/asn1-pdu/asn1_pdu.proto @@ -43,7 +43,6 @@ message TagNumber { } enum LowTagNumber { - VAL0 = 0; VAL1 = 1; VAL2 = 2; VAL3 = 3; 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..a2b996f 100644 --- a/proto/asn1-pdu/x509_certificate_to_der.cc +++ b/proto/asn1-pdu/x509_certificate_to_der.cc @@ -307,12 +307,18 @@ 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: |val| 0 is DEFAULT. + // (X.690 (2015), 11.5): DEFAULT value in a sequence field is not encoded. + if (val != 0) { + // |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 | kAsn1Constructed | 0x00, 0x03, kAsn1Integer, + 0x01, static_cast(val)}; + der.insert(der.end(), der_version.begin(), der_version.end()); + } } DECLARE_ENCODE_FUNCTION(TBSCertificateSequence) { @@ -342,8 +348,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()) { From 24d7f9dbef50fe57e0793f70a1f5ca33168685da Mon Sep 17 00:00:00 2001 From: Danny Halawi Date: Mon, 31 Aug 2020 17:06:40 +0000 Subject: [PATCH 2/4] 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_pdu.proto | 1 + proto/asn1-pdu/x509_certificate_to_der.cc | 13 +++++++------ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/proto/asn1-pdu/asn1_pdu.proto b/proto/asn1-pdu/asn1_pdu.proto index 524ed8c..551e285 100644 --- a/proto/asn1-pdu/asn1_pdu.proto +++ b/proto/asn1-pdu/asn1_pdu.proto @@ -43,6 +43,7 @@ message TagNumber { } enum LowTagNumber { + VAL0 = 0; VAL1 = 1; VAL2 = 2; VAL3 = 3; diff --git a/proto/asn1-pdu/x509_certificate_to_der.cc b/proto/asn1-pdu/x509_certificate_to_der.cc index a2b996f..3d6c9c7 100644 --- a/proto/asn1-pdu/x509_certificate_to_der.cc +++ b/proto/asn1-pdu/x509_certificate_to_der.cc @@ -307,13 +307,14 @@ DECLARE_ENCODE_FUNCTION(ValiditySequence) { } DECLARE_ENCODE_FUNCTION(VersionNumber) { - // RFC 5280, 4.1 & 4.1.2.1: |val| 0 is DEFAULT. - // (X.690 (2015), 11.5): DEFAULT value in a sequence field is not encoded. + // 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) { - // |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). + // 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)}; From 0f36b8bf45b2806aae472229bdb68b69d0f4c2f6 Mon Sep 17 00:00:00 2001 From: Danny Halawi Date: Wed, 2 Sep 2020 22:01:19 +0000 Subject: [PATCH 3/4] Adding Mutated Chain and Trust Parameter - Create a certificate chain and mutate it to create structural relationships between the certs. - Add a parameter to indicate if a cert is trusted or not. --- proto/asn1-pdu/mutated_x509_chain.proto | 63 ++++++++++++ proto/asn1-pdu/mutated_x509_chain_to_der.cc | 106 ++++++++++++++++++++ proto/asn1-pdu/mutated_x509_chain_to_der.h | 38 +++++++ 3 files changed, 207 insertions(+) create mode 100644 proto/asn1-pdu/mutated_x509_chain.proto create mode 100644 proto/asn1-pdu/mutated_x509_chain_to_der.cc create mode 100644 proto/asn1-pdu/mutated_x509_chain_to_der.h diff --git a/proto/asn1-pdu/mutated_x509_chain.proto b/proto/asn1-pdu/mutated_x509_chain.proto new file mode 100644 index 0000000..9ec7b26 --- /dev/null +++ b/proto/asn1-pdu/mutated_x509_chain.proto @@ -0,0 +1,63 @@ +// Copyright 2020 Google Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +//////////////////////////////////////////////////////////////////////////////// + +// This protobuf represents an X.509 certificate chain with a set of operations +// to mutate the chain to create structural relationships between the +// certificates. + +syntax = "proto2"; + +import "x509_certificate.proto"; + +package x509_certificate; + +message MutatedChain { + // An X.509 Certificate Chain comprises a list of certificates. + repeated x509_certificate.X509Certificate chain = 1; + repeated TrustParameter trust_parameters = 2; + repeated Mutation mutations = 3; +} + +message TrustParameter { + // Allow certificate to be trusted or not-trusted for fuzzing. + required bool trusted = 1; + // |index| represents the position of the certificate that will be mutated. + required CertIndex index = 2; +} + +// Mutate the certificate chain protobuf to force structural relationships +// between the certificates. +message Mutation { + oneof types { + MutateSignature mutate_signature = 1; + } +} + +message MutateSignature { + // Allow certificate to be valid or invalid for fuzzing. + required bool valid = 1; + // |index| represents the position of the certificate that will be mutated. + required CertIndex index = 2; +} + +// Contains the positions of the certiciate chain that will be mutated. +enum CertIndex { + CERT_0 = 0; + CERT_1 = 1; + CERT_2 = 2; + CERT_3 = 3; + CERT_4 = 4; +} \ No newline at end of file diff --git a/proto/asn1-pdu/mutated_x509_chain_to_der.cc b/proto/asn1-pdu/mutated_x509_chain_to_der.cc new file mode 100644 index 0000000..50f5d3e --- /dev/null +++ b/proto/asn1-pdu/mutated_x509_chain_to_der.cc @@ -0,0 +1,106 @@ +// Copyright 2020 Google Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +//////////////////////////////////////////////////////////////////////////////// + +#include "mutated_x509_chain_to_der.h" +#include "x509_certificate_to_der.h" + +#include + +#include +#include + +namespace x509_certificate { + +// DER encodes each |certificate| in |chain| and returns +// the encoded certificates in |der|. +std::vector> EncodeChain( + const google::protobuf::RepeatedPtrField& chain) { + std::vector> der; + + for (const auto& cert : chain) { + der.push_back(X509CertificateToDER(cert)); + } + + return der; +} + +void SetTrust( + std::vector& encoded_mutated_chain, + google::protobuf::RepeatedPtrField& trust_parameters) { + for (const auto& trust_parameter : trust_parameters) { + if (trust_parameter.index() >= encoded_mutated_chain.size()) { + continue; + } + + encoded_mutated_chain[trust_parameter.index()].trusted = + trust_parameter.trusted(); + } +} + +void Mutate(const MutateSignature& mutation, + google::protobuf::RepeatedPtrField& chain) { + // An |operation| on a certiciate cannot be executed if the |index| is greater + // than or euqal to the |size| of the certificate chain. + if (mutation.index() >= chain.size()) { + return; + } + + auto* signature_value = chain[mutation.index()].mutable_signature_value(); + signature_value->clear_pdu(); + signature_value->mutable_value()->set_unused_bits( + asn1_universal_types::UnusedBits::VAL0); + // Represent a valid signature value with 1 and invalid with 0. + if (mutation.valid()) { + signature_value->mutable_value()->set_val("1"); + } else { + signature_value->mutable_value()->set_val("0"); + } +} + +void Mutate(const Mutation& mutation, + google::protobuf::RepeatedPtrField& chain) { + switch (mutation.types_case()) { + case Mutation::TypesCase::kMutateSignature: + Mutate(mutation.mutate_signature(), chain); + break; + case Mutation::TypesCase::TYPES_NOT_SET: + return; + } +} + +std::vector MutatedChainToDER(const MutatedChain& mutated_chain) { + auto chain = mutated_chain.chain(); + auto trust_parameters = mutated_chain.trust_parameters(); + auto mutations = mutated_chain.mutations(); + + if (chain.empty()) { + return {{}}; + } + + for (const auto& mutation : mutations) { + Mutate(mutation, chain); + } + + std::vector encoded_mutated_chain; + for (const auto& encoded_cert : EncodeChain(chain)) { + encoded_mutated_chain.push_back({encoded_cert, false}); + } + SetTrust(encoded_mutated_chain, trust_parameters); + + return encoded_mutated_chain; +} + +} // namespace x509_certificate \ No newline at end of file diff --git a/proto/asn1-pdu/mutated_x509_chain_to_der.h b/proto/asn1-pdu/mutated_x509_chain_to_der.h new file mode 100644 index 0000000..49ec616 --- /dev/null +++ b/proto/asn1-pdu/mutated_x509_chain_to_der.h @@ -0,0 +1,38 @@ +// Copyright 2020 Google Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +//////////////////////////////////////////////////////////////////////////////// + +#ifndef PROTO_ASN1_PDU_MUTATE_X509_CHAIN_H_ +#define PROTO_ASN1_PDU_MUTATE_X509_CHAIN_H_ + +#include + +#include + +#include "mutated_x509_chain.pb.h" + +namespace x509_certificate { + +struct X509 { + std::vector cert; + bool trusted; +}; + +// Applies |operations| to |x509_chain| and returns the DER encoded chain. +std::vector MutatedChainToDER(const MutatedChain& mutated_chain); + +} // namespace x509_certificate + +#endif // PROTO_ASN1_PDU_X509_CERTIFICATE_TO_DER_H_ \ No newline at end of file From 92a6344936f74b4f93b68794ff3a73969c4b7320 Mon Sep 17 00:00:00 2001 From: Danny Halawi Date: Wed, 2 Sep 2020 22:16:54 +0000 Subject: [PATCH 4/4] nit: delete for fuzzing in comment --- proto/asn1-pdu/mutated_x509_chain.proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proto/asn1-pdu/mutated_x509_chain.proto b/proto/asn1-pdu/mutated_x509_chain.proto index 9ec7b26..cd5c70c 100644 --- a/proto/asn1-pdu/mutated_x509_chain.proto +++ b/proto/asn1-pdu/mutated_x509_chain.proto @@ -32,7 +32,7 @@ message MutatedChain { } message TrustParameter { - // Allow certificate to be trusted or not-trusted for fuzzing. + // Allow certificate to be trusted or not-trusted. required bool trusted = 1; // |index| represents the position of the certificate that will be mutated. required CertIndex index = 2;