From 6fc83e292b5c5d7961aea7bf8f44cbede27b856c Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Tue, 10 Mar 2026 18:04:26 +0100 Subject: [PATCH] Address code review --- examples/ocsp_responder/ocsp_responder.c | 13 ++--- src/ocsp.c | 13 +++-- tests/api/test_ocsp.c | 12 ++--- wolfcrypt/src/asn.c | 68 ++++++++++++++++++------ wolfssl/wolfcrypt/asn_public.h | 12 ++--- 5 files changed, 80 insertions(+), 38 deletions(-) diff --git a/examples/ocsp_responder/ocsp_responder.c b/examples/ocsp_responder/ocsp_responder.c index 7a800d3d84..28a4b2d243 100644 --- a/examples/ocsp_responder/ocsp_responder.c +++ b/examples/ocsp_responder/ocsp_responder.c @@ -391,8 +391,8 @@ static int PopulateResponderFromIndex(OcspResponder* responder, IndexEntry* inde DecodedCert* caCert) { IndexEntry* entry; - const char* caSubject; - word32 caSubjSz; + char caSubjectBuf[WC_ASN_NAME_MAX]; + word32 caSubjSz = sizeof(caSubjectBuf); int count = 0; int ret; @@ -400,8 +400,8 @@ static int PopulateResponderFromIndex(OcspResponder* responder, IndexEntry* inde return BAD_FUNC_ARG; } - caSubject = wc_GetDecodedCertSubject(caCert, &caSubjSz); - if (caSubject == NULL || caSubjSz == 0) { + ret = wc_GetDecodedCertSubject(caCert, caSubjectBuf, &caSubjSz); + if (ret != 0 || caSubjSz == 0) { LOG_ERROR("Could not get CA subject\n"); return BAD_FUNC_ARG; } @@ -467,7 +467,7 @@ static int PopulateResponderFromIndex(OcspResponder* responder, IndexEntry* inde } ret = wc_OcspResponder_SetCertStatus(responder, - caSubject, caSubjSz, + caSubjectBuf, caSubjSz, serial, serialLen, status, revTime, revReason, validity); if (ret == 0) { @@ -832,7 +832,8 @@ THREAD_RETURN WOLFSSL_THREAD ocsp_responder_test(void* args) ret = -1; goto cleanup; } - (void)wc_GetDecodedCertSubject(&caCert, &caSubjectSz); + (void)wc_GetDecodedCertSubject(&caCert, NULL, &caSubjectSz); + (void)caSubjectSz; /* Not used in current implementation */ (void)caSubjectSz; /* Not used in current implementation */ /* Load index file if provided */ diff --git a/src/ocsp.c b/src/ocsp.c index 6dcec8f02a..391c454022 100644 --- a/src/ocsp.c +++ b/src/ocsp.c @@ -633,9 +633,12 @@ static int CheckOcspResponderChain(OcspEntry* single, byte* issuerHash, /** * Enforce https://www.rfc-editor.org/rfc/rfc6960#section-4.2.2.2 - * @param bs The basic response to verify - * @param cert The decoded bs->cert - * @return + * @param bs The basic OCSP response to verify + * @param subjectHash The subject key hash of the OCSP responder certificate + * @param extExtKeyUsage The extended key usage bits of the responder certificate + * @param issuerHash The issuer key hash of the OCSP responder certificate + * @param vp Unused (reserved for future use) + * @return 1 if the responder is authorized to sign the response, 0 otherwise */ int CheckOcspResponder(OcspResponse *bs, byte* subjectHash, byte extExtKeyUsage, byte* issuerHash, void* vp) @@ -2263,7 +2266,8 @@ int wc_OcspResponder_AddSigner(OcspResponder* responder, WOLFSSL_ENTER("wc_OcspResponder_AddSigner"); if (responder == NULL || signerDer == NULL || signerDerSz == 0 || - keyDer == NULL || keyDerSz == 0) + keyDer == NULL || keyDerSz == 0 || + (issuerCertDerSz != 0 && issuerCertDer == NULL)) return BAD_FUNC_ARG; /* Allocate CA structure */ @@ -2431,7 +2435,6 @@ out: return ret; } -/* Find Auth CA by comparing cert DER */ /* Find Auth CA by issuer hashes from request */ static OcspResponderCa* FindCaByHashes(OcspResponder* responder, const byte* issuerHash, const byte* issuerKeyHash, int hashAlg) diff --git a/tests/api/test_ocsp.c b/tests/api/test_ocsp.c index 05c6ef73b6..1fe45f15f0 100644 --- a/tests/api/test_ocsp.c +++ b/tests/api/test_ocsp.c @@ -1278,10 +1278,10 @@ static int ocspResponderTest_Run(OcspResponderTestConfig* config, int sendCerts) word32 respSz = 0; byte reqBuf[1024]; int reqSz = 0; - const char* caSubject = NULL; - word32 caSubjectSz = 0; - const byte* serial = NULL; - word32 serialSz = 0; + char caSubject[WC_ASN_NAME_MAX]; + word32 caSubjectSz = sizeof(caSubject); + byte serial[EXTERNAL_SERIAL_SIZE]; + word32 serialSz = sizeof(serial); XFILE f = XBADFILE; byte usingAuthCa = XSTRCMP(config->caCertPath, config->responderCertPath) != 0; @@ -1362,9 +1362,9 @@ static int ocspResponderTest_Run(OcspResponderTestConfig* config, int sendCerts) usingAuthCa ? caCertDer : NULL, usingAuthCa ? caCertSz : 0), 0); /* Set certificate status */ - ExpectNotNull(caSubject = wc_GetDecodedCertSubject(&decodedCaCert, &caSubjectSz)); + ExpectIntEQ(wc_GetDecodedCertSubject(&decodedCaCert, caSubject, &caSubjectSz), 0); ExpectIntGT(caSubjectSz, 0); - ExpectNotNull(serial = wc_GetDecodedCertSerial(&targetCert, &serialSz)); + ExpectIntEQ(wc_GetDecodedCertSerial(&targetCert, serial, &serialSz), 0); ExpectIntGT(serialSz, 0); ExpectIntEQ(wc_OcspResponder_SetCertStatus(responder, diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index b616e8367c..1d53ef8b7a 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -25002,31 +25002,69 @@ int wc_ParseCert(DecodedCert* cert, int type, int verify, void* cm) return ParseCert(cert, type, verify, cm); } -const char* wc_GetDecodedCertSubject(struct DecodedCert* cert, word32* subjectSz) +int wc_GetDecodedCertSubject(const struct DecodedCert* cert, char* buf, + word32* bufSz) { - if (cert == NULL || subjectSz == NULL) { - return NULL; + word32 sz; + + if (cert == NULL || bufSz == NULL) + return BAD_FUNC_ARG; + + sz = (word32)XSTRLEN(cert->subject); + + if (buf == NULL) { + *bufSz = sz; + return WC_NO_ERR_TRACE(LENGTH_ONLY_E); } - *subjectSz = (word32)XSTRLEN(cert->subject); - return cert->subject; + + if (*bufSz < sz) + return BUFFER_E; + + XMEMCPY(buf, cert->subject, sz); + *bufSz = sz; + return 0; } -const char* wc_GetDecodedCertIssuer(struct DecodedCert* cert, word32* issuerSz) +int wc_GetDecodedCertIssuer(const struct DecodedCert* cert, char* buf, + word32* bufSz) { - if (cert == NULL || issuerSz == NULL) { - return NULL; + word32 sz; + + if (cert == NULL || bufSz == NULL) + return BAD_FUNC_ARG; + + sz = (word32)XSTRLEN(cert->issuer); + + if (buf == NULL) { + *bufSz = sz; + return WC_NO_ERR_TRACE(LENGTH_ONLY_E); } - *issuerSz = (word32)XSTRLEN(cert->issuer); - return cert->issuer; + + if (*bufSz < sz) + return BUFFER_E; + + XMEMCPY(buf, cert->issuer, sz); + *bufSz = sz; + return 0; } -const byte* wc_GetDecodedCertSerial(struct DecodedCert* cert, word32* serialSz) +int wc_GetDecodedCertSerial(const struct DecodedCert* cert, byte* buf, + word32* bufSz) { - if (cert == NULL || serialSz == NULL) { - return NULL; + if (cert == NULL || bufSz == NULL) + return BAD_FUNC_ARG; + + if (buf == NULL) { + *bufSz = (word32)cert->serialSz; + return WC_NO_ERR_TRACE(LENGTH_ONLY_E); } - *serialSz = (word32)cert->serialSz; - return cert->serial; + + if (*bufSz < (word32)cert->serialSz) + return BUFFER_E; + + XMEMCPY(buf, cert->serial, (size_t)cert->serialSz); + *bufSz = (word32)cert->serialSz; + return 0; } #ifdef WOLFCRYPT_ONLY diff --git a/wolfssl/wolfcrypt/asn_public.h b/wolfssl/wolfcrypt/asn_public.h index 656253e18e..8eda908fe8 100644 --- a/wolfssl/wolfcrypt/asn_public.h +++ b/wolfssl/wolfcrypt/asn_public.h @@ -930,12 +930,12 @@ WOLFSSL_API int wc_GetSubjectPubKeyInfoDerFromCert(const byte* certDer, word32 certDerSz, byte* pubKeyDer, word32* pubKeyDerSz); -WOLFSSL_API const char* wc_GetDecodedCertSubject(struct DecodedCert* cert, - word32* subjectSz); -WOLFSSL_API const char* wc_GetDecodedCertIssuer(struct DecodedCert* cert, - word32* issuerSz); -WOLFSSL_API const byte* wc_GetDecodedCertSerial(struct DecodedCert* cert, - word32* serialSz); +WOLFSSL_API int wc_GetDecodedCertSubject(const struct DecodedCert* cert, + char* buf, word32* bufSz); +WOLFSSL_API int wc_GetDecodedCertIssuer(const struct DecodedCert* cert, + char* buf, word32* bufSz); +WOLFSSL_API int wc_GetDecodedCertSerial(const struct DecodedCert* cert, + byte* buf, word32* bufSz); #ifdef WOLFSSL_FPKI WOLFSSL_API int wc_GetUUIDFromCert(struct DecodedCert* cert,