On Sun, May 08, 2022 at 08:05:08PM +0000, Job Snijders wrote:
> Dear Theo, fellow developers,
>
> Many thanks for the first review pass, much appreciated.
>
> > This is a good first step. I have a few initial comments inline. Once you
> > fix
> > those, review of the rest will be easier.
> >
> > The main thing is: please remove all the copy-pasted functions that had no
> > changes whatsoever or only very minor changes.
>
> As the various same-named-but-different 'parse' structs are not easily
> interchangeable without more refactoring, I marked them "XXX:". Perhaps
> we can work on that in tree?
>
> > [snip]
>
> The following is a list of changes compared to previous changeset
> proposal:
>
> * removed the __func__ references
> * added copyright lines
> * use d2i_X509_ALGOR() instead of deserializing by hand
> * removed all 'else if'
> * fixed spaces vs tabs
> * renamed elemz to elemsz, and made the check more readable
> * added default case in switch (aor->type)
> * removed superfluous parenthesis
See inline comments
> Index: usr.sbin/rpki-client/Makefile
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/Makefile,v
> retrieving revision 1.24
> diff -u -p -r1.24 Makefile
> --- usr.sbin/rpki-client/Makefile 21 Apr 2022 09:53:07 -0000 1.24
> +++ usr.sbin/rpki-client/Makefile 8 May 2022 20:01:25 -0000
> @@ -5,7 +5,7 @@ SRCS= as.c cert.c cms.c crl.c encoding.c
> log.c main.c mft.c mkdir.c output.c output-bgpd.c output-bird.c \
> output-csv.c output-json.c parser.c print.c repo.c roa.c rrdp.c \
> rrdp_delta.c rrdp_notification.c rrdp_snapshot.c rrdp_util.c \
> - rsync.c tal.c validate.c x509.c
> + rsc.c rsync.c tal.c validate.c x509.c
> MAN= rpki-client.8
>
> LDADD+= -lexpat -ltls -lssl -lcrypto -lutil
> Index: usr.sbin/rpki-client/extern.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
> retrieving revision 1.132
> diff -u -p -r1.132 extern.h
> --- usr.sbin/rpki-client/extern.h 21 Apr 2022 12:59:03 -0000 1.132
> +++ usr.sbin/rpki-client/extern.h 8 May 2022 20:01:25 -0000
> @@ -24,6 +24,14 @@
> #include <openssl/x509.h>
> #include <openssl/x509v3.h>
>
> +/*
> + * Enumeration for ASN.1 explicit tags in RSC eContent
> + */
> +enum rsc_resourceblock_tag {
> + RSRCBLK_TYPE_ASID,
> + RSRCBLK_TYPE_IPADDRBLK,
> +};
> +
> enum cert_as_type {
> CERT_AS_ID, /* single identifier */
> CERT_AS_INHERIT, /* inherit from parent */
> @@ -164,6 +172,7 @@ enum rtype {
> RTYPE_ASPA,
> RTYPE_REPO,
> RTYPE_FILE,
> + RTYPE_RSC,
> };
>
> enum location {
> @@ -232,6 +241,29 @@ struct roa {
> time_t expires; /* do not use after */
> };
>
> +struct rscfile {
> + char *filename; /* an optional filename on the checklist */
> + unsigned char hash[SHA256_DIGEST_LENGTH]; /* the digest */
> +};
> +
> +/*
> + * A Signed Checklist (RSC)
> + */
> +struct rsc {
> + int talid; /* RSC covered by what TAL */
> + int valid; /* eContent resources covered by EE's 3779? */
> + struct cert_ip *ips; /* IP prefixes */
> + size_t ipsz; /* number of IP prefixes */
> + struct cert_as *as; /* AS resources */
> + size_t asz; /* number of AS resources */
> + struct rscfile *files; /* FileAndHashes in the RSC */
> + size_t filesz; /* number of FileAndHashes */
> + char *aia; /* AIA */
> + char *aki; /* AKI */
> + char *ski; /* SKI */
> + time_t expires; /* Not After of the RSC EE */
> +};
> +
> /*
> * A single Ghostbuster record
> */
> @@ -450,6 +482,12 @@ void gbr_free(struct gbr *);
> struct gbr *gbr_parse(X509 **, const char *, const unsigned char *,
> size_t);
>
> +void rsc_buffer(struct ibuf *, const struct rsc *);
> +void rsc_free(struct rsc *);
> +struct rsc *rsc_parse(X509 **, const char *, const unsigned char *,
> + size_t);
> +struct rsc *rsc_read(struct ibuf *);
> +
Both rsc_buffer and rsc_read can be dropped here. These functions will not
be needed since .sig files are not part of a cache parse. See further
down.
> /* crl.c */
> struct crl *crl_parse(const char *, const unsigned char *, size_t);
> struct crl *crl_get(struct crl_tree *, const struct auth *);
> @@ -470,6 +508,7 @@ int valid_uri(const char *, size_t, co
> int valid_origin(const char *, const char *);
> int valid_x509(char *, X509_STORE_CTX *, X509 *, struct auth *,
> struct crl *, int);
> +int valid_rsc(const char *, struct auth *, struct rsc *);
>
> /* Working with CMS. */
> unsigned char *cms_parse_validate(X509 **, const char *,
> @@ -608,6 +647,7 @@ void crl_print(const struct crl *);
> void mft_print(const X509 *, const struct mft *);
> void roa_print(const X509 *, const struct roa *);
> void gbr_print(const X509 *, const struct gbr *);
> +void rsc_print(const X509 *, const struct rsc *);
>
> /* Output! */
>
> Index: usr.sbin/rpki-client/filemode.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/filemode.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 filemode.c
> --- usr.sbin/rpki-client/filemode.c 24 Apr 2022 22:26:44 -0000 1.5
> +++ usr.sbin/rpki-client/filemode.c 8 May 2022 20:01:25 -0000
> @@ -264,6 +264,7 @@ proc_parser_file(char *file, unsigned ch
> struct roa *roa = NULL;
> struct gbr *gbr = NULL;
> struct tal *tal = NULL;
> + struct rsc *rsc = NULL;
> char *aia = NULL, *aki = NULL;
> char filehash[SHA256_DIGEST_LENGTH];
> char *hash;
> @@ -356,6 +357,14 @@ proc_parser_file(char *file, unsigned ch
> if (tal == NULL)
> break;
> tal_print(tal);
> + break;
> + case RTYPE_RSC:
> + rsc = rsc_parse(&x509, file, buf, len);
> + if (rsc == NULL)
> + break;
> + rsc_print(x509, rsc);
> + aia = rsc->aia;
> + aki = rsc->aki;
> break;
> default:
> printf("%s: unsupported file type\n", file);
> Index: usr.sbin/rpki-client/mft.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/mft.c,v
> retrieving revision 1.60
> diff -u -p -r1.60 mft.c
> --- usr.sbin/rpki-client/mft.c 20 Apr 2022 10:46:20 -0000 1.60
> +++ usr.sbin/rpki-client/mft.c 8 May 2022 20:01:25 -0000
> @@ -120,6 +120,8 @@ rtype_from_file_extension(const char *fn
> return RTYPE_GBR;
> if (strcasecmp(fn + sz - 4, ".asa") == 0)
> return RTYPE_ASPA;
> + if (strcasecmp(fn + sz - 4, ".sig") == 0)
> + return RTYPE_RSC;
>
> return RTYPE_INVALID;
> }
> Index: usr.sbin/rpki-client/parser.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v
> retrieving revision 1.73
> diff -u -p -r1.73 parser.c
> --- usr.sbin/rpki-client/parser.c 21 Apr 2022 12:59:03 -0000 1.73
> +++ usr.sbin/rpki-client/parser.c 8 May 2022 20:01:25 -0000
> @@ -492,6 +492,39 @@ proc_parser_gbr(char *file, const unsign
> }
>
> /*
> + * Parse a Signed Checklist (RSC) file.
> + * Returns rsc struct (which must not be freed) or NULL
> + */
> +static struct rsc *
> +proc_parser_rsc(char *file, const unsigned char *der, size_t len)
> +{
> + struct rsc *rsc = NULL;
> + struct auth *a;
> + struct crl *crl;
> + X509 *x509;
> +
> + if ((rsc = rsc_parse(&x509, file, der, len)) == NULL)
> + return NULL;
> +
> + a = valid_ski_aki(file, &auths, rsc->ski, rsc->aki);
> + crl = crl_get(&crlt, a);
> +
> + if (!valid_x509(file, ctx, x509, a, crl, 0)) {
> + X509_free(x509);
> + rsc_free(rsc);
> + return NULL;
> + }
> + X509_free(x509);
> +
> + rsc->talid = a->cert->talid;
> +
> + if (valid_rsc(file, a, rsc))
> + rsc->valid = 1;
> +
> + return rsc;
> +}
> +
> +/*
> * Load the file specified by the entity information.
> */
> static char *
> @@ -522,6 +555,7 @@ parse_entity(struct entityq *q, struct m
> struct cert *cert;
> struct mft *mft;
> struct roa *roa;
> + struct rsc *rsc;
> struct ibuf *b;
> unsigned char *f;
> size_t flen;
> @@ -606,6 +640,16 @@ parse_entity(struct entityq *q, struct m
> file = parse_load_file(entp, &f, &flen);
> io_str_buffer(b, file);
> proc_parser_gbr(file, f, flen);
> + break;
> + case RTYPE_RSC:
> + file = parse_load_file(entp, &f, &flen);
> + io_str_buffer(b, file);
> + rsc = proc_parser_rsc(file, f, flen);
> + c = (rsc != NULL);
> + io_simple_buffer(b, &c, sizeof(int));
> + if (rsc != NULL)
> + rsc_buffer(b, rsc);
> + rsc_free(rsc);
> break;
> default:
> errx(1, "unhandled entity type %d", entp->type);
Since .sig files are not part of a RPKI repository you can drop all of
this code in parse.c. If you pass a RSC .sig file in a regular RPKI parse
it means it was included in the repository and added to an MFT which is
not conforming to the RFC. So all this code in parse.c is not needed.
> Index: usr.sbin/rpki-client/print.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/print.c,v
> retrieving revision 1.10
> diff -u -p -r1.10 print.c
> --- usr.sbin/rpki-client/print.c 24 Apr 2022 18:20:12 -0000 1.10
> +++ usr.sbin/rpki-client/print.c 8 May 2022 20:01:25 -0000
> @@ -455,3 +455,119 @@ gbr_print(const X509 *x, const struct gb
> printf("vcard:\n%s", p->vcard);
> }
> }
> +
> +void
> +rsc_print(const X509 *x, const struct rsc *p)
> +{
> + char buf1[64], buf2[64], tbuf[21];
> + char *hash;
> + int sockt;
> + size_t i, j;
> +
> + strftime(tbuf, sizeof(tbuf), "%FT%TZ", gmtime(&p->expires));
> +
> + if (outformats & FORMAT_JSON) {
> + printf("\t\"ski\": \"%s\",\n", pretty_key_id(p->ski));
> + printf("\t\"aki\": \"%s\",\n", pretty_key_id(p->aki));
> + x509_print(x);
> + printf("\t\"aia\": \"%s\",\n", p->aia);
> + printf("\t\"valid_until\": %lld,\n", (long long)p->expires);
> + printf("\t\"signed_with_resources\": [\n");
> + } else {
> + printf("Subject key identifier: %s\n", pretty_key_id(p->ski));
> + printf("Authority key identifier: %s\n", pretty_key_id(p->aki));
> + x509_print(x);
> + printf("Authority info access: %s\n", p->aia);
> + printf("Valid until: %s\n", tbuf);
> + printf("Signed with resources:\n");
> + }
> +
> + for (i = 0; i < p->asz; i++) {
> + switch (p->as[i].type) {
> + case CERT_AS_ID:
> + if (outformats & FORMAT_JSON)
> + printf("\t\t{ \"asid\": %u }", p->as[i].id);
> + else
> + printf("%5zu: AS: %u", i + 1, p->as[i].id);
> + break;
> + case CERT_AS_RANGE:
> + if (outformats & FORMAT_JSON)
> + printf("\t\t{ \"asrange\": { \"min\": %u, "
> + "\"max\": %u }}", p->as[i].range.min,
> + p->as[i].range.max);
> + else
> + printf("%5zu: AS: %u -- %u", i + 1,
> + p->as[i].range.min, p->as[i].range.max);
> + break;
> + case CERT_AS_INHERIT:
> + /* inheritance isn't possible in RSC */
> + break;
> + }
> + if (outformats & FORMAT_JSON && i + 1 < p->asz + p->ipsz)
> + printf(",\n");
> + else
> + printf("\n");
> + }
> +
> + for (j = 0; j < p->ipsz; j++) {
> + switch (p->ips[j].type) {
> + case CERT_IP_ADDR:
> + ip_addr_print(&p->ips[j].ip,
> + p->ips[j].afi, buf1, sizeof(buf1));
> + if (outformats & FORMAT_JSON)
> + printf("\t\t{ \"ip_prefix\": \"%s\" }", buf1);
> + else
> + printf("%5zu: IP: %s", i + j + 1, buf1);
> + break;
> + case CERT_IP_RANGE:
> + sockt = (p->ips[j].afi == AFI_IPV4) ?
> + AF_INET : AF_INET6;
> + inet_ntop(sockt, p->ips[j].min, buf1, sizeof(buf1));
> + inet_ntop(sockt, p->ips[j].max, buf2, sizeof(buf2));
> + if (outformats & FORMAT_JSON)
> + printf("\t\t{ \"ip_range\": { \"min\": \"%s\""
> + ", \"max\": \"%s\" }}", buf1, buf2);
> + else
> + printf("%5zu: IP: %s -- %s", i + j + 1, buf1,
> + buf2);
> + break;
> + case CERT_IP_INHERIT:
> + /* inheritance isn't possible in RSC */
> + break;
> + }
> + if (outformats & FORMAT_JSON && i + j + 1 < p->asz + p->ipsz)
> + printf(",\n");
> + else
> + printf("\n");
> + }
Not a JSON specialist but I find that the signed_with_resources array is a
collection of various objects (asid, as_range, ip_prefix, ip_range) and I
wonder if ASID and IP should be split into different arrays.
Guess like all other JSON outputs this format is probably not yet set in
stone.
> + if (outformats & FORMAT_JSON) {
> + printf("\t],\n");
> + printf("\t\"filenamesandhashes\": [\n");
> + } else
> + printf("Filenames and hashes:\n");
> +
> + for (i = 0; i < p->filesz; i++) {
> + if (base64_encode(p->files[i].hash, sizeof(p->files[i].hash),
> + &hash) == -1)
> + errx(1, "base64_encode failure");
> +
> + if (outformats & FORMAT_JSON) {
> + printf("\t\t{ \"filename\": \"%s\",",
> + p->files[i].filename ? p->files[i].filename : "");
> + printf(" \"hash_digest\": \"%s\" }", hash);
> + if (i + 1 < p->filesz)
> + printf(",");
> + printf("\n");
> + } else {
> + printf("%5zu: %s\n", i + 1, p->files[i].filename
> + ? p->files[i].filename : "no filename");
What if someone signes the file 'no filename'? Also in the JSON part
should the filename attribute be skipped if there is no filename?
Like:
{ "hash_digest": "xxx" },
{ "filename": "file2.txt", "hash_digest": "yyy" },
I have more of a rant below about this optional filename that makes
automated validation impossible.
> + printf("\thash %s\n", hash);
> + }
> +
> + free(hash);
> + }
> +
> + if (outformats & FORMAT_JSON)
> + printf("\t],\n");
> +}
> Index: usr.sbin/rpki-client/rpki-client.8
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/rpki-client.8,v
> retrieving revision 1.61
> diff -u -p -r1.61 rpki-client.8
> --- usr.sbin/rpki-client/rpki-client.8 20 Apr 2022 20:26:22 -0000
> 1.61
> +++ usr.sbin/rpki-client/rpki-client.8 8 May 2022 20:01:25 -0000
> @@ -269,6 +269,8 @@ A Profile for BGPsec Router Certificates
> Certification Requests.
> .It RFC 8630
> Resource Public Key Infrastructure (RPKI) Trust Anchor Locator.
> +.It draft-ietf-sidrops-rpki-rsc
> +A profile for Resource Public Key Infrastructure (RPKI) Signed Checklists
> (RSC).
> .El
> .Sh HISTORY
> .Nm
> Index: usr.sbin/rpki-client/rsc.c
> ===================================================================
> RCS file: usr.sbin/rpki-client/rsc.c
> diff -N usr.sbin/rpki-client/rsc.c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ usr.sbin/rpki-client/rsc.c 8 May 2022 20:01:25 -0000
> @@ -0,0 +1,818 @@
> +/* $OpenBSD: rsc.c,v 1.0 2021/03/29 06:50:44 job Exp $ */
> +/*
> + * Copyright (c) 2022 Job Snijders <[email protected]>
> + * Copyright (c) 2019 Kristaps Dzonsons <[email protected]>
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#include <assert.h>
> +#include <err.h>
> +#include <stdarg.h>
> +#include <stdint.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +
> +#include <openssl/asn1.h>
> +#include <openssl/x509.h>
> +
> +#include "extern.h"
> +
> +/*
> + * Parse results and data of the Signed Checklist file.
> + */
> +struct parse {
> + const char *fn; /* Signed Checklist file name */
> + struct rsc *res; /* results */
> +};
> +
> +extern ASN1_OBJECT *rsc_oid;
> +
> +/*
> + * Append an AS identifier structure to our list of results.
> + * Return zero on failure.
> + * XXX: merge with append_as() in cert.c
> + */
> +static int
> +append_as(struct parse *p, const struct cert_as *as)
> +{
> + if (!as_check_overlap(as, p->fn, p->res->as, p->res->asz))
> + return 0;
> + if (p->res->asz >= MAX_AS_SIZE)
> + return 0;
> + p->res->as = reallocarray(p->res->as, p->res->asz + 1,
> + sizeof(struct cert_as));
> + if (p->res->as == NULL)
> + err(1, NULL);
> + p->res->as[p->res->asz++] = *as;
> + return 1;
> +}
> +
> +/*
> + * Append an IP address structure to our list of results.
> + * return zero on failure.
> + * XXX: merge with append_ip() in cert.c
> + */
> +static int
> +append_ip(struct parse *p, const struct cert_ip *ip)
> +{
> + struct rsc *res = p->res;
> +
> + if (!ip_addr_check_overlap(ip, p->fn, p->res->ips, p->res->ipsz))
> + return 0;
> + if (res->ipsz >= MAX_IP_SIZE)
> + return 0;
> +
> + res->ips = reallocarray(res->ips, res->ipsz + 1,
> + sizeof(struct cert_ip));
> + if (res->ips == NULL)
> + err(1, NULL);
> +
> + res->ips[res->ipsz++] = *ip;
> + return 1;
> +}
Yes, this needs fixing. Also this realloc for every element should be
replaced (but that is a different story). There are many functions in here
that are mostly copies from cert.c
> +static int
> +rsc_check_digesttype(struct parse *p, const unsigned char *d, size_t dsz)
> +{
> + X509_ALGOR *alg;
> + const ASN1_OBJECT *obj;
> + int type, nid;
> + int rc = 0;
> +
> + if ((alg = d2i_X509_ALGOR(NULL, &d, dsz)) == NULL) {
> + cryptowarnx("%s: RSC DigestAlgorithmIdentifier faild to parse",
> + p->fn);
> + goto out;
> + }
> +
> + X509_ALGOR_get0(&obj, &type, NULL, alg);
> +
> + if (type != V_ASN1_UNDEF) {
> + warnx("%s: RSC DigestAlgorithmIdentifier unexpected parameters:"
> + " %d", p->fn, type);
> + goto out;
> + }
> +
> + if ((nid = OBJ_obj2nid(obj)) != NID_sha256) {
> + warnx("%s: RSC DigestAlgorithmIdentifier: want SHA256, have %s"
> + " (NID %d)", p->fn, ASN1_tag2str(nid), nid);
> + goto out;
> + }
> +
> + rc = 1;
> + out:
> + X509_ALGOR_free(alg);
> + return rc;
> +}
> +
> +/*
> + * Parse and individual "FileNameAndHash", draft-ietf-sidrops-rpki-rsc
> + * section 4.1.
> + * Return zero on failure, non-zero on success.
> + */
> +static int
> +rsc_parse_filenamehash(struct parse *p, const ASN1_OCTET_STRING *os)
> +{
> + ASN1_SEQUENCE_ANY *seq;
> + const ASN1_TYPE *file, *hash;
> + char *fn = NULL;
> + const unsigned char *d = os->data;
> + size_t dsz = os->length;
> + int i = 0, rc = 0, elemsz;
> + struct rscfile *rent;
> +
> + if ((seq = d2i_ASN1_SEQUENCE_ANY(NULL, &d, dsz)) == NULL) {
> + cryptowarnx("%s: RSC FileNameAndHash: failed ASN.1 sequence "
> + "parse", p->fn);
> + goto out;
> + }
> +
> + elemsz = sk_ASN1_TYPE_num(seq);
> + if (elemsz != 1 && elemsz != 2) {
> + warnx("%s: RSC FileNameAndHash: want 1 or 2 elements, have %d",
> + p->fn, elemsz);
> + goto out;
> + }
> +
> + if (elemsz == 2) {
> + file = sk_ASN1_TYPE_value(seq, i++);
> + if (file->type != V_ASN1_IA5STRING) {
> + warnx("%s: RSC FileNameAndHash: want ASN.1 IA5 string,"
> + " have %s (NID %d)", p->fn,
> + ASN1_tag2str(file->type), file->type);
> + goto out;
> + }
> + fn = strndup((const char *)file->value.ia5string->data,
> + file->value.ia5string->length);
> + if (fn == NULL)
> + err(1, NULL);
> +
> + /*
> + * filename must confirm to portable file name character set
> + * XXX: use valid_filename() instead
> + */
> + if (strchr(fn, '/') != NULL) {
> + warnx("%s: path components disallowed in filename: %s",
> + p->fn, fn);
> + goto out;
> + }
> + if (strchr(fn, '\n') != NULL) {
> + warnx("%s: newline disallowed in filename: %s",
> + p->fn, fn);
> + goto out;
> + }
> + }
God, why does the draft allow for optional filenames? What the heck is the
digest then covering some random gunk? This will make it close to
impossible to actually fully validate a RSC (which would include the
validation of all SHA256 signatures). Because of this RSC validation in
rpki-client will be close to impossible because how do you implement:
To verify a set of digital objects with an RSC:
* The message digest of each referenced digital object, using the
digest algorithm specified in the the digestAlgorithm field, MUST
be calculated and MUST match the value given in the messageDigest
field of the associated FileNameAndHash, for the digital object to
be considered as verified with the RSC.
When there is no filename to reference an object?
> + /* Now hash value. */
> +
> + hash = sk_ASN1_TYPE_value(seq, i);
> + if (hash->type != V_ASN1_OCTET_STRING) {
> + warnx("%s: RSC FileNameAndHash: want ASN.1 OCTET string, have "
> + "%s (NID %d)", p->fn, ASN1_tag2str(hash->type), hash->type);
> + goto out;
> + }
> +
> + if (hash->value.octet_string->length != SHA256_DIGEST_LENGTH) {
> + warnx("%s: RSC Digest: invalid SHA256 length, have %d",
> + p->fn, hash->value.octet_string->length);
> + goto out;
> + }
> +
> + p->res->files = recallocarray(p->res->files, p->res->filesz,
> + p->res->filesz + 1, sizeof(struct rscfile));
> + if (p->res->files == NULL)
> + err(1, NULL);
Another one of those cases where the size of the array is known
beforehands and should be allocated ahead of time. Constant reallocarray
calls are not that efficent.
> + rent = &p->res->files[p->res->filesz++];
> + rent->filename = fn;
> + fn = NULL;
> + memcpy(rent->hash, hash->value.octet_string->data,
> SHA256_DIGEST_LENGTH);
> +
> + rc = 1;
> + out:
> + free(fn);
> + sk_ASN1_TYPE_pop_free(seq, ASN1_TYPE_free);
> + return rc;
> +}
> +
> +/*
> + * Parse the FileNameAndHash sequence, draft-ietf-sidrops-rpki-rsc
> + * section 4.1
> + * Return zero on failure, non-zero on success.
> + */
> +static int
> +rsc_parse_checklist(struct parse *p, const ASN1_OCTET_STRING *os)
> +{
> + ASN1_SEQUENCE_ANY *seq;
> + const ASN1_TYPE *t;
> + const unsigned char *d = os->data;
> + size_t dsz = os->length;
> + int i, rc = 0;
> +
> + if ((seq = d2i_ASN1_SEQUENCE_ANY(NULL, &d, dsz)) == NULL) {
> + cryptowarnx("%s: RSC checkList: failed ASN.1 sequence parse",
> + p->fn);
> + goto out;
> + }
> +
> + for (i = 0; i < sk_ASN1_TYPE_num(seq); i++) {
> + t = sk_ASN1_TYPE_value(seq, i);
> + if (t->type != V_ASN1_SEQUENCE) {
> + warnx("%s: RSC checkList: want ASN.1 sequence, have %s"
> + " (NID %d)", p->fn, ASN1_tag2str(t->type), t->type);
> + goto out;
> + }
> + if (!rsc_parse_filenamehash(p, t->value.octet_string))
> + goto out;
> + }
> +
> + rc = 1;
> + out:
> + sk_ASN1_TYPE_pop_free(seq, ASN1_TYPE_free);
> + return rc;
> +}
> +
> +/*
> + * Convert ASN1 INTEGER and add it to parse results
> + * Return zero on failure.
> + * XXX: merge with sbgp_asid() in cert.c
> + */
> +static int
> +rsc_parse_asid(struct parse *p, const ASN1_INTEGER *i)
> +{
> + struct cert_as as;
> +
> + memset(&as, 0, sizeof(struct cert_as));
> + as.type = CERT_AS_ID;
> +
> + if (!as_id_parse(i, &as.id)) {
> + warnx("%s: RSC malformed AS identifier", p->fn);
> + return 0;
> + }
> + if (as.id == 0) {
> + warnx("%s: RSC AS identifier zero is reserved", p->fn);
> + return 0;
> + }
> +
> + return append_as(p, &as);
> +}
> +
> +/*
> + * Parse AS Range and add it to parse result
> + * Return zero on failure.
> + * XXX: merge with sbgp_asrange() in cert.c
> + */
> +static int
> +rsc_parse_asrange(struct parse *p, const unsigned char *d, size_t dsz)
> +{
> + struct cert_as as;
> + ASN1_SEQUENCE_ANY *seq;
> + const ASN1_TYPE *t;
> + int rc = 0;
> +
> + if ((seq = d2i_ASN1_SEQUENCE_ANY(NULL, &d, dsz)) == NULL) {
> + cryptowarnx("%s: ASRange failed ASN.1 seq parse", p->fn);
> + goto out;
> + }
> +
> + if (sk_ASN1_TYPE_num(seq) != 2) {
> + warnx("%s: expected 2 elements in RSC ASRange, have %d",
> + p->fn, sk_ASN1_TYPE_num(seq));
> + goto out;
> + }
> +
> + memset(&as, 0, sizeof(struct cert_as));
> + as.type = CERT_AS_RANGE;
> +
> + t = sk_ASN1_TYPE_value(seq, 0);
> + if (t->type != V_ASN1_INTEGER) {
> + warnx("%s: RSC ASRange: want ASN.1 integer, have %s (NID %d)",
> + p->fn, ASN1_tag2str(t->type), t->type);
> + goto out;
> + }
> + if (!as_id_parse(t->value.integer, &as.range.min)) {
> + warnx("%s: RSC malformed AS identifier", p->fn);
> + goto out;
> + }
> +
> + t = sk_ASN1_TYPE_value(seq, 1);
> + if (t->type != V_ASN1_INTEGER) {
> + warnx("%s: RSC ASRange: want ASN.1 integer, have %s (NID %d)",
> + p->fn, ASN1_tag2str(t->type), t->type);
> + goto out;
> + }
> + if (!as_id_parse(t->value.integer, &as.range.max)) {
> + warnx("%s: RSC malformed AS identifier", p->fn);
> + goto out;
> + }
> +
> + if (as.range.max == as.range.min) {
> + warnx("%s: RSC ASRange error: range is singular", p->fn);
> + goto out;
> + }
> + if (as.range.max < as.range.min) {
> + warnx("%s: RSC ASRange: range is out of order", p->fn);
> + goto out;
> + }
> +
> + if (!append_as(p, &as))
> + goto out;
> +
> + rc = 1;
> + out:
> + sk_ASN1_TYPE_pop_free(seq, ASN1_TYPE_free);
> + return rc;
> +}
> +
> +/*
> + * parse AsList (inside ResourceBlock)
> + * Return 0 on failure.
> + */
> +static int
> +rsc_parse_aslist(struct parse *p, const unsigned char *d, size_t dsz)
> +{
> + ASN1_SEQUENCE_ANY *seq;
> + const ASN1_TYPE *t;
> + int i, rc = 0;
> +
> + if ((seq = d2i_ASN1_SEQUENCE_ANY(NULL, &d, dsz)) == NULL) {
> + cryptowarnx("%s: RSC AsList: failed ASN.1 sequence parse",
> + p->fn);
> + goto out;
> + }
> +
> + for (i = 0; i < sk_ASN1_TYPE_num(seq); i++) {
> + t = sk_ASN1_TYPE_value(seq, i);
> + switch (t->type) {
> + case V_ASN1_INTEGER:
> + if (!rsc_parse_asid(p, t->value.integer))
> + goto out;
> + break;
> + case V_ASN1_SEQUENCE:
> + d = t->value.asn1_string->data;
> + dsz = t->value.asn1_string->length;
> + if (!rsc_parse_asrange(p, d, dsz))
> + goto out;
> + break;
> + default:
> + warnx("%s: RSC AsList expected INTEGER or SEQUENCE, "
> + "have %s (NID %d)", p->fn, ASN1_tag2str(t->type),
> + t->type);
> + goto out;
> + }
> + }
> +
> + rc = 1;
> + out:
> + sk_ASN1_TYPE_pop_free(seq, ASN1_TYPE_free);
> + return rc;
> +}
> +
> +/*
> + * parse IPAddressFamilyItem (inside IPList, inside ResourceBlock)
> + * Return 0 on failure.
> + */
> +static int
> +rsc_parse_ipaddrfamitem(struct parse *p, const ASN1_OCTET_STRING *os)
> +{
> + ASN1_OCTET_STRING *aos = NULL;
> + IPAddressOrRange *aor = NULL;
> + int tag;
> + const unsigned char *cnt = os->data;
> + long cntsz;
> + const unsigned char *d;
> + size_t dsz = os->length;
> + struct cert_ip ip;
> + int rc = 0;
> +
> + memset(&ip, 0, sizeof(struct cert_ip));
> +
> + /*
> + * IPAddressFamilyItem is a sequence containing an addressFamily and
> + * an IPAddressOrRange.
> + */
> + if (!ASN1_frame(p->fn, dsz, &cnt, &cntsz, &tag)) {
> + cryptowarnx("%s: ASN1_frame failed", p->fn);
> + goto out;
> + }
> + if (tag != V_ASN1_SEQUENCE) {
> + warnx("expected ASN.1 sequence, got %d", tag);
> + goto out;
> + }
> +
> + d = cnt;
> +
> + if ((aos = d2i_ASN1_OCTET_STRING(NULL, &cnt, cntsz)) == NULL) {
> + cryptowarnx("%s: d2i_ASN1_OCTET_STRING failed", p->fn);
> + goto out;
> + }
> +
> + cntsz -= cnt - d;
> + assert(cntsz >= 0);
> +
> + if (!ip_addr_afi_parse(p->fn, aos, &ip.afi)) {
> + warnx("%s: RSC invalid addressFamily", p->fn);
> + goto out;
> + }
> +
> + d = cnt;
> +
> + if ((aor = d2i_IPAddressOrRange(NULL, &cnt, cntsz)) == NULL) {
> + warnx("%s: d2i_IPAddressOrRange failed", p->fn);
> + goto out;
> + }
> +
> + cntsz -= cnt - d;
> + assert(cntsz >= 0);
> +
> + if (cntsz > 0) {
> + warnx("%s: trailing garbage in RSC IPAddressFamilyItem", p->fn);
> + goto out;
> + }
> +
> + switch (aor->type) {
> + case IPAddressOrRange_addressPrefix:
> + ip.type = CERT_IP_ADDR;
> + if (!ip_addr_parse(aor->u.addressPrefix, ip.afi, p->fn, &ip.ip))
> + goto out;
> + break;
> + case IPAddressOrRange_addressRange:
> + ip.type = CERT_IP_RANGE;
> + if (!ip_addr_parse(aor->u.addressRange->min, ip.afi, p->fn,
> + &ip.range.min))
> + goto out;
> + if (!ip_addr_parse(aor->u.addressRange->max, ip.afi, p->fn,
> + &ip.range.max))
> + goto out;
> + break;
> + default:
> + warnx("%s: unknown addressOrRange type %d\n", p->fn, aor->type);
> + goto out;
> + }
> +
> + if (!ip_cert_compose_ranges(&ip)) {
> + warnx("%s: RSC IP address range reversed", p->fn);
> + goto out;
> + }
> +
> + if (!append_ip(p, &ip))
> + goto out;
> +
> + rc = 1;
> + out:
> + ASN1_OCTET_STRING_free(aos);
> + IPAddressOrRange_free(aor);
> + return rc;
> +}
> +
> +static int
> +rsc_parse_iplist(struct parse *p, const unsigned char *d, size_t dsz)
> +{
> + ASN1_SEQUENCE_ANY *seq;
> + const ASN1_TYPE *t;
> + int i, rc = 0;
> +
> + if ((seq = d2i_ASN1_SEQUENCE_ANY(NULL, &d, dsz)) == NULL) {
> + cryptowarnx("%s: RSC IPList: failed ASN.1 sequence parse",
> + p->fn);
> + goto out;
> + }
> +
> + for (i = 0; i < sk_ASN1_TYPE_num(seq); i++) {
> + t = sk_ASN1_TYPE_value(seq, i);
> + if (t->type != V_ASN1_SEQUENCE) {
> + warnx("%s: RSC IPList: want ASN.1 sequence, have %s"
> + " (NID %d)", p->fn, ASN1_tag2str(t->type), t->type);
> + goto out;
> + }
> + if (!rsc_parse_ipaddrfamitem(p, t->value.octet_string))
> + goto out;
> + }
> +
> + rc = 1;
> + out:
> + sk_ASN1_TYPE_pop_free(seq, ASN1_TYPE_free);
> + return rc;
> +}
> +
> +/*
> + * Parse a ResourceBlock, draft-ietf-sidrops-rpki-rsc section 4
> + * Returns zero on failure, non-zero on success.
> + */
> +static int
> +rsc_parse_resourceblock(const ASN1_OCTET_STRING *os, struct parse *p)
> +{
> + ASN1_SEQUENCE_ANY *seq;
> + const unsigned char *d = os->data;
> + size_t dsz = os->length;
> + int i, ptag, rc = 0;
> + const ASN1_TYPE *t;
> + long plen;
> +
> + if ((seq = d2i_ASN1_SEQUENCE_ANY(NULL, &d, dsz)) == NULL) {
> + cryptowarnx("%s: RSC: ResourceBlock: failed ASN.1 sequence "
> + "parse", p->fn);
> + goto out;
> + }
> +
> + if (sk_ASN1_TYPE_num(seq) == 0) {
> + warnx("%s: ResourceBlock, there must be at least one of asID "
> + "or ipAddrBlocks", p->fn);
> + goto out;
> + }
> +
> + for (i = 0; i < sk_ASN1_TYPE_num(seq); i++) {
> + t = sk_ASN1_TYPE_value(seq, i);
> +
> + d = t->value.asn1_string->data;
> + dsz = t->value.asn1_string->length;
> + if (!ASN1_frame(p->fn, dsz, &d, &plen, &ptag))
> + goto out;
> + switch (ptag) {
> + case RSRCBLK_TYPE_ASID:
> + if (!rsc_parse_aslist(p, d, plen))
> + goto out;
> + break;
> + case RSRCBLK_TYPE_IPADDRBLK:
> + if (!rsc_parse_iplist(p, d, plen))
> + goto out;
> + break;
> + default:
> + warnx("%s: want ASN.1 context specific id, have %s"
> + " (NID %d)", p->fn, ASN1_tag2str(ptag), ptag);
> + goto out;
> + }
> + }
> +
> + rc = 1;
> + out:
> + sk_ASN1_TYPE_pop_free(seq, ASN1_TYPE_free);
> + return rc;
> +}
> +
> +/*
> + * Parses the eContent segment of a RSC file
> + * draft-ietf-sidrops-rpki-rsc, section 4
> + * Returns zero on failure, non-zero on success.
> + */
> +static int
> +rsc_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p)
> +{
> + ASN1_SEQUENCE_ANY *seq;
> + const ASN1_TYPE *t;
> + int i = 0, rc = 0, sz;
> + long rsc_version;
> +
> + /*
> + * draft-ietf-sidrops-rpki-rsc section 4
> + */
> +
> + if ((seq = d2i_ASN1_SEQUENCE_ANY(NULL, &d, dsz)) == NULL) {
> + cryptowarnx("%s: RSC: RpkiSignedChecklist: failed ASN.1 "
> + "sequence parse", p->fn);
> + goto out;
> + }
> +
> + if ((sz = sk_ASN1_TYPE_num(seq)) != 3 && sz != 4) {
> + warnx("%s: RSC RpkiSignedChecklist: want 3 or 4 elements, have"
> + "%d", p->fn, sk_ASN1_TYPE_num(seq));
> + goto out;
> + }
> +
> + /*
> + * if there are 4 elements, a version should be present: check it.
> + */
> + if (sz == 4) {
> + t = sk_ASN1_TYPE_value(seq, i++);
> + d = t->value.asn1_string->data;
> + dsz = t->value.asn1_string->length;
> +
> + if (cms_econtent_version(p->fn, &d, dsz, &rsc_version) == -1)
> + goto out;
> +
> + switch (rsc_version) {
> + case 0:
> + warnx("%s: invalid encoding for version 0", p->fn);
> + goto out;
> + default:
> + warnx("%s: version %ld not supported (yet)", p->fn,
> + rsc_version);
> + goto out;
> + }
> + }
> +
> + /*
> + * The RSC's eContent ResourceBlock indicates which Internet Number
> + * Resources are associated with the signature over the checkList.
> + */
> + t = sk_ASN1_TYPE_value(seq, i++);
> + if (t->type != V_ASN1_SEQUENCE) {
> + warnx("%s: RSC ResourceBlock: want ASN.1 sequence, have %s"
> + "(NID %d)", p->fn, ASN1_tag2str(t->type), t->type);
> + goto out;
> + }
> + if (!rsc_parse_resourceblock(t->value.octet_string, p))
> + goto out;
> +
> + /* digestAlgorithm */
> + t = sk_ASN1_TYPE_value(seq, i++);
> + if (t->type != V_ASN1_SEQUENCE) {
> + warnx("%s: RSC DigestAlgorithmIdentifier: want ASN.1 sequence,"
> + " have %s (NID %d)", p->fn, ASN1_tag2str(t->type), t->type);
> + goto out;
> + }
> + if (!rsc_check_digesttype(p, t->value.asn1_string->data,
> + t->value.asn1_string->length))
> + goto out;
> +
> + /*
> + * Now a sequence of FileNameAndHash
> + */
> + t = sk_ASN1_TYPE_value(seq, i++);
> + if (t->type != V_ASN1_SEQUENCE) {
> + warnx("%s: RSC checkList: want ASN.1 sequence, have %s "
> + "(NID %d)", p->fn, ASN1_tag2str(t->type), t->type);
> + goto out;
> + }
> + if (!rsc_parse_checklist(p, t->value.octet_string))
> + goto out;
> +
> + rc = 1;
> + out:
> + sk_ASN1_TYPE_pop_free(seq, ASN1_TYPE_free);
> + return rc;
> +}
> +
> +/*
> + * Parse a full draft-ietf-sidrops-rpki-rsc file.
> + * Returns the RSC or NULL if the object was malformed.
> + */
> +struct rsc *
> +rsc_parse(X509 **x509, const char *fn, const unsigned char *der, size_t len)
> +{
> + struct parse p;
> + size_t cmsz;
> + unsigned char *cms;
> + int rc = 0;
> + const ASN1_TIME *at;
> + struct tm expires_tm;
> + time_t expires;
> +
> + memset(&p, 0, sizeof(struct parse));
> + p.fn = fn;
> +
> + cms = cms_parse_validate(x509, fn, der, len, rsc_oid, &cmsz);
> + if (cms == NULL)
> + return NULL;
> +
> + if ((p.res = calloc(1, sizeof(struct rsc))) == NULL)
> + err(1, NULL);
> +
> + if (!x509_get_aia(*x509, fn, &p.res->aia))
> + goto out;
> + if (!x509_get_aki(*x509, fn, &p.res->aki))
> + goto out;
> + if (!x509_get_ski(*x509, fn, &p.res->ski))
> + goto out;
> + if (p.res->aia == NULL || p.res->aki == NULL || p.res->ski == NULL) {
> + warnx("%s: RFC 6487 section 4.8: "
> + "missing AIA, AKI or SKI X509 extension", fn);
> + goto out;
> + }
> +
> + at = X509_get0_notAfter(*x509);
> + if (at == NULL) {
> + warnx("%s: X509_get0_notAfter failed", fn);
> + goto out;
> + }
> + memset(&expires_tm, 0, sizeof(expires_tm));
> + if (ASN1_time_parse(at->data, at->length, &expires_tm, 0) == -1) {
> + warnx("%s: ASN1_time_parse failed", fn);
> + goto out;
> + }
> + if ((expires = mktime(&expires_tm)) == -1)
> + errx(1, "mktime failed");
> +
> + p.res->expires = expires;
> +
> + if (!rsc_parse_econtent(cms, cmsz, &p))
> + goto out;
> +
> + rc = 1;
> + out:
> + if (rc == 0) {
> + rsc_free(p.res);
> + p.res = NULL;
> + X509_free(*x509);
> + *x509 = NULL;
> + }
> + free(cms);
> + return p.res;
> +}
> +
> +/*
> + * Free an RSC pointer.
> + * Safe to call with NULL.
> + */
> +void
> +rsc_free(struct rsc *p)
> +{
> + if (p == NULL)
> + return;
> +
> + free(p->aia);
> + free(p->aki);
> + free(p->ski);
> + free(p->ips);
> + free(p->as);
> + free(p->files);
This leaks memory inside the files array. At least the filenames are also
allocated strings that need to be freed.
> + free(p);
> +}
> +
> +/*
> + * Serialise parsed RSC content.
> + * See rsc_read() for reader.
> + */
> +void
> +rsc_buffer(struct ibuf *b, const struct rsc *p)
> +{
> + size_t i;
> +
> + io_simple_buffer(b, &p->expires, sizeof(p->expires));
> + io_simple_buffer(b, &p->talid, sizeof(p->talid));
> + io_simple_buffer(b, &p->valid, sizeof(p->valid));
> +
> + io_str_buffer(b, p->ski);
> + io_str_buffer(b, p->aki);
> + io_str_buffer(b, p->aia);
> +
> + io_simple_buffer(b, &p->asz, sizeof(p->asz));
> + io_simple_buffer(b, &p->as, p->asz * sizeof(p->as[0]));
> +
> + io_simple_buffer(b, &p->ipsz, sizeof(p->ipsz));
> + io_simple_buffer(b, &p->ips, p->ipsz * sizeof(p->ips[0]));
> +
> + io_simple_buffer(b, &p->filesz, sizeof(p->filesz));
> + for (i = 0; i < p->filesz; i++) {
> + io_str_buffer(b, p->files[i].filename);
> + io_simple_buffer(b, p->files[i].hash, SHA256_DIGEST_LENGTH);
> + }
> +}
This function can be dropped, not needed.
> +/*
> + * Read parsed RSC content from descriptor.
> + * See rsc_buffer() for writer.
> + * Result must be passed to rsc_free().
> + */
> +struct rsc *
> +rsc_read(struct ibuf *b)
> +{
> + struct rsc *p;
> + size_t i;
> +
> + if ((p = calloc(1, sizeof(struct rsc))) == NULL)
> + err(1, NULL);
> +
> + io_read_buf(b, &p->expires, sizeof(p->expires));
> + io_read_buf(b, &p->talid, sizeof(p->talid));
> + io_read_buf(b, &p->valid, sizeof(p->valid));
> +
> + io_read_str(b, &p->ski);
> + io_read_str(b, &p->aki);
> + io_read_str(b, &p->aia);
> + assert(p->ski && p->aia && p->aia);
> +
> + io_read_buf(b, &p->asz, sizeof(p->asz));
> + p->as = calloc(p->asz, sizeof(struct cert_as));
> + if (p->as == NULL)
> + err(1, NULL);
> + io_read_buf(b, p->as, p->asz * sizeof(p->as[0]));
> +
> + io_read_buf(b, &p->ipsz, sizeof(p->ipsz));
> + p->ips = calloc(p->ipsz, sizeof(struct cert_ip));
> + if (p->ips == NULL)
> + err(1, NULL);
> + io_read_buf(b, p->ips, p->ipsz * sizeof(p->ips[0]));
> +
> + io_read_buf(b, &p->filesz, sizeof(size_t));
> + if ((p->files = calloc(p->filesz, sizeof(struct rscfile))) == NULL)
> + err(1, NULL);
> + for (i = 0; i < p->filesz; i++) {
> + io_read_str(b, &p->files[i].filename);
> + io_read_buf(b, p->files[i].hash, SHA256_DIGEST_LENGTH);
> + }
> +
> + return p;
> +}
Same as above.
> Index: usr.sbin/rpki-client/validate.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/validate.c,v
> retrieving revision 1.31
> diff -u -p -r1.31 validate.c
> --- usr.sbin/rpki-client/validate.c 21 Apr 2022 09:53:07 -0000 1.31
> +++ usr.sbin/rpki-client/validate.c 8 May 2022 20:01:25 -0000
> @@ -486,3 +486,65 @@ valid_x509(char *file, X509_STORE_CTX *s
> sk_X509_CRL_free(crls);
> return 1;
> }
> +
> +/*
> + * Validate our RSC: check that all items in the ResourceBlock are contained.
> + * Returns 1 if valid, 0 otherwise.
> + */
> +int
> +valid_rsc(const char *fn, struct auth *a, struct rsc *rsc)
> +{
> + size_t i;
> + uint32_t min, max;
> + char buf1[64], buf2[64];
> +
> + for (i = 0; i < rsc->asz; i++) {
> + switch (rsc->as[i].type) {
> + case CERT_AS_INHERIT:
> + return 0; /* RSC doesn't permit inheriting */
> + case CERT_AS_ID:
> + if (valid_as(a, rsc->as[i].id, rsc->as[i].id))
> + continue;
> + warnx("%s: RSC resourceBlock: uncovered AS Identifier: "
> + "%u", fn, rsc->as[i].id);
> + return 0;
> + case CERT_AS_RANGE:
> + min = rsc->as[i].range.min;
> + max = rsc->as[i].range.max;
> + if (valid_as(a, min, max))
> + continue;
> + warnx("%s: RSC resourceBlock: uncovered AS Range: "
> + "%u--%u", fn, min, max);
> + return 0;
> + }
> + }
> +
> + for (i = 0; i < rsc->ipsz; i++) {
> + if (valid_ip(a, rsc->ips[i].afi, rsc->ips[i].min,
> + rsc->ips[i].max))
> + continue;
> +
> + switch (rsc->ips[i].type) {
> + case CERT_IP_RANGE:
> + ip_addr_print(&rsc->ips[i].range.min,
> + rsc->ips[i].afi, buf1, sizeof(buf1));
> + ip_addr_print(&rsc->ips[i].range.max,
> + rsc->ips[i].afi, buf2, sizeof(buf2));
> + warnx("%s: RSC ResourceBlock: uncovered IP Range: "
> + "%s--%s", fn, buf1, buf2);
> + break;
> + case CERT_IP_ADDR:
> + ip_addr_print(&rsc->ips[i].ip,
> + rsc->ips[i].afi, buf1, sizeof(buf1));
> + warnx("%s: RSC ResourceBlock: uncovered IP: "
> + "%s", fn, buf1);
> + break;
> + case CERT_IP_INHERIT:
> + warnx("%s: RSC ResourceBlock: illegal inherit", fn);
> + break;
I'm not sure if this works the way you think it does. The switch case is
only triggers when valid_ip() fails but that may not be the case if
rsc->ips[i].type == CERT_IP_INHERIT.
I think the proper way to check this is above the valid_ip() call to make
sure CERT_IP_INHERIT can't be used. The parse kind of makes sure of this
(it does not handle NULL ipAddrOrRange objects so it will bork on that
with an error).
> + }
> + return 0;
> + }
> +
> + return 1;
> +}
Actually this function is not used anyway because the only place it was
called was in the parse.c code but that code is no needed.
Also this code should probably verify the fileandhash list if we are
serious about full RSC validation. rpki-client -f currently skips some
other validation steps so it mostly depends on proper x509 validation.
> Index: usr.sbin/rpki-client/x509.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/x509.c,v
> retrieving revision 1.41
> diff -u -p -r1.41 x509.c
> --- usr.sbin/rpki-client/x509.c 15 Apr 2022 12:59:44 -0000 1.41
> +++ usr.sbin/rpki-client/x509.c 8 May 2022 20:01:25 -0000
> @@ -42,6 +42,7 @@ ASN1_OBJECT *cnt_type_oid; /* pkcs-9 id-
> ASN1_OBJECT *msg_dgst_oid; /* pkcs-9 id-messageDigest */
> ASN1_OBJECT *sign_time_oid; /* pkcs-9 id-signingTime */
> ASN1_OBJECT *bin_sign_time_oid; /* pkcs-9 id-aa-binarySigningTime */
> +ASN1_OBJECT *rsc_oid; /* id-ct-signedChecklist */
>
> void
> x509_init_oid(void)
> @@ -76,6 +77,10 @@ x509_init_oid(void)
> OBJ_txt2obj("1.2.840.113549.1.9.16.2.46", 1)) == NULL)
> errx(1, "OBJ_txt2obj for %s failed",
> "1.2.840.113549.1.9.16.2.46");
> + if ((rsc_oid = OBJ_txt2obj("1.2.840.113549.1.9.16.1.48", 1)) == NULL)
> + errx(1, "OBJ_txt2obj for %s failed",
> + "1.2.840.113549.1.9.16.1.48");
> +
> }
>
> /*
> Index: regress/usr.sbin/rpki-client/test-rsc.c
> ===================================================================
> RCS file: regress/usr.sbin/rpki-client/test-rsc.c
> diff -N regress/usr.sbin/rpki-client/test-rsc.c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ regress/usr.sbin/rpki-client/test-rsc.c 8 May 2022 20:01:25 -0000
> @@ -0,0 +1,104 @@
> +/* $Id: test-rsc.c,v 1.18 2022/01/19 08:24:43 claudio Exp $ */
> +/*
> + * Copyright (c) 2019 Kristaps Dzonsons <[email protected]>
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#include <sys/types.h>
> +#include <netinet/in.h>
> +#include <assert.h>
> +#include <err.h>
> +#include <resolv.h> /* b64_ntop */
> +#include <stdio.h>
> +#include <stdint.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +
> +#include <openssl/err.h>
> +#include <openssl/evp.h>
> +#include <openssl/pem.h>
> +#include <openssl/x509v3.h>
> +
> +#include "extern.h"
> +
> +int outformats;
> +int verbose;
> +
> +int
> +main(int argc, char *argv[])
> +{
> + int c, i, ppem = 0, verb = 0;
> + struct rsc *p;
> + BIO *bio_out = NULL;
> + X509 *xp = NULL;
> + unsigned char *buf;
> + size_t len;
> +
> + ERR_load_crypto_strings();
> + OpenSSL_add_all_ciphers();
> + OpenSSL_add_all_digests();
> + x509_init_oid();
> +
> + while (-1 != (c = getopt(argc, argv, "pv")))
> + switch (c) {
> + case 'p':
> + if (ppem)
> + break;
> + ppem = 1;
> + if ((bio_out = BIO_new_fp(stdout, BIO_NOCLOSE)) == NULL)
> + errx(1, "BIO_new_fp");
> + break;
> + case 'v':
> + verb++;
> + break;
> + default:
> + errx(1, "bad argument %c", c);
> + }
> +
> + argv += optind;
> + argc -= optind;
> +
> + if (argc == 0)
> + errx(1, "argument missing");
> +
> + for (i = 0; i < argc; i++) {
> + buf = load_file(argv[i], &len);
> + if ((p = rsc_parse(&xp, argv[i], buf, len)) == NULL) {
> + free(buf);
> + continue;
> + }
> + if (verb)
> + rsc_print(xp, p);
> + if (ppem) {
> + if (!PEM_write_bio_X509(bio_out, xp))
> + errx(1,
> + "PEM_write_bio_X509: unable to write cert");
> + }
> + free(buf);
> + rsc_free(p);
> + X509_free(xp);
> + }
> +
> + BIO_free(bio_out);
> + EVP_cleanup();
> + CRYPTO_cleanup_all_ex_data();
> + ERR_free_strings();
> +
> + if (i < argc)
> + errx(1, "test failed for %s", argv[i]);
> +
> + printf("OK\n");
> + return 0;
> +}
> Index: regress/usr.sbin/rpki-client/Makefile.inc
> ===================================================================
> RCS file: /cvs/src/regress/usr.sbin/rpki-client/Makefile.inc,v
> retrieving revision 1.23
> diff -u -p -r1.23 Makefile.inc
> --- regress/usr.sbin/rpki-client/Makefile.inc 20 Apr 2022 17:37:53 -0000
> 1.23
> +++ regress/usr.sbin/rpki-client/Makefile.inc 8 May 2022 20:01:25 -0000
> @@ -7,6 +7,7 @@ PROGS += test-cert
> PROGS += test-gbr
> PROGS += test-mft
> PROGS += test-roa
> +PROGS += test-rsc
> PROGS += test-tal
> PROGS += test-rrdp
>
> @@ -42,6 +43,11 @@ SRCS_test-roa+= test-roa.c roa.c cms.c x
> encoding.c print.c validate.c cert.c mft.c
> run-regress-test-roa: test-roa
> ./test-roa -v ${.CURDIR}/../roa/*.roa
> +
> +SRCS_test-rsc+= test-rsc.c rsc.c cms.c x509.c ip.c as.c io.c log.c \
> + encoding.c print.c validate.c cert.c mft.c
> +run-regress-test-rsc: test-rsc
> + ./test-rsc -v ${.CURDIR}/../rsc/*.sig
>
> SRCS_test-gbr+= test-gbr.c gbr.c cms.c x509.c ip.c io.c log.c \
> encoding.c print.c validate.c as.c cert.c mft.c
>
--
:wq Claudio
