On Mon, Mar 29, 2021 at 10:38:54AM +0200, Claudio Jeker wrote:
> Replace a super strange way to translate some binary blob into a hex string.
> The code drops the : from the string but this is fine, the : is just
> visual fluff.  I used the same function in the not yet finished RRDP
> codebase and there I don't want the extra ':'.

Yes, that's much better.

Another detail that changes is that the hexadecimal digits will now be
lower case.  Trading one byte overallocation for a free overflow check
is reasonable.  The explicit NUL termination out[i * 2] = '\0' is not
really needed since we calloc, but it's probably fine to be explicit.

ok tb

for the diff as it is (also for moving the function to x509.c)


> 
> Works for me.
> -- 
> :wq Claudio
> 
> Index: extern.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
> retrieving revision 1.58
> diff -u -p -r1.58 extern.h
> --- extern.h  29 Mar 2021 06:50:44 -0000      1.58
> +++ extern.h  29 Mar 2021 08:09:34 -0000
> @@ -458,6 +458,7 @@ int                output_json(FILE *, struct vrp_tre
>  
>  void logx(const char *fmt, ...)
>                   __attribute__((format(printf, 1, 2)));
> +char *bin_to_hex(const unsigned char *, size_t);
>  
>  int  mkpath(const char *);
>  
> Index: main.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
> retrieving revision 1.126
> diff -u -p -r1.126 main.c
> --- main.c    29 Mar 2021 03:45:35 -0000      1.126
> +++ main.c    29 Mar 2021 08:09:23 -0000
> @@ -830,6 +830,25 @@ repo_cleanup(void)
>       return delsz;
>  }
>  
> +char *
> +bin_to_hex(const unsigned char *in, size_t insz)
> +{
> +     const char hex[] = "0123456789abcdef";
> +     size_t i;
> +     char *out;
> +
> +     if ((out = calloc(2, insz + 1)) == NULL)
> +             err(1, NULL);
> +
> +     for (i = 0; i < insz; i++) {
> +             out[i * 2] = hex[in[i] >> 4];
> +             out[i * 2 + 1] = hex[in[i] & 0xf];
> +     }
> +     out[i * 2] = '\0';
> +
> +     return out;
> +}
> +
>  void
>  suicide(int sig __attribute__((unused)))
>  {
> Index: x509.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/x509.c,v
> retrieving revision 1.19
> diff -u -p -r1.19 x509.c
> --- x509.c    29 Mar 2021 06:50:44 -0000      1.19
> +++ x509.c    29 Mar 2021 08:28:59 -0000
> @@ -32,7 +32,7 @@
>  /*
>   * Parse X509v3 authority key identifier (AKI), RFC 6487 sec. 4.8.3.
>   * Returns the AKI or NULL if it could not be parsed.
> - * The AKI is formatted as aa:bb:cc:dd, with each being a hex value.
> + * The AKI is formatted as a hex string.
>   */
>  char *
>  x509_get_aki(X509 *x, int ta, const char *fn)
> @@ -40,8 +40,7 @@ x509_get_aki(X509 *x, int ta, const char
>       const unsigned char     *d;
>       AUTHORITY_KEYID         *akid;
>       ASN1_OCTET_STRING       *os;
> -     int                      i, dsz, crit;
> -     char                     buf[4];
> +     int                      dsz, crit;
>       char                    *res = NULL;
>  
>       akid = X509_get_ext_d2i(x, NID_authority_key_identifier, &crit, NULL);
> @@ -80,16 +79,7 @@ x509_get_aki(X509 *x, int ta, const char
>               goto out;
>       }
>  
> -     /* Make room for [hex1, hex2, ":"]*, NUL. */
> -
> -     if ((res = calloc(dsz * 3 + 1, 1)) == NULL)
> -             err(1, NULL);
> -
> -     for (i = 0; i < dsz; i++) {
> -             snprintf(buf, sizeof(buf), "%02X:", d[i]);
> -             strlcat(res, buf, dsz * 3 + 1);
> -     }
> -     res[dsz * 3 - 1] = '\0';
> +     res = bin_to_hex(d, dsz);
>  out:
>       AUTHORITY_KEYID_free(akid);
>       return res;
> @@ -98,15 +88,14 @@ out:
>  /*
>   * Parse X509v3 subject key identifier (SKI), RFC 6487 sec. 4.8.2.
>   * Returns the SKI or NULL if it could not be parsed.
> - * The SKI is formatted as aa:bb:cc:dd, with each being a hex value.
> + * The SKI is formatted as a hex string.
>   */
>  char *
>  x509_get_ski(X509 *x, const char *fn)
>  {
>       const unsigned char     *d;
>       ASN1_OCTET_STRING       *os;
> -     int                      i, dsz, crit;
> -     char                     buf[4];
> +     int                      dsz, crit;
>       char                    *res = NULL;
>  
>       os = X509_get_ext_d2i(x, NID_subject_key_identifier, &crit, NULL);
> @@ -130,16 +119,7 @@ x509_get_ski(X509 *x, const char *fn)
>               goto out;
>       }
>  
> -     /* Make room for [hex1, hex2, ":"]*, NUL. */
> -
> -     if ((res = calloc(dsz * 3 + 1, 1)) == NULL)
> -             err(1, NULL);
> -
> -     for (i = 0; i < dsz; i++) {
> -             snprintf(buf, sizeof(buf), "%02X:", d[i]);
> -             strlcat(res, buf, dsz * 3 + 1);
> -     }
> -     res[dsz * 3 - 1] = '\0';
> +     res = bin_to_hex(d, dsz);
>  out:
>       ASN1_OCTET_STRING_free(os);
>       return res;
> @@ -270,14 +250,19 @@ out:
>       return crl;
>  }
>  
> +/*
> + * Parse X509v3 authority key identifier (AKI) from the CRL.
> + * This is matched against the string from x509_get_ski() above.
> + * Returns the AKI or NULL if it could not be parsed.
> + * The AKI is formatted as a hex string.
> + */
>  char *
>  x509_crl_get_aki(X509_CRL *crl, const char *fn)
>  {
>       const unsigned char     *d;
>       AUTHORITY_KEYID         *akid;
>       ASN1_OCTET_STRING       *os;
> -     int                      i, dsz, crit;
> -     char                     buf[4];
> +     int                      dsz, crit;
>       char                    *res = NULL;
>  
>       akid = X509_CRL_get_ext_d2i(crl, NID_authority_key_identifier, &crit,
> @@ -315,16 +300,7 @@ x509_crl_get_aki(X509_CRL *crl, const ch
>               goto out;
>       }
>  
> -     /* Make room for [hex1, hex2, ":"]*, NUL. */
> -
> -     if ((res = calloc(dsz * 3 + 1, 1)) == NULL)
> -             err(1, NULL);
> -
> -     for (i = 0; i < dsz; i++) {
> -             snprintf(buf, sizeof(buf), "%02X:", d[i]);
> -             strlcat(res, buf, dsz * 3 + 1);
> -     }
> -     res[dsz * 3 - 1] = '\0';
> +     res = bin_to_hex(d, dsz);
>  out:
>       AUTHORITY_KEYID_free(akid);
>       return res;
> 

Reply via email to