Skip to content

Enable TFM mp_sqr even when HAVE_ECC disabled#7333

Merged
dgarske merged 1 commit intowolfSSL:masterfrom
gojimmypi:PR-tfm-mp_sqr
Mar 14, 2024
Merged

Enable TFM mp_sqr even when HAVE_ECC disabled#7333
dgarske merged 1 commit intowolfSSL:masterfrom
gojimmypi:PR-tfm-mp_sqr

Conversation

@gojimmypi
Copy link
Copy Markdown
Contributor

@gojimmypi gojimmypi commented Mar 13, 2024

Description

This may not be essential to release... however during ESP8266 testing, I realized the wolfssl_test expects to see the TFM mp_sqr() even when HAVE_ECC is not defined.

  • edit:

See my user_settings.h used in the Espressif wolfssl_test example.

Here's the error I see otherwise:

App "wolfssl_test" version: v5.6.6-stable-2210-g1b1306875-d
CC build/Debug/wolfssl/wolfcrypt/test/test.o
AR build/Debug/wolfssl/libwolfssl.a
Generating esp8266.project.ld
LD build/Debug/wolfssl_test.elf
c:/sysgcc/esp8266/opt/xtensa-lx106-elf/bin/../lib/gcc/xtensa-lx106-elf/8.4.0/../../../../xtensa-lx106-elf/bin/ld.exe: C:/workspace/wolfssl-gojimmypi/IDE/Espressif/ESP-IDF/examples/wolfssl_test/build/Debug/wolfssl\libwolfssl.a(test.o):(.literal.mp_test+0x38c): undefined reference to `mp_sqr'
c:/sysgcc/esp8266/opt/xtensa-lx106-elf/bin/../lib/gcc/xtensa-lx106-elf/8.4.0/../../../../xtensa-lx106-elf/bin/ld.exe: C:/workspace/wolfssl-gojimmypi/IDE/Espressif/ESP-IDF/examples/wolfssl_test/build/Debug/wolfssl\libwolfssl.a(test.o): in function `mp_test_prime':
C:/workspace/wolfssl-gojimmypi/wolfcrypt/test/test.c:47134: undefined reference to `mp_sqr'
c:/sysgcc/esp8266/opt/xtensa-lx106-elf/bin/../lib/gcc/xtensa-lx106-elf/8.4.0/../../../../xtensa-lx106-elf/bin/ld.exe: C:/workspace/wolfssl-gojimmypi/wolfcrypt/test/test.c:47147: undefined reference to `mp_sqr'
c:/sysgcc/esp8266/opt/xtensa-lx106-elf/bin/../lib/gcc/xtensa-lx106-elf/8.4.0/../../../../xtensa-lx106-elf/bin/ld.exe: C:/workspace/wolfssl-gojimmypi/IDE/Espressif/ESP-IDF/examples/wolfssl_test/build/Debug/wolfssl\libwolfssl.a(test.o): in function `mp_test_invmod':
C:/workspace/wolfssl-gojimmypi/wolfcrypt/test/test.c:47507: undefined reference to `mp_sqr'
collect2.exe: error: ld returned 1 exit status
make: *** [/rtos-sdk/v3.4/make/project.mk:510: /C/workspace/wolfssl-gojimmypi/IDE/Espressif/ESP-IDF/examples/wolfssl_test/build/Debug/wolfssl_test.elf] Error 1
-------------------------------------------------------------
Command exited with code 2

In particular line 47147 of test.c when calling mp_prime_is_prime_ex. See also test.c line 47416.

Fixes zd# n/a

Testing

How did you test?

Not fully tested. Proposed last-minute inclusion in next release.

Checklist

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

@JacobBarthelmeh
Copy link
Copy Markdown
Contributor

Hi @gojimmypi , I suspect we might need additional macro gating in the test cases? To only test mp_sqr() when it is available. Reproduced with ./configure --enable-opensslextra --disable-ecc --enable-fastmath CPPFLAGS=-DWOLFSSL_PUBLIC_MP

@dgarske
Copy link
Copy Markdown
Member

dgarske commented Mar 14, 2024

Reproduced with ./configure --disable-ecc --enable-fastmath --enable-keygen --enable-opensslextra CFLAGS="-DWOLFSSL_PUBLIC_MP" && make

Copy link
Copy Markdown
Member

@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.

Thanks Jim.

It was incredibly difficult to find the right combination of options to reproduce. But was able to get it ./configure --disable-ecc --enable-fastmath --enable-keygen --enable-opensslextra CFLAGS="-DWOLFSSL_PUBLIC_MP" && make.

Copy link
Copy Markdown
Contributor

@JacobBarthelmeh JacobBarthelmeh left a comment

Choose a reason for hiding this comment

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

On second thought, having the math libraries consistent with provided API when building with WOLFSSL_PUBLIC_MP is likely better than having the test case conditionally test the mp_sqr API. Rerunning this without --enable-fastmath hits the same test case (no compile errors).

@dgarske dgarske merged commit b7b6752 into wolfSSL:master Mar 14, 2024
@gojimmypi
Copy link
Copy Markdown
Contributor Author

It was incredibly difficult to find the right combination of options to reproduce.

Sorry for the challenge. I was hoping the referenced user_setings.h would have sufficed.

only test mp_sqr() when it is available.

Let me know if I can polish this up with any additional tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

For This Release Release version 5.9.1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants