On Thu, Aug 25, 2022 at 04:04:27PM +0000, Job Snijders wrote:
> On Thu, Aug 25, 2022 at 03:38:45PM +0200, Claudio Jeker wrote:
> > I wonder why is PEM printing not part of -f? It seems to be something
> > that should be part of filemode.
>
> OK, how about this?
That's a lot better. However X509_print() generates much visual noise
and hides the things I'm primarily interested in. I like the current
concise output of -f mode.
Can we put the X509_print() behind the -v flag and keep the
PEM_write_bio_X509() behind -p?
More comments inline.
>
> Kind regards,
>
> Job
>
> Index: filemode.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/filemode.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 filemode.c
> --- filemode.c 25 Aug 2022 11:07:28 -0000 1.9
> +++ filemode.c 25 Aug 2022 16:01:16 -0000
> @@ -32,13 +32,17 @@
> #include <imsg.h>
>
> #include <openssl/asn1.h>
> +#include <openssl/bio.h>
> #include <openssl/err.h>
> #include <openssl/evp.h>
> +#include <openssl/pem.h>
> #include <openssl/x509.h>
> #include <openssl/x509v3.h>
>
> #include "extern.h"
>
> +extern int printpem;
> +
> static X509_STORE_CTX *ctx;
> static struct auth_tree auths = RB_INITIALIZER(&auths);
> static struct crl_tree crlt = RB_INITIALIZER(&crlt);
> @@ -258,6 +262,7 @@ proc_parser_file(char *file, unsigned ch
> {
> static int num;
> X509 *x509 = NULL;
> + BIO *bio_out = NULL;
> struct cert *cert = NULL;
> struct crl *crl = NULL;
> struct mft *mft = NULL;
> @@ -421,9 +426,24 @@ proc_parser_file(char *file, unsigned ch
>
> if (outformats & FORMAT_JSON)
> printf("\"\n}");
Unrelated to your diff, but I think this lacks an \n after }.
If you agree, ok tb for that change alone.
> - else
> + else {
> printf("\n");
>
> + if (type == RTYPE_TAL || type == RTYPE_CRL)
> + goto out;
x509 can be NULL here (e.g., when mft_parse() fails), so this needs an
if (x509 == NULL)
goto out;
to avoid a crash.
> +
> + if ((bio_out = BIO_new_fp(stdout, BIO_NOCLOSE)) == NULL)
> + errx(1, "BIO_new_fp");
> +
> + if (!X509_print(bio_out, x509))
> + errx(1, "X509_print");
There is actually no need for a BIO:
if (!X509_print_fp(stdout, x509))
errx(1, "X509_print_fp");
> +
> + if (printpem)
> + if (!PEM_write_bio_X509(bio_out, x509))
Similarly here:
if (!PEM_write_X509(stdout, x509))
> + errx(1, "PEM_write_bio_X509");
> + }
> +
> + out:
> X509_free(x509);
> cert_free(cert);
> crl_free(crl);
> @@ -432,6 +452,7 @@ proc_parser_file(char *file, unsigned ch
> gbr_free(gbr);
> tal_free(tal);
> rsc_free(rsc);
> + BIO_free(bio_out);
> }
>
> /*
> 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 25 Aug 2022 16:01:16 -0000
> @@ -64,6 +64,7 @@ const char *bird_tablename = "ROAS";
> int verbose;
> int noop;
> int filemode;
> +int printpem;
> 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':
> + printpem = 1;
> + break;
> case 'R':
> rrdpon = 0;
> break;
> @@ -888,6 +892,9 @@ main(int argc, char *argv[])
> argv += optind;
> argc -= optind;
>
> + if ((!filemode && printpem) || (printpem && outformats))
> + goto usage;
Let's take care of this in a separate diff. I think we need to do more
than just that.
> +
> if (!filemode) {
> if (argc == 1)
> outputdir = argv[0];
> @@ -1278,6 +1285,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] [-j | -p] [-t tal] -f file"
> + " ...\n");
> return 1;
> }
> 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 25 Aug 2022 16:01:16 -0000
> @@ -34,6 +34,7 @@
> .Nm
> .Op Fl Vv
> .Op Fl d Ar cachedir
> +.Op Fl j | p
> .Op Fl t Ar tal
> .Fl f
> .Ar
> @@ -144,6 +145,8 @@ If the
> and
> .Fl j
> options are not specified this is the default.
> +.It Fl p
> +Print the encapsulated X.509 certificate in PEM format.
> .It Fl R
> Synchronize via RSYNC only.
> .It Fl r
>
