From 9ddfe93c434470f578a69674b1e0e16ccdee960f Mon Sep 17 00:00:00 2001 From: David Garske Date: Tue, 2 Aug 2016 16:47:21 -0700 Subject: [PATCH] Fixed issue with CRL check and zero pad (the GetRevoked function was not trimming pad). Added new ASN "GetSerialNumber" function and implemented it in three places in asn.c. --- wolfcrypt/src/asn.c | 126 +++++++++++++++++----------------------- wolfssl/wolfcrypt/asn.h | 2 + 2 files changed, 55 insertions(+), 73 deletions(-) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index b151f16a9..4a75f5e6f 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -2318,13 +2318,6 @@ void FreeDecodedCert(DecodedCert* cert) static int GetCertHeader(DecodedCert* cert) { int ret = 0, len; - byte serialTmp[EXTERNAL_SERIAL_SIZE]; -#if defined(WOLFSSL_SMALL_STACK) && defined(USE_FAST_MATH) - mp_int* mpi = NULL; -#else - mp_int stack_mpi; - mp_int* mpi = &stack_mpi; -#endif if (GetSequence(cert->source, &cert->srcIdx, &len, cert->maxIdx) < 0) return ASN_PARSE_E; @@ -2338,31 +2331,9 @@ static int GetCertHeader(DecodedCert* cert) if (GetExplicitVersion(cert->source, &cert->srcIdx, &cert->version) < 0) return ASN_PARSE_E; -#if defined(WOLFSSL_SMALL_STACK) && defined(USE_FAST_MATH) - mpi = (mp_int*)XMALLOC(sizeof(mp_int), NULL, DYNAMIC_TYPE_TMP_BUFFER); - if (mpi == NULL) - return MEMORY_E; -#endif - - if (GetInt(mpi, cert->source, &cert->srcIdx, cert->maxIdx) < 0) { -#if defined(WOLFSSL_SMALL_STACK) && defined(USE_FAST_MATH) - XFREE(mpi, NULL, DYNAMIC_TYPE_TMP_BUFFER); -#endif + if (GetSerialNumber(cert->source, &cert->srcIdx, cert->serial, + &cert->serialSz, cert->maxIdx) < 0) return ASN_PARSE_E; - } - - len = mp_unsigned_bin_size(mpi); - if (len < (int)sizeof(serialTmp)) { - if ( (ret = mp_to_unsigned_bin(mpi, serialTmp)) == MP_OKAY) { - XMEMCPY(cert->serial, serialTmp, len); - cert->serialSz = len; - } - } - mp_clear(mpi); - -#if defined(WOLFSSL_SMALL_STACK) && defined(USE_FAST_MATH) - XFREE(mpi, NULL, DYNAMIC_TYPE_TMP_BUFFER); -#endif return ret; } @@ -5307,6 +5278,49 @@ WOLFSSL_LOCAL int SetSerialNumber(const byte* sn, word32 snSz, byte* output) return result; } +WOLFSSL_LOCAL int GetSerialNumber(const byte* input, word32* inOutIdx, + byte* serial, int* serialSz, word32 maxIdx) +{ + int result = 0; + byte b; + + WOLFSSL_ENTER("GetSerialNumber"); + + if (serial == NULL || input == NULL || serialSz == NULL) { + return BAD_FUNC_ARG; + } + + /* First byte is ASN type */ + b = input[*inOutIdx]; + *inOutIdx += 1; + + if (b != ASN_INTEGER) { + WOLFSSL_MSG("Expecting Integer"); + return ASN_PARSE_E; + } + + if (GetLength(input, inOutIdx, serialSz, maxIdx) < 0) { + return ASN_PARSE_E; + } + + if (*serialSz > EXTERNAL_SERIAL_SIZE) { + WOLFSSL_MSG("Serial Size too big"); + return ASN_PARSE_E; + } + + /* skip padding */ + if (input[*inOutIdx] == 0x00) { + *serialSz -= 1; + *inOutIdx += 1; + } + + /* return serial */ + XMEMCPY(serial, &input[*inOutIdx], *serialSz); + *inOutIdx += *serialSz; + + return result; +} + const char* BEGIN_CERT = "-----BEGIN CERTIFICATE-----"; @@ -8922,28 +8936,9 @@ static int DecodeSingleResponse(byte* source, resp->issuerKeyHash = source + idx; idx += length; - /* Read the serial number, it is handled as a string, not as a - * proper number. Just XMEMCPY the data over, rather than load it - * as an mp_int. */ - if (source[idx++] != ASN_INTEGER) + /* Get serial number */ + if (GetSerialNumber(source, &idx, cs->serial, &cs->serialSz, size) < 0) return ASN_PARSE_E; - if (GetLength(source, &idx, &length, size) < 0) - return ASN_PARSE_E; - if (length <= EXTERNAL_SERIAL_SIZE) - { - if (source[idx] == 0) - { - idx++; - length--; - } - XMEMCPY(cs->serial, source + idx, length); - cs->serialSz = length; - } - else - { - return ASN_GETINT_E; - } - idx += length; /* CertStatus */ switch (source[idx++]) @@ -9655,39 +9650,24 @@ static int GetRevoked(const byte* buff, word32* idx, DecodedCRL* dcrl, end = *idx + len; - /* get serial number */ - b = buff[*idx]; - *idx += 1; - - if (b != ASN_INTEGER) { - WOLFSSL_MSG("Expecting Integer"); - return ASN_PARSE_E; - } - - if (GetLength(buff, idx, &len, maxIdx) < 0) - return ASN_PARSE_E; - - if (len > EXTERNAL_SERIAL_SIZE) { - WOLFSSL_MSG("Serial Size too big"); - return ASN_PARSE_E; - } - rc = (RevokedCert*)XMALLOC(sizeof(RevokedCert), dcrl->heap, - DYNAMIC_TYPE_CRL); + DYNAMIC_TYPE_CRL); if (rc == NULL) { WOLFSSL_MSG("Alloc Revoked Cert failed"); return MEMORY_E; } - XMEMCPY(rc->serialNumber, &buff[*idx], len); - rc->serialSz = len; + if (GetSerialNumber(buff, idx, rc->serialNumber, &rc->serialSz, + maxIdx) < 0) { + XFREE(rc, dcrl->heap, DYNAMIC_TYPE_CRL); + return ASN_PARSE_E; + } /* add to list */ rc->next = dcrl->certs; dcrl->certs = rc; dcrl->totalCerts++; - *idx += len; /* get date */ b = buff[*idx]; diff --git a/wolfssl/wolfcrypt/asn.h b/wolfssl/wolfcrypt/asn.h index 293adfd4d..366a652d2 100644 --- a/wolfssl/wolfcrypt/asn.h +++ b/wolfssl/wolfcrypt/asn.h @@ -670,6 +670,8 @@ WOLFSSL_LOCAL word32 SetSet(word32 len, byte* output); WOLFSSL_LOCAL word32 SetAlgoID(int algoOID,byte* output,int type,int curveSz); WOLFSSL_LOCAL int SetMyVersion(word32 version, byte* output, int header); WOLFSSL_LOCAL int SetSerialNumber(const byte* sn, word32 snSz, byte* output); +WOLFSSL_LOCAL int GetSerialNumber(const byte* input, word32* inOutIdx, + byte* serial, int* serialSz, word32 maxIdx); WOLFSSL_LOCAL int GetNameHash(const byte* source, word32* idx, byte* hash, int maxIdx);