On Tue, Aug 19, 2025 at 10:08:53AM +0000, Job Snijders wrote:
> For upcoming work the type definitions need to be accessible by multiple
> translation units. The order of rpki-asn1.h is alphabetical. While
> there, update references.

Not a huge fan of all the doubled empty lines, but since you did that
consistently, I can live with them.

I'm generally ok with this, but see nits inline and a d a diff to go
on top of yours at the end.

> 
> Index: aspa.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/aspa.c,v
> diff -u -p -r1.37 aspa.c
> --- aspa.c	1 Aug 2025 14:57:15 -0000	1.37
> +++ aspa.c	19 Aug 2025 10:05:16 -0000
> @@ -33,25 +33,19 @@
>  #include "extern.h"
>  
>  /*
> - * Types and templates for ASPA eContent draft-ietf-sidrops-aspa-profile-15
> + * ASPA eContent definition in draft-ietf-sidrops-aspa-profile-20.

I'd add ", section 3." at the end.

> Index: extern.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
> diff -u -p -r1.257 extern.h
> --- extern.h	14 Aug 2025 15:12:00 -0000	1.257
> +++ extern.h	19 Aug 2025 10:05:16 -0000
> @@ -24,6 +24,8 @@
>  #include <openssl/x509.h>
>  #include <openssl/x509v3.h>
>  
> +#include "rpki-asn1.h"

I think we're better off including this in the files that actually need it.
See diff at the end.

> +
>  #define CTASSERT(x)	extern char  _ctassert[(x) ? 1 : -1 ] \
>  			    __attribute__((__unused__))
>  
> Index: mft.c
> Index: roa.c
> Index: rpki-asn1.h
> ===================================================================
> RCS file: rpki-asn1.h
> diff -N rpki-asn1.h
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ rpki-asn1.h	19 Aug 2025 10:05:16 -0000
> @@ -0,0 +1,238 @@
> +/* $OpenBSD$ */
> +/*
> + * Copyright (c) 2025 Job Snijders <job@openbsd.org>
> + * Copyright (c) 2025 Theo Buehler <tb@openbsd.org>
> + *
> + * 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.
> + */
> +
> +#ifndef RPKI_ASN1_H
> +#define RPKI_ASN1_H
> +
> +#include <unistd.h>
> +
> +#include <openssl/asn1.h>
> +#include <openssl/asn1t.h>
> +
> +/*
> + * Autonomous System Provider Authorization (ASPA)

Consider adding a reference here (and in all other similar comments).

> Index: rsc.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/rsc.c,v
> diff -u -p -r1.40 rsc.c
> --- rsc.c	1 Aug 2025 14:57:15 -0000	1.40
> +++ rsc.c	19 Aug 2025 10:05:16 -0000
> @@ -33,7 +33,7 @@
>  #include "extern.h"
>  
>  /*
> - * Types and templates for RSC eContent - RFC 9323
> + * RSC eContent definition in RFC 9323 section 4.

missing comma

> Index: spl.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/spl.c,v
> diff -u -p -r1.13 spl.c
> --- spl.c	1 Aug 2025 14:57:15 -0000	1.13
> +++ spl.c	19 Aug 2025 10:05:16 -0000
> @@ -34,51 +34,26 @@
>  #include "extern.h"
>  
>  /*
> - * Types and templates for the SPL eContent.
> + * SPL eContent definition in draft-ietf-sidrops-rpki-prefixlist-04 section 3.

ditto

> Index: tak.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/tak.c,v
> diff -u -p -r1.26 tak.c
> --- tak.c	1 Aug 2025 14:57:15 -0000	1.26
> +++ tak.c	19 Aug 2025 10:05:16 -0000
> @@ -34,32 +34,12 @@
>  #include "extern.h"
>  
>  /*
> - * ASN.1 templates for Trust Anchor Keys (RFC 9691)
> + * TAK eContent definition in RFC 9691 section 2.

ditto

I'd probably refer to Appendix A since unlike all other RFCs this one doesn't
inline the ASN.1 definition in the main body of the text.

As mentioned, I'd prefer not to include rpki-asn1.h from extern.h.
Regress needs no change (except the header guards for openssl/unistd.h
which you already have in your tree).

diff --git a/usr.sbin/rpki-client/aspa.c b/usr.sbin/rpki-client/aspa.c
index c4a4676b03..1cacf35511 100644
--- a/usr.sbin/rpki-client/aspa.c
+++ b/usr.sbin/rpki-client/aspa.c
@@ -31,6 +31,7 @@
 #include <openssl/x509.h>
 
 #include "extern.h"
+#include "rpki-asn1.h"
 
 /*
  * ASPA eContent definition in draft-ietf-sidrops-aspa-profile-20.
diff --git a/usr.sbin/rpki-client/extern.h b/usr.sbin/rpki-client/extern.h
index b8de7cd426..721196fb56 100644
--- a/usr.sbin/rpki-client/extern.h
+++ b/usr.sbin/rpki-client/extern.h
@@ -24,8 +24,6 @@
 #include <openssl/x509.h>
 #include <openssl/x509v3.h>
 
-#include "rpki-asn1.h"
-
 #define CTASSERT(x)	extern char  _ctassert[(x) ? 1 : -1 ] \
 			    __attribute__((__unused__))
 
diff --git a/usr.sbin/rpki-client/mft.c b/usr.sbin/rpki-client/mft.c
index c1f75096b6..58d2333aca 100644
--- a/usr.sbin/rpki-client/mft.c
+++ b/usr.sbin/rpki-client/mft.c
@@ -33,6 +33,7 @@
 #include <openssl/x509.h>
 
 #include "extern.h"
+#include "rpki-asn1.h"
 
 /*
  * Manifest eContent definition in RFC 9286, section 4.2.
diff --git a/usr.sbin/rpki-client/roa.c b/usr.sbin/rpki-client/roa.c
index 6e01838356..081a9b83fd 100644
--- a/usr.sbin/rpki-client/roa.c
+++ b/usr.sbin/rpki-client/roa.c
@@ -30,6 +30,7 @@
 #include <openssl/x509.h>
 
 #include "extern.h"
+#include "rpki-asn1.h"
 
 /*
  * ROA eContent definition in RFC 9582, section 4.
diff --git a/usr.sbin/rpki-client/rsc.c b/usr.sbin/rpki-client/rsc.c
index a7d4bf8172..9bd1e1737f 100644
--- a/usr.sbin/rpki-client/rsc.c
+++ b/usr.sbin/rpki-client/rsc.c
@@ -31,6 +31,7 @@
 #include <openssl/x509v3.h>
 
 #include "extern.h"
+#include "rpki-asn1.h"
 
 /*
  * RSC eContent definition in RFC 9323 section 4.
diff --git a/usr.sbin/rpki-client/spl.c b/usr.sbin/rpki-client/spl.c
index 0d9f239d20..3c1125f89b 100644
--- a/usr.sbin/rpki-client/spl.c
+++ b/usr.sbin/rpki-client/spl.c
@@ -32,6 +32,7 @@
 #include <openssl/x509v3.h>
 
 #include "extern.h"
+#include "rpki-asn1.h"
 
 /*
  * SPL eContent definition in draft-ietf-sidrops-rpki-prefixlist-04 section 3.
diff --git a/usr.sbin/rpki-client/tak.c b/usr.sbin/rpki-client/tak.c
index ce06901ace..d271bf1c2b 100644
--- a/usr.sbin/rpki-client/tak.c
+++ b/usr.sbin/rpki-client/tak.c
@@ -32,6 +32,7 @@
 #include <openssl/x509v3.h>
 
 #include "extern.h"
+#include "rpki-asn1.h"
 
 /*
  * TAK eContent definition in RFC 9691 section 2.