ASN macro simplification #7798

dgarske · 2024-07-26T19:18:42Z

Description

ASN macro simplification.

Added new --enable-asn=all and WOLFSSL_ASN_ALL option.
Added granular macros for ASN features like: WOLFSSL_ASN_CA_ISSUER, WOLFSSL_ASN_PARSE_KEYUSAGE, WOLFSSL_ASN_TIME_STRING, WOLFSSL_OCSP_PARSE_STATUS.
Fixed issues with SetDNSEntry and GenerateDNSEntryRIDString with possible leaks and return code checking.
Split up the WOLFSSL_SEP and WOLFSSL_CERT_EXT logic so they can be used individually.
Fix bad C89 XSNPRINTF remap.
Improved logic on ASN_BER_TO_DER.
Improved logic on unknown extension callback (new WC_ASN_UNKNOWN_EXT_CB gate).
Fail with NOT_COMPILED_IN if someone tries to use ConfirmSignature with NO_ASN_CRYPT.
Fixed benchmark.c error without ChaCha and unused encrypt_only.

Testing

./configure --enable-asn=all && make check
./configure --enable-asn=all --enable-opensslall && make check
./configure --enable-asn=all --enable-all && make check

./configure --enable-asn=all,original && make check
./configure --enable-asn=all,original --enable-opensslall && make check
./configure --enable-asn=all,original --enable-all && make check

./configure --enable-asn=all --enable-cryptonly && make check
./configure --enable-asn=all,original --enable-cryptonly && make check

./configure --disable-shared --enable-asn=template,nocrypt --enable-cryptonly && make check

Checklist

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

bandi13 · 2024-07-29T14:24:34Z

retest this please

dgarske · 2024-07-29T20:58:18Z

Retest this please

…SN_ALL` option. Added granular macros for ASN features like: `WOLFSSL_ASN_CA_ISSUER`, `WOLFSSL_ASN_PARSE_KEYUSAGE`, `WOLFSSL_ASN_TIME_STRING`, `WOLFSSL_OCSP_PARSE_STATUS`.

…-curl` and `--enable-openssh`).

dgarske · 2024-07-30T18:40:44Z

Retest this please. Found cause for intermittent test_wolfSSL_Tls12_Key_Logging_test failure (new PR to follow).

dgarske · 2024-07-30T18:57:04Z

@SparkiDev and @bandi13 this PR is ready. It also resolves a C89 issue detected in @douzzer multi-test:

[allcryptonly-gcc-c89] [14 of 271] [7da6149250]
    configure...   real 0m10.701s  user 0m3.762s  sys 0m8.085s
    build...In file included from ./wolfssl/wolfcrypt/error-crypt.h:34,
                 from wolfcrypt/src/asn.c:105:
wolfcrypt/src/asn.c: In function ‘GetFormattedTime’:
f9dc5e9f4d (<[[email protected]](mailto:[email protected])> 2024-07-29 13:26:04 -0700 835)                 #define XSNPRINTF(f, len, ...) sprintf(f, ...)
./wolfssl/wolfcrypt/types.h:835:59: error: expected expression before ‘...’ token
  835 |                 #define XSNPRINTF(f, len, ...) sprintf(f, ...)
      |                                                           ^~~

SparkiDev · 2024-07-30T23:43:04Z

./configure --disable-shared --enable-asn=template,nocrypt

'RsaPssHashOidTosigOid' defined but not used.

…`. Improved logic on unknown extension callback (new `WC_ASN_UNKNOWN_EXT_CB` gate).

SparkiDev · 2024-07-31T22:43:49Z

./configure --disable-shared --enable-asn=template,nocrypt
make test fails - unit.test is running API tests that fail (test_wc_CreateEncryptedPKCS8Key, test_EccSigFailure_cm, test_RsaSigFailure_cm)

dgarske · 2024-07-31T22:59:25Z

./configure --disable-shared --enable-asn=template,nocrypt make test fails - unit.test is running API tests that fail (test_wc_CreateEncryptedPKCS8Key, test_EccSigFailure_cm, test_RsaSigFailure_cm)

Doubt that works on master with CFLAGS="-DNO_ASN_CRYPT", but I'll get it passing. Thanks @SparkiDev

…tificate signature checking, so make check TLS expected failures do not pass. Cleanup of the api.c headers / macros.

dgarske · 2024-08-01T18:14:35Z

Retest this please.

Seems to be unstable FIPS test:

./configure --enable-fips=v5 --enable-sha3 --enable-aesni CFLAGS="-DWOLFSSL_PUBLIC_MP -g -O0"
Error: wolfacvp_client -D 140-3-known-tests -K

douzzer

This is some nice cleanup!

testing uncovered this:

[quantum-safe-wolfssl-all-g++-latest] [6 of 32] [9911392b22]
    configure...   real 0m23.619s  user 0m10.294s  sys 0m15.108s
    build...In file included from tests/api.c:63:
tests/api.c: In function ‘int test_GENERAL_NAME_set0_othername()’:
e542e51d9f (<[email protected]> 2023-06-05 17:39:39 +1000 57494)     ExpectNotNull(gns = X509_get_ext_d2i(x509, NID_subject_alt_name, NULL,
tests/api.c:57494:41: error: invalid conversion from ‘void*’ to ‘GENERAL_NAMES*’ {aka ‘WOLFSSL_STACK*’} [-fpermissive]
57494 |     ExpectNotNull(gns = X509_get_ext_d2i(x509, NID_subject_alt_name, NULL,
e467112a93 (<[email protected]> 2023-06-13 16:36:10 +1000 143)     if (_ret != TEST_FAIL) { if (!(test)) ExpFail(description, result);        \
./tests/unit.h:143:36: note: in definition of macro ‘Expect’
  143 |     if (_ret != TEST_FAIL) { if (!(test)) ExpFail(description, result);        \
      |                                    ^~~~
e542e51d9f (<[email protected]> 2023-06-05 17:39:39 +1000 57494)     ExpectNotNull(gns = X509_get_ext_d2i(x509, NID_subject_alt_name, NULL,
tests/api.c:57494:5: note: in expansion of macro ‘ExpectNotNull’
57494 |     ExpectNotNull(gns = X509_get_ext_d2i(x509, NID_subject_alt_name, NULL,
      |     ^~~~~~~~~~~~~
tests/api.c: In function ‘int test_othername_and_SID_ext()’:
e542e51d9f (<[email protected]> 2023-06-05 17:39:39 +1000 57658)     ExpectNotNull(gns = X509_get_ext_d2i(x509, NID_subject_alt_name, NULL,
tests/api.c:57658:41: error: invalid conversion from ‘void*’ to ‘GENERAL_NAMES*’ {aka ‘WOLFSSL_STACK*’} [-fpermissive]
57658 |     ExpectNotNull(gns = X509_get_ext_d2i(x509, NID_subject_alt_name, NULL,
e467112a93 (<[email protected]> 2023-06-13 16:36:10 +1000 143)     if (_ret != TEST_FAIL) { if (!(test)) ExpFail(description, result);        \
./tests/unit.h:143:36: note: in definition of macro ‘Expect’
  143 |     if (_ret != TEST_FAIL) { if (!(test)) ExpFail(description, result);        \
      |                                    ^~~~
e542e51d9f (<[email protected]> 2023-06-05 17:39:39 +1000 57658)     ExpectNotNull(gns = X509_get_ext_d2i(x509, NID_subject_alt_name, NULL,
tests/api.c:57658:5: note: in expansion of macro ‘ExpectNotNull’
57658 |     ExpectNotNull(gns = X509_get_ext_d2i(x509, NID_subject_alt_name, NULL,
      |     ^~~~~~~~~~~~~

(note that line numbers are after rebase on current master, 15e99c8)

Also several overlong lines, some of which are probably best left overlong:

/src/x509.c:5677                 case NID_certificate_policies: crit = x509->certPolicyCrit; break;
/tests/api.c:341     #if (defined(HAVE_ECC112) || defined(HAVE_ALL_CURVES)) && ECC_MIN_KEY_SZ <= 112
/tests/api.c:346     #if (defined(HAVE_ECC128) || defined(HAVE_ALL_CURVES)) && ECC_MIN_KEY_SZ <= 128
/tests/api.c:351     #if (defined(HAVE_ECC160) || defined(HAVE_ALL_CURVES)) && ECC_MIN_KEY_SZ <= 160
/tests/api.c:356     #if (defined(HAVE_ECC192) || defined(HAVE_ALL_CURVES)) && ECC_MIN_KEY_SZ <= 192
/wolfssl/ocsp.h:72                                               WOLFSSL_OCSP_CERTID *id, int *status, int *reason,
/wolfssl/ocsp.h:73                                               WOLFSSL_ASN1_TIME **revtime, WOLFSSL_ASN1_TIME **thisupd,
/wolfssl/wolfcrypt/settings.h:2943     /* Store pointers to issuer name components and their lengths and encodings. */
/wolfssl/wolfcrypt/settings.h:3033         #error ASN unknown extension callback is only supported with ASN template

…nchmark.c error without ChaCha and unused encrypt_only.

SparkiDev · 2024-08-01T23:21:29Z

./configure --disable-shared --enable-asn=template,nocrypt
Fails in unit.test when using server-cert-rsa-badsig.pem as the handshake goes through.
Shouldn't certificates be disabled or fail because there is no support for decoding keys?

dgarske · 2024-08-01T23:58:06Z

./configure --disable-shared --enable-asn=template,nocrypt Fails in unit.test when using server-cert-rsa-badsig.pem as the handshake goes through. Shouldn't certificates be disabled or fail because there is no support for decoding keys?

That is correct. With NO_ASN_CRYPT the signature check always report success. I am not sure we should allow this build option with TLS. To get make check to pass I had to disable all of the TLS tests. Thoughts?

dgarske · 2024-08-01T23:58:58Z

My preference would be to return NOT_COMPILED_IN and only support it for wolfCrypt only.

dgarske · 2024-08-02T15:06:07Z

I've updated the PR to return NOT_COMPILED_IN (instead of 0=success) in ConfirmSignature for NO_ASYNC_CRYPT and updated the test description for nocrypt to use ./configure --disable-shared --enable-asn=template,nocrypt --enable-cryptonly && make check

…th NO_ASN_CRYPT. Also default to signature failed.

dgarske requested review from bandi13 and SparkiDev July 26, 2024 19:18

dgarske self-assigned this Jul 26, 2024

dgarske force-pushed the asn_macros branch from 0989f41 to c5eb361 Compare July 26, 2024 22:28

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

dgarske force-pushed the asn_macros branch 3 times, most recently from 3ea0fa5 to ad497dd Compare July 27, 2024 01:53

dgarske force-pushed the asn_macros branch 2 times, most recently from b20605c to 85f6b6a Compare July 29, 2024 20:31

dgarske assigned SparkiDev and unassigned dgarske Jul 29, 2024

dgarske added 4 commits July 30, 2024 10:35

ASN macro simplification. Added new --enable-asn=all and `WOLFSSL_A…

20f7d6f

…SN_ALL` option. Added granular macros for ASN features like: `WOLFSSL_ASN_CA_ISSUER`, `WOLFSSL_ASN_PARSE_KEYUSAGE`, `WOLFSSL_ASN_TIME_STRING`, `WOLFSSL_OCSP_PARSE_STATUS`.

Fixes for building due to missing OCSP and DecodePolicyOID (`--enable…

afb6fe6

…-curl` and `--enable-openssh`).

Fixes for clang-tidy.

bbbc1e0

Fix bad C89 XSNPRINTF remap.

877c1d7

dgarske force-pushed the asn_macros branch from 85f6b6a to 877c1d7 Compare July 30, 2024 17:42

dgarske assigned bandi13 Jul 30, 2024

dgarske unassigned bandi13 Jul 30, 2024

SparkiDev previously approved these changes Jul 30, 2024

View reviewed changes

SparkiDev assigned dgarske and unassigned SparkiDev Jul 30, 2024

Fixed issues building with nocrypt. Improved logic on `ASN_BER_TO_DER…

548a2c6

…`. Improved logic on unknown extension callback (new `WC_ASN_UNKNOWN_EXT_CB` gate).

dgarske assigned SparkiDev and unassigned SparkiDev Jul 31, 2024

SparkiDev assigned dgarske and unassigned SparkiDev Jul 31, 2024

Improvement for the --enable-asn=nocrypt. Note: This option skips cer…

65283fb

…tificate signature checking, so make check TLS expected failures do not pass. Cleanup of the api.c headers / macros.

dgarske assigned SparkiDev Aug 1, 2024

dgarske assigned bandi13 and unassigned dgarske Aug 1, 2024

douzzer requested changes Aug 1, 2024

View reviewed changes

dgarske assigned SparkiDev and dgarske and unassigned bandi13 and SparkiDev Aug 1, 2024

Fixes for minor implicit cast warnings and line length. Also fixed be…

a18d016

…nchmark.c error without ChaCha and unused encrypt_only.

dgarske requested a review from douzzer August 1, 2024 22:34

dgarske assigned SparkiDev Aug 1, 2024

SparkiDev removed their assignment Aug 1, 2024

Fail with NOT_COMPILED_IN if someone tries to use ConfirmSignature wi…

25d14f1

…th NO_ASN_CRYPT. Also default to signature failed.

dgarske force-pushed the asn_macros branch from a72156f to 25d14f1 Compare August 2, 2024 15:25

douzzer approved these changes Aug 2, 2024

View reviewed changes

douzzer merged commit 9aa0742 into wolfSSL:master Aug 2, 2024
93 of 125 checks passed

dgarske deleted the asn_macros branch August 13, 2024 19:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ASN macro simplification #7798

ASN macro simplification #7798

dgarske commented Jul 26, 2024 •

edited

Loading

bandi13 commented Jul 29, 2024

dgarske commented Jul 29, 2024

dgarske commented Jul 30, 2024

dgarske commented Jul 30, 2024

SparkiDev commented Jul 30, 2024

SparkiDev commented Jul 31, 2024

dgarske commented Jul 31, 2024

dgarske commented Aug 1, 2024

douzzer left a comment

SparkiDev commented Aug 1, 2024

dgarske commented Aug 1, 2024

dgarske commented Aug 1, 2024

dgarske commented Aug 2, 2024

ASN macro simplification #7798

ASN macro simplification #7798

Conversation

dgarske commented Jul 26, 2024 • edited Loading

Description

Testing

Checklist

bandi13 commented Jul 29, 2024

dgarske commented Jul 29, 2024

dgarske commented Jul 30, 2024

dgarske commented Jul 30, 2024

SparkiDev commented Jul 30, 2024

SparkiDev commented Jul 31, 2024

dgarske commented Jul 31, 2024

dgarske commented Aug 1, 2024

douzzer left a comment

Choose a reason for hiding this comment

SparkiDev commented Aug 1, 2024

dgarske commented Aug 1, 2024

dgarske commented Aug 1, 2024

dgarske commented Aug 2, 2024

dgarske commented Jul 26, 2024 •

edited

Loading