Skip to content

Add --enable-quic to --enable-all #6957

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

Merged
merged 1 commit into from
Nov 10, 2023
Merged

Conversation

lealem47
Copy link
Contributor

@lealem47 lealem47 commented Nov 9, 2023

Description

Add --enable-quic to --enable-all

Testing

Make check

Checklist

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

@lealem47 lealem47 self-assigned this Nov 9, 2023
@lealem47 lealem47 changed the title Add --enable-haproxy and --enable-quic to --enable-all Add --enable-quic to --enable-all Nov 9, 2023
@lealem47
Copy link
Contributor Author

lealem47 commented Nov 9, 2023

Jenkins Retest this please

@lealem47 lealem47 requested a review from wolfSSL-Bot November 10, 2023 00:04
@lealem47 lealem47 assigned wolfSSL-Bot and unassigned lealem47 Nov 10, 2023
WOLFSSL_MSG("Got Peer cert ASN PARSE_E, BUFFER E, MEMORY_E");
ret == MEMORY_E || ret == BAD_FUNC_ARG) {
WOLFSSL_MSG("Got Peer cert ASN_PARSE_E, BUFFER_E, MEMORY_E,"
" BAD_FUNC_ARG");
Copy link
Contributor

Choose a reason for hiding this comment

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

Was there a configure option failing with BAD_FUNC_ARG once quic was added to enable-all. (tried with --enable-all --enable-quic && make check and did not see it). The change alters a lot of behavior such as ssl->peerVerifyRet value, verify callback calls, and potentially sending fatal cert alert.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It wasn't failing make check. It was a clang sanitizer error in the multi-test. If ProcessPeerCertParse() returned BAD_FUNC_ARG, the code would enter the XMEMCPY on line 14494 with NULL parameters. I can move the BAD_FUNC_ARG check below the verify callback calls. I could also remove this change all together (since the error showed up when --enable-haproxy was part of this PR, and it no longer is) but we should definitely be checking for BAD_FUNC_ARG here

Copy link
Contributor

@JacobBarthelmeh JacobBarthelmeh Nov 10, 2023

Choose a reason for hiding this comment

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

Looking at the BAD_FUNC_ARG return from ProcessPeerCertParse it looks like the case where that would be encountered is if args->dCert is NULL. In that case I think this is the right change in that the verify callbacks would not be used since there is no dCert set.

WOLFSSL_MSG("Got Peer cert ASN PARSE_E, BUFFER E, MEMORY_E");
ret == MEMORY_E || ret == BAD_FUNC_ARG) {
WOLFSSL_MSG("Got Peer cert ASN_PARSE_E, BUFFER_E, MEMORY_E,"
" BAD_FUNC_ARG");
Copy link
Contributor

@JacobBarthelmeh JacobBarthelmeh Nov 10, 2023

Choose a reason for hiding this comment

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

Looking at the BAD_FUNC_ARG return from ProcessPeerCertParse it looks like the case where that would be encountered is if args->dCert is NULL. In that case I think this is the right change in that the verify callbacks would not be used since there is no dCert set.

@dgarske dgarske merged commit 12878fc into wolfSSL:master Nov 10, 2023
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