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; >