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

Not add a cert to CA cache if it doesn't set "CA:TRUE" as basic constraints #8060

Merged
merged 1 commit into from
Nov 16, 2024

Conversation

miyazakh
Copy link
Contributor

Description

Not add a cert to CA cache if it doesn't have CA:TRUE as basic constraints. The behavior is enabled when OPENSSL_ALL is defined. This change is needed for qt nightly Jenkins test failure fix.

Fix trusted peer cert cache
It could not add a cert to trusted peer cert cache if the cert has the same subject as pre-added cert. For example, ./certs/server-ecc-self.pem
The cert above has the same subject to server-ecc.pem. Therefore, it could not add the cert to cache if there is "server-ecc.pem" in trusted peer cert cache already. This was revealed after changing "Not add a cert to CA cache".

Testing

Qt jenkins test. Unit test

Checklist

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

@miyazakh miyazakh self-assigned this Oct 10, 2024
@miyazakh miyazakh force-pushed the qt_jenkins_failure branch 2 times, most recently from 9696f39 to e8074ed Compare October 11, 2024 03:58
@miyazakh miyazakh changed the title Not add a cert to CA cache if it doesn't have "CA:TRUE" as basic constraints Not add a cert to CA cache if it doesn't set "CA:TRUE" as basic constraints Oct 11, 2024
@miyazakh
Copy link
Contributor Author

retest this please

@miyazakh miyazakh assigned wolfSSL-Bot and unassigned miyazakh Oct 11, 2024
Copy link
Contributor

@douzzer douzzer left a comment

Choose a reason for hiding this comment

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

Found a few nits in certs/intermediate/ca_false_intermediate/gentestcert.sh.

With --enable-all CFLAGS='-DWOLFSSL_X509_STRICT', I'm seeing the follow unit test failures (rebase on 3e1f365 to assure line numbers match):

   686: test_wolfSSL_OCSP_REQ_CTX                           :
ERROR - tests/api.c line 76037 failed with:
    expected: wolfSSL_X509_LOOKUP_load_file(lookup, "certs/ocsp/server1-cert.pem", WOLFSSL_FILETYPE_PEM) == 1
    result:   0 != 1

[...]

   793: test_wolfSSL_CertManagerLoadCABuffer_ex             :
ERROR - tests/api.c line 3037 failed with:
    expected: ret == (ASN_AFTER_DATE_E)
    result:   -357 != -151

[...]

   848: test_wolfSSL_CTX_load_verify_locations              :
ERROR - tests/api.c line 2573 failed with:
    expected: wolfSSL_CTX_load_verify_locations_ex(ctx, ((void *)0), load_expired_path, 0x00000002 | 0x00000004) == WOLFSSL_SUCCESS
    result:   0 != 1


# Script for generating RSA CA and server certs based on it.
#
SEVER_PEM='test_sign_bynoca_srv.pem'
Copy link
Contributor

Choose a reason for hiding this comment

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

misspelled -- should be SERVER_PEM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks.

# Server
openssl x509 -in $SRV_CRT_HOME/server.crt -inform PEM -noout -text > $SRV_CRT_HOME/server.pem
cat $SRV_CRT_HOME/server.crt >> $SRV_CRT_HOME/server.pem
mv $SRV_CRT_HOME/server.pem $SEVER_PEM
Copy link
Contributor

Choose a reason for hiding this comment

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

misspelled -- should be SERVER_PEM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks

rm -f wolfssl_ca.conf
rm -f wolfssl_int_ca.conf
rm -rf pki/
exit 0
Copy link
Contributor

Choose a reason for hiding this comment

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

the exit 0 here is masking the echo "Completed" at the end of the script.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved "exit"

@douzzer douzzer assigned miyazakh and unassigned wolfSSL-Bot Oct 15, 2024
@miyazakh
Copy link
Contributor Author

Fixed unit test failures with --enable-all CFLAGS='-DWOLFSSL_X509_STRICT'

@miyazakh miyazakh assigned douzzer and unassigned miyazakh Oct 16, 2024
@cconlon
Copy link
Member

cconlon commented Oct 28, 2024

@miyazakh Please fix merge conflicts, then re-assign to @douzzer / @wolfSSL-Bot, thanks.

@cconlon cconlon assigned miyazakh and unassigned douzzer Oct 28, 2024
@miyazakh miyazakh force-pushed the qt_jenkins_failure branch 2 times, most recently from 68cb00c to 370b925 Compare November 1, 2024 02:11
@miyazakh
Copy link
Contributor Author

miyazakh commented Nov 1, 2024

Re-visited this PR based on PR8087.
Removed WOLFSSL_X509_STRICT macro and WOLFSSL_MUST_BE_CA enum

@miyazakh miyazakh assigned douzzer and wolfSSL-Bot and unassigned miyazakh Nov 1, 2024
@dgarske dgarske requested a review from douzzer November 1, 2024 17:56
Copy link
Contributor

@douzzer douzzer left a comment

Choose a reason for hiding this comment

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

found some more spelling errors that need fixing before merge:

certs/intermediate/ca_false_intermediate/gentestcert.sh:142: genereated ==> generated
certs/intermediate/ca_false_intermediate/wolfssl_base.conf:29: Extention ==> Extension
certs/intermediate/ca_false_intermediate/wolfssl_base.conf:57: internaly ==> internally
certs/intermediate/ca_false_intermediate/wolfssl_srv.conf:2: genrate ==> generate
src/x509_str.c:409: curren ==> current

@douzzer douzzer assigned miyazakh and unassigned wolfSSL-Bot Nov 4, 2024
@miyazakh
Copy link
Contributor Author

miyazakh commented Nov 5, 2024

Thanks!

fix trusted peer cert cache
@dgarske
Copy link
Contributor

dgarske commented Nov 15, 2024

Retest this please. History for PRB lost

@douzzer douzzer merged commit 49393ec into wolfSSL:master Nov 16, 2024
143 checks passed
@miyazakh miyazakh deleted the qt_jenkins_failure branch November 17, 2024 11:51
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.

5 participants