On Wed, Nov 05, 2008 at 11:51:44AM +0100, Christophe Mac? wrote:

> Hi,
> I noticed a different behaviour between v0.9.8h and v0.9.8i when
> printing dates of my certificates and crls.
> for example (I patched file crypto/asn1/t_x509.c to print value of
> tm->length after the date) :
> 
> [EMAIL PROTECTED]/usr/local/src/openssl-0.9.8i/apps/openssl x509 -in
> /etc/ssl/stunnel/serveur-cert.pem -startdate -noout
> notBefore=Oct 24 09:29:00 2008 GMT (i=10)
> 
> [EMAIL PROTECTED]/usr/local/src/openssl-0.9.8h/apps/openssl x509 -in
> /etc/ssl/stunnel/serveur-cert.pem -startdate -noout
> notBefore=Oct 24 09:29:19 2008 GMT (i=10)
> 
> Seconds are not printed in v0.9.8i, which is normal since there is a
> check if tm->length < 12 (line 432 of crypto/asn1/t_x509.c).

I see the same behaviour:

    $ /.../openssl/0.9.8h/bin/openssl x509 \
        -in /tmp/serveur-cert.pem -noout -dates
    notBefore=Oct 24 09:29:19 2008 GMT
    notAfter=Oct 24 09:29:19 2009 GMT

    $ /.../openssl/0.9.8i/bin/openssl x509 \
        -in /tmp/serveur-cert.pem -noout -dates
    notBefore=Oct 24 09:29:00 2008 GMT
    notAfter=Oct 24 09:29:00 2009 GMT

And it appears to be the result of the length check you noticed.

> I conclude that the field 'tm->length' seems not to be coherent with
> the field 'tm->data'. I cannot go further in my investigation for the
> moment.

The problem is that while "i" in the code is initialized to the ASN1
string length, it is also used as a loop control variable, clobbering the
initial value:

    int ASN1_UTCTIME_print(BIO *bp, ASN1_UTCTIME *tm)
            {
            char *v;
            int gmt=0;
            int i;
            int y=0,M=0,d=0,h=0,m=0,s=0;

            i=tm->length;
            v=(char *)tm->data;

            if (i < 10) goto err;
            if (v[i-1] == 'Z') gmt=1;
---------------------------------------------------------------
            for (i=0; i<10; i++)
                    if ((v[i] > '9') || (v[i] < '0')) goto err;
---------------------------------------------------------------
            y= (v[0]-'0')*10+(v[1]-'0');
            if (y < 50) y+=100;
            M= (v[2]-'0')*10+(v[3]-'0');
            if ((M > 12) || (M < 1)) goto err;
            d= (v[4]-'0')*10+(v[5]-'0');
            h= (v[6]-'0')*10+(v[7]-'0');
            m=  (v[8]-'0')*10+(v[9]-'0');
            if (i >=12 &&
                (v[10] >= '0') && (v[10] <= '9') &&
                (v[11] >= '0') && (v[11] <= '9'))
                    s=  (v[10]-'0')*10+(v[11]-'0');

            if (BIO_printf(bp,"%s %2d %02d:%02d:%02d %d%s",
                    mon[M-1],d,h,m,s,y+1900,(gmt)?" GMT":"") <= 0)
                    return(0);
            else
                    return(1);
    err:
            BIO_write(bp,"Bad time value",14);
            return(0);
            }

Also, the validation is sloppy, garbage (non-digits) in the year, month,
day, hour or minutes triggers an error, while garbage in the seconds
field is silently treated as "00". The correct solution is likely to
ensure that *all* characters up to an optional final "Z" are numeric,
and to not clobber "i".

I could volunteer a patch, but perhaps the OpenSSL team wants to solve
this in slightly different way.

-- 
        Viktor.
______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
User Support Mailing List                    openssl-users@openssl.org
Automated List Manager                           [EMAIL PROTECTED]

Reply via email to