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

Add support for nginx-1.25.0 #6515

Merged
merged 1 commit into from
Jul 7, 2023
Merged

Conversation

julek-wolfssl
Copy link
Member

@julek-wolfssl julek-wolfssl commented Jun 15, 2023

  • Add nginx github action test
  • Implement Certificate Authorities for TLS 1.3
  • Implement secret logging for TLS 1.3
    • Can be used for example with ./configure CPPFLAGS="-DWOLFSSL_SSLKEYLOGFILE -DSHOW_SECRETS -DHAVE_SECRET_CALLBACK -DWOLFSSL_SSLKEYLOGFILE_OUTPUT='\"/tmp/secrets\"'"
  • Implement session context checking for tickets
  • Check for authorized responder in OCSP basic response
  • Fix handling call to ocsp->statusCb
  • compat: Translate SOCKET_PEER_CLOSED_E to WOLFSSL_ERROR_SYSCALL
  • Fix wolfSSL_CTX_set_session_cache_mode
    • WOLFSSL_SESS_CACHE_OFF means nothing should be on
    • WOLFSSL_SESS_CACHE_NO_INTERNAL turns off only the internal cache
  • Respect ssl->options.internalCacheOff
  • Implement SSL_SESSION_set_time
  • wolfSSL_SESSION_get_id: Return sess->altSessionID when available
  • nginx: add necessary defines and function plus some misc additions
  • Fix memory leaks
  • wolfSSL_OCSP_basic_verify: implement OCSP_TRUSTOTHER flag
  • AKID: implement matching on issuer name and serial number

src/ocsp.c Show resolved Hide resolved
src/ssl.c Outdated Show resolved Hide resolved
src/tls.c Show resolved Hide resolved
src/tls.c Show resolved Hide resolved
src/tls.c Show resolved Hide resolved
src/tls.c Show resolved Hide resolved
@julek-wolfssl
Copy link
Member Author

julek-wolfssl commented Jun 22, 2023

This PR now depends on wolfSSL/osp#146. It fixes the stunnel error.

@julek-wolfssl
Copy link
Member Author

Once wolfSSL/wolfssl-nginx#22 is merged, the nginx action in this PR will need to be changed to point at the master branch of https://github.com/wolfSSL/wolfssl-nginx.

kareem-wolfssl
kareem-wolfssl previously approved these changes Jun 23, 2023
Copy link
Contributor

@kareem-wolfssl kareem-wolfssl left a comment

Choose a reason for hiding this comment

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

Looks good to me. Would like to see the nginx patch merged first so this PR's workflow can be updated to point to the master repo as you mentioned. Once that is done and the workflow is updated, I will merge this.

@kareem-wolfssl
Copy link
Contributor

I'm ready to merge this, but there are some conflicts in api.c now, please resolve them.

@julek-wolfssl
Copy link
Member Author

retest this please

1 similar comment
@julek-wolfssl
Copy link
Member Author

retest this please

@julek-wolfssl
Copy link
Member Author

retest this please

Copy link
Contributor

@dgarske dgarske left a comment

Choose a reason for hiding this comment

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

Great work on this. Very close, just a few minor cleanups.

src/tls.c Outdated
@@ -3013,6 +3016,10 @@ static void TLSX_CSR_Free(CertificateStatusRequest* csr, void* heap)
break;
}

#ifdef WOLFSSL_TLS13
if (csr->response.buffer != NULL)
XFREE(csr->response.buffer, NULL, DYNAMIC_TYPE_TMP_BUFFER);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add heap

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a way to use a heap hint. Also removed responseBuffer from internal API's that don't use it anywhere.

src/tls.c Outdated
@@ -3187,7 +3194,13 @@ static int TLSX_CSR_Parse(WOLFSSL* ssl, const byte* input, word16 length,
ret = BUFFER_ERROR;
}
if (ret == 0) {
csr->response.buffer = (byte*)(input + offset);
csr->response.buffer = (byte*)XMALLOC(resp_length, NULL,
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add heap hint ssl->heap.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a way to use a heap hint. Also removed responseBuffer from internal API's that don't use it anywhere.

src/ocsp.c Show resolved Hide resolved
src/tls.c Show resolved Hide resolved
src/tls13.c Outdated
}
fprintf(fp, "\n");

if (fp != stderr) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Prefer just wrap this with #ifdef WOLFSSL_SSLKEYLOGFILE_OUTPUT

Copy link
Member Author

Choose a reason for hiding this comment

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

Just using WOLFSSL_SSLKEYLOGFILE_OUTPUT gating without if's.

src/tls13.c Show resolved Hide resolved
src/tls13.c Outdated
{
int i;
const char* str = NULL;
unsigned char clientRandom[32];
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use byte and RAN_LEN.

Copy link
Member Author

Choose a reason for hiding this comment

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

Changing.

@dgarske
Copy link
Contributor

dgarske commented Jul 6, 2023

@julek-wolfssl Recommend a squash on merge or on next push (28 is a lot)

@dgarske dgarske removed their assignment Jul 6, 2023
wolfssl/internal.h Show resolved Hide resolved
wolfssl/wolfcrypt/asn.h Show resolved Hide resolved
- nginx: add necessary defines and function
- Implement Certificate Authorities for TLS 1.3
- Implement secret logging for TLS 1.3. Can be used for example with:
  ./configure CPPFLAGS="-DWOLFSSL_SSLKEYLOGFILE -DSHOW_SECRETS -DHAVE_SECRET_CALLBACK -DWOLFSSL_SSLKEYLOGFILE_OUTPUT='\"/tmp/secrets\"'"
- Implement session context checking for tickets
- Check for authorized responder in OCSP basic response
- Fix handling call to ocsp->statusCb
- compat: Translate SOCKET_PEER_CLOSED_E to WOLFSSL_ERROR_SYSCALL
- Fix wolfSSL_CTX_set_session_cache_mode
  - WOLFSSL_SESS_CACHE_OFF means nothing should be on
  - WOLFSSL_SESS_CACHE_NO_INTERNAL turns off only the internal cache
- Respect ssl->options.internalCacheOff
- Implement SSL_SESSION_set_time
- wolfSSL_SSL_in_init: fix detection for TLS 1.3
- Fix handling call to ssl->alpnSelect
- SendTls13NewSessionTicket: always generate new ID
  - When we send a new ticket for the same session (for example we resumed a connection and are sending a new ticket so that the client can resume in the future), we need to generate a new ID so that we don't overwrite the old session in the cache. Overwriting the session results in the `diff` calculation in `DoClientTicketCheck()` producing the wrong value and failing to resume.
Add nginx github action test
- Fix memory leaks
- wolfSSL_OCSP_basic_verify: implement OCSP_TRUSTOTHER flag
- AKID: implement matching on issuer name and serial number
- ocsp: check for a chain match for OCSP responder
- Split CreateTicket into CreateTicket and SetupTicket
- SendCertificateStatus: free response.buffer
- Use heap hint when allocating responseBuffer
- Remove responseBuffer from internal API's that don't use it anywhere
@julek-wolfssl
Copy link
Member Author

@dgarske Rebased and squashed to one commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants