Use BER_Decoder::get_next to cleanup X.509 cert decoding code

3 files changed, 24 insertions, 15 deletions

diff --git a/src/lib/asn1/ber_dec.cpp b/src/lib/asn1/ber_dec.cpp
index 28443f2d8..35f1a53b5 100644
--- a/src/lib/asn1/ber_dec.cpp
+++ b/src/lib/asn1/ber_dec.cpp
@@ -162,8 +162,16 @@ bool BER_Decoder::more_items() const
 */
 BER_Decoder& BER_Decoder::verify_end()
    {
+   return verify_end("BER_Decoder::verify_end called, but data remains");
+   }
+
+/*
+* Verify that no bytes remain in the source
+*/
+BER_Decoder& BER_Decoder::verify_end(const std::string& err)
+   {
    if(!m_source->end_of_data() || m_pushed.is_set())
-      throw Invalid_State("BER_Decoder::verify_end called, but data remains");
+      throw Decoding_Error(err);
    return (*this);
    }
 
diff --git a/src/lib/asn1/ber_dec.h b/src/lib/asn1/ber_dec.h
index ab2516175..d454c45d6 100644
--- a/src/lib/asn1/ber_dec.h
+++ b/src/lib/asn1/ber_dec.h
@@ -27,6 +27,8 @@ class BOTAN_PUBLIC_API(2,0) BER_Decoder final
 
       bool more_items() const;
       BER_Decoder& verify_end();
+      BER_Decoder& verify_end(const std::string& err_msg);
+
       BER_Decoder& discard_remaining();
 
       BER_Decoder  start_cons(ASN1_Tag type_tag, ASN1_Tag class_tag = UNIVERSAL);
diff --git a/src/lib/x509/x509cert.cpp b/src/lib/x509/x509cert.cpp
index 8d74d614e..ddfe5d5b2 100644
--- a/src/lib/x509/x509cert.cpp
+++ b/src/lib/x509/x509cert.cpp
@@ -106,11 +106,12 @@ std::unique_ptr<X509_Certificate_Data> parse_x509_cert_body(const X509_Object& o
    {
    std::unique_ptr<X509_Certificate_Data> data(new X509_Certificate_Data);
 
-   BER_Decoder tbs_cert(obj.signed_body());
    BigInt serial_bn;
+   BER_Object public_key;
+   BER_Object v3_exts_data;
 
-   tbs_cert.decode_optional(data->m_version, ASN1_Tag(0),
-                            ASN1_Tag(CONSTRUCTED | CONTEXT_SPECIFIC))
+   BER_Decoder(obj.signed_body())
+      .decode_optional(data->m_version, ASN1_Tag(0), ASN1_Tag(CONSTRUCTED | CONTEXT_SPECIFIC))
       .decode(serial_bn)
       .decode(data->m_sig_algo_inner)
       .decode(data->m_issuer_dn)
@@ -118,12 +119,20 @@ std::unique_ptr<X509_Certificate_Data> parse_x509_cert_body(const X509_Object& o
          .decode(data->m_not_before)
          .decode(data->m_not_after)
       .end_cons()
-      .decode(data->m_subject_dn);
+      .decode(data->m_subject_dn)
+      .get_next(public_key)
+      .decode_optional_string(data->m_v2_issuer_key_id, BIT_STRING, 1)
+      .decode_optional_string(data->m_v2_subject_key_id, BIT_STRING, 2)
+      .get_next(v3_exts_data)
+      .verify_end("TBSCertificate has extra data after extensions block");
 
    if(data->m_version > 2)
       throw Decoding_Error("Unknown X.509 cert version " + std::to_string(data->m_version));
    if(obj.signature_algorithm() != data->m_sig_algo_inner)
       throw Decoding_Error("X.509 Certificate had differing algorithm identifers in inner and outer ID fields");
+
+   public_key.assert_is_a(SEQUENCE, CONSTRUCTED, "X.509 certificate public key");
+
    // crude method to save the serial's sign; will get lost during decoding, otherwise
    data->m_serial_negative = serial_bn.is_negative();
 
@@ -134,9 +143,6 @@ std::unique_ptr<X509_Certificate_Data> parse_x509_cert_body(const X509_Object& o
    data->m_subject_dn_bits = ASN1::put_in_sequence(data->m_subject_dn.get_bits());
    data->m_issuer_dn_bits = ASN1::put_in_sequence(data->m_issuer_dn.get_bits());
 
-   BER_Object public_key = tbs_cert.get_next_object();
-   public_key.assert_is_a(SEQUENCE, CONSTRUCTED, "X.509 certificate public key");
-
    // validate_public_key_params(public_key.value);
    AlgorithmIdentifier public_key_alg_id;
    BER_Decoder(public_key).decode(public_key_alg_id).discard_remaining();
@@ -193,10 +199,6 @@ std::unique_ptr<X509_Certificate_Data> parse_x509_cert_body(const X509_Object& o
       .decode(data->m_subject_public_key_algid)
       .decode(data->m_subject_public_key_bitstring, BIT_STRING);
 
-   tbs_cert.decode_optional_string(data->m_v2_issuer_key_id, BIT_STRING, 1);
-   tbs_cert.decode_optional_string(data->m_v2_subject_key_id, BIT_STRING, 2);
-
-   BER_Object v3_exts_data = tbs_cert.get_next_object();
    if(v3_exts_data.is_a(3, ASN1_Tag(CONSTRUCTED | CONTEXT_SPECIFIC)))
       {
       BER_Decoder(v3_exts_data).decode(data->m_v3_extensions).verify_end();
@@ -204,9 +206,6 @@ std::unique_ptr<X509_Certificate_Data> parse_x509_cert_body(const X509_Object& o
    else if(v3_exts_data.is_set())
       throw BER_Bad_Tag("Unknown tag in X.509 cert", v3_exts_data.tagging());
 
-   if(tbs_cert.more_items())
-      throw Decoding_Error("TBSCertificate has extra data after extensions block");
-
    // Now cache some fields from the extensions
    if(auto ext = data->m_v3_extensions.get_extension_object_as<Cert_Extension::Key_Usage>())
       {