Re: rpki-client: implement rsc-08.txt with templates
On Tue, May 31, 2022 at 04:16:20PM +0200, Claudio Jeker wrote: > On Tue, May 31, 2022 at 01:16:19PM +0200, Theo Buehler wrote: > > I chose to implement the constrained versions of the RFC 3779 types from > > the draft because the OpenSSL RFC 3779 code has static IPAddrBlocks_it, > > so we have to work around that anyway. This isn't quite minimal, but it > > avoids asymmetry between ASIdentifiers and IPAddrBlocks and it's cleaner > > than reusing as many of the available RFC 3779 types as possible (which > > also means additional checks either when walking the structs or after). > > > > The diff has three parts that build on top of each other. There is no > > overlap outside of extern.h, so it should not make the review harder. > > > > The mechanical cert.c diff adjusts some sbgp_addr_*() and sbgp_as_*() to > > remove the struct parse argument so that we can use them from rsc.c. > > > > The rsc.c diff is the tricky part: it switches to templates and uses the > > cert.c functions. rsc_parse_aslist() and rsc_parse_iplist() are similar > > to sbgp_assysnum() and sbgp_ipaddrblk(), but somewhat easier. We get > > rid of the copy-paste XXXs and the last bit of low level ASN.1 fiddling. > > > > Remove the unused ASN1_frame() and cms_econtent_version() from cms.c. > > I checked the changes outside of rsc.c and am OK with those. > I also looked at the new version of rsc.c and think it is much nicer code. > I did not test the rsc.c changes with an RSC file though. I tested with an RSC! :-) OK job@ Kind regards, Job
Re: rpki-client: implement rsc-08.txt with templates
On Tue, May 31, 2022 at 01:16:19PM +0200, Theo Buehler wrote:
> I chose to implement the constrained versions of the RFC 3779 types from
> the draft because the OpenSSL RFC 3779 code has static IPAddrBlocks_it,
> so we have to work around that anyway. This isn't quite minimal, but it
> avoids asymmetry between ASIdentifiers and IPAddrBlocks and it's cleaner
> than reusing as many of the available RFC 3779 types as possible (which
> also means additional checks either when walking the structs or after).
>
> The diff has three parts that build on top of each other. There is no
> overlap outside of extern.h, so it should not make the review harder.
>
> The mechanical cert.c diff adjusts some sbgp_addr_*() and sbgp_as_*() to
> remove the struct parse argument so that we can use them from rsc.c.
>
> The rsc.c diff is the tricky part: it switches to templates and uses the
> cert.c functions. rsc_parse_aslist() and rsc_parse_iplist() are similar
> to sbgp_assysnum() and sbgp_ipaddrblk(), but somewhat easier. We get
> rid of the copy-paste XXXs and the last bit of low level ASN.1 fiddling.
>
> Remove the unused ASN1_frame() and cms_econtent_version() from cms.c.
I checked the changes outside of rsc.c and am OK with those.
I also looked at the new version of rsc.c and think it is much nicer code.
I did not test the rsc.c changes with an RSC file though.
OK claudio@
> Index: cert.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> retrieving revision 1.82
> diff -u -p -r1.82 cert.c
> --- cert.c15 May 2022 15:00:53 - 1.82
> +++ cert.c31 May 2022 10:46:00 -
> @@ -58,11 +58,12 @@ extern ASN1_OBJECT*notify_oid;/* 1.3.6
> * Returns zero on failure (IP overlap) non-zero on success.
> */
> static int
> -append_ip(struct parse *p, const struct cert_ip *ip)
> +append_ip(const char *fn, struct cert_ip *ips, size_t *ipsz,
> +const struct cert_ip *ip)
> {
> - if (!ip_addr_check_overlap(ip, p->fn, p->res->ips, p->res->ipsz))
> + if (!ip_addr_check_overlap(ip, fn, ips, *ipsz))
> return 0;
> - p->res->ips[p->res->ipsz++] = *ip;
> + ips[(*ipsz)++] = *ip;
> return 1;
> }
>
> @@ -72,11 +73,12 @@ append_ip(struct parse *p, const struct
> * as defined by RFC 3779 section 3.3.
> */
> static int
> -append_as(struct parse *p, const struct cert_as *as)
> +append_as(const char *fn, struct cert_as *ases, size_t *asz,
> +const struct cert_as *as)
> {
> - if (!as_check_overlap(as, p->fn, p->res->as, p->res->asz))
> + if (!as_check_overlap(as, fn, ases, *asz))
> return 0;
> - p->res->as[p->res->asz++] = *as;
> + ases[(*asz)++] = *as;
> return 1;
> }
>
> @@ -84,8 +86,9 @@ append_as(struct parse *p, const struct
> * Parse a range of AS identifiers as in 3.2.3.8.
> * Returns zero on failure, non-zero on success.
> */
> -static int
> -sbgp_asrange(struct parse *p, const ASRange *range)
> +int
> +sbgp_as_range(const char *fn, struct cert_as *ases, size_t *asz,
> +const ASRange *range)
> {
> struct cert_as as;
>
> @@ -94,34 +97,35 @@ sbgp_asrange(struct parse *p, const ASRa
>
> if (!as_id_parse(range->min, )) {
> warnx("%s: RFC 3779 section 3.2.3.8 (via RFC 1930): "
> - "malformed AS identifier", p->fn);
> + "malformed AS identifier", fn);
> return 0;
> }
>
> if (!as_id_parse(range->max, )) {
> warnx("%s: RFC 3779 section 3.2.3.8 (via RFC 1930): "
> - "malformed AS identifier", p->fn);
> + "malformed AS identifier", fn);
> return 0;
> }
>
> if (as.range.max == as.range.min) {
> warnx("%s: RFC 3379 section 3.2.3.8: ASRange: "
> - "range is singular", p->fn);
> + "range is singular", fn);
> return 0;
> } else if (as.range.max < as.range.min) {
> warnx("%s: RFC 3379 section 3.2.3.8: ASRange: "
> - "range is out of order", p->fn);
> + "range is out of order", fn);
> return 0;
> }
>
> - return append_as(p, );
> + return append_as(fn, ases, asz, );
> }
>
> /*
> * Parse an entire 3.2.3.10 integer type.
> */
> -static int
> -sbgp_asid(struct parse *p, const ASN1_INTEGER *i)
> +int
> +sbgp_as_id(const char *fn, struct cert_as *ases, size_t *asz,
> +const ASN1_INTEGER *i)
> {
> struct cert_as as;
>
> @@ -130,27 +134,27 @@ sbgp_asid(struct parse *p, const ASN1_IN
>
> if (!as_id_parse(i, )) {
> warnx("%s: RFC 3779 section 3.2.3.10 (via RFC 1930): "
> - "malformed AS identifier", p->fn);
> + "malformed AS identifier", fn);
> return 0;
> }
> if (as.id == 0) {
> warnx("%s: RFC 3779 section 3.2.3.10 (via RFC 1930): "
>
rpki-client: implement rsc-08.txt with templates
I chose to implement the constrained versions of the RFC 3779 types from
the draft because the OpenSSL RFC 3779 code has static IPAddrBlocks_it,
so we have to work around that anyway. This isn't quite minimal, but it
avoids asymmetry between ASIdentifiers and IPAddrBlocks and it's cleaner
than reusing as many of the available RFC 3779 types as possible (which
also means additional checks either when walking the structs or after).
The diff has three parts that build on top of each other. There is no
overlap outside of extern.h, so it should not make the review harder.
The mechanical cert.c diff adjusts some sbgp_addr_*() and sbgp_as_*() to
remove the struct parse argument so that we can use them from rsc.c.
The rsc.c diff is the tricky part: it switches to templates and uses the
cert.c functions. rsc_parse_aslist() and rsc_parse_iplist() are similar
to sbgp_assysnum() and sbgp_ipaddrblk(), but somewhat easier. We get
rid of the copy-paste XXXs and the last bit of low level ASN.1 fiddling.
Remove the unused ASN1_frame() and cms_econtent_version() from cms.c.
Index: cert.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
retrieving revision 1.82
diff -u -p -r1.82 cert.c
--- cert.c 15 May 2022 15:00:53 - 1.82
+++ cert.c 31 May 2022 10:46:00 -
@@ -58,11 +58,12 @@ extern ASN1_OBJECT *notify_oid;/* 1.3.6
* Returns zero on failure (IP overlap) non-zero on success.
*/
static int
-append_ip(struct parse *p, const struct cert_ip *ip)
+append_ip(const char *fn, struct cert_ip *ips, size_t *ipsz,
+const struct cert_ip *ip)
{
- if (!ip_addr_check_overlap(ip, p->fn, p->res->ips, p->res->ipsz))
+ if (!ip_addr_check_overlap(ip, fn, ips, *ipsz))
return 0;
- p->res->ips[p->res->ipsz++] = *ip;
+ ips[(*ipsz)++] = *ip;
return 1;
}
@@ -72,11 +73,12 @@ append_ip(struct parse *p, const struct
* as defined by RFC 3779 section 3.3.
*/
static int
-append_as(struct parse *p, const struct cert_as *as)
+append_as(const char *fn, struct cert_as *ases, size_t *asz,
+const struct cert_as *as)
{
- if (!as_check_overlap(as, p->fn, p->res->as, p->res->asz))
+ if (!as_check_overlap(as, fn, ases, *asz))
return 0;
- p->res->as[p->res->asz++] = *as;
+ ases[(*asz)++] = *as;
return 1;
}
@@ -84,8 +86,9 @@ append_as(struct parse *p, const struct
* Parse a range of AS identifiers as in 3.2.3.8.
* Returns zero on failure, non-zero on success.
*/
-static int
-sbgp_asrange(struct parse *p, const ASRange *range)
+int
+sbgp_as_range(const char *fn, struct cert_as *ases, size_t *asz,
+const ASRange *range)
{
struct cert_as as;
@@ -94,34 +97,35 @@ sbgp_asrange(struct parse *p, const ASRa
if (!as_id_parse(range->min, )) {
warnx("%s: RFC 3779 section 3.2.3.8 (via RFC 1930): "
- "malformed AS identifier", p->fn);
+ "malformed AS identifier", fn);
return 0;
}
if (!as_id_parse(range->max, )) {
warnx("%s: RFC 3779 section 3.2.3.8 (via RFC 1930): "
- "malformed AS identifier", p->fn);
+ "malformed AS identifier", fn);
return 0;
}
if (as.range.max == as.range.min) {
warnx("%s: RFC 3379 section 3.2.3.8: ASRange: "
- "range is singular", p->fn);
+ "range is singular", fn);
return 0;
} else if (as.range.max < as.range.min) {
warnx("%s: RFC 3379 section 3.2.3.8: ASRange: "
- "range is out of order", p->fn);
+ "range is out of order", fn);
return 0;
}
- return append_as(p, );
+ return append_as(fn, ases, asz, );
}
/*
* Parse an entire 3.2.3.10 integer type.
*/
-static int
-sbgp_asid(struct parse *p, const ASN1_INTEGER *i)
+int
+sbgp_as_id(const char *fn, struct cert_as *ases, size_t *asz,
+const ASN1_INTEGER *i)
{
struct cert_as as;
@@ -130,27 +134,27 @@ sbgp_asid(struct parse *p, const ASN1_IN
if (!as_id_parse(i, )) {
warnx("%s: RFC 3779 section 3.2.3.10 (via RFC 1930): "
- "malformed AS identifier", p->fn);
+ "malformed AS identifier", fn);
return 0;
}
if (as.id == 0) {
warnx("%s: RFC 3779 section 3.2.3.10 (via RFC 1930): "
- "AS identifier zero is reserved", p->fn);
+ "AS identifier zero is reserved", fn);
return 0;
}
- return append_as(p, );
+ return append_as(fn, ases, asz, );
}
static int
-sbgp_asinherit(struct parse *p)
+sbgp_as_inherit(const char *fn, struct cert_as *ases, size_t *asz)
{
struct cert_as as;
memset(, 0, sizeof(struct
