On Fri, Oct 21, 2022 at 01:08:49PM +0000, Job Snijders wrote:
> Dear all,
>
> I've authored an extension to rpki-client(8) to support validation of
> Signed Objects containing Trust Anchor Keys (aka 'Signed TALs'). Signed
> TALs provide a mechanism for RIRs to distribute and sign the next Trust
> Anchor with the current Trust Anchor. This might be an improvement over
> going to RIR websites and copy+pasting TAL data.
>
> The Signed TAL specification is available here:
> https://datatracker.ietf.org/doc/html/draft-ietf-sidrops-signed-tal
>
> A testbed is available here:
> https://github.com/APNIC-net/rpki-signed-tal-demo/blob/master/testbed-tas.md
>
> This implementation deviates from the specification in a small number of
> ways:
>
> * omitted to implement the notion of acceptance timers.
> * omitted to check (and disallow) multiple TAK objects under a single
> TA.
> * Because of the unveil() write barrier between the parser subprocess
> and the system operator 'owned' roots in /etc/rpki, this
> implementation does not use successor TAKeys for top-down validation.
>
> Below is a terminal transcript of the workflow I envision distributors
> of RPKI TALs would use to cryptographically verify a new TAL originated
> from a given Trust Anchor operator.
Apologies for the long wait. There are a few things that I still need
to think through more thoroughly, especially your unveil-related
omission and your suggested workflow.
In the interest of moving forward with this, here's a first round of
comments. We can address some of that in tree if you prefer.
>
> Kind regards,
>
> Job
>
> $ doas rpki-client -R -t two_taks.tal
> Processing time 12 seconds (0 seconds user, 0 seconds system)
> Skiplist entries: 0
> Route Origin Authorizations: 0 (0 failed parse, 0 invalid)
> AS Provider Attestations: 0 (0 failed parse, 0 invalid)
> BGPsec Router Certificates: 0
> Certificates: 2 (0 invalid)
> Trust Anchor Locators: 1 (0 invalid)
> Manifests: 2 (0 failed parse, 0 stale)
> Certificate revocation lists: 2
> Ghostbuster records: 0
> Repositories: 2
> Cleanup: removed 531 files, 228 directories, 0 superfluous
> VRP Entries: 0 (0 unique)
> VAP Entries: 0 (0 unique)
>
> $ cd /var/cache/rpki-client/
>
> $ find * -name '*.tak'
> rpki-testbed.apnic.net/repository/ED9D6A424DA911EDB9AC0C5B9E174E93/05F53BCE4DAA11EDB9AC0C5B9E174E93.tak
>
> $ cd rpki-testbed.apnic.net/repository/ED9D6A424DA911EDB9AC0C5B9E174E93/
>
> $ rpki-client -t two_taks.tal -f 05F53BCE4DAA11EDB9AC0C5B9E174E93.tak
> File: 05F53BCE4DAA11EDB9AC0C5B9E174E93.tak
> Hash identifier: jTSYpCn4ceH4eSfpvehCtg9B4vr2pZYfyhAZg1z34L8=
> Subject key identifier:
> 37:AF:B3:64:3A:EA:0F:50:82:E9:20:D4:F2:72:C3:5B:88:A2:23:3A
> Certificate serial: 05
> Authority key identifier:
> 08:C4:85:FC:A8:A3:59:F2:AD:09:47:E8:0F:CD:1F:48:52:93:4D:8F
> Authority info access:
> rsync://rpki-testbed.apnic.net/repository/ED9D5F8E4DA911EDB9AC0C5B9E174E93/root.cer
> TAK EE certificate valid until: 2037-01-01T00:00:00Z
> TAL derived from the 'current' Trust Anchor Key:
>
> # Current key for original TAL
>
> rsync://rpki-testbed.apnic.net/repository/ED9D5F8E4DA911EDB9AC0C5B9E174E93/root.cer
>
> MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAyYT3tLDtrBW4xeBnhNWM
> jgRkJJKlUyh3TodVDLxF+7r2CZOpTqSjkNO68G7HEmlRJnIxT6OyY+E4vg9kHmIU
> cYyy9Cf3AP1GN9PbgR2TD6BCZzinxhQZSvm8tN0D16jjWBLh23FXbNr0a0lhnbPa
> MGuLVIN2mHQ1eLyUSqhx6G40f+yVfOvmjW2vgkfiYkOSjbDPIGSVSn8jHzwDAVMY
> QZdqPgHniw3RHlCmJRJWFVGxgG4qBdjeZfnlCRZamv3/MA8HJMMu6knLc4xj7uFC
> kd/+I40pmZHrubsyWgLN4RBfx62hAOF/TTKzCqoncc45PYe8KOFHeEIR5fna0RjW
> sQIDAQAB
>
> TAL derived from the 'successor' Trust Anchor Key:
>
> # Successor key for original TAL
>
> rsync://rpki-testbed.apnic.net/repository/F785A7404DA911EDB9AC0C5B9E174E93/root.cer
>
> MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA1lGORSJFEjbf8t6zEANM
> F6ROAjSGs9mFMX5DnPUFoMzRcjc6mdgE1T4XsThm+oHc7lxlr4mhaZ08QPePHGuT
> UilXOBNZTyZS2Ed75fQTN12biVaOdP/XQCUEEllqpCiNpKt7+C0YjMmbzS1jEK76
> 3WgaXQsKSiDolyHr4xfxmhf+/e3F8C6nU9OYaVSKU3grlr/rpfTZl7CSe7Gq1BNW
> A3BWuYHA/wmlGIQdV60GcvkHWZPzJTdxkRQLgeCQUHY22YHpiRtqckVCx9jHOU8v
> ygp62bERlCRT7PtGmgdUJE7Co40289/sgWWo2R0CncrvhPFdBEoaDNT/s+I+eTtx
> UQIDAQAB
>
> Validation: OK
>
> End of terminal transcript, proposed changeset follows below:
>
> Index: Makefile
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/Makefile,v
> retrieving revision 1.26
> diff -u -p -r1.26 Makefile
> --- Makefile 30 Aug 2022 18:56:49 -0000 1.26
> +++ Makefile 21 Oct 2022 13:07:29 -0000
> @@ -5,7 +5,7 @@ SRCS= as.c aspa.c cert.c cms.c crl.c enc
> ip.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 \
> - rsc.c rsync.c tal.c validate.c x509.c
> + rsc.c rsync.c tak.c tal.c validate.c x509.c
> MAN= rpki-client.8
>
> LDADD+= -lexpat -ltls -lssl -lcrypto -lutil
> Index: extern.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
> retrieving revision 1.156
> diff -u -p -r1.156 extern.h
> --- extern.h 3 Sep 2022 21:24:02 -0000 1.156
> +++ extern.h 21 Oct 2022 13:07:29 -0000
> @@ -183,6 +183,7 @@ enum rtype {
> RTYPE_FILE,
> RTYPE_RSC,
> RTYPE_ASPA,
> + RTYPE_TAK,
> };
>
> enum location {
> @@ -275,6 +276,33 @@ struct rsc {
> };
>
> /*
> + * Datastructure representing the TAKey sequence inside TAKs.
> + */
> +struct takey {
> + char **comments; /* Comments */
> + size_t commentsz; /* number of Comments */
> + char **uris; /* CertificateURI */
> + size_t urisz; /* number of CertificateURIs */
> + unsigned char *pubkey; /* DER encoded SubjectPublicKeyInfo */
> + size_t pubkeysz;
> + char *ski; /* hex encoded SubjectKeyIdentifier of pubkey */
> +};
> +
> +/*
> + * A Signed TAL (TAK) draft-ietf-sidrops-signed-tal-12
> + */
> +struct tak {
> + int talid; /* TAK covered by what TAL */
> + struct takey *current;
> + struct takey *predecessor;
> + struct takey *successor;
> + char *aia; /* AIA */
> + char *aki; /* AKI */
> + char *ski; /* SKI */
> + time_t expires; /* Not After of the TAK EE */
> +};
> +
> +/*
> * A single Ghostbuster record
> */
> struct gbr {
> @@ -474,6 +502,7 @@ struct stats {
> size_t rrdp_fails; /* failed rrdp repositories */
> size_t crls; /* revocation lists */
> size_t gbrs; /* ghostbuster records */
> + size_t taks; /* signed TAL objects */
> size_t aspas; /* ASPA objects */
> size_t aspas_fail; /* ASPA objects failing syntactic parse */
> size_t aspas_invalid; /* ASPAs with invalid customerASID */
> @@ -544,6 +573,11 @@ void rsc_free(struct rsc *);
> struct rsc *rsc_parse(X509 **, const char *, const unsigned char *,
> size_t);
>
> +void tak_free(struct tak *);
> +struct tak *tak_parse(X509 **, const char *, const unsigned char *,
> + size_t);
> +struct tak *tak_read(struct ibuf *);
> +
> void aspa_buffer(struct ibuf *, const struct aspa *);
> void aspa_free(struct aspa *);
> void aspa_insert_vaps(struct vap_tree *, struct aspa *, size_t *,
> @@ -726,6 +760,7 @@ void roa_print(const X509 *, const str
> void gbr_print(const X509 *, const struct gbr *);
> void rsc_print(const X509 *, const struct rsc *);
> void aspa_print(const X509 *, const struct aspa *);
> +void tak_print(const X509 *, const struct tak *);
>
> /* Output! */
>
> Index: filemode.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/filemode.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 filemode.c
> --- filemode.c 6 Sep 2022 11:16:51 -0000 1.14
> +++ filemode.c 21 Oct 2022 13:07:29 -0000
> @@ -269,6 +269,7 @@ proc_parser_file(char *file, unsigned ch
> struct tal *tal = NULL;
> struct rsc *rsc = NULL;
> struct aspa *aspa = NULL;
> + struct tak *tak = NULL;
> char *aia = NULL, *aki = NULL;
> char filehash[SHA256_DIGEST_LENGTH];
> char *hash;
> @@ -376,6 +377,14 @@ proc_parser_file(char *file, unsigned ch
> aia = aspa->aia;
> aki = aspa->aki;
> break;
> + case RTYPE_TAK:
> + tak = tak_parse(&x509, file, buf, len);
> + if (tak == NULL)
> + break;
> + tak_print(x509, tak);
> + aia = tak->aia;
> + aki = tak->aki;
> + break;
> default:
> printf("%s: unsupported file type\n", file);
> break;
> @@ -469,6 +478,7 @@ proc_parser_file(char *file, unsigned ch
> tal_free(tal);
> rsc_free(rsc);
> aspa_free(aspa);
> + tak_free(tak);
> }
>
> /*
> Index: main.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
> retrieving revision 1.219
> diff -u -p -r1.219 main.c
> --- main.c 3 Sep 2022 09:22:25 -0000 1.219
> +++ main.c 21 Oct 2022 13:07:30 -0000
> @@ -612,6 +612,9 @@ entity_process(struct ibuf *b, struct st
> st->aspas_invalid++;
> aspa_free(aspa);
> break;
> + case RTYPE_TAK:
> + st->taks++;
> + break;
> default:
> errx(1, "unknown entity type %d", type);
> }
> @@ -1311,6 +1314,8 @@ main(int argc, char *argv[])
> stats.mfts, stats.mfts_fail, stats.mfts_stale);
> printf("Certificate revocation lists: %zu\n", stats.crls);
> printf("Ghostbuster records: %zu\n", stats.gbrs);
> + if (verbose)
> + printf("Trust Anchor Keys: %zu\n", stats.taks);
Why is this hidden under verbose?
> printf("Repositories: %zu\n", stats.repos);
> printf("Cleanup: removed %zu files, %zu directories, %zu superfluous\n",
> stats.del_files, stats.del_dirs, stats.extra_files);
> Index: mft.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/mft.c,v
> retrieving revision 1.75
> diff -u -p -r1.75 mft.c
> --- mft.c 13 Oct 2022 04:43:32 -0000 1.75
> +++ mft.c 21 Oct 2022 13:07:30 -0000
> @@ -1,4 +1,4 @@
> -/* $OpenBSD: mft.c,v 1.75 2022/10/13 04:43:32 job Exp $ */
> +/* $OpenBSD: mft.c,v 1.74 2022/08/30 18:56:49 job Exp $ */
> /*
> * Copyright (c) 2022 Theo Buehler <[email protected]>
> * Copyright (c) 2019 Kristaps Dzonsons <[email protected]>
> @@ -168,6 +168,8 @@ rtype_from_file_extension(const char *fn
> return RTYPE_RSC;
> if (strcasecmp(fn + sz - 4, ".asa") == 0)
> return RTYPE_ASPA;
> + if (strcasecmp(fn + sz - 4, ".tak") == 0)
> + return RTYPE_TAK;
>
> return RTYPE_INVALID;
> }
> @@ -208,6 +210,7 @@ rtype_from_mftfile(const char *fn)
> case RTYPE_GBR:
> case RTYPE_ROA:
> case RTYPE_ASPA:
> + case RTYPE_TAK:
> return type;
> default:
> return RTYPE_INVALID;
> Index: output-json.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/output-json.c,v
> retrieving revision 1.28
> diff -u -p -r1.28 output-json.c
> --- output-json.c 30 Aug 2022 23:40:37 -0000 1.28
> +++ output-json.c 21 Oct 2022 13:07:30 -0000
> @@ -52,6 +52,7 @@ outputheader_json(FILE *out, struct stat
> "\t\t\"bgpsec_pubkeys\": %zu,\n"
> "\t\t\"certificates\": %zu,\n"
> "\t\t\"invalidcertificates\": %zu,\n"
> + "\t\t\"taks\": %zu,\n"
> "\t\t\"tals\": %zu,\n"
> "\t\t\"invalidtals\": %zu,\n"
> "\t\t\"talfiles\": [\n",
> @@ -59,7 +60,7 @@ outputheader_json(FILE *out, struct stat
> (long long)st->user_time.tv_sec, (long long)st->system_time.tv_sec,
> st->roas, st->roas_fail, st->roas_invalid,
> st->aspas, st->aspas_fail, st->aspas_invalid,
> - st->brks, st->certs, st->certs_fail,
> + st->brks, st->certs, st->certs_fail, st->taks,
> st->tals, talsz - st->tals) < 0)
> return -1;
>
> Index: parser.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v
> retrieving revision 1.77
> diff -u -p -r1.77 parser.c
> --- parser.c 3 Sep 2022 21:24:02 -0000 1.77
> +++ parser.c 21 Oct 2022 13:07:30 -0000
> @@ -521,6 +521,42 @@ proc_parser_aspa(char *file, const unsig
> }
>
> /*
> + * Parse a TAK object.
> + */
> +static struct tak *
> +proc_parser_tak(char *file, const unsigned char *der, size_t len)
> +{
> + struct tak *tak;
> + X509 *x509;
> + struct crl *crl;
> + struct auth *a;
> + int rc = 0;
> +
> + if ((tak = tak_parse(&x509, file, der, len)) == NULL)
> + return NULL;
> +
> + a = valid_ski_aki(file, &auths, tak->ski, tak->aki);
> + crl = crl_get(&crlt, a);
> +
> + /* TAK EE must be signed by self-signed CA */
> + if (a->parent != NULL)
> + goto out;
> +
> + if (!valid_x509(file, ctx, x509, a, crl, 0))
> + goto out;
> +
> + tak->talid = a->cert->talid;
> + rc = 1;
> + out:
> + if (rc == 0) {
> + tak_free(tak);
> + tak = NULL;
> + }
> + X509_free(x509);
> + return tak;
> +}
> +
> +/*
> * Load the file specified by the entity information.
> */
> static char *
> @@ -648,6 +684,11 @@ parse_entity(struct entityq *q, struct m
> if (aspa != NULL)
> aspa_buffer(b, aspa);
> aspa_free(aspa);
> + break;
> + case RTYPE_TAK:
> + file = parse_load_file(entp, &f, &flen);
> + io_str_buffer(b, file);
> + proc_parser_tak(file, f, flen);
> break;
> default:
> errx(1, "unhandled entity type %d", entp->type);
> Index: print.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/print.c,v
> retrieving revision 1.16
> diff -u -p -r1.16 print.c
> --- print.c 30 Aug 2022 23:41:53 -0000 1.16
> +++ print.c 21 Oct 2022 13:07:30 -0000
> @@ -617,3 +617,95 @@ aspa_print(const X509 *x, const struct a
> }
> }
> }
> +
> +static void
> +takey_print(char *name, const struct takey *t)
> +{
> + char *spki = NULL;
> + size_t i, j = 0;
> +
> + if (base64_encode(t->pubkey, t->pubkeysz, &spki) != 0)
> + errx(1, "base64_encode failed in %s", __func__);
> +
> + if (outformats & FORMAT_JSON) {
> + printf("\t\t{\n\t\t\t\"name\": \"%s\",\n", name);
> + printf("\t\t\t\"comments\": [");
> + for (i = 0; i < t->commentsz; i++) {
> + printf("\"%s\"", t->comments[i]);
> + if (i + 1 < t->commentsz)
> + printf(", ");
> + }
> + printf("],\n");
> + printf("\t\t\t\"uris\": [");
> + for (i = 0; i < t->urisz; i++) {
> + printf("\"%s\"", t->uris[i]);
> + if (i + 1 < t->urisz)
> + printf(", ");
> + }
> + printf("],\n");
> + printf("\t\t\t\"spki\": \"%s\"\n\t\t}", spki);
> + } else {
> + printf("TAL derived from the '%s' Trust Anchor Key:\n\n", name);
> +
> + for (i = 0; i < t->commentsz; i++) {
> + printf("\t# %s\n", t->comments[i]);
> + }
> +
> + for (i = 0; i < t->urisz; i++) {
> + printf("\t%s\n\n\t", t->uris[i]);
> + }
> +
> + for (i = 0; i < strlen(spki); i++) {
> + printf("%c", spki[i]);
> + j++;
> + if (j == 64) {
> + printf("\n\t");
> + j = 0;
> + }
> + }
> +
> + printf("\n\n");
> + }
> +
> + free(spki);
> +}
> +
> +void
> +tak_print(const X509 *x, const struct tak *p)
> +{
> + char tbuf[21];
> +
> + if (outformats & FORMAT_JSON) {
> + printf("\t\"type\": \"tak\",\n");
> + printf("\t\"ski\": \"%s\",\n", pretty_key_id(p->ski));
> + x509_print(x);
> + printf("\t\"aki\": \"%s\",\n", pretty_key_id(p->aki));
> + printf("\t\"aia\": \"%s\",\n", p->aia);
> + printf("\t\"valid_until\": %lld,\n", (long long)p->expires);
> + printf("\t\"takeys\": [\n");
> + } else {
> + strftime(tbuf, sizeof(tbuf), "%FT%TZ", gmtime(&p->expires));
> + printf("Subject key identifier: %s\n", pretty_key_id(p->ski));
> + x509_print(x);
> + printf("Authority key identifier: %s\n", pretty_key_id(p->aki));
> + printf("Authority info access: %s\n", p->aia);
> + printf("TAK EE certificate valid until: %s\n", tbuf);
> + }
> +
> + takey_print("current", p->current);
> +
> + if (p->predecessor != NULL) {
> + if (outformats & FORMAT_JSON)
> + printf(",\n");
> + takey_print("predecessor", p->predecessor);
> + }
> +
> + if (p->successor != NULL) {
> + if (outformats & FORMAT_JSON)
> + printf(",\n");
> + takey_print("successor", p->successor);
> + }
> +
> + if (outformats & FORMAT_JSON)
> + printf("\n\t],\n");
> +}
> Index: rpki-client.8
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/rpki-client.8,v
> retrieving revision 1.73
> diff -u -p -r1.73 rpki-client.8
> --- rpki-client.8 5 Sep 2022 20:08:26 -0000 1.73
> +++ rpki-client.8 21 Oct 2022 13:07:30 -0000
> @@ -305,6 +305,8 @@ Resource Public Key Infrastructure (RPKI
> A profile for Resource Public Key Infrastructure (RPKI) Signed Checklists
> (RSC).
> .It draft-ietf-sidrops-aspa-profile-10
> A Profile for Autonomous System Provider Authorization (ASPA).
> +.It draft-ietf-sidrops-signed-tal-12
> +RPKI Signed Object for Trust Anchor Key.
> .El
> .Sh HISTORY
> .Nm
> Index: rsync.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/rsync.c,v
> retrieving revision 1.43
> diff -u -p -r1.43 rsync.c
> --- rsync.c 2 Sep 2022 17:39:51 -0000 1.43
> +++ rsync.c 21 Oct 2022 13:07:30 -0000
> @@ -158,6 +158,7 @@ exec_rsync(const char *prog, const char
> args[i++] = "--include=*.mft";
> args[i++] = "--include=*.roa";
> args[i++] = "--include=*.asa";
> + args[i++] = "--include=*.tak";
> args[i++] = "--exclude=*";
> if (bind_addr != NULL) {
> args[i++] = "--address";
> Index: tak.c
> ===================================================================
> RCS file: tak.c
> diff -N tak.c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ tak.c 21 Oct 2022 13:07:30 -0000
> @@ -0,0 +1,336 @@
> +/* $OpenBSD: tak.c,v 1.15 2022/09/03 14:40:09 job Exp $ */
> +/*
> + * Copyright (c) 2022 Job Snijders <[email protected]>
> + * Copyright (c) 2022 Theo Buehler <[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 <err.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +
> +#include <openssl/asn1.h>
> +#include <openssl/asn1t.h>
> +#include <openssl/safestack.h>
> +#include <openssl/stack.h>
> +#include <openssl/x509.h>
> +#include <openssl/x509v3.h>
> +
> +#include "extern.h"
> +
> +/*
> + * Parse results and data of the Trust Anchor Key file.
> + */
> +struct parse {
> + const char *fn; /* TAK file name */
> + struct tak *res; /* results */
> +};
> +
> +extern ASN1_OBJECT *tak_oid;
> +
> +/*
> + * ASN.1 templates for Trust Anchor Keys (draft-ietf-sidrops-signed-tal-12)
> + */
> +
> +DECLARE_STACK_OF(ASN1_IA5STRING);
Ugh, yuck. That looks like a bug in libcrypto :S
OpenSSL appears to omit this as well.
> +
> +#ifndef DEFINE_STACK_OF
> +#define sk_ASN1_IA5STRING_num(st) SKM_sk_num(ASN1_IA5STRING, (st))
> +#define sk_ASN1_IA5STRING_value(st, i) SKM_sk_value(ASN1_IA5STRING, (st),
> (i))
> +#endif
> +
> +typedef struct {
> + STACK_OF(ASN1_UTF8STRING) *comments;
> + STACK_OF(ASN1_IA5STRING) *certificateURIs;
> + X509_PUBKEY *subjectPublicKeyInfo;
> +} TAKey;
> +
> +typedef struct {
> + ASN1_INTEGER *version;
> + TAKey *current;
> + TAKey *predecessor;
> + TAKey *successor;
> +} TAK;
> +
> +ASN1_SEQUENCE(TAKey) = {
> + ASN1_SEQUENCE_OF(TAKey, comments, ASN1_UTF8STRING),
> + ASN1_SEQUENCE_OF(TAKey, certificateURIs, ASN1_IA5STRING),
> + ASN1_SIMPLE(TAKey, subjectPublicKeyInfo, X509_PUBKEY),
> +} ASN1_SEQUENCE_END(TAKey);
> +
> +ASN1_SEQUENCE(TAK) = {
> + ASN1_OPT(TAK, version, ASN1_INTEGER),
> + ASN1_SIMPLE(TAK, current, TAKey),
> + ASN1_EXP_OPT(TAK, predecessor, TAKey, 0),
> + ASN1_EXP_OPT(TAK, successor, TAKey, 1),
> +} ASN1_SEQUENCE_END(TAK);
> +
> +DECLARE_ASN1_FUNCTIONS(TAK);
> +IMPLEMENT_ASN1_FUNCTIONS(TAK);
> +
> +/*
> + * Return zero on failure, non-zero on success.
> + */
> +static int
> +parse_takey(const char *fn, const TAKey *takey, struct takey *out)
I'm not a fan of this API. I think it should be either
/* Return NULL on error and allocated, valid struct takey on success. */
struct takey *
parse_takey(const char *fn, const TAKey *takey)
or
/* Return 0 on error and 1 on success. */
int
parse_takey(const char *fn, const TAKEY *takey, struct takey **out_takey)
Either way, it should be parse_takey()'s responsibility to allocate the
new struct takey and to free it on error.
> +{
> + const ASN1_UTF8STRING *comment;
> + const ASN1_IA5STRING *certURI;
> + EVP_PKEY *pkey;
> + RSA *r;
> + unsigned char *der = NULL, *rder = NULL;
> + unsigned char md[SHA_DIGEST_LENGTH];
> + size_t i;
> + int rdersz, rc = 0;
space before tab here: ^
(previous two lines)
> +
> + if (takey == NULL)
> + goto out;
> +
> + out->commentsz = sk_ASN1_UTF8STRING_num(takey->comments);
> + if ((out->comments = calloc(out->commentsz, sizeof(char *))) == NULL)
> + err(1, NULL);
The comment is allowed to be empty, and if that is the case, libc can
return NULL without it being an error, so I think we should only call
calloc() if out->commentsz > 0. (We may be doing this elsewhere).
> +
> + for (i = 0; i < out->commentsz; i++) {
> + comment = sk_ASN1_UTF8STRING_value(takey->comments, i);
> + out->comments[i] = strndup(comment->data, comment->length);
> + if (out->comments[i] == NULL)
> + err(1, NULL);
> + }
> +
> + out->urisz = sk_ASN1_IA5STRING_num(takey->certificateURIs);
> + if (out->urisz == 0) {
> + warnx("%s: Signed TAL requires at least 1 CertificateURI", fn);
> + goto out;
> + }
> + if ((out->uris = calloc(out->urisz, sizeof(char *))) == NULL)
> + err(1, NULL);
> +
> + for (i = 0; i < out->urisz; i++) {
> + certURI = sk_ASN1_IA5STRING_value(takey->certificateURIs, i);
as you mentioned, the uris should be validated - fine to leave this
as a followup.
> + out->uris[i] = strndup(certURI->data, certURI->length);
> + if (out->uris[i] == NULL)
> + err(1, NULL);
> + }
> +
> + if ((pkey = X509_PUBKEY_get0(takey->subjectPublicKeyInfo)) == NULL) {
> + warnx("%s: X509_PUBKEY_get0 failed", fn);
> + goto out;
> + }
> +
> + if ((r = EVP_PKEY_get0_RSA(pkey)) == NULL) {
> + warnx("%s: EVP_PKEY_get0_RSA failed", fn);
> + goto out;
> + }
> +
> + if ((rdersz = i2d_RSAPublicKey(r, &rder)) <= 0) {
> + warnx("%s: i2d_RSAPublicKey failed", fn);
> + goto out;
> + }
> +
> + if (!EVP_Digest(rder, rdersz, md, NULL, EVP_sha1(), NULL)) {
> + warnx("%s: EVP_Digest failed", fn);
> + goto out;
> + }
> + out->ski = hex_encode(md, SHA_DIGEST_LENGTH);
> +
> + if ((out->pubkeysz = i2d_PUBKEY(pkey, &der)) <= 0) {
> + warnx("%s: i2d_PUBKEY failed", fn);
> + goto out;
> + }
> +
> + out->pubkey = der;
> + der = NULL;
> +
> + rc = 1;
> + out:
> + free(der);
> + free(rder);
> + return rc;
> +}
> +
> +/*
> + * Parses the eContent segment of an TAK file
> + * Returns zero on failure, non-zero on success.
> + */
> +static int
> +tak_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p)
> +{
> + TAK *tak;
> + const char *fn;
> + int rc = 0;
> +
> + fn = p->fn;
> +
> + if ((tak = d2i_TAK(NULL, &d, dsz)) == NULL) {
> + cryptowarnx("%s: failed to parse Trust Anchor Key", fn);
> + goto out;
> + }
> +
> + if (!valid_econtent_version(fn, tak->version))
> + goto out;
> +
> + p->res->current = calloc(1, sizeof(struct takey));
> + if (p->res->current == NULL)
> + err(1, NULL);
> +
> + if (!parse_takey(fn, tak->current, p->res->current)) {
> + warnx("%s: invalid current TAKey", fn);
> + goto out;
> + }
> +
> + if (tak->predecessor != NULL) {
> + p->res->predecessor = calloc(1, sizeof(struct takey));
> + if (p->res->predecessor == NULL)
> + err(1, NULL);
> +
> + if (!parse_takey(fn, tak->predecessor, p->res->predecessor)) {
> + warnx("%s: invalid predecessor TAKey", fn);
> + goto out;
> + }
> + }
> +
> + if (tak->successor != NULL) {
> + p->res->successor = calloc(1, sizeof(struct takey));
> + if (p->res->successor == NULL)
> + err(1, NULL);
> +
> + if (!parse_takey(fn, tak->successor, p->res->successor)) {
> + warnx("%s: invalid successor TAKey", fn);
> + goto out;
> + }
> + }
> +
> + rc = 1;
> + out:
> + TAK_free(tak);
> + return rc;
> +}
> +
> +/*
> + * Parse a full draft-ietf-sidrops-signed-tal file.
> + * Returns the TAK or NULL if the object was malformed.
> + */
> +struct tak *
> +tak_parse(X509 **x509, const char *fn, const unsigned char *der, size_t len)
> +{
> + struct parse p;
> + unsigned char *cms;
> + size_t cmsz;
> + const ASN1_TIME *at;
> + struct cert *cert = NULL;
cert isn't used
> + int rc = 0;
> +
> + memset(&p, 0, sizeof(struct parse));
> + p.fn = fn;
> +
> + cms = cms_parse_validate(x509, fn, der, len, tak_oid, &cmsz);
> + if (cms == NULL)
> + return NULL;
> +
> + if ((p.res = calloc(1, sizeof(struct tak))) == 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;
> + }
> + if (x509_get_time(at, &p.res->expires) == -1) {
> + warnx("%s: ASN1_time_parse failed", fn);
> + goto out;
> + }
> +
> + if (!x509_inherits(*x509)) {
> + warnx("%s: RFC 3779 extension not set to inherit", fn);
> + goto out;
> + }
> +
> + if (!tak_parse_econtent(cms, cmsz, &p))
> + goto out;
> +
> + if (strncmp(p.res->aki, p.res->current->ski, strlen(p.res->aki)) != 0) {
I think this should be plain strcmp(), otherwise we'd ignore trailing garbage
in p.res->current->ski.
> + warnx("%s: current TAKey's SKI does not match EE AKI", fn);
> + goto out;
> + }
> +
> + rc = 1;
> + out:
> + if (rc == 0) {
> + tak_free(p.res);
> + p.res = NULL;
> + X509_free(*x509);
> + *x509 = NULL;
> + }
> + cert_free(cert);
> + free(cms);
> + return p.res;
> +}
> +
> +/*
> + * Free TAKey pointer.
> + */
> +static void
> +takey_free(struct takey *t)
> +{
> + size_t i;
> +
> + if (t == NULL)
> + return;
> +
> + for (i = 0; i < t->commentsz; i++)
> + free(t->comments[i]);
> +
> + for (i = 0; i < t->urisz; i++)
> + free(t->uris[i]);
> +
> + free(t->comments);
> + free(t->uris);
> + free(t->ski);
> + free(t->pubkey);
> + free(t);
> +}
> +
> +/*
> + * Free an TAK pointer.
> + * Safe to call with NULL.
> + */
> +void
> +tak_free(struct tak *t)
> +{
> + if (t == NULL)
> + return;
> +
> + takey_free(t->current);
> + takey_free(t->predecessor);
> + takey_free(t->successor);
> +
> + free(t->aia);
> + free(t->aki);
> + free(t->ski);
> + free(t);
> +}
> Index: x509.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/x509.c,v
> retrieving revision 1.50
> diff -u -p -r1.50 x509.c
> --- x509.c 3 Sep 2022 14:40:09 -0000 1.50
> +++ x509.c 21 Oct 2022 13:07:30 -0000
> @@ -45,6 +45,7 @@ ASN1_OBJECT *sign_time_oid; /* pkcs-9 id
> ASN1_OBJECT *bin_sign_time_oid; /* pkcs-9 id-aa-binarySigningTime */
> ASN1_OBJECT *rsc_oid; /* id-ct-signedChecklist */
> ASN1_OBJECT *aspa_oid; /* id-ct-ASPA */
> +ASN1_OBJECT *tak_oid; /* id-ct-SignedTAL */
>
> void
> x509_init_oid(void)
> @@ -85,6 +86,10 @@ x509_init_oid(void)
> if ((aspa_oid = OBJ_txt2obj("1.2.840.113549.1.9.16.1.49", 1)) == NULL)
> errx(1, "OBJ_txt2obj for %s failed",
> "1.2.840.113549.1.9.16.1.49");
> + if ((tak_oid = OBJ_txt2obj("1.2.840.113549.1.9.16.1.50", 1)) == NULL)
> + errx(1, "OBJ_txt2obj for %s failed",
> + "1.2.840.113549.1.9.16.1.50");
> +
After x509.c r1.51 this will need a new table entry. Use the diff below
instead.
> }
>
> /*
>
Index: x509.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/x509.c,v
retrieving revision 1.51
diff -u -p -r1.51 x509.c
--- x509.c 24 Oct 2022 10:26:59 -0000 1.51
+++ x509.c 1 Nov 2022 09:49:55 -0000
@@ -45,6 +45,7 @@ ASN1_OBJECT *sign_time_oid; /* pkcs-9 id
ASN1_OBJECT *bin_sign_time_oid; /* pkcs-9 id-aa-binarySigningTime */
ASN1_OBJECT *rsc_oid; /* id-ct-signedChecklist */
ASN1_OBJECT *aspa_oid; /* id-ct-ASPA */
+ASN1_OBJECT *tak_oid; /* id-ct-SignedTAL */
static const struct {
const char *oid;
@@ -105,6 +106,10 @@ static const struct {
{
.oid = "1.2.840.113549.1.9.16.1.49",
.ptr = &aspa_oid,
+ },
+ {
+ .oid = "1.2.840.113549.1.9.16.1.50",
+ .ptr = &tak_oid,
},
};