wolfSSL / Arduino-wolfSSL

This repository is a restructured copy of https://github.com/wolfSSL/wolfssl/ for the Arduino environment. Any Pull Requests for code changes should be opened there.
https://www.wolfssl.com
GNU General Public License v2.0
11 stars 7 forks source link

Initial Official wolfSSL Arduino Library #1

Closed gojimmypi closed 6 months ago

gojimmypi commented 7 months ago

Official wolfSSL Arduino Library

This PR extracts wolfSSL from https://github.com/wolfSSL/wolfssl and restructures the files in a format needed for Arduino libraries as found at https://github.com/arduino/library-registry.

image

This is related to the transition of the 3rd party onelife/Arduino_wolfssl and the PlatformIO https://github.com/platformio/platformio-registry/issues/85 namespace request to an Official wolfSSL repository. This is also related to Arduino support ticket #282646.

The wolfSSL Arduino examples can be found at wolfssl/IDE/ARDUINO are still being improved at the time of this PR and will be updated soon. Please see the additional wolfSSL platforms available.

Are you an Arduino fan? Please take this initial library for a test drive and let us know how it goes. Pull requests and new issues are welcome. Want to install your own library? I have this install script update that will be coming soon to a PR. See the README.md file there for how this file set was generated.

We at wolfSSL would like to thank everyone that has been involved in helping spread the word about the value of wolfSSL in various maker platforms such as Arduino and PlatformIO. We look forward to taking over the control, responsibility, updates, and support of Official wolfSSL source code for those environments.

dgarske commented 7 months ago

Why not use a submodule for the wolfSSL files? Maintaining a copy of our wolfSSL sources is not ideal.

gojimmypi commented 7 months ago

That's an excellent question. I agree that maintaining a copy is not ideal. However, the current structure of the core wolfSSL library is not compatible with the requirements of Arduino libraries as described in the Arduino Library Specification.

Even if the structure would work, the build system of Arduino does not lend itself to excluding source files, such as the assembly source code in wolfcrypt. See the wolfssl forum for a typical problem encountered when using the wolfSSL repo as-is, and my suggested solution that is needed to massage wolfSSL.

dgarske commented 7 months ago

I believe Arduino has a library setup for wolfSSL and we have a script that combines all the sources and headers together. Might it be easy to require / have a script to do this from a sub-module or wolfssl directory environment variable. Would recommend a .bat and .sh version.

gojimmypi commented 7 months ago

I believe Arduino has a library setup for wolfSSL

Well, that's not an official wolfSSL library. It points to https://github.com/onelife/Arduino_wolfssl that does a similar restructuring of source code as found in this new repo. However, that one doesn't even compile, as noted in https://github.com/wolfSSL/wolfssl/issues/6360

See the Library Layout diagram in the Arduino Library Specifications:

image

we have a script that combines all the sources and headers together

Yes, I've updated that script in https://github.com/wolfSSL/wolfssl/pull/7177. It now can be used to create a local Arduino library manually, in addition to publishing files for this repository.

To actually have a wolfSSL library available in the IDE like this, a repository is needed:

image

Notice the hover text for the "more info" link. Once updated, that link will point to this repository.

I could also update the script to also have a .bat version for Windows users to manually install, but it will never be as easy as simply using the Arduino IDE to install a library.

gojimmypi commented 7 months ago

Hi @dgarske I've been considering your comment on the undesired nature of having duplicate wolfSSL code just for the sake of publishing an Arduino library. I'm in complete agreement... but there remains that pesky problem of not being able to exclude files from the Arduino build process.

As an example, I dropped just one of each of the non - C code files into the equivalent src directory for my local wolfSSL Arduino library. I received hundred of messages like these:

C:\Users\gojimmypi\Documents\Arduino\libraries\wolfSSL\src\wolfcrypt\src\aes_gcm_x86_asm.S:10194: Error: bad instruction `movl 152(%esp),%edx'
C:\Users\gojimmypi\Documents\Arduino\libraries\wolfSSL\src\wolfcrypt\src\aes_gcm_x86_asm.S:10195: Error: bad instruction `andl $15,%ecx'
C:\Users\gojimmypi\Documents\Arduino\libraries\wolfSSL\src\wolfcrypt\src\aes_gcm_x86_asm.S:10196: Error: bad instruction `jz L_AES_GCM_encrypt_avx2_done_enc'
C:\Users\gojimmypi\Documents\Arduino\libraries\wolfSSL\src\wolfcrypt\src\aes_gcm_x86_asm.S:10198: Error: bad instruction `vpshufb L_aes_gcm_avx2_bswap_epi64,%xmm4,%xmm4'

We're not the first ones to encounter that. See https://github.com/arduino/arduino-cli/issues/631

However, that does not mean there are not possible alternative solutions. How would you feel about moving all of the non-C source code (e.g. all those wolfcrypt *.i, *.asm and *.S files) ... into a new and different directory location?

If there were only C Source files in the wolfcrypt/src directory, I think your idea of the submodules may be feasible.

gojimmypi commented 7 months ago

I have another idea that might meet the requirements of Arduino library specification and yet still use submodules and not restructure or move any wolfSSL source files.

What about a GitHub action that changes the first and last lines of assembly code in the wolfcrypt/src directory at check-in and check-out time?

Imagine for example, the aes_asm.asm file, which uses the ; character for comments. In particular, these files do not recognize # as an assembler / compiler directive the way C does, causing the problem with Arduino noted above.

We could add the first line in the file with ;#ifdef WOLFSSL_ASM and the last line in the file with ;#endif /* WOLFSSL_ASM */

At check-in time, the ;#ifdef WOLFSSL_ASM would be replaced with #ifdef WOLFSSL_ASM and the ;#endif /* WOLFSSL_ASM */ would be replaced with #endif /* WOLFSSL_ASM */.

By removing the assembly comments and making it a C-friendly file... the Arduino library would technically compile, but the contents of course ignored unless WOLFSSL_ASM is manually assigned. The files in the repository at a given time appear to be C-friendly.

Then, at check-out time, the reverse would happen: The beginning C-friendly comment #ifdef WOLFSSL_ASM would be prepended with a ; for the assembler, making it again ;#ifdef WOLFSSL_ASM. (similarly for the #endif). At check-out time the files would be assembler-friendly.

As the Arduino library repository submodule would not actually be triggering the checkout process once in a submodule, the as-found, C-friendly comments would remain in place, allowing the problematic assembly files to remain in wolfcrypt/src and not cause Arduino build failures.

This would of course also need to be a step added to the commercial distribution package. Not ideal, for sure.

I realize this is quite an eyebrow-raising proposal... changing source files like this and all. Just an idea. Thoughts?

Open to any suggestions.

dgarske commented 6 months ago

Before I merge... @gojimmypi did you figure out a plan for how to avoid keeping duplicate files? Is this going to be a new release step?

gojimmypi commented 6 months ago

Hi @dgarske -

figure out a plan for how to avoid keeping duplicate files?

No, not at this time. We'll just have to consider this repository an "Arduino Library Distribution Container" for now.

The only alternatives I see are using submodules as you suggested with either:

1) Having the Arduino IDE be able to ignore files as noted in arduino/631.

2) Move or otherwise make the non-C files such as those in wolfcrypt/src to be "C compiler friendly".

Is this going to be a new release step?

Yes. It would be nice, but not essential to have the Arduino library updated at the same time as our normal release.

There should be a validation step, rather than blindly pushing new code here. I was planning on some sort of GitHub / Jenkins action. It will be a manual testing process until then.

Note the repo home page has new About text, added since I first created this PR:

I envision the only pull requests merged here will be for codebase refresh from wolfssl and nothing else.

Thanks for reviewing this. We are in agreement that the copy is far from ideal, but it will be awesome to finally have an official wolfSSL Arduino library.