ARIA cipher cmake #6600

gojimmypi · 2023-07-12T00:54:45Z

Description

This is an initial cmake PR for the MagicCrypto ARIA cipher suite as a supplement to #6400.

Currently the ARIA build automatically includes the MagicCrypto ciphers when the ARIA_DIR environment
variable is defined with a valid path to the directory structure extracted with tar -xvf MagicCrypto.tar.gz.

Reminder: Copy the contents of the MagicCrypto directory into the root of the wolfssl directory. Otherwise this error is observed during ./configure

configure: error: Error including mcapi_error.h. Please put the MagicCrypto library in /mnt/c/test/wolfssl.

To use cmake, the MagicCrypto directory can be either in the wolfSSL tree, or specified with an environment variable: ARIA_DIR.

Note this PR has since been updated to NOT enable ARIA just because an environment variable ARIA_DIR is defined.

Enable ARIA with -DWOLFSSL_ARIA=yes cmake parameter.

Build wolfSSL

For reference only, here is how to build using make

git clone https://github.com/wolfSSL/wolfssl.git
cd wolfssl
# copy MagicCrypto directory into wolfssl or set LD_LIBRARY_PATH:
# export LD_LIBRARY_PATH=/mnt/c/workspace/MagicCrypto/lib
./autogen.sh
./configure --enable-all --enable-aria
make

Run CMake build

# set to your path
export ARIA_DIR=/mnt/c/workspace/MagicCrypto

mkdir -p out
pushd out
cmake .. -DWOLFSSL_ARIA=yes
cmake --build .

# View the available ciphers with:
./examples/client/client -e

# or with grep:
./examples/client/client -e | grep -i ARIA

popd

Fixes zd# n/a

Testing

There's a new ARIA CMake test script that checks:

No ARIA Environment Variable, ARIA not enabled.
No ARIA Environment Variable, ARIA Enabled
ARIA Environment Variable with MagicCrypto in local user directory, ARIA not enabled.
ARIA Environment Variable with MagicCrypto in local user directory, ARIA Enabled
ARIA Environment Variable with MagicCrypto in wolfssl directory, ARIA not enabled
ARIA Environment Variable with MagicCrypto in wolfssl, ARIA Enabled
ARIA Environment Variable with bad directory, ARIA not enabled.
ARIA Environment Variable with bad directory, ARIA Enabled (failure expected here)

Windows Server Testing:

Unzip your MagicCrypto.tar.gz, shown here for C:\workspace\MagicCrypto

# set to your path
export ARIA_DIR=c:\\workspace\\MagicCrypto

mkdir -p out
pushd out
cmake .. -DWOLFSSL_ARIA=yes
cmake --build .

# View the available ciphers with:
./examples/client/client -e

popd

Linux Server Testing:

# From the root of the wolfSSL repo:

# set to your path
export ARIA_DIR=/mnt/c/workspace/MagicCrypto

mkdir -p out
pushd out
cmake .. -DWOLFSSL_ARIA=yes
cmake --build .

# View the available ciphers with:
./examples/client/client -e
popd

Server:

# reminder this is in the CMake `out` directory, NOT the root of wolfssl that is build with regular make.
# we also do NOT need the `export ARIA_DIR` here, as that's only for CMake building.

./examples/server/server -i -x -v 3 -A ./certs/ca-ecc-cert.pem -k ./certs/ecc-key.pem -c ./certs/intermediate/server-chain-ecc.pem -V -l ECDHE-ECDSA-ARIA128-GCM-SHA256

Client:

./examples/client/client -v 3 -l ECDHE-ECDSA-ARIA128-GCM-SHA256 -A ./certs/ca-ecc-cert.pem -k ./certs/ecc-client-key.pem -c ./certs/intermediate/client-chain-ecc.pem -C

Checklist

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

bandi13

It's strange to me that you need to modify .C and .H files to get CMake to build with ARIA. Feels like something is not being set right.

examples/server/server.c

README-aria.md

bandi13 · 2023-07-12T17:18:23Z

CMakeLists.txt

 if(BUILD_SHARED_LIBS)
+ # message(STATUS "ARIA Check: SHARED! ${LIB_SOURCES}")


Probably should remove debug code

bandi13 · 2023-07-12T17:19:49Z

CMakeLists.txt

 list(APPEND WOLFSSL_DEFINITIONS
- "-DHAVE_PTHREAD"
+ # "-DHAVE_PTHREAD"


This PR should possibly be based on the PTHREAD issue being resolved

I believe this is resolved. I will pull the threading correction to a different PR.

bandi13 · 2023-07-12T17:20:08Z

CMakeLists.txt

 cmake_minimum_required(VERSION 3.16)
+# set(CMAKE_BINARY_DIR "${CMAKE_BINARY_DIR}/build")


Remove dead code

bandi13 · 2023-07-12T17:20:32Z

CMakeLists.txt

+ "no" "yes;no")
+
+if(0)
+ get_cmake_property(_variableNames VARIABLES)


Remove debug code

CMakeLists.txt

bandi13 · 2023-07-12T17:22:13Z

CMakeLists.txt

@@ -1882,6 +1918,9 @@ if (WOLFSSL_CAAM)
 list(APPEND WOLFSSL_DEFINITIONS "-DWOLFSSL_CAAM")
 endif()

+if (WOLFSSL_ARIA)
+ list(APPEND WOLFSSL_DEFINITIONS "-DWOLFSSL_ARIA")


Gut feeling is that this should be HAVE_ARIA instead of WOLFSSL_ARIA.

ooph, good catch!

test-aria.sh

README_cmake_notes.md

README-aria.md

gojimmypi · 2023-07-12T22:33:33Z

I've removed the stray debug info files, added a couple of CMake README files, and put the CMake threading logic change in a separate PR (see #6606).

I've confirmed this now works with Windows CMake (Visual Studio 2022 command prompt). For example the server.exe ends up in out\examples\server\Debug

C:\workspace\wolfssl-gojimmypi-bandi13\out>cmake ..
-- Building for: Visual Studio 17 2022
-- Selecting Windows SDK version 10.0.22000.0 to target Windows 10.0.22621.
-- The C compiler identification is MSVC 19.36.32535.0
-- The ASM compiler identification is MSVC
-- Found assembler: C:/Program Files/Microsoft Visual Studio/2022/Enterprise/VC/Tools/MSVC/14.36.32532/bin/Hostx64/x64/cl.exe
...etc

A separate issue that may need to be addressed: the Cmake-compiled examples for Windows do not immediately work fresh out of the box.

Trying to run the executable, there's no error message, but nothing happens. This occurs when the wolfSSL dll's are not installed in the system. The current debug environment does not find them in the build directory. If I manually copy them, for example the 4 files created in the \out\debug directory:

         1,158,656 wolfssl.dll
            90,094 wolfssl.exp
           148,848 wolfssl.lib
         2,510,848 wolfssl.pdb

... to each of the respective example output directories: out\examples\client\Debug and out\examples\server\Debug. After this copy, then the Windows examples all work well. Perhaps it would be a good idea for CMake to copy them?

gojimmypi · 2023-07-13T01:03:03Z

As noted in #6606 (comment) there may be a known problem in master unrelated to this PR. I've reverted the WOLFSSL_SINGLE_THREADED change here and squashed the commits.

The result is that all of the examples do not compile successfully with CMake in Windows for the ARIA ciphers.

The test client and server do compile successfully with CMake, in Windows and Linux, with and without the ARIA ciphers.

JacobBarthelmeh · 2023-08-21T20:41:49Z

Please check the additional includes in other example files. For example client.c.

gojimmypi · 2023-08-22T00:31:27Z

ok, I've removed all references of the ARIA #include that I added, retested everything, resync from upstream & pushed changes along with squash.

Please let me know if you see anything else that can be improved.

dgarske

Works as described. Just a few minor documentation fixes needed.

dgarske · 2023-09-18T17:35:52Z

INSTALL

@@ -110,6 +110,55 @@

 To build with debugging use: `cmake .. -DCMAKE_BUILD_TYPE=Debug`.

+ In the simplest form:
+
+ # create a root directory for wolfssl repo


Perhaps not include the ./autogen.sh and ./configure example step for CMake?

ah yes, good. I removed those lines from the docs.

dgarske · 2023-09-18T18:26:38Z

cmake/README.md

+
+This directory contains some supplementary functions for the [CMakeLists.txt](../CMakeLists.txt) in the root.
+
+See also the [README_cmake.md](../README_cmake.md) file.


Fix. No longer README_cmake.md.

I updated that to instead reference the root INSTALL document.

gojimmypi · 2023-09-19T17:56:34Z

All checks pass. @dgarske let me know if you'd prefer I squash the 14 commits in this PR.

fixed

dgarske · 2023-09-19T17:57:29Z

All checks pass. @dgarske let me know if you'd prefer I squash the 14 commits in this PR.

I squashed on merge. Thanks

gojimmypi marked this pull request as draft July 12, 2023 00:54

gojimmypi requested review from dgarske, JacobBarthelmeh and bandi13 July 12, 2023 00:55

gojimmypi self-assigned this Jul 12, 2023

gojimmypi force-pushed the aria branch from abf8fcc to ce547c2 Compare July 12, 2023 01:03

bandi13 requested changes Jul 12, 2023

View reviewed changes

dgarske requested changes Jul 12, 2023

View reviewed changes

README_cmake_notes.md Outdated Show resolved Hide resolved

README-aria.md Outdated Show resolved Hide resolved

gojimmypi force-pushed the aria branch from 7f04963 to 09bdc6d Compare July 12, 2023 19:55

gojimmypi requested a review from bandi13 July 12, 2023 19:58

gojimmypi mentioned this pull request Jul 12, 2023

CMake SINGLE_THREADED not WOLFSSL_SINGLE_THREADED #6606

Closed

4 tasks

gojimmypi force-pushed the aria branch from a14e76d to a9fd5eb Compare July 13, 2023 01:00

gojimmypi marked this pull request as ready for review July 13, 2023 01:03

gojimmypi requested a review from dgarske July 13, 2023 01:03

gojimmypi requested review from wolfSSL-Bot and removed request for dgarske August 15, 2023 19:15

gojimmypi assigned wolfSSL-Bot Aug 15, 2023

JacobBarthelmeh unassigned gojimmypi Aug 15, 2023

gojimmypi requested review from dgarske and removed request for wolfSSL-Bot August 15, 2023 21:16

JacobBarthelmeh assigned gojimmypi and unassigned wolfSSL-Bot Aug 21, 2023

ARIA Cipher CMake support with code review changes (+commit squash)

0379526

gojimmypi force-pushed the aria branch from 8c685af to 0379526 Compare August 21, 2023 23:48

gojimmypi assigned JacobBarthelmeh and wolfSSL-Bot Aug 22, 2023

JacobBarthelmeh unassigned wolfSSL-Bot Aug 23, 2023

gojimmypi mentioned this pull request Aug 24, 2023

ARIA no longer needs OpenSSL enabled #6730

Merged

4 tasks

gojimmypi added 5 commits August 24, 2023 16:35

upstream merge

15d6b6f

changes from wolfSSL#6730

d62f425

ignore cmake out & out_temp

04b6df4

improved aria cmake build script test

392ebca

Merge branch 'master' of https://github.com/wolfSSL/wolfssl into aria

5065649

gojimmypi mentioned this pull request Sep 7, 2023

implement gettime_secs for Windows (_MSC_VER) in tests/api.c #6733

Merged

4 tasks

gojimmypi added 3 commits September 12, 2023 16:40

Merge branch 'master' of https://github.com/wolfSSL/wolfssl into aria

49cad09

rename aria cmake build tests script

851f7a2

move CMake and ARIA instructions to INSTALL file

7ba830e

gojimmypi assigned JacobBarthelmeh and wolfSSL-Bot Sep 13, 2023

gojimmypi added 2 commits September 12, 2023 20:22

add new cmake readme and aria test script files to include.am

c597470

remove commented examples from aria cmake function

fa8dee9

gojimmypi requested a review from JacobBarthelmeh September 13, 2023 03:31

dgarske requested changes Sep 18, 2023

View reviewed changes

dgarske assigned gojimmypi and unassigned bandi13, JacobBarthelmeh, gojimmypi and wolfSSL-Bot Sep 18, 2023

minor ARIA cmake doc update

19d966a

gojimmypi assigned dgarske Sep 19, 2023

gojimmypi requested a review from dgarske September 19, 2023 17:54

dgarske approved these changes Sep 19, 2023

View reviewed changes

dgarske merged commit 5830f92 into wolfSSL:master Sep 19, 2023
95 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ARIA cipher cmake #6600

ARIA cipher cmake #6600

gojimmypi commented Jul 12, 2023 •

edited

Loading

bandi13 left a comment

bandi13 Jul 12, 2023

gojimmypi Jul 12, 2023

bandi13 Jul 12, 2023

gojimmypi Jul 12, 2023

bandi13 Jul 12, 2023

gojimmypi Jul 12, 2023

bandi13 Jul 12, 2023

gojimmypi Jul 12, 2023

bandi13 Jul 12, 2023

gojimmypi Jul 12, 2023

gojimmypi commented Jul 12, 2023

gojimmypi commented Jul 13, 2023

JacobBarthelmeh commented Aug 21, 2023

gojimmypi commented Aug 22, 2023

dgarske left a comment

dgarske Sep 18, 2023

gojimmypi Sep 19, 2023

dgarske Sep 18, 2023

gojimmypi Sep 19, 2023

gojimmypi commented Sep 19, 2023

dgarske commented Sep 19, 2023

		if(BUILD_SHARED_LIBS)
		# message(STATUS "ARIA Check: SHARED! ${LIB_SOURCES}")

		cmake_minimum_required(VERSION 3.16)
		# set(CMAKE_BINARY_DIR "${CMAKE_BINARY_DIR}/build")


		This directory contains some supplementary functions for the [CMakeLists.txt](../CMakeLists.txt) in the root.

		See also the [README_cmake.md](../README_cmake.md) file.

ARIA cipher cmake #6600

ARIA cipher cmake #6600

Conversation

gojimmypi commented Jul 12, 2023 • edited Loading

Description

Build wolfSSL

Run CMake build

Testing

Windows Server Testing:

Linux Server Testing:

Server:

Client:

Checklist

bandi13 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gojimmypi commented Jul 12, 2023

gojimmypi commented Jul 13, 2023

JacobBarthelmeh commented Aug 21, 2023

gojimmypi commented Aug 22, 2023

dgarske left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gojimmypi commented Sep 19, 2023

dgarske commented Sep 19, 2023

gojimmypi commented Jul 12, 2023 •

edited

Loading