Commit 80770da3 authored by Emilia Kasper's avatar Emilia Kasper
Browse files

X509 time: tighten validation per RFC 5280 



- Reject fractional seconds
- Reject offsets
- Check that the date/time digits are in valid range.
- Add documentation for X509_cmp_time

GH issue 2620

Reviewed-by: default avatarRichard Levitte <levitte@openssl.org>
Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
parent b169c0ec

CHANGES

+5 −0
Original line number Original line Diff line number Diff line
@@ -4,6 +4,11 @@ 
















 Changes between 1.1.0e and 1.1.1 [xx XXX xxxx]







 Changes between 1.1.0e and 1.1.1 [xx XXX xxxx]






























  *) Certificate time validation (X509_cmp_time) enforces stricter














     compliance with RFC 5280. Fractional seconds and timezone offsets














     are no longer allowed.














     [Emilia Käsper]
























  *) Add support for SipHash







  *) Add support for SipHash











     [Todd Short]







     [Todd Short]

















































crypto/x509/x509_vfy.c



+48
−99






























Original line number
Original line
Diff line number
Diff line








@@ -7,6 +7,7 @@









 * https://www.openssl.org/source/license.html







 * https://www.openssl.org/source/license.html











 */







 */


































#include <ctype.h>











#include <stdio.h>







#include <stdio.h>











#include <time.h>







#include <time.h>











#include <errno.h>







#include <errno.h>










@@ -1754,119 +1755,67 @@ int X509_cmp_current_time(const ASN1_TIME *ctm)





























int X509_cmp_time(const ASN1_TIME *ctm, time_t *cmp_time)







int X509_cmp_time(const ASN1_TIME *ctm, time_t *cmp_time)











{







{











    char *str;







    static const size_t utctime_length = sizeof("YYMMDDHHMMSSZ") - 1;











    ASN1_TIME atm;







    static const size_t generalizedtime_length = sizeof("YYYYMMDDHHMMSSZ") - 1;











    long offset;







    ASN1_TIME *asn1_cmp_time = NULL;











    char buff1[24], buff2[24], *p;







    int i, day, sec, ret = 0;











    int i, j, remaining;


































    p = buff1;














    remaining = ctm->length;














    str = (char *)ctm->data;














    /*







    /*











     * Note that the following (historical) code allows much more slack in the







     * Note that ASN.1 allows much more slack in the time format than RFC5280.











     * time format than RFC5280. In RFC5280, the representation is fixed:







     * In RFC5280, the representation is fixed:











     * UTCTime: YYMMDDHHMMSSZ







     * UTCTime: YYMMDDHHMMSSZ











     * GeneralizedTime: YYYYMMDDHHMMSSZ







     * GeneralizedTime: YYYYMMDDHHMMSSZ














     *














     * We do NOT currently enforce the following RFC 5280 requirement:














     * "CAs conforming to this profile MUST always encode certificate














     *  validity dates through the year 2049 as UTCTime; certificate validity














     *  dates in 2050 or later MUST be encoded as GeneralizedTime."











     */







     */











    if (ctm->type == V_ASN1_UTCTIME) {







    switch (ctm->type) {











        /* YYMMDDHHMM[SS]Z or YYMMDDHHMM[SS](+-)hhmm */







    case V_ASN1_UTCTIME:











        int min_length = sizeof("YYMMDDHHMMZ") - 1;







        if (ctm->length != (int)(utctime_length))











        int max_length = sizeof("YYMMDDHHMMSS+hhmm") - 1;














        if (remaining < min_length || remaining > max_length)














            return 0;







            return 0;











        memcpy(p, str, 10);







        break;











        p += 10;







    case V_ASN1_GENERALIZEDTIME:











        str += 10;







        if (ctm->length != (int)(generalizedtime_length))











        remaining -= 10;














    } else {














        /* YYYYMMDDHHMM[SS[.fff]]Z or YYYYMMDDHHMM[SS[.f[f[f]]]](+-)hhmm */














        int min_length = sizeof("YYYYMMDDHHMMZ") - 1;














        int max_length = sizeof("YYYYMMDDHHMMSS.fff+hhmm") - 1;














        if (remaining < min_length || remaining > max_length)














            return 0;







            return 0;











        memcpy(p, str, 12);














        p += 12;














        str += 12;














        remaining -= 12;














    }





























    if ((*str == 'Z') || (*str == '-') || (*str == '+')) {














        *(p++) = '0';














        *(p++) = '0';














    } else {














        /* SS (seconds) */














        if (remaining < 2)














            return 0;














        *(p++) = *(str++);














        *(p++) = *(str++);














        remaining -= 2;














        /*














         * Skip any (up to three) fractional seconds...














         * TODO(emilia): in RFC5280, fractional seconds are forbidden.














         * Can we just kill them altogether?














         */














        if (remaining && *str == '.') {














            str++;














            remaining--;














            for (i = 0; i < 3 && remaining; i++, str++, remaining--) {














                if (*str < '0' || *str > '9')














        break;







        break;














    default:














        return 0;











    }







    }











        }





























    }














    *(p++) = 'Z';














    *(p++) = '\0';


































    /* We now need either a terminating 'Z' or an offset. */







    /**











    if (!remaining)







     * Verify the format: the ASN.1 functions we use below allow a more











        return 0;







     * flexible format than what's mandated by RFC 5280.











    if (*str == 'Z') {







     * Digit and date ranges will be verified in the conversion methods.











        if (remaining != 1)







     */











            return 0;







    for (i = 0; i < ctm->length - 1; i++) {











        offset = 0;







        if (!isdigit(ctm->data[i]))











    } else {














        /* (+-)HHMM */














        if ((*str != '+') && (*str != '-'))














            return 0;














        /* Historical behaviour: the (+-)hhmm offset is forbidden in RFC5280. */














        if (remaining != 5)














            return 0;














        if (str[1] < '0' || str[1] > '9' || str[2] < '0' || str[2] > '9' ||














            str[3] < '0' || str[3] > '9' || str[4] < '0' || str[4] > '9')














            return 0;







            return 0;











        offset = ((str[1] - '0') * 10 + (str[2] - '0')) * 60;














        offset += (str[3] - '0') * 10 + (str[4] - '0');














        if (*str == '-')














            offset = -offset;














    }







    }











    atm.type = ctm->type;







    if (ctm->data[ctm->length - 1] != 'Z')











    atm.flags = 0;














    atm.length = sizeof(buff2);














    atm.data = (unsigned char *)buff2;





























    if (X509_time_adj(&atm, offset * 60, cmp_time) == NULL)














        return 0;







        return 0;































    if (ctm->type == V_ASN1_UTCTIME) {







    /*











        i = (buff1[0] - '0') * 10 + (buff1[1] - '0');







     * There is ASN1_UTCTIME_cmp_time_t but no











        if (i < 50)







     * ASN1_GENERALIZEDTIME_cmp_time_t or ASN1_TIME_cmp_time_t,











            i += 100;           /* cf. RFC 2459 */







     * so we go through ASN.1











        j = (buff2[0] - '0') * 10 + (buff2[1] - '0');







     */











        if (j < 50)







    asn1_cmp_time = X509_time_adj(NULL, 0, cmp_time);











            j += 100;







    if (asn1_cmp_time == NULL)














        goto err;














    if (!ASN1_TIME_diff(&day, &sec, ctm, asn1_cmp_time))














        goto err;































        if (i < j)







    /*











            return -1;







     * X509_cmp_time comparison is <=.











        if (i > j)







     * The return value 0 is reserved for errors.











            return 1;







     */











    }







    ret = (day >= 0 && sec >= 0) ? -1 : 1;











    i = strcmp(buff1, buff2);



















    if (i == 0)                 /* wait a second then return younger :-) */







 err:











        return -1;







    ASN1_TIME_free(asn1_cmp_time);











    else







    return ret;











        return i;














}







}































ASN1_TIME *X509_gmtime_adj(ASN1_TIME *s, long adj)







ASN1_TIME *X509_gmtime_adj(ASN1_TIME *s, long adj)

































doc/man3/X509_cmp_time.pod
0 → 100644



+39
−0






























Original line number
Original line
Diff line number
Diff line














=pod





























=head1 NAME





























X509_cmp_time - X509 time functions





























=head1 SYNOPSIS





























 X509_cmp_time(const ASN1_TIME *asn1_time, time_t *cmp_time);





























=head1 DESCRIPTION





























X509_cmp_time() compares the ASN1_TIME in B<asn1_time> with the time in














<cmp_time>.





























B<asn1_time> must satisfy the ASN1_TIME format mandated by RFC 5280, i.e.,














its format must be either YYMMDDHHMMSSZ or YYYYMMDDHHMMSSZ.





























If B<cmp_time> is NULL the current time is used.





























=head1 BUGS





























Unlike many standard comparison functions, X509_cmp_time returns 0 on error.





























=head1 RETURN VALUES





























X509_cmp_time() returns -1 if B<asn1_time> is earlier than, or equal to,














B<cmp_time>, and 1 otherwise. It returns 0 on error.





























=head1 COPYRIGHT





























Copyright 2017 The OpenSSL Project Authors. All Rights Reserved.





























Licensed under the OpenSSL license (the "License").  You may not use














this file except in compliance with the License.  You can obtain a copy














in the file LICENSE in the source distribution or at














L<https://www.openssl.org/source/license.html>.





























=cut
























test/build.info



+5
−1






























Original line number
Original line
Diff line number
Diff line








@@ -28,7 +28,7 @@ IF[{- !$disabled{tests} -}]









          dtlsv1listentest ct_test threadstest afalgtest d2i_test \







          dtlsv1listentest ct_test threadstest afalgtest d2i_test \











          ssl_test_ctx_test ssl_test x509aux cipherlist_test asynciotest \







          ssl_test_ctx_test ssl_test x509aux cipherlist_test asynciotest \











          bioprinttest sslapitest dtlstest sslcorrupttest bio_enc_test \







          bioprinttest sslapitest dtlstest sslcorrupttest bio_enc_test \











          pkey_meth_test uitest cipherbytes_test







          pkey_meth_test uitest cipherbytes_test x509_time_test































  SOURCE[aborttest]=aborttest.c







  SOURCE[aborttest]=aborttest.c











  INCLUDE[aborttest]=../include







  INCLUDE[aborttest]=../include










@@ -295,6 +295,10 @@ IF[{- !$disabled{tests} -}]









  INCLUDE[pkey_meth_test]=../include







  INCLUDE[pkey_meth_test]=../include











  DEPEND[pkey_meth_test]=../libcrypto







  DEPEND[pkey_meth_test]=../libcrypto


































  SOURCE[x509_time_test]=x509_time_test.c testutil.c test_main.c














  INCLUDE[x509_time_test]=.. ../include














  DEPEND[x509_time_test]=../libcrypto


























  IF[{- !$disabled{psk} -}]







  IF[{- !$disabled{psk} -}]











    PROGRAMS_NO_INST=dtls_mtu_test







    PROGRAMS_NO_INST=dtls_mtu_test











    SOURCE[dtls_mtu_test]=dtls_mtu_test.c ssltestlib.c







    SOURCE[dtls_mtu_test]=dtls_mtu_test.c ssltestlib.c

































test/recipes/60-test_x509_time.t
0 → 100644



+12
−0






























Original line number
Original line
Diff line number
Diff line














#! /usr/bin/env perl














# Copyright 2017 The OpenSSL Project Authors. All Rights Reserved.














#














# Licensed under the OpenSSL license (the "License").  You may not use














# this file except in compliance with the License.  You can obtain a copy














# in the file LICENSE in the source distribution or at














# https://www.openssl.org/source/license.html












































use OpenSSL::Test::Simple;





























simple_test("test_x509_time", "x509_time_test");