From: Alberto Leiva Popper Date: Wed, 15 May 2024 18:05:11 +0000 (-0600) Subject: Privatize the asn_codec_ctx_t into the ASN.1 code X-Git-Tag: 1.6.2~16 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=a0e575cb584a3a5a8fd0ed28bef88aa4cb86600e;p=thirdparty%2FFORT-validator.git Privatize the asn_codec_ctx_t into the ASN.1 code Fort has `--asn1-decode-max-stack`, a global configuration option for the maximum stack usage. So there's no need to pass this as an argument. --- diff --git a/src/asn1/asn1c/ANY.c b/src/asn1/asn1c/ANY.c index a478f9df..cde0cf9e 100644 --- a/src/asn1/asn1c/ANY.c +++ b/src/asn1/asn1c/ANY.c @@ -138,7 +138,7 @@ ANY_to_type(ANY_t *st, asn_TYPE_descriptor_t *td, void **struct_ptr) { return 0; } - rval = ber_decode(0, td, (void **)&newst, st->buf, st->size); + rval = ber_decode(td, (void **)&newst, st->buf, st->size); if(rval.code == RC_OK) { *struct_ptr = newst; return 0; diff --git a/src/asn1/asn1c/CMSAttribute.c b/src/asn1/asn1c/CMSAttribute.c index 5a21ca1c..262c6792 100644 --- a/src/asn1/asn1c/CMSAttribute.c +++ b/src/asn1/asn1c/CMSAttribute.c @@ -21,7 +21,7 @@ attr2json(asn_TYPE_descriptor_t const *td, CMSAttributeValue_t const *ber) json_t *json; attr = NULL; - rval = ber_decode(NULL, td, &attr, ber->buf, ber->size); + rval = ber_decode(td, &attr, ber->buf, ber->size); json = (rval.code == RC_OK) ? td->op->json_encoder(td, attr) : NULL; diff --git a/src/asn1/asn1c/ContentInfo.c b/src/asn1/asn1c/ContentInfo.c index bb3faf35..55f554e1 100644 --- a/src/asn1/asn1c/ContentInfo.c +++ b/src/asn1/asn1c/ContentInfo.c @@ -19,7 +19,7 @@ content2json(const asn_TYPE_descriptor_t *td, ANY_t const *ber) json_t *json; decoded = NULL; - rval = ber_decode(NULL, td, &decoded, ber->buf, ber->size); + rval = ber_decode(td, &decoded, ber->buf, ber->size); json = (rval.code == RC_OK) ? td->op->json_encoder(td, decoded) : NULL; diff --git a/src/asn1/asn1c/EncapsulatedContentInfo.c b/src/asn1/asn1c/EncapsulatedContentInfo.c index 3c551498..54ae8c2f 100644 --- a/src/asn1/asn1c/EncapsulatedContentInfo.c +++ b/src/asn1/asn1c/EncapsulatedContentInfo.c @@ -20,7 +20,7 @@ econtent2json(asn_TYPE_descriptor_t const *td, OCTET_STRING_t *eContent) json_t *json; decoded = NULL; - rval = ber_decode(NULL, td, &decoded, eContent->buf, eContent->size); + rval = ber_decode(td, &decoded, eContent->buf, eContent->size); json = (rval.code == RC_OK) ? td->op->json_encoder(td, decoded) : NULL; diff --git a/src/asn1/asn1c/asn_application.c b/src/asn1/asn1c/asn_application.c index 1106160f..469a0a48 100644 --- a/src/asn1/asn1c/asn_application.c +++ b/src/asn1/asn1c/asn_application.c @@ -290,24 +290,3 @@ asn_encode_internal(const asn_codec_ctx_t *opt_codec_ctx, return er; } - -asn_dec_rval_t -asn_decode(const asn_codec_ctx_t *opt_codec_ctx, - enum asn_transfer_syntax syntax, const asn_TYPE_descriptor_t *td, - void **sptr, const void *buffer, size_t size) { - if(!td || !td->op || !sptr || (size && !buffer)) { - ASN__DECODE_FAILED; - } - - switch(syntax) { - case ATS_CER: - case ATS_NONSTANDARD_PLAINTEXT: - default: - errno = ENOENT; - ASN__DECODE_FAILED; - - case ATS_DER: - case ATS_BER: - return ber_decode(opt_codec_ctx, td, sptr, buffer, size); - } -} diff --git a/src/asn1/asn1c/asn_application.h b/src/asn1/asn1c/asn_application.h index 8382ca80..e59ab437 100644 --- a/src/asn1/asn1c/asn_application.h +++ b/src/asn1/asn1c/asn_application.h @@ -106,18 +106,6 @@ asn_enc_rval_t asn_encode( asn_app_consume_bytes_f *callback, void *callback_key); -/* - * A generic decoder for any supported transfer syntax. - */ -asn_dec_rval_t asn_decode( - const asn_codec_ctx_t *opt_codec_parameters, enum asn_transfer_syntax, - const struct asn_TYPE_descriptor_s *type_to_decode, - void **structure_ptr, /* Pointer to a target structure's pointer */ - const void *buffer, /* Data to be decoded */ - size_t size /* Size of that buffer */ -); - - /* * A callback of this type is called whenever constraint validation fails * on some ASN.1 type. See "constraints.h" for more details on constraint diff --git a/src/asn1/asn1c/asn_codecs.h b/src/asn1/asn1c/asn_codecs.h index b7f25d1e..0b3519ab 100644 --- a/src/asn1/asn1c/asn_codecs.h +++ b/src/asn1/asn1c/asn_codecs.h @@ -35,6 +35,24 @@ typedef struct asn_codec_ctx_s { * A value from getrlimit(RLIMIT_STACK) may be used to initialize * this variable. Be careful in multithreaded environments, as the * stack size is rather limited. + * + * 2024-05-15: None of the RPKI objects employ recursive structures, and + * they're all somewhat shallow. So this feature seems to be clutter. + * + * In my test environment, the highest effective ASN1 stack size + * reported in a full run was 1296. Fort defaults + * `--asn1-decode-max-stack` to 4096. The asn1c default recommended + * maximum was 30000. My soft getrlimit(RLIMIT_STACK) is 8192... + * I don't feel threatened by these numbers. + * + * Also, this seems naive. The "ASN1 stack size" isn't really comparable + * to RLIMIT_STACK to begin with. + * + * TODO (fine) Consider (from worst to best) + * - switching `--asn1-decode-max-stack` to getrlimit(RLIMIT_STACK), + * - drop this feature altogether, + * - or replace it with a gcc -fstack-usage analysis/monitor. (See + * https://stackoverflow.com/a/74769668/1735458.) */ size_t max_stack_size; /* 0 disables stack bounds checking */ } asn_codec_ctx_t; diff --git a/src/asn1/asn1c/asn_internal.h b/src/asn1/asn1c/asn_internal.h index 2bc3d9b5..ae8157f6 100644 --- a/src/asn1/asn1c/asn_internal.h +++ b/src/asn1/asn1c/asn_internal.h @@ -89,10 +89,6 @@ asn__format_to_callback( return -1; \ } while(0) -/* - * Check stack against overflow, if limit is set. - */ -#define ASN__DEFAULT_STACK_MAX (30000) static int CC_NOTUSED ASN__STACK_OVERFLOW_CHECK(const asn_codec_ctx_t *ctx) { if(ctx && ctx->max_stack_size) { diff --git a/src/asn1/asn1c/ber_decoder.c b/src/asn1/asn1c/ber_decoder.c index a3d2797f..b7ba96dd 100644 --- a/src/asn1/asn1c/ber_decoder.c +++ b/src/asn1/asn1c/ber_decoder.c @@ -6,6 +6,7 @@ #include #include "asn1/asn1c/asn_internal.h" +#include "config.h" #undef ADVANCE #define ADVANCE(num_bytes) do { \ @@ -30,35 +31,20 @@ * The BER decoder of any type. */ asn_dec_rval_t -ber_decode(const asn_codec_ctx_t *opt_codec_ctx, - const asn_TYPE_descriptor_t *type_descriptor, void **struct_ptr, - const void *ptr, size_t size) { - asn_codec_ctx_t s_codec_ctx; +ber_decode(const asn_TYPE_descriptor_t *type_descriptor, void **struct_ptr, + const void *ptr, size_t size) +{ + /* Needs to be allocated on the stack! */ + asn_codec_ctx_t s_codec_ctx = { 0 }; - /* - * Stack checker requires that the codec context - * must be allocated on the stack. - */ - if(opt_codec_ctx) { - if(opt_codec_ctx->max_stack_size) { - s_codec_ctx = *opt_codec_ctx; - opt_codec_ctx = &s_codec_ctx; - } - } else { - /* If context is not given, be security-conscious anyway */ - memset(&s_codec_ctx, 0, sizeof(s_codec_ctx)); - s_codec_ctx.max_stack_size = ASN__DEFAULT_STACK_MAX; - opt_codec_ctx = &s_codec_ctx; - } + s_codec_ctx.max_stack_size = config_get_asn1_decode_max_stack(); - /* - * Invoke type-specific decoder. - */ - return type_descriptor->op->ber_decoder(opt_codec_ctx, type_descriptor, + /* Invoke type-specific decoder. */ + return type_descriptor->op->ber_decoder(&s_codec_ctx, type_descriptor, struct_ptr, /* Pointer to the destination structure */ ptr, size, /* Buffer and its size */ 0 /* Default tag mode is 0 */ - ); + ); } /* diff --git a/src/asn1/asn1c/ber_decoder.h b/src/asn1/asn1c/ber_decoder.h index 64e2be57..72fb98b7 100644 --- a/src/asn1/asn1c/ber_decoder.h +++ b/src/asn1/asn1c/ber_decoder.h @@ -19,7 +19,6 @@ struct asn_codec_ctx_s; /* Forward declaration */ * which is compliant with ber_decode(). */ asn_dec_rval_t ber_decode( - const struct asn_codec_ctx_s *opt_codec_ctx, const struct asn_TYPE_descriptor_s *type_descriptor, void **struct_ptr, /* Pointer to a target structure's pointer */ const void *buffer, /* Data to be decoded */ diff --git a/src/asn1/decode.c b/src/asn1/decode.c index 7b0a2bec..48abd16a 100644 --- a/src/asn1/decode.c +++ b/src/asn1/decode.c @@ -1,7 +1,6 @@ #include "asn1/decode.h" #include "common.h" -#include "config.h" #include "log.h" #include "incidence/incidence.h" @@ -36,15 +35,12 @@ int asn1_decode(const void *buffer, size_t buffer_size, asn_TYPE_descriptor_t const *descriptor, void **result, bool log) { - asn_codec_ctx_t s_codec_ctx; asn_dec_rval_t rval; int error; *result = NULL; - s_codec_ctx.max_stack_size = config_get_asn1_decode_max_stack(); - rval = ber_decode(&s_codec_ctx, descriptor, result, buffer, - buffer_size); + rval = ber_decode(descriptor, result, buffer, buffer_size); if (rval.code != RC_OK) { /* Must free partial object according to API contracts. */ ASN_STRUCT_FREE(*descriptor, *result); diff --git a/src/print_file.c b/src/print_file.c index 3a1cb1b3..06ce5438 100644 --- a/src/print_file.c +++ b/src/print_file.c @@ -209,7 +209,7 @@ bio2ci(BIO *bio) goto fail; } - res = ber_decode(NULL, &asn_DEF_ContentInfo, (void **)&ci, + res = ber_decode(&asn_DEF_ContentInfo, (void **)&ci, buffer, consumed); pr_op_debug("Consumed: %zu", res.consumed);