Small stack compress -- 3000line reduction

[effbiae]

Description

There are many lines of small stack conditional compiles (#ifdef WOLFSSL_SMALL_STACK) that have been compressed to fewer lines using the macros in ./wolfssl/wolfcrypt/types.h. This work was automated with a python script and reduces the code base by 3000 lines. The first commit is the result of the script and subsequent commits are hand edits.

I added two macros to ./wolfssl/wolfcrypt/types.h:

• WC_ALLOC_VAR_EX to take 2 extra parameters
• WC_FREE_VAR_EX to take 1 extra parameter

Testing

./configure --enable-all && make check && \
./configure --enable-all --enable-smallstack  && make check && \
./configure --enable-all --enable-asynccrypt  && make check && \
./configure --enable-all  --enable-smallstack --enable-asynccrypt && make check

Checklist

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

[wolfSSL-Bot]

Can one of the admins verify this patch?

[dgarske]

Okay to test. Contributor agreement on file.

[effbiae]

please retest. fixed an error picked up in CI

[effbiae]

hmmm, what's happened in the failing test?

[dgarske]

hmmm, what's happened in the failing test?

All of the FIPS 140-3 tests are failing. You'd have to get the fips-ready bundle and build it with the --enabe-fips=ready option to reproduce. But hopefully you can spot it with the compile error:

If something fails check the log file: /tmp/workspace/wolfSSL/PRB-140-3-tests/wolfssl-fips/test-#1.log
configure step: #1
make step: #1
fips-hash.sh: #1
make check: #1
Updated 1 path from the index
+ sed -i.bak 's%^INSTALL_PATH.*%INSTALL_PATH=/tmp/workspace/wolfSSL/PRB-140-3-tests/wolfssl-fips/fips140-3-install%' /tmp/workspace/wolfSSL/PRB-140-3-tests/wolfssl-fips/fips/optest-140-3/Makefile
+ sed -i.bak 's%^#CPPFLAGS+=%CPPFLAGS+=%' /tmp/workspace/wolfSSL/PRB-140-3-tests/wolfssl-fips/fips/optest-140-3/Makefile
+ set +x
/tmp/workspace/wolfSSL/PRB-140-3-tests/wolfssl-fips/fips140-3-install/include/wolfssl/wolfcrypt/types.h:742: note: macro "WC_DECLARE_VAR" defined here
  742 |     #define WC_DECLARE_VAR(VAR_NAME, VAR_TYPE, VAR_SIZE) \
      | 
test.c:32378:54: error: ‘sharedA’ undeclared (first use in this function)
32378 |             ret = wc_ecc_shared_secret(userA, userB, sharedA, &x);
      |                                                      ^~~~~~~
test.c:32390:54: error: ‘sharedB’ undeclared (first use in this function)
32390 |             ret = wc_ecc_shared_secret(userB, userA, sharedB, &y);
      |                                                      ^~~~~~~
test.c:32446:37: error: ‘exportBuf’ undeclared (first use in this function)
32446 |     ret = wc_ecc_export_x963(userA, exportBuf, &x);
      |                                     ^~~~~~~~~
test.c:32578:9: error: ‘digest’ undeclared (first use in this function); did you mean ‘dirent’?
32578 |         digest[i] = 0;
      |         ^~~~~~
      |         dirent
test.c:32587:61: error: ‘sig’ undeclared (first use in this function)
32587 |             ret = wc_ecc_sign_hash(digest, ECC_DIGEST_SIZE, sig, &x, rng,
      |                                                             ^~~
make: *** [<builtin>: test.o] Error 1
Error: optest build
script returned exit code 1

[effbiae]

my changes to wolfssl/wolfcrypt/types.h may be conflicting with the fips version

i've got the fips-ready zip and can configure and make install it according to instructions

can you share the steps to make check the latest source with the fips library? or maybe the script that produced the error output above?

[effbiae]

All of the FIPS 140-3 tests are failing. You'd have to get the fips-ready bundle and build it with the --enabe-fips=ready option to reproduce.

can you please send me the script that fails so i can reproduce?

[dgarske]

Jenkins retest this please. History lost. @effbiae I'll work on trying to provide you a way to reproduce, but it might not be possible. In fips and self test modes it uses an older snapshot of the wolfCrypt code.

[effbiae]

i reduced my impact on wolfssl/wolfcrypt/types.h -- is PRB-master-job still failing on FIPS tests?

[dgarske]

Jenkins retest this please

[dgarske]

This is a great refactor!

Not a critical issue, but we prefer a 80 character limit:

overlong lines added:
src/internal.c:15902                         WC_FREE_VAR_EX(dCertAdd, ssl->heap, DYNAMIC_TYPE_TMP_BUFFER);
src/internal.c:33148                                 WC_DECLARE_VAR(encodedSig, byte, MAX_ENCODED_SIG_SZ, 0);
src/internal.c:33165                                 WC_FREE_VAR_EX(encodedSig, ssl->heap, DYNAMIC_TYPE_SIGNATURE);
src/internal.c:39038                             WC_FREE_VAR_EX(encodedSig, ssl->heap, DYNAMIC_TYPE_SIGNATURE);
wolfcrypt/src/pkcs7.c:4446                     WC_FREE_VAR_EX(digest, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER);
wolfcrypt/src/pkcs7.c:4561                 WC_FREE_VAR_EX(pkcs7Digest, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER);
wolfcrypt/src/pkcs7.c:4574                 WC_FREE_VAR_EX(pkcs7Digest, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER);
wolfcrypt/src/pkcs7.c:4587                 WC_FREE_VAR_EX(pkcs7Digest, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER);
wolfcrypt/src/pkcs7.c:10166                     WC_FREE_VAR_EX(serialNum, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER);
wolfcrypt/src/pkcs7.c:10344                                     WC_FREE_VAR_EX(privKey, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER);
[...and more...]

[effbiae]

This is a great refactor!

thanks, i like being useful :)

Not a critical issue, but we prefer a 80 character limit:

fixed, thanks!

what happened in PRB-master-job?

[dgarske]

Jenkins retest this please. Some tests failed with "AgentOfflineException" and another "RequestAbortedException"

[dgarske]

Jenkins retest this please. History lost

[SparkiDev]

retest this please

[douzzer]

This looks really good.

cppcheck discovered one new defect, easy to fix:

src/ssl.c:5916:19: warning: Either the condition 'cert==((void*)0)' is redundant or there is possible null pointer dereference: cert. [nullPointerRedundantCheck]
    subjectHash = cert->extSubjKeyId;
                  ^
src/ssl.c:5892:14: note: Assuming that condition 'cert==((void*)0)' is not redundant
    if (cert == NULL) {
             ^
src/ssl.c:5916:19: note: Null pointer dereference
    subjectHash = cert->extSubjKeyId;
                  ^

It's definitely a false positive -- your WC_ALLOC_VAR_EX() does the right thing -- but cppcheck isn't grokking the expanded macro there (common failure mode for that tool, alas). You can fix it by passing an empty failure expression to your new macro, and right after, explicitly checking for and returning failure on null, as in the incumbent code.

Note the line numbers are after rebasing the PR on current master.

[douzzer]

Note there's also some indentation flubs after your Python script had its way with the source -- worth cleaning up.

[effbiae]

cppcheck discovered one new defect, easy to fix:

hopefully fixed.

indentation problems

i've looked through the diffs again but couldn't pick up anything

[effbiae]

indentation problems

i've looked through the diffs again but couldn't pick up anything

ah, i think i know what you mean. david picked it up too, but let it through. have manually edited the few cases.

[dgarske]

Jenkins retest this please

[dgarske]

Jenkins retest this please. FIPS v2

[dgarske]

Jenkins retest this please: "fipsv2-OE-ready" -> Error: OE2: without PAA test failed. This is an recurring intermittent issue.