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

Missing CMake target_include_directory for Kyber768 #899

Closed
jschanck opened this issue Feb 8, 2021 · 1 comment · Fixed by #905
Closed

Missing CMake target_include_directory for Kyber768 #899

jschanck opened this issue Feb 8, 2021 · 1 comment · Fixed by #905
Assignees

Comments

@jschanck
Copy link
Contributor

jschanck commented Feb 8, 2021

Upstream Kyber768_META.yml is missing a "common_dep: common_ref" line, which results in OQS_ENABLE_KEM_kyber_768 being omitted from this if statement:

if(OQS_ENABLE_KEM_kyber_512 OR OQS_ENABLE_KEM_kyber_1024 OR OQS_ENABLE_KEM_kyber_512_90s OR OQS_ENABLE_KEM_kyber_768_90s OR OQS_ENABLE_KEM_kyber_1024_90s)
add_library(kyber_common_ref OBJECT pqcrystals-kyber_common_ref/aes256ctr.c pqcrystals-kyber_common_ref/fips202.c)
target_include_directories(kyber_common_ref PRIVATE ${CMAKE_CURRENT_LIST_DIR}/pqcrystals-kyber_common_ref)
set(_KYBER_OBJS ${_KYBER_OBJS} $<TARGET_OBJECTS:kyber_common_ref>)
endif()

If you try to build liboqs with only Kyber768 (and none of the other Kyber parameter sets), the build will fail with undefined references.

This can be reproduced using my feat.minimal-build branch (jschanck/liboqs@4feed617).

cmake -GNinja -DOQS_MINIMAL_BUILD=ON -DOQS_KEM_DEFAULT=OQS_KEM_alg_kyber_768  ..
@baentsch
Copy link
Member

baentsch commented Feb 9, 2021

Bug confirmed. Probably something for @bhess?

Side note:

-DOQS_MINIMAL_BUILD=ON

This is a really nice new build switch: hope you intend to do a PR for this: Fast(er) build, (comparatively) small lib, (nearly) fully functioning. Nearly: tests/example_kem|sig fail due to "alg hardcoding" - logical as it's an example; but probably needs to be skipped in testing if this option is set.

@bhess bhess self-assigned this Feb 9, 2021
bhess added a commit to bhess/liboqs that referenced this issue Feb 9, 2021
bhess added a commit to bhess/liboqs that referenced this issue Feb 10, 2021
bhess added a commit to bhess/liboqs that referenced this issue Feb 10, 2021
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 a pull request may close this issue.

3 participants