Add OCSP response for intermediate cert into Certificate extension on TLS1.3 #7766

miyazakh · 2024-07-19T09:02:05Z

Description

Similar to OCSP Stapling v2 Multi, Request OCSP response for Intermediate cert of server certificate chain and include the results to extension of CertificateEntry in Certificate Message

This behavior is default when enabled OCSP Stapling and TLS13.
./configure --enable-ocspstapling

By specifying no-multi to --enable-ocspstapling, it behaves the same as before.
./configure --enable-ocspstapling=no-multi

Fixes zd#18141

Testing

Run unit test that is newly added

Checklist

added tests
updated/added doxygen
updated appropriate READMEs
Updated manual and documentation

dgarske · 2024-07-19T21:34:53Z

wolfssl/wolfcrypt/asn.h

@@ -2630,6 +2632,8 @@ WOLFSSL_LOCAL int InitOcspRequest(OcspRequest* req, DecodedCert* cert,
 WOLFSSL_LOCAL void FreeOcspRequest(OcspRequest* req);
 WOLFSSL_LOCAL int EncodeOcspRequest(OcspRequest* req, byte* output,
 word32 size);
+WOLFSSL_LOCAL int CreateOcspRequest(struct WOLFSSL* ssl, OcspRequest* request,


This function declaration should not be in wolfCrypt... please move to internal.h and delete struct WOLFSSL;

miyazakh · 2024-07-24T13:03:45Z

retest this please

miyazakh · 2024-07-24T21:56:44Z

retest this please

miyazakh · 2024-07-26T01:59:59Z

retest this please

dgarske · 2024-07-26T18:50:14Z

Retest this please

src/tls.c

src/tls13.c

src/tls.c

dgarske

[check-source-text] [2 of 7] [wolfssl]
overlong lines added:
/src/internal.c:14898                     #if defined(WOLFSSL_TLS13) && defined(HAVE_CERTIFICATE_STATUS_REQUEST)
/src/tls.c:3205                     ret = (int)EncodeOcspRequestExtensions(&csr->request.ocsp[0],
/src/tls.c:3393                                                     csr->request.ocsp[0].nonceSz);
/src/tls.c:3657                                               &csr->request.ocsp[0], NULL, NULL);
/src/tls13.c:8750                 word32 copySz = AddCertExt(ssl, ssl->buffers.certificate->buffer,
    check-source-text OK
[check-shell-scripts] [3 of 7] [wolfssl]
    bash -n... done.
    shellcheck scripts...
In ./scripts/ocsp-stapling_tls13multi.test line 48:
if [[ ("$tls13" == "no") && ("$dtsl13" == "no") ]]; then
                              ^-----^ SC2154 (warning): dtsl13 is referenced but not assigned.
For more information:
  https://www.shellcheck.net/wiki/SC2154 -- dtsl13 is referenced but not assi...
   real 0m8.639s  user 0m5.715s  sys 0m0.056s
    check-shell-scripts fail_analysis

dgarske · 2024-08-02T15:10:42Z

wolfssl/wolfcrypt/asn.h

@@ -2630,6 +2630,8 @@ struct OcspRequest {
 void* ssl;
 };

+struct WOLFSSL;


Please remove this artifact. Thanks.

dgarske · 2024-08-02T15:14:10Z

examples/client/client.c

@@ -1221,66 +1221,70 @@ static const char* client_usage_msg[][78] = {
 || defined(HAVE_CERTIFICATE_STATUS_REQUEST_V2)
 "-W <num> Use OCSP Stapling (1 v1, 2 v2, 3 v2 multi)\n", /* 41 */
 " With 'm' at end indicates MUST staple\n", /* 42 */
+#if defined(WOLFSSL_TLS13) && defined(HAVE_CSR_TLS13MULTI)


Does this support need gated? Should it be on by default for TLS v1.3 with OCSP?
Perhaps the name could be improved like WOLFSSL_TLS_OCSP_MULTI?
If the macro is kept can you make sure it is available as an option in configure.ac and also documented?

Agree. Default is multi OCSP stapling on TLS13. It is able to disable it by --enable-ocspstapling=no-multi

Doc update
PR157

JacobBarthelmeh · 2024-09-17T16:44:18Z

wolfssl/error-ssl.h

@@ -102,7 +102,7 @@ enum wolfSSL_ErrorCodes {
 COOKIE_ERROR = -369, /* dtls cookie error */
 SEQUENCE_ERROR = -370, /* dtls sequence error */
 SUITES_ERROR = -371, /* suites pointer error */
-
+ MAX_CERT_EXTENSIONS_ERR = -372, /* max cert extension exceeded */


Fails error tests for me:

./configure --enable-all --enable-ocspstapling=no-multi

./scripts/unit.test -833 starting unit tests... Begin API Tests 833: test_wolfSSL_ERR_strings : ERROR - tests/api.c line 83240 failed with: expected: strcmp((errStr),(unknownStr)) == 0 result: -40 != 0 ERROR - tests/api.c line 83286 failed with: expected: error_test() == 1 result: -372 != 1 failed ( 0.00006) ERROR - tests/api.c line 91637 failed with: expected: Test failed result: ret 0 FAILURES: 833: test_wolfSSL_ERR_strings End API Tests

Hi @JacobBarthelmeh
I cannot see the error. Is there any pre-setting needed for testing? I checked the following gcc and clang version:

$ gcc --version gcc (Ubuntu 9.5.0-1ubuntu1~22.04) 9.5.0 $ gcc --version gcc (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0 $ clang --version Ubuntu clang version 14.0.0-1ubuntu1.1 $ lsb_release -d Description: Ubuntu 22.04.5 LTS

Used command:

./configure --enable-all --enable-ocspstapling=no-multi && make check

Could you share your test environment?

After rebasing to the latest master, I am seeing the errors now.

wolfssl/internal.h

dgarske · 2024-09-17T17:08:53Z

SCRIPTS-LIST

@@ -35,6 +35,7 @@ scripts/
 google.test - example client test against google, part of tests
 resume.test - example sessoin resume test, part of tests
 ocsp-stapling.test - example client test against globalsign, part of tests
+ ocsp-stapling1_tls13.text - example client test against example server, part of tests


Should this be ./scripts/ocsp-stapling_tls13multi.test?

Yes, that is true.

miyazakh · 2024-09-19T06:59:20Z

retest this please

dgarske · 2024-09-20T16:07:30Z

src/tls13.c

@@ -8459,8 +8529,10 @@ static int SendTls13Certificate(WOLFSSL* ssl)
 {
 int ret = 0;
 word32 certSz, certChainSz, headerSz, listSz, payloadSz;
- word16 extSz = 0;
+ word16 extSz[1 + MAX_CERT_EXTENSIONS];


Does this actually need to be MAX_CERT_EXTENSIONS + 1? Isn't the +1 already handled by the macro? I reviewed code below and don't see reason. I do see that it needs to be at least 1.

I thin we don't need + 1 by disabling index increment when not using multi.

if (MAX_CERT_EXTENSIONS > extIdx) extIdx++;

dgarske · 2024-09-20T16:10:33Z

src/tls13.c

@@ -8404,6 +8404,75 @@ static word32 NextCert(byte* data, word32 length, word32* idx)
 return len;
 }

+#if defined(HAVE_CERTIFICATE_STATUS_REQUEST)


Fairly certain you can wrap this in !defined(NO_WOLFSSL_SERVER) to reduce code size for client only.

dgarske · 2024-09-20T16:10:54Z

src/tls13.c

+ /* We only send CSR on the server side. On client side, the CSR data
+ * is populated with the server response. We would be sending the server
+ * its own stapling data. */
+ if (ssl->options.side == WOLFSSL_SERVER_END) {


Please wrap with !defined(NO_WOLFSSL_SERVER) for reduced code size with client only.

dgarske · 2024-09-20T16:13:05Z

wolfssl/internal.h

@@ -3235,10 +3244,11 @@ typedef struct {
 byte options;
 WOLFSSL* ssl;
 union {
- OcspRequest ocsp;
+ OcspRequest ocsp[MAX_CERT_EXTENSIONS];


Building HAVE_CERTIFICATE_STATUS_REQUEST without HAVE_OCSP causes these errors:
Guessing configure.ac has protections for this, but I was building with user_settings.h.

../Middlewares/Third_Party/wolfSSL_wolfSSL_wolfSSL/wolfssl/wolfssl/internal.h:3250:9: error: unknown type name 'OcspRequest' 3250 | OcspRequest ocsp[MAX_CERT_EXTENSIONS]; | ^~~~~~~~~~~ arm-none-eabi-gcc "../Middlewares/Third_Party/wolfSSL_wolfSSL_wolfSSL/wolfssl/wolfcrypt/src/ed448.c" -mcpu=cortex-m7 -std=gnu11 -DUSE_HAL_DRIVER -DSTM32H743xx -c -I../Core/Inc -I../Drivers/STM32H7xx_HAL_Driver/Inc -I../Drivers/STM32H7xx_HAL_Driver/Inc/Legacy -I../Drivers/CMSIS/Device/ST/STM32H7xx/Include -I../Drivers/CMSIS/Include -I../LWIP/App -I../LWIP/Target -I../Middlewares/Third_Party/LwIP/src/include -I../Middlewares/Third_Party/LwIP/system -I../Middlewares/Third_Party/FreeRTOS/Source/include -I../Middlewares/Third_Party/FreeRTOS/Source/CMSIS_RTOS_V2 -I../Middlewares/Third_Party/FreeRTOS/Source/portable/GCC/ARM_CM4F -I../Drivers/BSP/Components/lan8742 -I../Middlewares/Third_Party/LwIP/src/include/netif/ppp -I../Middlewares/Third_Party/LwIP/src/include/lwip -I../Middlewares/Third_Party/LwIP/src/include/lwip/apps -I../Middlewares/Third_Party/LwIP/src/include/lwip/priv -I../Middlewares/Third_Party/LwIP/src/include/lwip/prot -I../Middlewares/Third_Party/LwIP/src/include/netif -I../Middlewares/Third_Party/LwIP/src/include/compat/posix -I../Middlewares/Third_Party/LwIP/src/include/compat/posix/arpa -I../Middlewares/Third_Party/LwIP/src/include/compat/posix/net -I../Middlewares/Third_Party/LwIP/src/include/compat/posix/sys -I../Middlewares/Third_Party/LwIP/system/arch -I../Middlewares/Third_Party/LwIP/src/include/compat/stdc -I../Middlewares/Third_Party/LwIP/src/apps/http -I../wolfSSL -I../wolfTPM -I../Middlewares/Third_Party/wolfSSL_wolfSSL_wolfSSL/wolfssl/ -I../Middlewares/Third_Party/wolfSSL_wolfTPM_wolfTPM/wolftpm/ -Os -ffunction-sections -fdata-sections -Wall -fstack-usage -fcyclomatic-complexity -MMD -MP -MF"Middlewares/Third_Party/wolfSSL_wolfSSL_wolfSSL/wolfssl/wolfcrypt/src/ed448.d" -MT"Middlewares/Third_Party/wolfSSL_wolfSSL_wolfSSL/wolfssl/wolfcrypt/src/ed448.o" --specs=nano.specs -mfpu=fpv5-d16 -mfloat-abi=hard -mthumb -o "Middlewares/Third_Party/wolfSSL_wolfSSL_wolfSSL/wolfssl/wolfcrypt/src/ed448.o" ../Middlewares/Third_Party/wolfSSL_wolfSSL_wolfSSL/wolfssl/wolfssl/internal.h:3278:51: error: unknown type name 'OcspRequest' 3278 | WOLFSSL_LOCAL int CreateOcspRequest(WOLFSSL* ssl, OcspRequest* request, | ^~~~~~~~~~~ ../Middlewares/Third_Party/wolfSSL_wolfSSL_wolfSSL/wolfssl/wolfssl/internal.h:3992:13: error: unknown type name 'OcspRequest' 3992 | OcspRequest* certOcspRequest; | ^~~~~~~~~~~ ../Middlewares/Third_Party/wolfSSL_wolfSSL_wolfSSL/wolfssl/wolfssl/internal.h:6376:52: error: unknown type name 'OcspRequest' 6376 | WOLFSSL_LOCAL int CreateOcspResponse(WOLFSSL* ssl, OcspRequest** ocspRequest, | ^~~~~~~~~~~

Added to check if defined HAVE_CERTIFICATE_STATUS_REQUEST and _V2, but not defined HAVE_OCSP

miyazakh self-assigned this Jul 19, 2024

dgarske requested changes Jul 19, 2024

View reviewed changes

SparkiDev changed the title ~~Add OCSP responce for intermediate cert into Certificate extension on TLS1.3~~ Add OCSP response for intermediate cert into Certificate extension on TLS1.3 Jul 21, 2024

miyazakh force-pushed the zd18141_tls13_ocsp branch from 3e5553d to 2119f15 Compare July 25, 2024 00:13

miyazakh assigned wolfSSL-Bot and unassigned miyazakh Jul 26, 2024

miyazakh force-pushed the zd18141_tls13_ocsp branch from 9938dbe to 4bf9895 Compare July 26, 2024 05:43

dgarske self-requested a review July 26, 2024 18:50

dgarske assigned dgarske and SparkiDev Jul 26, 2024

SparkiDev requested changes Jul 29, 2024

View reviewed changes

SparkiDev assigned miyazakh and unassigned SparkiDev Jul 29, 2024

dgarske removed their assignment Jul 29, 2024

miyazakh force-pushed the zd18141_tls13_ocsp branch 2 times, most recently from 5ee488b to ad44b12 Compare July 31, 2024 22:02

SparkiDev reviewed Jul 31, 2024

View reviewed changes

src/tls.c Outdated Show resolved Hide resolved

src/tls.c Outdated Show resolved Hide resolved

dgarske requested changes Aug 1, 2024

View reviewed changes

miyazakh removed their assignment Aug 2, 2024

dgarske requested review from SparkiDev and julek-wolfssl August 2, 2024 15:08

dgarske assigned SparkiDev and julek-wolfssl Aug 2, 2024

dgarske requested changes Aug 2, 2024

View reviewed changes

SparkiDev assigned miyazakh and unassigned SparkiDev Aug 5, 2024

miyazakh assigned wolfSSL-Bot and unassigned miyazakh Aug 30, 2024

julek-wolfssl previously approved these changes Aug 30, 2024

View reviewed changes

dgarske requested review from douzzer and removed request for dgarske September 9, 2024 15:32

JacobBarthelmeh requested changes Sep 17, 2024

View reviewed changes

dgarske requested changes Sep 17, 2024

View reviewed changes

JacobBarthelmeh assigned miyazakh and unassigned wolfSSL-Bot Sep 17, 2024

miyazakh dismissed julek-wolfssl’s stale review via d56469b September 18, 2024 09:30

miyazakh and others added 6 commits September 18, 2024 19:00

works OCSP Stapling with TLS1.3 like OCSPv2Multi

a5ecfcf

fix pr test

7f71656

TLSX_CSR_InitRequest_ex: decrement csr->requests when reusing

d7b4aac

SendTls13Certificate: set variables directly instead of incrementing

c070a45

addressed review comment

b44e8e6

fix pr unit test

fca6a40

miyazakh force-pushed the zd18141_tls13_ocsp branch from d56469b to fca6a40 Compare September 19, 2024 05:55

miyazakh assigned wolfSSL-Bot and unassigned miyazakh Sep 19, 2024

JacobBarthelmeh previously approved these changes Sep 20, 2024

View reviewed changes

JacobBarthelmeh assigned dgarske and unassigned wolfSSL-Bot Sep 20, 2024

dgarske requested changes Sep 20, 2024

View reviewed changes

dgarske assigned miyazakh and wolfSSL-Bot and unassigned dgarske Sep 20, 2024

addressed review comments

2b6ffec

miyazakh dismissed JacobBarthelmeh’s stale review via 2b6ffec September 21, 2024 07:03

miyazakh removed their assignment Sep 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add OCSP response for intermediate cert into Certificate extension on TLS1.3 #7766

Add OCSP response for intermediate cert into Certificate extension on TLS1.3 #7766

miyazakh commented Jul 19, 2024 •

edited

Loading

dgarske Jul 19, 2024

miyazakh commented Jul 24, 2024

miyazakh commented Jul 24, 2024

miyazakh commented Jul 26, 2024

dgarske commented Jul 26, 2024

dgarske left a comment

dgarske Aug 2, 2024

miyazakh Aug 4, 2024

dgarske Aug 2, 2024

miyazakh Aug 4, 2024

miyazakh Aug 4, 2024

JacobBarthelmeh Sep 17, 2024

miyazakh Sep 18, 2024

miyazakh Sep 18, 2024

dgarske Sep 17, 2024

miyazakh Sep 18, 2024

miyazakh commented Sep 19, 2024

dgarske Sep 20, 2024

miyazakh Sep 21, 2024

dgarske Sep 20, 2024

miyazakh Sep 21, 2024

dgarske Sep 20, 2024

miyazakh Sep 21, 2024

dgarske Sep 20, 2024

miyazakh Sep 21, 2024

Add OCSP response for intermediate cert into Certificate extension on TLS1.3 #7766

Are you sure you want to change the base?

Add OCSP response for intermediate cert into Certificate extension on TLS1.3 #7766

Conversation

miyazakh commented Jul 19, 2024 • edited Loading

Description

Testing

Checklist

Choose a reason for hiding this comment

miyazakh commented Jul 24, 2024

miyazakh commented Jul 24, 2024

miyazakh commented Jul 26, 2024

dgarske commented Jul 26, 2024

dgarske left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miyazakh commented Sep 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miyazakh commented Jul 19, 2024 •

edited

Loading