Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OCSP openssl compat fixes #8498

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
237 changes: 198 additions & 39 deletions src/ocsp.c
Original file line number Diff line number Diff line change
Expand Up @@ -727,13 +727,23 @@ WOLFSSL_OCSP_CERTID* wolfSSL_OCSP_cert_to_id(
WOLFSSL_CERT_MANAGER* cm = NULL;
int ret = -1;
DerBuffer* derCert = NULL;
int dgstType;
#ifdef WOLFSSL_SMALL_STACK
DecodedCert *cert = NULL;
#else
DecodedCert cert[1];
#endif

(void)dgst;
if (dgst == NULL) {
dgstType = WC_HASH_TYPE_SHA;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not OCSP_DIGEST?

}
else if (wolfSSL_EVP_get_hashinfo(dgst, &dgstType, NULL) !=
WOLFSSL_SUCCESS) {
return NULL;
}

if (dgstType != OCSP_DIGEST)
return NULL;

cm = wolfSSL_CertManagerNew();
if (cm == NULL
Expand Down Expand Up @@ -785,6 +795,7 @@ WOLFSSL_OCSP_CERTID* wolfSSL_OCSP_cert_to_id(
goto out;
}
else {
certId->hashAlgoOID = wc_HashGetOID(OCSP_DIGEST);
XMEMCPY(certId->issuerHash, cert->issuerHash, OCSP_DIGEST_SIZE);
XMEMCPY(certId->issuerKeyHash, cert->issuerKeyHash, OCSP_DIGEST_SIZE);
XMEMCPY(certId->status->serial, cert->serial, (size_t)cert->serialSz);
Expand Down Expand Up @@ -822,6 +833,103 @@ void wolfSSL_OCSP_BASICRESP_free(WOLFSSL_OCSP_BASICRESP* basicResponse)
wolfSSL_OCSP_RESPONSE_free(basicResponse);
}

/* Calculate size needed for CertID DER encoding following RFC 6960:
CertID ::= SEQUENCE {
hashAlgorithm AlgorithmIdentifier,
issuerNameHash OCTET STRING,
issuerKeyHash OCTET STRING,
serialNumber CertificateSerialNumber }
*/
static int OcspGetCertIdRawSize(WOLFSSL_OCSP_CERTID* id, word32* totalSz,
word32* intSize)
{
word32 idx = 0;
int ret;

if (id == NULL || totalSz == NULL || id->status == NULL)
return BAD_FUNC_ARG;

ret = SetAlgoID(id->hashAlgoOID, NULL, oidHashType, 0);
if (ret < 0)
return ret;
idx += ret;

/* issuerNameHash */
ret = SetOctetString(OCSP_DIGEST_SIZE, NULL);
if (ret < 0)
return ret;
idx += ret + OCSP_DIGEST_SIZE;

/* issuerKeyHash */
ret = SetOctetString(OCSP_DIGEST_SIZE, NULL);
if (ret < 0)
return ret;
idx += ret + OCSP_DIGEST_SIZE;

/* serialNumber */
ret = SetASNInt(id->status->serialSz, id->status->serial[0], NULL);
if (ret < 0)
return ret;
idx += ret + id->status->serialSz;

/* Set sequence - get size */
ret = SetSequence(idx, NULL);
if (ret < 0)
return ret;

*totalSz = idx + ret;
if (intSize)
*intSize = idx;
return 0;
}

/* Encode CertID following RFC 6960 ASN.1 structure */
static int OcspEncodeCertID(WOLFSSL_OCSP_CERTID* id, byte* output, word32 size)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation can be combined with above.
Avoids two functions getting out of sync with each other.

{
word32 idx = 0;
int ret;

if (id == NULL || output == NULL || id->status == NULL)
return BAD_FUNC_ARG;

ret = SetSequence(size, output);
if (ret < 0)
return ret;
idx += ret;

/* hashAlgorithm */
ret = SetAlgoID(id->hashAlgoOID, output + idx, oidHashType, 0);
if (ret < 0)
return ret;
idx += ret;

/* issuerNameHash */
ret = SetOctetString(OCSP_DIGEST_SIZE, output + idx);
if (ret < 0)
return ret;
idx += ret;
XMEMCPY(output + idx, id->issuerHash, OCSP_DIGEST_SIZE);
idx += OCSP_DIGEST_SIZE;

/* issuerKeyHash */
ret = SetOctetString(OCSP_DIGEST_SIZE, output + idx);
if (ret < 0)
return ret;
idx += ret;
XMEMCPY(output + idx, id->issuerKeyHash, OCSP_DIGEST_SIZE);
idx += OCSP_DIGEST_SIZE;

/* serialNumber */
ret = SetASNInt(id->status->serialSz, id->status->serial[0], output + idx);
if (ret < 0)
return ret;
idx += ret;
XMEMCPY(output + idx, id->status->serial, id->status->serialSz);
idx += id->status->serialSz;

return idx;
}

static int OcspRespIdMatches(OcspResponse* resp, const byte* NameHash,
const byte* keyHash)
{
Expand Down Expand Up @@ -1284,67 +1392,118 @@ int wolfSSL_i2d_OCSP_REQUEST_bio(WOLFSSL_BIO* out,

int wolfSSL_i2d_OCSP_CERTID(WOLFSSL_OCSP_CERTID* id, unsigned char** data)
{
if (id == NULL || data == NULL)
return WOLFSSL_FAILURE;
int allocated = 0;
word32 derSz = 0;
word32 intSz = 0;
int ret;
WOLFSSL_ENTER("wolfSSL_i2d_OCSP_CERTID");

if (*data != NULL) {
XMEMCPY(*data, id->rawCertId, (size_t)id->rawCertIdSize);
*data = *data + id->rawCertIdSize;
if (id == NULL) {
return -1;
}

if (data == NULL) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't do this here.
Return derSz when data is NULL after line 1427.
Code is similar enough that I can't see why not.

if (id->rawCertId != NULL)
return id->rawCertIdSize;

ret = OcspGetCertIdRawSize(id, &derSz, NULL);
if (ret != 0) {
WOLFSSL_MSG("Failed to calculate CertID size");
return -1;
}
return derSz;
}

if (id->rawCertId == NULL) {
/* Calculate required size */
ret = OcspGetCertIdRawSize(id, &derSz, &intSz);
if (ret != 0) {
WOLFSSL_MSG("Failed to calculate CertID size");
return -1;
}
}
else {
*data = (unsigned char*)XMALLOC((size_t)id->rawCertIdSize, NULL, DYNAMIC_TYPE_OPENSSL);
derSz = id->rawCertIdSize;
}

if (*data == NULL) {
/* Allocate buffer for DER encoding */
*data = (byte*)XMALLOC(derSz, NULL, DYNAMIC_TYPE_OPENSSL);
if (*data == NULL) {
return WOLFSSL_FAILURE;
WOLFSSL_MSG("Failed to allocate memory for CertID DER encoding");
return -1;
}
allocated = 1;
}

if (id->rawCertId != NULL) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Swap the condition check to match the code above.

XMEMCPY(*data, id->rawCertId, id->rawCertIdSize);
}
else {
ret = OcspEncodeCertID(id, *data, intSz);
if (ret < 0) {
WOLFSSL_MSG("Failed to encode CertID");
if (allocated) {
XFREE(data, NULL, DYNAMIC_TYPE_OPENSSL);
*data = NULL;
}
return -1;
}
XMEMCPY(*data, id->rawCertId, (size_t)id->rawCertIdSize);
}

return id->rawCertIdSize;
if (!allocated)
*data += derSz;

return derSz;
}

WOLFSSL_OCSP_CERTID* wolfSSL_d2i_OCSP_CERTID(WOLFSSL_OCSP_CERTID** cidOut,
const unsigned char** derIn,
int length)
{
WOLFSSL_OCSP_CERTID *cid = NULL;
int isAllocated = 0;
word32 idx = 0;
int ret;

if ((cidOut != NULL) && (derIn != NULL) && (*derIn != NULL) &&
(length > 0)) {
if (derIn == NULL || *derIn == NULL || length <= 0)
return NULL;

if (cidOut != NULL && *cidOut != NULL) {
cid = *cidOut;
FreeOcspEntry(cid, NULL);
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
else {

cid = (WOLFSSL_OCSP_CERTID*)XMALLOC(sizeof(WOLFSSL_OCSP_CERTID),
NULL, DYNAMIC_TYPE_OPENSSL);
if (cid == NULL)
return NULL;
isAllocated = 1;
}

/* If a NULL is passed we allocate the memory for the caller. */
if (cid == NULL) {
cid = (WOLFSSL_OCSP_CERTID*)XMALLOC(sizeof(*cid), NULL,
DYNAMIC_TYPE_OPENSSL);
}
else if (cid->rawCertId != NULL) {
XFREE(cid->rawCertId, NULL, DYNAMIC_TYPE_OPENSSL);
cid->rawCertId = NULL;
cid->rawCertIdSize = 0;
}

if (cid != NULL) {
cid->rawCertId = (byte*)XMALLOC((size_t)length + 1, NULL, DYNAMIC_TYPE_OPENSSL);
if (cid->rawCertId != NULL) {
XMEMCPY(cid->rawCertId, *derIn, (size_t)length);
cid->rawCertIdSize = length;

/* Per spec. advance past the data that is being returned
* to the caller. */
*cidOut = cid;
*derIn = *derIn + length;
XMEMSET(cid, 0, sizeof(WOLFSSL_OCSP_CERTID));
cid->status = XMALLOC(sizeof(CertStatus), NULL, DYNAMIC_TYPE_OCSP_STATUS);
if (cid->status == NULL) {
XFREE(cid, NULL, DYNAMIC_TYPE_OPENSSL);
return NULL;
}
XMEMSET(cid->status, 0, sizeof(CertStatus));
cid->ownStatus = 1;

return cid;
}
ret = OcspDecodeCertID(*derIn, &idx, length, cid);
if (ret != 0) {
FreeOcspEntry(cid, NULL);
if (isAllocated) {
XFREE(cid, NULL, DYNAMIC_TYPE_OPENSSL);
}
return NULL;
}

if ((cid != NULL) && ((cidOut == NULL) || (cid != *cidOut))) {
XFREE(cid, NULL, DYNAMIC_TYPE_OPENSSL);
}
*derIn += idx;

return NULL;
if (isAllocated && cidOut != NULL)
*cidOut = cid;

return cid;
}

const WOLFSSL_OCSP_CERTID* wolfSSL_OCSP_SINGLERESP_get0_id(
Expand Down
9 changes: 3 additions & 6 deletions tests/api.c
Original file line number Diff line number Diff line change
Expand Up @@ -71456,10 +71456,9 @@ static int test_wolfSSL_d2i_OCSP_CERTID(void)
WOLFSSL_OCSP_CERTID* certId = NULL;
ExpectNotNull(certId = wolfSSL_d2i_OCSP_CERTID(&certId, &rawCertIdPtr,
sizeof(rawCertId)));
ExpectIntEQ(certId->rawCertIdSize, sizeof(rawCertId));
if (certId != NULL) {
XFREE(certId->rawCertId, NULL, DYNAMIC_TYPE_OPENSSL);
XFREE(certId, NULL, DYNAMIC_TYPE_OPENSSL);
wolfSSL_OCSP_CERTID_free(certId);
}
}

Expand All @@ -71477,10 +71476,9 @@ static int test_wolfSSL_d2i_OCSP_CERTID(void)
ExpectNotNull(certIdGood = wolfSSL_d2i_OCSP_CERTID(&certId, &rawCertIdPtr,
sizeof(rawCertId)));
ExpectPtrEq(certIdGood, certId);
ExpectIntEQ(certId->rawCertIdSize, sizeof(rawCertId));
if (certId != NULL) {
XFREE(certId->rawCertId, NULL, DYNAMIC_TYPE_OPENSSL);
XFREE(certId, NULL, DYNAMIC_TYPE_TMP_BUFFER);
wolfSSL_OCSP_CERTID_free(certId);
certId = NULL;
}
}
Expand All @@ -71489,8 +71487,6 @@ static int test_wolfSSL_d2i_OCSP_CERTID(void)
* always be returned. */
{
WOLFSSL_OCSP_CERTID* certId = NULL;
ExpectNull(certIdBad = wolfSSL_d2i_OCSP_CERTID(NULL, &rawCertIdPtr,
sizeof(rawCertId)));
ExpectNull(certIdBad = wolfSSL_d2i_OCSP_CERTID(&certId, NULL,
sizeof(rawCertId)));
ExpectNull(certIdBad = wolfSSL_d2i_OCSP_CERTID(&certId, &rawCertIdPtr, 0));
Expand Down Expand Up @@ -95500,6 +95496,7 @@ TEST_CASE testCases[] = {
TEST_DECL(test_ocsp_status_callback),
TEST_DECL(test_ocsp_basic_verify),
TEST_DECL(test_ocsp_response_parsing),
TEST_DECL(test_ocsp_certid_enc_dec),
/* This test needs to stay at the end to clean up any caches allocated. */
TEST_DECL(test_wolfSSL_Cleanup)
};
Expand Down
2 changes: 1 addition & 1 deletion tests/api/create_ocsp_test_blobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ def create_bad_response(rd: dict) -> bytes:
},
]

with open('./tests/api/ocsp_test_blobs.h', 'w') as f:
with open('./tests/api/test_ocsp_test_blobs.h', 'w') as f:
f.write(
"""/*
* This file is generated automatically by running ./tests/api/create_ocsp_test_blobs.py.
Expand Down
Loading
Loading