rpki-client cleanup

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

rpki-client cleanup

Claudio Jeker
This diff reduces the number of rsync_uri_parse() calls to one.

The cert parser just checks for rsync:// and a file extension of .mft.
This is done similar to the way the notification URL is checked and is
streight forward.

Additonally all the extra check for RTYPE_MFT / RTYPE_CERT in the main
process are not needed. These checks have already been done by the parser
and the main code can assume that the cert or tal was properly parsed
(including these checks).

All in all this simplifies the code a bit further and may allow the
removal of rsync_uri_parse() at a later stage.

--
:wq Claudio

Index: cert.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
retrieving revision 1.18
diff -u -p -r1.18 cert.c
--- cert.c 12 Sep 2020 15:46:48 -0000 1.18
+++ cert.c 23 Oct 2020 13:04:29 -0000
@@ -149,7 +149,8 @@ sbgp_sia_resource_notify(struct parse *p
 
  /* Make sure it's a https:// address. */
  if (dsz <= 8 || strncasecmp(d, "https://", 8)) {
- warnx("%s: RFC8182 section 3.2: not using https schema", p->fn);
+ warnx("%s: RFC 8182 section 3.2: not using https schema",
+    p->fn);
  return 0;
  }
 
@@ -167,32 +168,28 @@ static int
 sbgp_sia_resource_mft(struct parse *p,
  const unsigned char *d, size_t dsz)
 {
- enum rtype rt;
-
  if (p->res->mft != NULL) {
  warnx("%s: RFC 6487 section 4.8.8: SIA: "
     "MFT location already specified", p->fn);
  return 0;
  }
- if ((p->res->mft = strndup((const char *)d, dsz)) == NULL)
- err(1, NULL);
 
  /* Make sure it's an MFT rsync address. */
- if (!rsync_uri_parse(NULL, NULL, NULL,
-    NULL, NULL, NULL, &rt, p->res->mft)) {
- warnx("%s: RFC 6487 section 4.8.8: SIA: "
-    "failed to parse rsync URI", p->fn);
- free(p->res->mft);
- p->res->mft = NULL;
+ if (dsz <= 8 || strncasecmp(d, "rsync://", 8)) {
+ warnx("%s: RFC 6487 section 4.8.8: not using rsync schema",
+    p->fn);
  return 0;
  }
- if (rt != RTYPE_MFT) {
- warnx("%s: RFC 6487 section 4.8.8: SIA: "
+ if (strcasecmp(d + dsz - 4, ".mft") != 0) {
+         warnx("%s: RFC 6487 section 4.8.8: SIA: "
     "invalid rsync URI suffix", p->fn);
- free(p->res->mft);
- p->res->mft = NULL;
  return 0;
  }
+
+
+ if ((p->res->mft = strndup((const char *)d, dsz)) == NULL)
+ err(1, NULL);
+
  return 1;
 }
 
Index: main.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
retrieving revision 1.83
diff -u -p -r1.83 main.c
--- main.c 11 Oct 2020 12:35:24 -0000 1.83
+++ main.c 23 Oct 2020 13:01:41 -0000
@@ -202,19 +202,6 @@ filepath_exists(char *file)
 
 RB_GENERATE(filepath_tree, filepath, entry, filepathcmp);
 
-/*
- * Resolve the media type of a resource by looking at its suffice.
- * Returns the type of RTYPE_EOF if not found.
- */
-static enum rtype
-rtype_resolve(const char *uri)
-{
- enum rtype rp;
-
- rsync_uri_parse(NULL, NULL, NULL, NULL, NULL, NULL, &rp, uri);
- return rp;
-}
-
 static void
 entity_free(struct entity *ent)
 {
@@ -580,8 +567,6 @@ queue_add_from_tal(int proc, int rsync,
  errx(1, "TAL file has no rsync:// URI");
 
  /* Look up the repository. */
- assert(rtype_resolve(uri) == RTYPE_CER);
-
  repo = repo_lookup(rsync, uri);
  nfile = repo_filename(repo, uri);
 
@@ -590,29 +575,23 @@ queue_add_from_tal(int proc, int rsync,
 }
 
 /*
- * Add a manifest (MFT) or CRL found in an X509 certificate, RFC 6487.
+ * Add a manifest (MFT) found in an X509 certificate, RFC 6487.
  */
 static void
 queue_add_from_cert(int proc, int rsync, struct entityq *q,
     const char *rsyncuri, const char *rrdpuri, size_t *eid)
 {
  char *nfile;
- enum rtype type;
  const struct repo *repo;
 
  if (rsyncuri == NULL)
  return;
- if ((type = rtype_resolve(rsyncuri)) == RTYPE_EOF)
- errx(1, "%s: unknown file type", rsyncuri);
- if (type != RTYPE_MFT)
- errx(1, "%s: invalid file type", rsyncuri);
 
  /* Look up the repository. */
-
  repo = repo_lookup(rsync, rsyncuri);
  nfile = repo_filename(repo, rsyncuri);
 
- entityq_add(proc, q, nfile, type, repo, NULL, NULL, 0, NULL, eid);
+ entityq_add(proc, q, nfile, RTYPE_MFT, repo, NULL, NULL, 0, NULL, eid);
 }
 
 /*

Reply | Threaded
Open this post in threaded view
|

Re: rpki-client cleanup

Theo Buehler-3
On Fri, Oct 23, 2020 at 03:49:23PM +0200, Claudio Jeker wrote:

> This diff reduces the number of rsync_uri_parse() calls to one.
>
> The cert parser just checks for rsync:// and a file extension of .mft.
> This is done similar to the way the notification URL is checked and is
> streight forward.
>
> Additonally all the extra check for RTYPE_MFT / RTYPE_CERT in the main
> process are not needed. These checks have already been done by the parser
> and the main code can assume that the cert or tal was properly parsed
> (including these checks).
>
> All in all this simplifies the code a bit further and may allow the
> removal of rsync_uri_parse() at a later stage.

ok tb

>
> --
> :wq Claudio
>
> Index: cert.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> retrieving revision 1.18
> diff -u -p -r1.18 cert.c
> --- cert.c 12 Sep 2020 15:46:48 -0000 1.18
> +++ cert.c 23 Oct 2020 13:04:29 -0000
> @@ -149,7 +149,8 @@ sbgp_sia_resource_notify(struct parse *p
>  
>   /* Make sure it's a https:// address. */
>   if (dsz <= 8 || strncasecmp(d, "https://", 8)) {
> - warnx("%s: RFC8182 section 3.2: not using https schema", p->fn);
> + warnx("%s: RFC 8182 section 3.2: not using https schema",
> +    p->fn);
>   return 0;
>   }
>  
> @@ -167,32 +168,28 @@ static int
>  sbgp_sia_resource_mft(struct parse *p,
>   const unsigned char *d, size_t dsz)
>  {
> - enum rtype rt;
> -
>   if (p->res->mft != NULL) {
>   warnx("%s: RFC 6487 section 4.8.8: SIA: "
>      "MFT location already specified", p->fn);
>   return 0;
>   }
> - if ((p->res->mft = strndup((const char *)d, dsz)) == NULL)
> - err(1, NULL);
>  
>   /* Make sure it's an MFT rsync address. */
> - if (!rsync_uri_parse(NULL, NULL, NULL,
> -    NULL, NULL, NULL, &rt, p->res->mft)) {
> - warnx("%s: RFC 6487 section 4.8.8: SIA: "
> -    "failed to parse rsync URI", p->fn);
> - free(p->res->mft);
> - p->res->mft = NULL;
> + if (dsz <= 8 || strncasecmp(d, "rsync://", 8)) {
> + warnx("%s: RFC 6487 section 4.8.8: not using rsync schema",
> +    p->fn);
>   return 0;
>   }
> - if (rt != RTYPE_MFT) {
> - warnx("%s: RFC 6487 section 4.8.8: SIA: "
> + if (strcasecmp(d + dsz - 4, ".mft") != 0) {
> +         warnx("%s: RFC 6487 section 4.8.8: SIA: "
>      "invalid rsync URI suffix", p->fn);
> - free(p->res->mft);
> - p->res->mft = NULL;
>   return 0;
>   }
> +
> +
> + if ((p->res->mft = strndup((const char *)d, dsz)) == NULL)
> + err(1, NULL);
> +
>   return 1;
>  }
>  
> Index: main.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
> retrieving revision 1.83
> diff -u -p -r1.83 main.c
> --- main.c 11 Oct 2020 12:35:24 -0000 1.83
> +++ main.c 23 Oct 2020 13:01:41 -0000
> @@ -202,19 +202,6 @@ filepath_exists(char *file)
>  
>  RB_GENERATE(filepath_tree, filepath, entry, filepathcmp);
>  
> -/*
> - * Resolve the media type of a resource by looking at its suffice.
> - * Returns the type of RTYPE_EOF if not found.
> - */
> -static enum rtype
> -rtype_resolve(const char *uri)
> -{
> - enum rtype rp;
> -
> - rsync_uri_parse(NULL, NULL, NULL, NULL, NULL, NULL, &rp, uri);
> - return rp;
> -}
> -
>  static void
>  entity_free(struct entity *ent)
>  {
> @@ -580,8 +567,6 @@ queue_add_from_tal(int proc, int rsync,
>   errx(1, "TAL file has no rsync:// URI");
>  
>   /* Look up the repository. */
> - assert(rtype_resolve(uri) == RTYPE_CER);
> -
>   repo = repo_lookup(rsync, uri);
>   nfile = repo_filename(repo, uri);
>  
> @@ -590,29 +575,23 @@ queue_add_from_tal(int proc, int rsync,
>  }
>  
>  /*
> - * Add a manifest (MFT) or CRL found in an X509 certificate, RFC 6487.
> + * Add a manifest (MFT) found in an X509 certificate, RFC 6487.
>   */
>  static void
>  queue_add_from_cert(int proc, int rsync, struct entityq *q,
>      const char *rsyncuri, const char *rrdpuri, size_t *eid)
>  {
>   char *nfile;
> - enum rtype type;
>   const struct repo *repo;
>  
>   if (rsyncuri == NULL)
>   return;
> - if ((type = rtype_resolve(rsyncuri)) == RTYPE_EOF)
> - errx(1, "%s: unknown file type", rsyncuri);
> - if (type != RTYPE_MFT)
> - errx(1, "%s: invalid file type", rsyncuri);
>  
>   /* Look up the repository. */
> -
>   repo = repo_lookup(rsync, rsyncuri);
>   nfile = repo_filename(repo, rsyncuri);
>  
> - entityq_add(proc, q, nfile, type, repo, NULL, NULL, 0, NULL, eid);
> + entityq_add(proc, q, nfile, RTYPE_MFT, repo, NULL, NULL, 0, NULL, eid);
>  }
>  
>  /*
>