From: job Date: Mon, 5 Feb 2024 19:23:58 +0000 (+0000) Subject: Check whether all data in eContent has been consumed X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=d115f50d217404079b89805fc41a8002bc6376ba;p=openbsd Check whether all data in eContent has been consumed It is possible that a given ASN.1 template generated d2i_*() function didn't consume all data, so there is a potential for malleability. The econtent is a sequence (which means it could be the concatenation of several DER "blobs"). d2i_*() would only deserialize the first one and not notice blobs following it. OK tb@ --- diff --git a/usr.sbin/rpki-client/aspa.c b/usr.sbin/rpki-client/aspa.c index 8d81c161b0c..e857c3068e5 100644 --- a/usr.sbin/rpki-client/aspa.c +++ b/usr.sbin/rpki-client/aspa.c @@ -1,4 +1,4 @@ -/* $OpenBSD: aspa.c,v 1.24 2023/10/13 12:06:49 job Exp $ */ +/* $OpenBSD: aspa.c,v 1.25 2024/02/05 19:23:58 job Exp $ */ /* * Copyright (c) 2022 Job Snijders * Copyright (c) 2022 Theo Buehler @@ -129,13 +129,20 @@ aspa_parse_providers(struct parse *p, const STACK_OF(ASN1_INTEGER) *providers) static int aspa_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p) { + const unsigned char *oder; ASProviderAttestation *aspa; int rc = 0; + oder = d; if ((aspa = d2i_ASProviderAttestation(NULL, &d, dsz)) == NULL) { warnx("%s: ASPA: failed to parse ASProviderAttestation", p->fn); goto out; } + if (d != oder + dsz) { + warnx("%s: %td bytes trailing garbage in eContent", p->fn, + oder + dsz - d); + goto out; + } if (!valid_econtent_version(p->fn, aspa->version, 1)) goto out; diff --git a/usr.sbin/rpki-client/mft.c b/usr.sbin/rpki-client/mft.c index d6891157edd..0effb92a5c8 100644 --- a/usr.sbin/rpki-client/mft.c +++ b/usr.sbin/rpki-client/mft.c @@ -1,4 +1,4 @@ -/* $OpenBSD: mft.c,v 1.105 2024/02/04 00:53:27 job Exp $ */ +/* $OpenBSD: mft.c,v 1.106 2024/02/05 19:23:58 job Exp $ */ /* * Copyright (c) 2022 Theo Buehler * Copyright (c) 2019 Kristaps Dzonsons @@ -232,15 +232,22 @@ mft_parse_filehash(struct parse *p, const FileAndHash *fh) static int mft_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p) { + const unsigned char *oder; Manifest *mft; FileAndHash *fh; int i, rc = 0; + oder = d; if ((mft = d2i_Manifest(NULL, &d, dsz)) == NULL) { warnx("%s: RFC 6486 section 4: failed to parse Manifest", p->fn); goto out; } + if (d != oder + dsz) { + warnx("%s: %td bytes trailing garbage in eContent", p->fn, + oder + dsz - d); + goto out; + } if (!valid_econtent_version(p->fn, mft->version, 0)) goto out; diff --git a/usr.sbin/rpki-client/roa.c b/usr.sbin/rpki-client/roa.c index 6e19ca7c53a..5728189cbc1 100644 --- a/usr.sbin/rpki-client/roa.c +++ b/usr.sbin/rpki-client/roa.c @@ -1,4 +1,4 @@ -/* $OpenBSD: roa.c,v 1.72 2023/12/14 07:52:53 tb Exp $ */ +/* $OpenBSD: roa.c,v 1.73 2024/02/05 19:23:58 job Exp $ */ /* * Copyright (c) 2022 Theo Buehler * Copyright (c) 2019 Kristaps Dzonsons @@ -101,6 +101,7 @@ IMPLEMENT_ASN1_FUNCTIONS(RouteOriginAttestation); static int roa_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p) { + const unsigned char *oder; RouteOriginAttestation *roa; const ROAIPAddressFamily *addrfam; const STACK_OF(ROAIPAddress) *addrs; @@ -113,11 +114,17 @@ roa_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p) int ipaddrblocksz; int i, j, rc = 0; + oder = d; if ((roa = d2i_RouteOriginAttestation(NULL, &d, dsz)) == NULL) { warnx("%s: RFC 6482 section 3: failed to parse " "RouteOriginAttestation", p->fn); goto out; } + if (d != oder + dsz) { + warnx("%s: %td bytes trailing garbage in eContent", p->fn, + oder + dsz - d); + goto out; + } if (!valid_econtent_version(p->fn, roa->version, 0)) goto out; diff --git a/usr.sbin/rpki-client/rsc.c b/usr.sbin/rpki-client/rsc.c index c0fd9d8dfb0..cb58f73795a 100644 --- a/usr.sbin/rpki-client/rsc.c +++ b/usr.sbin/rpki-client/rsc.c @@ -1,4 +1,4 @@ -/* $OpenBSD: rsc.c,v 1.29 2023/10/13 12:06:49 job Exp $ */ +/* $OpenBSD: rsc.c,v 1.30 2024/02/05 19:23:58 job Exp $ */ /* * Copyright (c) 2022 Theo Buehler * Copyright (c) 2022 Job Snijders @@ -325,6 +325,7 @@ rsc_parse_checklist(struct parse *p, const STACK_OF(FileNameAndHash) *checkList) static int rsc_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p) { + const unsigned char *oder; RpkiSignedChecklist *rsc; ResourceBlock *resources; int rc = 0; @@ -333,10 +334,16 @@ rsc_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p) * RFC 9323 section 4 */ + oder = d; if ((rsc = d2i_RpkiSignedChecklist(NULL, &d, dsz)) == NULL) { warnx("%s: RSC: failed to parse RpkiSignedChecklist", p->fn); goto out; } + if (d != oder + dsz) { + warnx("%s: %td bytes trailing garbage in eContent", p->fn, + oder + dsz - d); + goto out; + } if (!valid_econtent_version(p->fn, rsc->version, 0)) goto out; diff --git a/usr.sbin/rpki-client/tak.c b/usr.sbin/rpki-client/tak.c index a3f0934e39e..522d1b0952a 100644 --- a/usr.sbin/rpki-client/tak.c +++ b/usr.sbin/rpki-client/tak.c @@ -1,4 +1,4 @@ -/* $OpenBSD: tak.c,v 1.13 2023/10/13 12:06:49 job Exp $ */ +/* $OpenBSD: tak.c,v 1.14 2024/02/05 19:23:58 job Exp $ */ /* * Copyright (c) 2022 Job Snijders * Copyright (c) 2022 Theo Buehler @@ -184,16 +184,23 @@ parse_takey(const char *fn, const TAKey *takey) static int tak_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p) { - TAK *tak; - const char *fn; - int rc = 0; + const unsigned char *oder; + TAK *tak; + const char *fn; + int rc = 0; fn = p->fn; + oder = d; if ((tak = d2i_TAK(NULL, &d, dsz)) == NULL) { warnx("%s: failed to parse Trust Anchor Key", fn); goto out; } + if (d != oder + dsz) { + warnx("%s: %td bytes trailing garbage in eContent", p->fn, + oder + dsz - d); + goto out; + } if (!valid_econtent_version(fn, tak->version, 0)) goto out;