Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LMS wolfBoot support. #350

Merged
merged 1 commit into from
Sep 6, 2023
Merged

Conversation

philljj
Copy link
Contributor

@philljj philljj commented Aug 21, 2023

Description

Adds support for LMS/HSS signatures to wolfBoot.

LMS/HSS is a post-quantum stateful hash-based signature scheme.

Documentation

Added new docs post-quantum readme:

docs/PQ.md

Building

See procedure in docs/PQ.md.

Config

Added new LMS sim example

config/examples/sim-lms.config

The LMS_LEVELS, LMS_HEIGHT, and LMS_WINTERNITZ, IMAGE_SIGNATURE_SIZE, and (optionally) IMAGE_HEADER_SIZE must be set here.

SIGN?=LMS
...
LMS_LEVELS=2
LMS_HEIGHT=5
LMS_WINTERNITZ=8
...
IMAGE_SIGNATURE_SIZE=2644
IMAGE_HEADER_SIZE?=5288

In LMS the signature size is a function of the parameters. Use the added helper script tools/lms/lms_siglen to calculate your signature length given your LMS parameters:

$./tools/lms/lms_siglen 
levels:      3
height:      5
winternitz:  8
#
total_len:   3992

More Info

See these links for more info on LMS and wolfSSL/wolfCrypt:

@philljj philljj self-assigned this Aug 21, 2023
@dgarske dgarske self-assigned this Aug 22, 2023
@dgarske
Copy link
Contributor

dgarske commented Aug 22, 2023

Please resolve conflicts.

@dgarske dgarske removed their assignment Aug 22, 2023
# The number of available signatures is:
# N = 2 ** (levels * height)
#
# LMS/HSS Signature sizes are directly proportional to the levels parm,
Copy link
Member

Choose a reason for hiding this comment

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

Remove parm.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reworded it.

IMAGE_SIGNATURE_SIZE=2644
IMAGE_HEADER_SIZE?=5288

# it should be multiple of system page size
Copy link
Member

Choose a reason for hiding this comment

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

"it" what is "it"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Outdated comment I copy-pasted from other example configs. Will update it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated it to be similar to the other sim*.config files.

options.mk Outdated
@@ -286,6 +286,53 @@ ifeq ($(SIGN),RSA4096)
endif
endif

ifeq ($(SIGN),LMS)
# In LMS the signature size is a function of the LMS parameters.
Copy link
Member

Choose a reason for hiding this comment

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

In -> For

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

fprintf(stderr, "error: fclose returned %d\n", err);
return WC_LMS_RC_WRITE_FAIL;
}

Copy link
Member

Choose a reason for hiding this comment

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

What about read back and verify content written matches priv ?
We should show best practices and in this case speed/efficiency isn't priority.

Copy link
Contributor Author

@philljj philljj Aug 22, 2023

Choose a reason for hiding this comment

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

Agree but probably should close and check return first before re-opening and reading to increase likelihood the write has actually reached persistent storage.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added additional logic to re-open, read and compare priv key data.

@@ -0,0 +1,76 @@
#!/bin/bash
#
Copy link
Member

Choose a reason for hiding this comment

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

Do we need GPL boiler plate here? I don't know the answer so I'm asking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure. The shell scripts in tools/scripts/ don't have GPL, so was using that as ref. But lms_siglen should probably have .sh suffix like them.

Copy link
Contributor Author

@philljj philljj Aug 22, 2023

Choose a reason for hiding this comment

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

I originally experimented with putting the sig len calculation logic in makefile scripts but was too messy. And even if sig len is automatically set, the user will likely still need to manually config the header length for their platform. Hence I settled on this helper script.

# Globals

# The lm_pub_len is 4 less than the pub len value of 60
# returned from hash-sigs HSS api.
Copy link
Member

Choose a reason for hiding this comment

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

api is all caps (API)

@dgarske
Copy link
Contributor

dgarske commented Aug 22, 2023

@philljj , please add a docs/PQ.md or docs/LMS.md with the details from the PR description. Please include a link to the hash-sigs repo.

Copy link
Member

@anhu anhu left a comment

Choose a reason for hiding this comment

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

Just one little punctuation problem.

docs/PQ.md Outdated

Stateful HBS schemes are based on the security of their underlying hash
functions and Merkle trees, which are not expected to be broken by the advent
of cryptographically relevant quantum computers
Copy link
Member

Choose a reason for hiding this comment

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

Missing period at end of paragraph.

tools/keytools/Makefile Show resolved Hide resolved
dgarske
dgarske previously approved these changes Aug 24, 2023
Copy link
Contributor

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

Over to @danielinux

@dgarske dgarske removed their assignment Aug 24, 2023
@danielinux
Copy link
Member

Unfortunately libhss can only be linked on a POSIX-C system and won't work baremetal:

/usr/lib/gcc/arm-none-eabi/12.2.1/../../../arm-none-eabi/bin/ld: /usr/lib/gcc/arm-none-eabi/12.2.1/../../../arm-none-eabi/lib/thumb/v7-m/nofp/libc.a(lib_a-sbrkr.o): in function `_sbrk_r':
/build/newlib-afIbHz/newlib-3.3.0/build/arm-none-eabi/thumb/v7-m/nofp/newlib/libc/reent/../../../../../../../../newlib/libc/reent/sbrkr.c:51: undefined reference to `_sbrk'
/usr/lib/gcc/arm-none-eabi/12.2.1/../../../arm-none-eabi/bin/ld: /usr/lib/gcc/arm-none-eabi/12.2.1/../../../arm-none-eabi/lib/thumb/v7-m/nofp/libc.a(lib_a-writer.o): in function `_write_r':
/build/newlib-afIbHz/newlib-3.3.0/build/arm-none-eabi/thumb/v7-m/nofp/newlib/libc/reent/../../../../../../../../newlib/libc/reent/writer.c:49: undefined reference to `_write'
/usr/lib/gcc/arm-none-eabi/12.2.1/../../../arm-none-eabi/bin/ld: /usr/lib/gcc/arm-none-eabi/12.2.1/../../../arm-none-eabi/lib/thumb/v7-m/nofp/libc.a(lib_a-closer.o): in function `_close_r':
/build/newlib-afIbHz/newlib-3.3.0/build/arm-none-eabi/thumb/v7-m/nofp/newlib/libc/reent/../../../../../../../../newlib/libc/reent/closer.c:47: undefined reference to `_close'
/usr/lib/gcc/arm-none-eabi/12.2.1/../../../arm-none-eabi/bin/ld: /usr/lib/gcc/arm-none-eabi/12.2.1/../../../arm-none-eabi/lib/thumb/v7-m/nofp/libc.a(lib_a-fstatr.o): in function `_fstat_r':
/build/newlib-afIbHz/newlib-3.3.0/build/arm-none-eabi/thumb/v7-m/nofp/newlib/libc/reent/../../../../../../../../newlib/libc/reent/fstatr.c:55: undefined reference to `_fstat'
/usr/lib/gcc/arm-none-eabi/12.2.1/../../../arm-none-eabi/bin/ld: /usr/lib/gcc/arm-none-eabi/12.2.1/../../../arm-none-eabi/lib/thumb/v7-m/nofp/libc.a(lib_a-isattyr.o): in function `_isatty_r':
/build/newlib-afIbHz/newlib-3.3.0/build/arm-none-eabi/thumb/v7-m/nofp/newlib/libc/reent/../../../../../../../../newlib/libc/reent/isattyr.c:52: undefined reference to `_isatty'
/usr/lib/gcc/arm-none-eabi/12.2.1/../../../arm-none-eabi/bin/ld: /usr/lib/gcc/arm-none-eabi/12.2.1/../../../arm-none-eabi/lib/thumb/v7-m/nofp/libc.a(lib_a-lseekr.o): in function `_lseek_r':
/build/newlib-afIbHz/newlib-3.3.0/build/arm-none-eabi/thumb/v7-m/nofp/newlib/libc/reent/../../../../../../../../newlib/libc/reent/lseekr.c:49: undefined reference to `_lseek'
/usr/lib/gcc/arm-none-eabi/12.2.1/../../../arm-none-eabi/bin/ld: /usr/lib/gcc/arm-none-eabi/12.2.1/../../../arm-none-eabi/lib/thumb/v7-m/nofp/libc.a(lib_a-readr.o): in function `_read_r':
/build/newlib-afIbHz/newlib-3.3.0/build/arm-none-eabi/thumb/v7-m/nofp/newlib/libc/reent/../../../../../../../../newlib/libc/reent/readr.c:49: undefined reference to `_read'
/usr/lib/gcc/arm-none-eabi/12.2.1/../../../arm-none-eabi/bin/ld: /usr/lib/gcc/arm-none-eabi/12.2.1/../../../arm-none-eabi/lib/thumb/v7-m/nofp/libc.a(lib_a-abort.o): in function `abort':
/build/newlib-afIbHz/newlib-3.3.0/build/arm-none-eabi/thumb/v7-m/nofp/newlib/libc/stdlib/../../../../../../../../newlib/libc/stdlib/abort.c:59: undefined reference to `_exit'
/usr/lib/gcc/arm-none-eabi/12.2.1/../../../arm-none-eabi/bin/ld: /usr/lib/gcc/arm-none-eabi/12.2.1/../../../arm-none-eabi/lib/thumb/v7-m/nofp/libc.a(lib_a-signalr.o): in function `_kill_r':
/build/newlib-afIbHz/newlib-3.3.0/build/arm-none-eabi/thumb/v7-m/nofp/newlib/libc/reent/../../../../../../../../newlib/libc/reent/signalr.c:53: undefined reference to `_kill'
/usr/lib/gcc/arm-none-eabi/12.2.1/../../../arm-none-eabi/bin/ld: /usr/lib/gcc/arm-none-eabi/12.2.1/../../../arm-none-eabi/lib/thumb/v7-m/nofp/libc.a(lib_a-signalr.o): in function `_getpid_r':
/build/newlib-afIbHz/newlib-3.3.0/build/arm-none-eabi/thumb/v7-m/nofp/newlib/libc/reent/../../../../../../../../newlib/libc/reent/signalr.c:83: undefined reference to `_getpid'

Copy link
Member

@danielinux danielinux left a comment

Choose a reason for hiding this comment

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

Cannot merge until we figure out how to link a cross-compiled hss_lib.a that does not carry POSIX-C, OS system calls and other dependencies.

@danielinux danielinux removed their assignment Aug 25, 2023
@dgarske
Copy link
Contributor

dgarske commented Aug 25, 2023

Also we should have a CI test added for it so the example config is built (including that hash-sigs lib).

@philljj
Copy link
Contributor Author

philljj commented Aug 30, 2023

Note: these changes require wolfSSL LMS_VERIFY_ONLY support from this PR:
wolfSSL/wolfssl#6738

Also in this last change, the hash-sigs lib is separated like this:

ls lib/hash-sigs/
lib  src

The building of hash-sigs libs/objects is handled automatically with wolfBoot/keytools Makefiles.

The keytools build links with hss_lib.a.
The wolfboot build links with the subset of objects in the hss_verify.a build rule, with -DLMS_VERIFY_ONLY to guard out non-verify code from wolfCrypt LMS.

@philljj
Copy link
Contributor Author

philljj commented Aug 31, 2023

This last change introduced a small build error on mac m1 clang, fixing now.

@philljj philljj requested a review from dgarske August 31, 2023 19:33
tools/config.mk Show resolved Hide resolved
Copy link
Member

@danielinux danielinux left a comment

Choose a reason for hiding this comment

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

POSIX dependencies now solved. Successfully tested on embedded target.

Once the wolfSSL PR is merged, please add non-regression tests + github automations as suggested by @dgarske, and discussed internally.

@danielinux danielinux dismissed dgarske’s stale review September 5, 2023 22:38

Change requests have been addressed

@danielinux danielinux assigned anhu and dgarske and unassigned philljj Sep 5, 2023
@danielinux
Copy link
Member

Happy to merge if you think your change requests have been addressed properly @anhu @dgarske

@anhu
Copy link
Member

anhu commented Sep 6, 2023

I'm good!! Thank you!

@danielinux danielinux merged commit e23d450 into wolfSSL:master Sep 6, 2023
58 checks passed
@philljj philljj deleted the lms_wolfboot_support branch September 6, 2023 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants