On Thu, Apr 09, 2020 at 07:44:51PM +0100, Stuart Henderson wrote:
> On 2020/04/09 20:13, Theo Buehler wrote:
> > On Thu, Apr 09, 2020 at 05:56:55PM +0100, Stuart Henderson wrote:
> > > Not new - this happened somewhere between 6.5 and 6.6 - but some
> > > certificates are now showing up with bad serial numbers in "openssl x509".
> > > Example below, a few other certs are affected.
> > >
> > > From the current /etc/ssl/cert.pem, these are the ones showing the same:
> > >
> > > - Serial Number: 11806822484801597146 (0xa3da427ea4b1aeda)
> > > - Serial Number: 14541511773111788494 (0xc9cdd3e9d57d23ce)
> > > - Serial Number: 18364802974209362175 (0xfedce3010fc948ff)
> > > - Serial Number: 10572350602393338211 (0x92b888dbb08ac163)
> > > - Serial Number: 14014712776195784473 (0xc27e43044e473f19)
> > > - Serial Number: 13492815561806991280 (0xbb401c43f55e4fb0)
> > > - Serial Number: 9548242946988625984 (0x84822c5f1c62d040)
> > > - Serial Number: 15752444095811006489 (0xda9bec71f303b019)
> >
> > This is due to r1.34 of src/lib/libcrypto/asn1/a_int.c which changed
> > ASN1_INTEGER_get() to avoid undefined behavior. It now returns -1 more
> > often. Note that your examples are all larger than LONG_MAX.
> >
> > Minimal fix for this issue is to use the fallback to colon separated hex
> > digits in X509_print_ex() in case ASN1_INTEGER_get() returns an error so
> > that your example cert yields:
> >
> > Serial Number:
> > da:9b:ec:71:f3:03:b0:19
>
> Aha - this matches what OpenSSL does. I'm OK with this.
>
> > Index: asn1/t_x509.c
> > ===================================================================
> > RCS file: /var/cvs/src/lib/libcrypto/asn1/t_x509.c,v
> > retrieving revision 1.31
> > diff -u -p -r1.31 t_x509.c
> > --- asn1/t_x509.c 18 May 2018 18:23:24 -0000 1.31
> > +++ asn1/t_x509.c 9 Apr 2020 17:53:51 -0000
> > @@ -145,8 +145,10 @@ X509_print_ex(BIO *bp, X509 *x, unsigned
> > goto err;
> >
> > bs = X509_get_serialNumber(x);
> > - if (bs->length <= (int)sizeof(long)) {
> > + l = -1;
> > + if (bs->length <= (int)sizeof(long))
> > l = ASN1_INTEGER_get(bs);
> > + if (l != -1) {
> > if (bs->type == V_ASN1_NEG_INTEGER) {
> > l = -l;
> > neg = "-";
> >
I'm fine with this fix, but 1 comment.
How about doing below rather than assign -1 to 'l' for readability ?
Index: t_x509.c
===================================================================
RCS file: /cvs/src/lib/libcrypto/asn1/t_x509.c,v
retrieving revision 1.31
diff -u -p -u -p -r1.31 t_x509.c
--- t_x509.c 18 May 2018 18:23:24 -0000 1.31
+++ t_x509.c 10 Apr 2020 07:08:43 -0000
@@ -145,8 +145,8 @@ X509_print_ex(BIO *bp, X509 *x, unsigned
goto err;
bs = X509_get_serialNumber(x);
- if (bs->length <= (int)sizeof(long)) {
- l = ASN1_INTEGER_get(bs);
+ if (bs->length <= (int)sizeof(long) &&
+ (l = ASN1_INTEGER_get(bs)) != -1) {
if (bs->type == V_ASN1_NEG_INTEGER) {
l = -l;
neg = "-";
