On Wed, Aug 24, 2022 at 11:35:01PM +0000, Job Snijders wrote:
> Hi all,
>
> Scratching an itch: When debugging RPKI things, I've grown tired of
> typing stuff like the below 2 commands to get to the CMS encapsulated
> DER encoded EE certificate in RPKI Signed Objects.
>
> $ openssl cms -verify -noverify -inform DER -signer signer.pem \
> -in OOFPkv3HzPv8GCNhUjrifWl-lS8.mft > /dev/zero
> $ openssl x509 -in signer.pem -text
>
> Life is too short to type that many letters - instead, I'd rather:
>
> $ rpki-client -p OOFPkv3HzPv8GCNhUjrifWl-lS8.mft
>
> For completeness' sake, also print all other (not-CMS encapsulated)
> file types such as CA certs, BGPsec Router Keys, and CRLs.
Kind of makes sense to me. Instead of adding another mode, could this be
done as a -p flag in file mode (or even be hidden behind the -v flag)?
Or is this too ugly due to repeated info?
Some comments inline.
>
> OK?
>
> Kind regards,
>
> Job
>
> Index: extern.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
> retrieving revision 1.150
> diff -u -p -r1.150 extern.h
> --- extern.h 19 Aug 2022 12:45:53 -0000 1.150
> +++ extern.h 24 Aug 2022 23:33:24 -0000
> @@ -664,6 +664,7 @@ void mft_print(const X509 *, const str
> void roa_print(const X509 *, const struct roa *);
> void gbr_print(const X509 *, const struct gbr *);
> void rsc_print(const X509 *, const struct rsc *);
> +int print_pem(char *);
This should probably be pem_print() to match all the other print functions.
Any reason it can't take a const char *?
>
> /* Output! */
>
> Index: main.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
> retrieving revision 1.209
> diff -u -p -r1.209 main.c
> --- main.c 4 Aug 2022 13:44:07 -0000 1.209
> +++ main.c 24 Aug 2022 23:33:25 -0000
> @@ -64,6 +64,7 @@ const char *bird_tablename = "ROAS";
> int verbose;
> int noop;
> int filemode;
> +int pemmode;
> int rrdpon = 1;
> int repo_timeout;
>
> @@ -819,7 +820,7 @@ main(int argc, char *argv[])
> "proc exec unveil", NULL) == -1)
> err(1, "pledge");
>
> - while ((c = getopt(argc, argv, "b:Bcd:e:fjnorRs:S:t:T:vV")) != -1)
> + while ((c = getopt(argc, argv, "b:Bcd:e:fjnoprRs:S:t:T:vV")) != -1)
> switch (c) {
> case 'b':
> bind_addr = optarg;
> @@ -849,6 +850,9 @@ main(int argc, char *argv[])
> case 'o':
> outformats |= FORMAT_OPENBGPD;
> break;
> + case 'p':
> + pemmode = 1;
> + break;
> case 'R':
> rrdpon = 0;
> break;
> @@ -888,6 +892,17 @@ main(int argc, char *argv[])
> argv += optind;
> argc -= optind;
>
> + if (pemmode) {
> + if (pledge("stdio rpath", NULL) == -1)
> + err(1, "pledge");
> +
> + if (argc > 1)
> + goto usage;
This should be argc > 0 to match your synopsis.
Or should this behave like file mode and accept an arbitrary number of
file arguments? For example you could drop the argc check and do:
for (i = 0; i < argc; i++) {
if ((rc = print_pem(argv[i])) != 0)
return rc
}
> +
> + rc = print_pem(argv[0]);
> + return rc;
> + }
> +
> if (!filemode) {
> if (argc == 1)
> outputdir = argv[0];
> @@ -1278,6 +1293,7 @@ usage:
> " [-e rsync_prog]\n"
> " [-S skiplist] [-s timeout] [-T table] [-t tal]"
> " [outputdir]\n"
> - " rpki-client [-Vv] [-d cachedir] [-t tal] -f file ...\n");
> + " rpki-client [-Vv] [-d cachedir] [-t tal] -f file ...\n"
> + " rpki-client -p file\n");
> return 1;
> }
> Index: print.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/print.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 print.c
> --- print.c 14 Jul 2022 13:24:56 -0000 1.14
> +++ print.c 24 Aug 2022 23:33:25 -0000
> @@ -1,5 +1,6 @@
> /* $OpenBSD: print.c,v 1.14 2022/07/14 13:24:56 job Exp $ */
> /*
> + * Copyright (c) 2022 Job Snijders <[email protected]>
> * Copyright (c) 2021 Claudio Jeker <[email protected]>
> * Copyright (c) 2019 Kristaps Dzonsons <[email protected]>
> *
> @@ -26,6 +27,8 @@
> #include <time.h>
>
> #include <openssl/evp.h>
> +#include <openssl/pem.h>
> +#include <openssl/x509.h>
>
> #include "extern.h"
>
> @@ -567,4 +570,100 @@ rsc_print(const X509 *x, const struct rs
>
> if (outformats & FORMAT_JSON)
> printf("\t],\n");
> +}
> +
> +/*
> + * Read a file, extract the encapsulated X509 cert and print in PEM format.
> + * Return zero on success, non-zero on failure.
> + */
> +int
> +print_pem(char *fn)
> +{
> + BIO *bio_out = NULL;
> + X509 *x = NULL;
> + X509_CRL *c = NULL;
It's not 100% consistent, but most rpki-client code aligns variables:
BIO *bio_out = NULL;
X509 *x = NULL:
etc.
> + struct gbr *gbr = NULL;
> + struct mft *mft = NULL;
> + struct roa *roa = NULL;
> + struct rsc *rsc = NULL;
> + unsigned char *buf;
Initialize buf to NULL here.
> + size_t len;
> + enum rtype type;
> + int rc = 1;
> +
> + x509_init_oid();
> +
> + type = rtype_from_file_extension(fn);
> + if (type == RTYPE_INVALID) {
> + buf = NULL;
Drop buf = NULL here.
> + warnx("%s: unsupported file type", fn);
> + goto out;
> + }
> +
> + if ((buf = load_file(fn, &len)) == NULL) {
> + warnx("load_file failed");
> + goto out;
> + }
> +
> + if ((bio_out = BIO_new_fp(stdout, BIO_NOCLOSE)) == NULL)
> + errx(1, "BIO_new_fp");
> +
> + switch (type) {
> + case RTYPE_CER:
> + if ((x = d2i_X509(NULL, (const unsigned char **)&buf, len))
> + == NULL) {
d2i functions have a trap: they advance the buf pointer you pass in past
what they parse (so that you can pass buf to the next d2i function if
necessary). That's why you got a crash due to a bogus free at the end
for CER and CRL files.
The right way to handle this is to add "const unsigned char *der;" next
to buf at the top and do
der = buf;
if ((x = d2i_X509(NULL, &der, len)) == NULL) {
then you can free(buf) unconditionally at the end.
> + warnx("d2i_X509 failed");
> + goto out;
> + }
> + break;
> + case RTYPE_CRL:
> + if ((c = d2i_X509_CRL(NULL, (const unsigned char **)&buf, len))
> + == NULL) {
Similarly here with der = buf.
> + warnx("d2i_X509_CRL failed");
> + goto out;
> + }
> + if (!X509_CRL_print(bio_out, c))
> + errx(1, "X509_CRL_print");
> + if (!PEM_write_bio_X509_CRL(bio_out, c))
> + errx(1, "PEM_write_bio_X509_CRL");
> + rc = 0;
> + goto out;
> + case RTYPE_GBR:
> + if ((gbr = gbr_parse(&x, fn, buf, len)) == NULL)
> + goto out;
> + break;
> + case RTYPE_MFT:
> + if ((mft = mft_parse(&x, fn, buf, len)) == NULL)
> + goto out;
> + break;
> + case RTYPE_ROA:
> + if ((roa = roa_parse(&x, fn, buf, len)) == NULL)
> + goto out;
> + break;
> + case RTYPE_RSC:
> + if ((rsc = rsc_parse(&x, fn, buf, len)) == NULL)
> + goto out;
> + break;
> + default:
> + errx(1, "%s: unhandled", fn);
> + }
> +
> + if (!X509_print(bio_out, x))
> + errx(1, "X509_print");
> +
> + if (!PEM_write_bio_X509(bio_out, x))
> + errx(1, "PEM_write_bio_X509");
> +
> + rc = 0;
> + out:
> + BIO_free(bio_out);
> + if (type != RTYPE_CER && type != RTYPE_CRL)
> + free(buf);
buf should be freed unconditionally.
> + X509_free(x);
> + X509_CRL_free(c);
> + gbr_free(gbr);
> + mft_free(mft);
> + roa_free(roa);
> + rsc_free(rsc);
> + return rc;
> }
> Index: rpki-client.8
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/rpki-client.8,v
> retrieving revision 1.68
> diff -u -p -r1.68 rpki-client.8
> --- rpki-client.8 30 Jun 2022 10:27:52 -0000 1.68
> +++ rpki-client.8 24 Aug 2022 23:33:25 -0000
> @@ -37,6 +37,8 @@
> .Op Fl t Ar tal
> .Fl f
> .Ar
> +.Nm
> +.Fl p Ar file
> .Sh DESCRIPTION
> The
> .Nm
> @@ -144,6 +146,12 @@ If the
> and
> .Fl j
> options are not specified this is the default.
> +.It Fl p Ar file
> +Print the encapsulated DER encoded X.509 certificate or CRL in
> +.Ar file
> +in both human-readable and
> +.Em PEM
> +format.
> .It Fl R
> Synchronize via RSYNC only.
> .It Fl r
>
