Re: more rpki-client refactor

2022-05-11 Thread Theo Buehler
On Wed, May 11, 2022 at 06:54:32PM +0200, Claudio Jeker wrote:
> I took the liberty and refactored the sbgp_assysnum() code a bit more.
> 
> Main goal is to replace the reallocarray() in append_as() with an upfront
> calloc() call since now the size is known. Also I decided to collaps
> sbgp_asnum() into sbgp_assysnum().
> 

Cool. As you can imagine, this quite close to some diffs I have in my
tree :)

I haven't thought about getting rid of the reallocarray() yet. I like
this approach a lot.

ok tb

> One could also inline the now very simple append_as() but I guess the
> compiler already does that for us.

I also considered removing the overlap and other checks. At this point
we know that the extension is in canonical form thanks to the early
caching (OpenSSL would need an extra X509v3_asid_is_canonical() call).

If the RFC 3779 ideas in the RSC draft materialize, everything after
the successful X509V3_EXT_d2i() call until the end should end up in a
function of the form

int
sbgp_parse_asidentifiers(const char *fn, struct cert_as **as, size_t *asz,
const ASIdentifiers *asidentifiers)

(maybe no const so that it can free asidentifiers itself).

This way we can call it from the cert and the RSC side and get rid of
all the XXX. Assuming we do the same IPAddrBlocks, of course.



more rpki-client refactor

2022-05-11 Thread Claudio Jeker
I took the liberty and refactored the sbgp_assysnum() code a bit more.

Main goal is to replace the reallocarray() in append_as() with an upfront
calloc() call since now the size is known. Also I decided to collaps
sbgp_asnum() into sbgp_assysnum().

One could also inline the now very simple append_as() but I guess the
compiler already does that for us.

-- 
:wq Claudio

Index: cert.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
retrieving revision 1.78
diff -u -p -r1.78 cert.c
--- cert.c  11 May 2022 16:13:05 -  1.78
+++ cert.c  11 May 2022 16:49:03 -
@@ -84,12 +84,6 @@ append_as(struct parse *p, const struct 
 {
if (!as_check_overlap(as, p->fn, p->res->as, p->res->asz))
return 0;
-   if (p->res->asz >= MAX_AS_SIZE)
-   return 0;
-   p->res->as = reallocarray(p->res->as, p->res->asz + 1,
-   sizeof(struct cert_as));
-   if (p->res->as == NULL)
-   err(1, NULL);
p->res->as[p->res->asz++] = *as;
return 1;
 }
@@ -156,51 +150,15 @@ sbgp_asid(struct parse *p, const ASN1_IN
return append_as(p, );
 }
 
-/*
- * Parse one of RFC 3779 3.2.3.2.
- * Returns zero on failure, non-zero on success.
- */
 static int
-sbgp_asnum(struct parse *p, const ASIdentifierChoice *aic)
+sbgp_asinherit(struct parse *p)
 {
-   struct cert_as   as;
-   const ASIdOrRanges  *aors = NULL;
-   const ASIdOrRange   *aor;
-   int  i;
-
-   switch (aic->type) {
-   case ASIdentifierChoice_inherit:
-   memset(, 0, sizeof(struct cert_as));
-   as.type = CERT_AS_INHERIT;
-   return append_as(p, );
-   case ASIdentifierChoice_asIdsOrRanges:
-   aors = aic->u.asIdsOrRanges;
-   break;
-   default:
-   warnx("%s: RFC 3779 section 3.2.3.2: ASIdentifierChoice: "
-   "unknown type %d", p->fn, aic->type);
-   return 0;
-   }
+   struct cert_as as;
 
-   for (i = 0; i < sk_ASIdOrRange_num(aors); i++) {
-   aor = sk_ASIdOrRange_value(aors, i);
-   switch (aor->type) {
-   case ASIdOrRange_id:
-   if (!sbgp_asid(p, aor->u.id))
-   return 0;
-   break;
-   case ASIdOrRange_range:
-   if (!sbgp_asrange(p, aor->u.range))
-   return 0;
-   break;
-   default:
-   warnx("%s: RFC 3779 section 3.2.3.5: ASIdOrRange: "
-   "unknown type %d", p->fn, aor->type);
-   return 0;
-   }
-   }
+   memset(, 0, sizeof(struct cert_as));
+   as.type = CERT_AS_INHERIT;
 
-   return 1;
+   return append_as(p, );
 }
 
 /*
@@ -212,7 +170,9 @@ static int
 sbgp_assysnum(struct parse *p, X509_EXTENSION *ext)
 {
ASIdentifiers   *asidentifiers = NULL;
-   int  rc = 0;
+   const ASIdOrRanges  *aors = NULL;
+   size_t   asz;
+   int  i, rc = 0;
 
if (!X509_EXTENSION_get_critical(ext)) {
cryptowarnx("%s: RFC 6487 section 4.8.11: autonomousSysNum: "
@@ -238,8 +198,53 @@ sbgp_assysnum(struct parse *p, X509_EXTE
goto out;
}
 
-   if (!sbgp_asnum(p, asidentifiers->asnum))
+   switch (asidentifiers->asnum->type) {
+   case ASIdentifierChoice_inherit:
+   asz = 1;
+   break;
+   case ASIdentifierChoice_asIdsOrRanges:
+   aors = asidentifiers->asnum->u.asIdsOrRanges;
+   asz = sk_ASIdOrRange_num(aors);
+   break;
+   default:
+   warnx("%s: RFC 3779 section 3.2.3.2: ASIdentifierChoice: "
+   "unknown type %d", p->fn, asidentifiers->asnum->type);
+   goto out;
+   }
+
+   if (asz >= MAX_AS_SIZE) {
+   warnx("%s: too many AS number entries: limit %d",
+   p->fn, MAX_AS_SIZE);
goto out;
+   }
+   p->res->as = calloc(asz, sizeof(struct cert_as));
+   if (p->res->as == NULL)
+   err(1, NULL);
+
+   if (aors == NULL) {
+   if (!sbgp_asinherit(p))
+   goto out;
+   }
+
+   for (i = 0; i < sk_ASIdOrRange_num(aors); i++) {
+   const ASIdOrRange *aor;
+
+   aor = sk_ASIdOrRange_value(aors, i);
+   switch (aor->type) {
+   case ASIdOrRange_id:
+   if (!sbgp_asid(p, aor->u.id))
+   goto out;
+   break;
+   case ASIdOrRange_range:
+   if (!sbgp_asrange(p, aor->u.range))
+   goto out;
+   break;
+