Remove the DER validator

rfc6488#3.1.l states we need to check "the signed object is DER
encoded." But that's not what this code was doing.

First, the validation was only kicking in specifically during the
decoding of the ContentInfo, which is just the outermost layer of the
signed object.

Second, the validation was incorrect. This seems to be the intended
algorithm in pseudocode:

	boolean is_der_encoded(original_bytes):
		der_bytes = der_encode(ber_decode(original_bytes));
		return (original_bytes equal der_bytes);

This is what the code was actually doing:

	boolean is_der_encoded(original_bytes):
		der_bytes = der_encode(ber_decode(original_bytes));
		return (original_bytes.length equals der_bytes.length);

These two quirks made the validation mostly a no-op.

There's also the issue that this implementation seems inefficient,
especially since Fort doesn't need to DER-encode anywhere else. By
checking the encoding while parsing, I would save a lot of memory
in addition to being able to delete that mess of encoding functions.

But I'm going to have to push that to the future. This is growing more
ambitious than I can afford during a release review, and given that the
code wasn't really doing anything productive in the first place, I'm not
losing much by simply axing it for now.

diff --git a/docs/incidence.md b/docs/incidence.md
index 4fd56c0f679009646b5d95c4e5ff4fc6b259f074..e389ea8dff79e47738b2a4b61617b62486e6e613 100644 (file)
--- a/docs/incidence.md
+++ b/docs/incidence.md
@@ -98,20 +98,7 @@ This only applies to digest parameters that have been defined as NULL objects; a
 - **Name:** `incid-obj-not-der-encoded`
 - **Default action:** `ignore`
 
-
-[RFC 6488](https://tools.ietf.org/html/rfc6488) mandates that all signed objects must be DER-encoded (see [section 3](https://tools.ietf.org/html/rfc6488#section-3)):
-
-```
-      l.  The signed object is DER encoded.
-```
-
-Altough this is mandatory, quite a few signed objects in the global RPKI ignore this rule and are simply BER-encoded.
-
-If not `ignore`d, FORT will report this incidence with the following error message at the validation log:
-
-```
-<log level>: <offending file name>: '<object>' isn't DER encoded
-```
+Deprecated. `incid-obj-not-der-encoded` does nothing as of Fort 1.6.2.
 
 ### File listed at manifest doesn't exist
 

diff --git a/src/asn1/content_info.c b/src/asn1/content_info.c
index 1aacb7939ede304da3006826743783674e03b5fd..fa6900bb6a8cd9ea78cee4cae179fe3aaf6b6e40 100644 (file)
--- a/src/asn1/content_info.c
+++ b/src/asn1/content_info.c
@@ -32,12 +32,13 @@ decode(struct file_contents *fc, struct ContentInfo **result)
        struct ContentInfo *cinfo;
        int error;
 
-       /* Validate DER encoding rfc6488#section3 bullet 1.l */
        error = asn1_decode_fc(fc, &asn_DEF_ContentInfo, (void **) &cinfo,
-           true, true);
+           true);
        if (error)
                return error;
 
+       /* TODO (asn1c) rfc6488#3.1.l: Validate DER encoding */
+
        error = validate(cinfo);
        if (error) {
                content_info_free(cinfo);

diff --git a/src/asn1/decode.c b/src/asn1/decode.c
index b416e3397b1c262ab12d253424baf926200e2bd8..7b0a2bec05f6b22f8fcbe4929d99a73ad033d8c0 100644 (file)
--- a/src/asn1/decode.c
+++ b/src/asn1/decode.c
@@ -32,58 +32,9 @@ validate(asn_TYPE_descriptor_t const *descriptor, void *result, bool log)
        return 0;
 }
 
-static int
-der_coder(const void *buf, size_t size, void *app_key)
-{
-       struct ber_data *data = app_key;
-
-       if (data->consumed + size > data->src_size) {
-               pr_val_debug("DER encoding will consume more bytes than expected (expected %lu, will get %lu)",
-                   data->consumed + size, data->src_size);
-               return -1;
-       }
-
-       if (memcmp(data->src + data->consumed, buf, size) != 0)
-               return -1;
-
-       data->consumed += size;
-       return 0;
-}
-
-/*
- * TODO (performance) This isn't efficient, consider implement DER decoding
- * or something better.
- */
-static int
-validate_der(size_t ber_consumed, asn_TYPE_descriptor_t const *descriptor,
-    const void *original, void *result)
-{
-       struct ber_data data;
-       asn_enc_rval_t eval;
-
-       data.src = (unsigned char *) original;
-       data.src_size = ber_consumed;
-       data.consumed = 0;
-
-       eval = der_encode(descriptor, result, der_coder, &data);
-       if (eval.encoded == -1)
-               return incidence(INID_OBJ_NOT_DER,
-                   "'%s' isn't DER encoded", eval.failed_type->name);
-
-       if (ber_consumed != eval.encoded) {
-               pr_val_debug("DER encoding consumed less bytes than expected (expected %lu, got %lu)",
-                   ber_consumed, eval.encoded);
-               return incidence(INID_OBJ_NOT_DER, "'%s' isn't DER encoded",
-                   descriptor->name);
-       }
-
-       return 0;
-}
-
 int
 asn1_decode(const void *buffer, size_t buffer_size,
-    asn_TYPE_descriptor_t const *descriptor, void **result, bool log,
-    bool dec_as_der)
+    asn_TYPE_descriptor_t const *descriptor, void **result, bool log)
 {
        asn_codec_ctx_t s_codec_ctx;
        asn_dec_rval_t rval;
@@ -103,17 +54,6 @@ asn1_decode(const void *buffer, size_t buffer_size,
                        rval.code, rval.consumed));
        }
 
-       /* Validate DER encoding, only if wanted and incidence isn't ignored */
-       if (dec_as_der &&
-           incidence_get_action(INID_OBJ_NOT_DER) != INAC_IGNORE) {
-               error = validate_der(rval.consumed, descriptor, buffer,
-                   *result);
-               if (error) {
-                       ASN_STRUCT_FREE(*descriptor, *result);
-                       return error;
-               }
-       }
-
        error = validate(descriptor, *result, log);
        if (error) {
                ASN_STRUCT_FREE(*descriptor, *result);
@@ -125,19 +65,16 @@ asn1_decode(const void *buffer, size_t buffer_size,
 
 int
 asn1_decode_any(ANY_t *any, asn_TYPE_descriptor_t const *descriptor,
-    void **result, bool log, bool dec_as_der)
+    void **result, bool log)
 {
-       return asn1_decode(any->buf, any->size, descriptor, result, log,
-           dec_as_der);
+       return asn1_decode(any->buf, any->size, descriptor, result, log);
 }
 
 int
 asn1_decode_octet_string(OCTET_STRING_t *string,
-    asn_TYPE_descriptor_t const *descriptor, void **result, bool log,
-    bool dec_as_der)
+    asn_TYPE_descriptor_t const *descriptor, void **result, bool log)
 {
-       return asn1_decode(string->buf, string->size, descriptor, result, log,
-           dec_as_der);
+       return asn1_decode(string->buf, string->size, descriptor, result, log);
 }
 
 /*
@@ -147,9 +84,7 @@ asn1_decode_octet_string(OCTET_STRING_t *string,
  */
 int
 asn1_decode_fc(struct file_contents *fc,
-    asn_TYPE_descriptor_t const *descriptor, void **result, bool log,
-    bool dec_as_der)
+    asn_TYPE_descriptor_t const *descriptor, void **result, bool log)
 {
-       return asn1_decode(fc->buffer, fc->buffer_size, descriptor, result,
-           log, dec_as_der);
+       return asn1_decode(fc->buffer, fc->buffer_size, descriptor, result, log);
 }

diff --git a/src/asn1/decode.h b/src/asn1/decode.h
index 977742e5a8c1093ce6d8c7a62cc1eb90d7acb6bc..217a4fb15868c3f8e281a62d18cf76eaf5d04037 100644 (file)
--- a/src/asn1/decode.h
+++ b/src/asn1/decode.h
@@ -6,12 +6,11 @@
 #include "asn1/asn1c/constr_TYPE.h"
 
 int asn1_decode(const void *, size_t, asn_TYPE_descriptor_t const *, void **,
-    bool, bool);
-int asn1_decode_any(ANY_t *, asn_TYPE_descriptor_t const *, void **, bool,
     bool);
+int asn1_decode_any(ANY_t *, asn_TYPE_descriptor_t const *, void **, bool);
 int asn1_decode_octet_string(OCTET_STRING_t *, asn_TYPE_descriptor_t const *,
-    void **, bool, bool);
+    void **, bool);
 int asn1_decode_fc(struct file_contents *, asn_TYPE_descriptor_t const *,
-    void **, bool, bool);
+    void **, bool);
 
 #endif /* SRC_ASN1_DECODE_H_ */

diff --git a/src/asn1/signed_data.c b/src/asn1/signed_data.c
index 0fbaf89bae7abe6b14552e778ee9f30228cc7dc4..8dd1bcff62c30118d5308e722b8e5eff49e3c4d0 100644 (file)
--- a/src/asn1/signed_data.c
+++ b/src/asn1/signed_data.c
@@ -126,7 +126,7 @@ validate_content_type_attribute(CMSAttributeValue_t *value,
        int error;
 
        error = asn1_decode_any(value, &asn_DEF_OBJECT_IDENTIFIER,
-           (void **) &attrValues, true, false);
+           (void **) &attrValues, true);
        if (error)
                return error;
        eContentType = &eci->eContentType;
@@ -149,7 +149,7 @@ validate_message_digest_attribute(CMSAttributeValue_t *value,
                return pr_val_err("There's no content being signed.");
 
        error = asn1_decode_any(value, &asn_DEF_MessageDigest,
-           (void **) &digest, true, false);
+           (void **) &digest, true);
        if (error)
                return error;
 
@@ -406,7 +406,7 @@ signed_data_decode_pkcs7(ANY_t *coded, struct SignedData **result)
        int error;
 
        error = asn1_decode_any(coded, &asn_DEF_SignedDataPKCS7,
-           (void **) &sdata_pkcs7, true, false);
+           (void **) &sdata_pkcs7, true);
        if (error)
                return error;
 
@@ -415,7 +415,7 @@ signed_data_decode_pkcs7(ANY_t *coded, struct SignedData **result)
        /* Parse content as OCTET STRING */
        error = asn1_decode_any(sdata_pkcs7->encapContentInfo.eContent,
            &asn_DEF_ContentTypePKCS7,
-           (void **) &sdata->encapContentInfo.eContent, true, false);
+           (void **) &sdata->encapContentInfo.eContent, true);
        if (error)
                goto release_sdata;
 
@@ -447,7 +447,7 @@ signed_data_decode(ANY_t *encoded, struct SignedData **decoded)
        int error;
 
        error = asn1_decode_any(encoded, &asn_DEF_SignedData,
-           (void **) decoded, false, false);
+           (void **) decoded, false);
        if (error) {
                /* Try to decode as PKCS content (RFC 5652 section 5.2.1) */
                error = signed_data_decode_pkcs7(encoded, decoded);
@@ -494,7 +494,7 @@ get_content_type_attr(struct SignedData *sdata, OBJECT_IDENTIFIER_t **result)
                                return -EINVAL;
                        return asn1_decode_any(attr->attrValues.list.array[0],
                            &asn_DEF_OBJECT_IDENTIFIER,
-                           (void **) result, true, false);
+                           (void **) result, true);
                }
        }
 

diff --git a/src/object/certificate.c b/src/object/certificate.c
index 67c328a22c139ba7117eab291879476cf7295fab..58a6359c55d2d07b4842cbd0c81727b4e0f5ab80 100644 (file)
--- a/src/object/certificate.c
+++ b/src/object/certificate.c
@@ -1025,7 +1025,7 @@ handle_ip_extension(X509_EXTENSION *ext, struct resources *resources)
 
        string = X509_EXTENSION_get_data(ext);
        error = asn1_decode(string->data, string->length, &asn_DEF_IPAddrBlocks,
-           (void **) &blocks, true, false);
+           (void **) &blocks, true);
        if (error)
                return error;
 
@@ -1073,7 +1073,7 @@ handle_asn_extension(X509_EXTENSION *ext, struct resources *resources,
 
        string = X509_EXTENSION_get_data(ext);
        error = asn1_decode(string->data, string->length,
-           &asn_DEF_ASIdentifiers, (void **) &ids, true, false);
+           &asn_DEF_ASIdentifiers, (void **) &ids, true);
        if (error)
                return error;
 

diff --git a/src/object/manifest.c b/src/object/manifest.c
index 9d42dc72bb6a9d41c492ae5bcdb2afe7af4edd31..d3267644d1bb23c8291e7a3c5faba0526e49163e 100644 (file)
--- a/src/object/manifest.c
+++ b/src/object/manifest.c
@@ -35,8 +35,7 @@ decode_manifest(struct signed_object *sobj, struct Manifest **result)
                sobj->sdata->encapContentInfo.eContent,
                &asn_DEF_Manifest,
                (void **) result,
-               true,
-               false
+               true
        );
 }
 

diff --git a/src/object/roa.c b/src/object/roa.c
index eeda8108a503b1335c74364c514e0e3326daa504..99ff75f97772543127c3cbc1c3070d01b0c8856c 100644 (file)
--- a/src/object/roa.c
+++ b/src/object/roa.c
@@ -15,8 +15,7 @@ decode_roa(struct signed_object *sobj, struct RouteOriginAttestation **result)
                sobj->sdata->encapContentInfo.eContent,
                &asn_DEF_RouteOriginAttestation,
                (void **) result,
-               true,
-               false
+               true
        );
 }