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,
        },
 };
 

Reply via email to