Re: rpki-client: deserialize ASIdentifiers in libcrypto
On Tue, May 10, 2022 at 08:43:45PM +0200, Theo Buehler wrote:
> The ASIdentifiers code is a bit strangely factored presumably due to
> constraints of the low-level shoveling. I kept the coarse structure
> of the code and left some house keeping for later. The changes in
> sbgp_asrange() and sbgp_asid() should be easy.
>
> The in-tree sbgp_assysnum() is tricky to follow. The main thing to
> note is that the for loop at the end really only inspects asnum (and
> ignores rdi). Since RDI is forbidden in RFC 6487, I added a check
> and error. All the complicated gymnastics can be dropped and what
> remains is a bit stricter and what it does should be rather obvious.
>
> With this diff, ASN1_TYPE and ASN1_frame() uses are gone from cert.c.
Works for me, the result is again already much cleaner.
OK claudio@
> Index: cert.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> retrieving revision 1.74
> diff -u -p -r1.74 cert.c
> --- cert.c10 May 2022 16:43:53 - 1.74
> +++ cert.c10 May 2022 18:13:02 -
> @@ -34,13 +34,6 @@
> #include "extern.h"
>
> /*
> - * Type of ASIdentifier (RFC 3779, 3.2.3).
> - */
> -#define ASID_TYPE_ASNUM 0x00
> -#define ASID_TYPE_RDI0x01
> -#define ASID_TYPE_MAXASID_TYPE_RDI
> -
> -/*
> * A parsing sequence of a file (which may just be ).
> */
> struct parse {
> @@ -128,70 +121,36 @@ sbgp_addr(struct parse *p, struct cert_i
> * Returns zero on failure, non-zero on success.
> */
> static int
> -sbgp_asrange(struct parse *p, const unsigned char *d, size_t dsz)
> +sbgp_asrange(struct parse *p, const ASRange *range)
> {
> struct cert_as as;
> - ASN1_SEQUENCE_ANY *seq;
> - const ASN1_TYPE *t;
> - int rc = 0;
> -
> - if ((seq = d2i_ASN1_SEQUENCE_ANY(NULL, , dsz)) == NULL) {
> - cryptowarnx("%s: RFC 3779 section 3.2.3.8: ASRange: "
> - "failed ASN.1 sequence parse", p->fn);
> - goto out;
> - }
> - if (sk_ASN1_TYPE_num(seq) != 2) {
> - warnx("%s: RFC 3779 section 3.2.3.8: ASRange: "
> - "want 2 elements, have %d", p->fn,
> - sk_ASN1_TYPE_num(seq));
> - goto out;
> - }
>
> memset(, 0, sizeof(struct cert_as));
> as.type = CERT_AS_RANGE;
>
> - t = sk_ASN1_TYPE_value(seq, 0);
> - if (t->type != V_ASN1_INTEGER) {
> - warnx("%s: RFC 3779 section 3.2.3.8: ASRange: "
> - "want ASN.1 integer, have %s (NID %d)",
> - p->fn, ASN1_tag2str(t->type), t->type);
> - goto out;
> - }
> - if (!as_id_parse(t->value.integer, )) {
> + if (!as_id_parse(range->min, )) {
> warnx("%s: RFC 3779 section 3.2.3.8 (via RFC 1930): "
> "malformed AS identifier", p->fn);
> - goto out;
> + return 0;
> }
>
> - t = sk_ASN1_TYPE_value(seq, 1);
> - if (t->type != V_ASN1_INTEGER) {
> - warnx("%s: RFC 3779 section 3.2.3.8: ASRange: "
> - "want ASN.1 integer, have %s (NID %d)",
> - p->fn, ASN1_tag2str(t->type), t->type);
> - goto out;
> - }
> - if (!as_id_parse(t->value.integer, )) {
> + if (!as_id_parse(range->max, )) {
> warnx("%s: RFC 3779 section 3.2.3.8 (via RFC 1930): "
> "malformed AS identifier", p->fn);
> - goto out;
> + return 0;
> }
>
> if (as.range.max == as.range.min) {
> warnx("%s: RFC 3379 section 3.2.3.8: ASRange: "
> "range is singular", p->fn);
> - goto out;
> + 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);
> - goto out;
> + return 0;
> }
>
> - if (!append_as(p, ))
> - goto out;
> - rc = 1;
> -out:
> - sk_ASN1_TYPE_pop_free(seq, ASN1_TYPE_free);
> - return rc;
> + return append_as(p, );
> }
>
> /*
> @@ -224,80 +183,46 @@ sbgp_asid(struct parse *p, const ASN1_IN
> * Returns zero on failure, non-zero on success.
> */
> static int
> -sbgp_asnum(struct parse *p, const unsigned char *d, size_t dsz)
> +sbgp_asnum(struct parse *p, const ASIdentifierChoice *aic)
> {
> struct cert_as as;
> - ASN1_TYPE *t, *tt;
> - ASN1_SEQUENCE_ANY *seq = NULL;
> - int i, rc = 0;
> - const unsigned char *sv = d;
> -
> - /* We can either be a null (inherit) or sequence. */
> -
> - if ((t = d2i_ASN1_TYPE(NULL, , dsz)) == NULL) {
> - cryptowarnx("%s: RFC 3779 section 3.2.3.2: ASIdentifierChoice: "
> - "failed ASN.1 type parse", p->fn);
> - goto out;
> - }
> -
> -
rpki-client: deserialize ASIdentifiers in libcrypto
The ASIdentifiers code is a bit strangely factored presumably due to
constraints of the low-level shoveling. I kept the coarse structure
of the code and left some house keeping for later. The changes in
sbgp_asrange() and sbgp_asid() should be easy.
The in-tree sbgp_assysnum() is tricky to follow. The main thing to
note is that the for loop at the end really only inspects asnum (and
ignores rdi). Since RDI is forbidden in RFC 6487, I added a check
and error. All the complicated gymnastics can be dropped and what
remains is a bit stricter and what it does should be rather obvious.
With this diff, ASN1_TYPE and ASN1_frame() uses are gone from cert.c.
Index: cert.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
retrieving revision 1.74
diff -u -p -r1.74 cert.c
--- cert.c 10 May 2022 16:43:53 - 1.74
+++ cert.c 10 May 2022 18:13:02 -
@@ -34,13 +34,6 @@
#include "extern.h"
/*
- * Type of ASIdentifier (RFC 3779, 3.2.3).
- */
-#defineASID_TYPE_ASNUM 0x00
-#define ASID_TYPE_RDI 0x01
-#define ASID_TYPE_MAX ASID_TYPE_RDI
-
-/*
* A parsing sequence of a file (which may just be ).
*/
struct parse {
@@ -128,70 +121,36 @@ sbgp_addr(struct parse *p, struct cert_i
* Returns zero on failure, non-zero on success.
*/
static int
-sbgp_asrange(struct parse *p, const unsigned char *d, size_t dsz)
+sbgp_asrange(struct parse *p, const ASRange *range)
{
struct cert_as as;
- ASN1_SEQUENCE_ANY *seq;
- const ASN1_TYPE *t;
- int rc = 0;
-
- if ((seq = d2i_ASN1_SEQUENCE_ANY(NULL, , dsz)) == NULL) {
- cryptowarnx("%s: RFC 3779 section 3.2.3.8: ASRange: "
- "failed ASN.1 sequence parse", p->fn);
- goto out;
- }
- if (sk_ASN1_TYPE_num(seq) != 2) {
- warnx("%s: RFC 3779 section 3.2.3.8: ASRange: "
- "want 2 elements, have %d", p->fn,
- sk_ASN1_TYPE_num(seq));
- goto out;
- }
memset(, 0, sizeof(struct cert_as));
as.type = CERT_AS_RANGE;
- t = sk_ASN1_TYPE_value(seq, 0);
- if (t->type != V_ASN1_INTEGER) {
- warnx("%s: RFC 3779 section 3.2.3.8: ASRange: "
- "want ASN.1 integer, have %s (NID %d)",
- p->fn, ASN1_tag2str(t->type), t->type);
- goto out;
- }
- if (!as_id_parse(t->value.integer, )) {
+ if (!as_id_parse(range->min, )) {
warnx("%s: RFC 3779 section 3.2.3.8 (via RFC 1930): "
"malformed AS identifier", p->fn);
- goto out;
+ return 0;
}
- t = sk_ASN1_TYPE_value(seq, 1);
- if (t->type != V_ASN1_INTEGER) {
- warnx("%s: RFC 3779 section 3.2.3.8: ASRange: "
- "want ASN.1 integer, have %s (NID %d)",
- p->fn, ASN1_tag2str(t->type), t->type);
- goto out;
- }
- if (!as_id_parse(t->value.integer, )) {
+ if (!as_id_parse(range->max, )) {
warnx("%s: RFC 3779 section 3.2.3.8 (via RFC 1930): "
"malformed AS identifier", p->fn);
- goto out;
+ return 0;
}
if (as.range.max == as.range.min) {
warnx("%s: RFC 3379 section 3.2.3.8: ASRange: "
"range is singular", p->fn);
- goto out;
+ 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);
- goto out;
+ return 0;
}
- if (!append_as(p, ))
- goto out;
- rc = 1;
-out:
- sk_ASN1_TYPE_pop_free(seq, ASN1_TYPE_free);
- return rc;
+ return append_as(p, );
}
/*
@@ -224,80 +183,46 @@ sbgp_asid(struct parse *p, const ASN1_IN
* Returns zero on failure, non-zero on success.
*/
static int
-sbgp_asnum(struct parse *p, const unsigned char *d, size_t dsz)
+sbgp_asnum(struct parse *p, const ASIdentifierChoice *aic)
{
struct cert_as as;
- ASN1_TYPE *t, *tt;
- ASN1_SEQUENCE_ANY *seq = NULL;
- int i, rc = 0;
- const unsigned char *sv = d;
-
- /* We can either be a null (inherit) or sequence. */
-
- if ((t = d2i_ASN1_TYPE(NULL, , dsz)) == NULL) {
- cryptowarnx("%s: RFC 3779 section 3.2.3.2: ASIdentifierChoice: "
- "failed ASN.1 type parse", p->fn);
- goto out;
- }
-
- /*
-* Section 3779 3.2.3.3 is to inherit with an ASN.1 NULL type,
-* which is the easy case.
-*/
+ const ASIdOrRanges *aors = NULL;
+ const ASIdOrRange *aor;
+ int i;
- switch
