Remove s2n's internal Kyber512 implementation, and rely on AWS-LC for Kyber support (#4283)

---------

Co-authored-by: Lindsay Stewart <[email protected]>

Author: alexw91

CMakeLists.txt

@@ -19,10 +19,6 @@ set(VERSION_MAJOR 1)
 set(VERSION_MINOR 0)
 set(VERSION_PATCH 0)
 
-option(S2N_NO_PQ "Disables all Post Quantum Crypto code. You likely want this
-for older compilers or uncommon platforms." OFF)
-option(S2N_NO_PQ_ASM "Turns off the ASM for PQ Crypto even if it's available for the toolchain.
-You likely want this on older compilers." OFF)
 option(SEARCH_LIBCRYPTO "Set this if you want to let S2N search libcrypto for you,
 otherwise a crypto target needs to be defined." ON)
 option(UNSAFE_TREAT_WARNINGS_AS_ERRORS "Compiler warnings are treated as errors. Warnings may
@@ -62,47 +58,25 @@ file(GLOB_RECURSE TLS_SRC "tls/*.c")
 file(GLOB UTILS_HEADERS "utils/*.h")
 file(GLOB UTILS_SRC "utils/*.c")
 
-# Always include the top-level pq-crypto/ files
-file(GLOB PQ_HEADERS "pq-crypto/*.h")
-file(GLOB PQ_SRC "pq-crypto/*.c")
-
 message(STATUS "Detected CMAKE_SYSTEM_PROCESSOR as ${CMAKE_SYSTEM_PROCESSOR}")
 
 if(CMAKE_SIZEOF_VOID_P EQUAL 4)
-  message(STATUS "Detected 32-Bit system - disabling PQ crypto assembly optimizations")
-  set(S2N_NO_PQ_ASM ON)
+  message(STATUS "Detected 32-Bit system")
 else()
     message(STATUS "Detected 64-Bit system")
 endif()
 
-if(S2N_NO_PQ)
-    # PQ is disabled, so we do not include any PQ crypto code
-    message(STATUS "S2N_NO_PQ flag was detected - disabling PQ crypto")
-    set(S2N_NO_PQ_ASM ON)
-else()
-    # PQ is enabled, so include all of the PQ crypto code
-    file(GLOB PQ_HEADERS
-        "pq-crypto/*.h"
-        "pq-crypto/kyber_r3/*.h")
-
-    file(GLOB PQ_SRC
-        "pq-crypto/*.c"
-        "pq-crypto/kyber_r3/*.c")
-endif()
-
 ##be nice to visual studio users
 if(MSVC)
     source_group("Header Files\\s2n\\api" FILES ${API_HEADERS} ${API_UNSTABLE_HEADERS})
     source_group("Header Files\\s2n\\crypto" FILES ${CRYPTO_HEADERS})
     source_group("Header Files\\s2n\\error" FILES ${ERROR_HEADERS})
-    source_group("Header Files\\s2n\\pq-crypto" FILES ${PQ_HEADERS})
     source_group("Header Files\\s2n\\stuffer" FILES ${STUFFER_HEADERS})
     source_group("Header Files\\s2n\\tls" FILES ${TLS_HEADERS})
     source_group("Header Files\\s2n\\utils" FILES ${UTILS_HEADERS})
 
     source_group("Source Files\\crypto" FILES ${CRYPTO_SRC})
     source_group("Source Files\\error" FILES ${ERROR_SRC})
-    source_group("Source Files\\pq-crypto" FILES ${PQ_SRC})
     source_group("Source Files\\stuffer" FILES ${STUFFER_SRC})
     source_group("Source Files\\tls" FILES ${TLS_SRC})
     source_group("Source Files\\utils" FILES ${UTILS_SRC})
@@ -135,7 +109,6 @@ file(GLOB S2N_HEADERS
     ${API_UNSTABLE_HEADERS}
     ${CRYPTO_HEADERS}
     ${ERROR_HEADERS}
-    ${PQ_HEADERS}
     ${STUFFER_HEADERS}
     ${TLS_HEADERS}
     ${UTILS_HEADERS}
@@ -144,7 +117,6 @@ file(GLOB S2N_HEADERS
 file(GLOB S2N_SRC
     ${CRYPTO_SRC}
     ${ERROR_SRC}
-    ${PQ_SRC}
     ${STUFFER_SRC}
     ${TLS_SRC}
     ${UTILS_SRC}
@@ -186,10 +158,6 @@ if(NOT APPLE)
     set(CMAKE_SHARED_LINKER_FLAGS -Wl,-z,noexecstack,-z,relro,-z,now)
 endif()
 
-if(S2N_NO_PQ)
-    add_definitions(-DS2N_NO_PQ)
-endif()
-
 # Whether to fail the build when compiling s2n's portable C code with non-portable assembly optimizations. Doing this
 # can lead to runtime crashes if build artifacts are built on modern hardware, but deployed to older hardware without
 # newer CPU instructions. s2n, by default, should be backwards compatible with older CPU types so this flag should be
@@ -367,32 +335,6 @@ if (NOT S2N_EXECINFO_AVAILABLE)
 endif()
 feature_probe_result(S2N_STACKTRACE ${S2N_STACKTRACE})
 
-set(S2N_KYBER512R3_AVX2_BMI2 FALSE)
-if(NOT S2N_NO_PQ_ASM)
-    # Kyber Round-3 code has several different optimizations which require
-    # specific compiler flags to be supported by the compiler.
-    # So for each needed instruction set extension we check if the compiler
-    # supports it and set proper compiler flags to be added later to the
-    # Kyber compilation units.
-    if(${CMAKE_SYSTEM_PROCESSOR} MATCHES "^(x86_64|amd64|AMD64)$")
-        # Some platforms support -mavx2 flag but not m256 intrinsics required to use them. Only enable Kyber assembly
-        # optimizations if both are supported. See https://github.com/aws/s2n-tls/pull/3005 for more info.
-        if(S2N_KYBER512R3_AVX2_BMI2_SUPPORTED AND S2N_KYBER512R3_M256_INTRINSICS_SUPPORTED)
-            set(S2N_KYBER512R3_AVX2_BMI2 TRUE)
-            enable_language(ASM)
-
-            # add the assembly files to the project
-            FILE(GLOB KYBER512R3_AVX2_BMI2_ASM_SRCS "pq-crypto/kyber_r3/*_avx2.S")
-            target_sources(${PROJECT_NAME} PRIVATE ${KYBER512R3_AVX2_BMI2_ASM_SRCS})
-
-            # compile the C files with avx flags
-            FILE(GLOB KYBER512R3_AVX2_BMI2_SRCS "pq-crypto/kyber_r3/*_avx2.c")
-            set_source_files_properties(${KYBER512R3_AVX2_BMI2_SRCS} PROPERTIES COMPILE_FLAGS ${S2N_KYBER512R3_AVX2_BMI2_SUPPORTED_FLAGS})
-        endif()
-    endif()
-endif()
-feature_probe_result(S2N_KYBER512R3_AVX2_BMI2 ${S2N_KYBER512R3_AVX2_BMI2})
-
 if (S2N_INTERN_LIBCRYPTO)
 
     # Check if the AWS::crypto target has beeen added and handle it

LICENSE

@@ -200,25 +200,3 @@
    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    See the License for the specific language governing permissions and
    limitations under the License.
-   
-   
-============================================================================
-    S2N SUBCOMPONENTS:
-
-    The s2n Project contains subcomponents with separate copyright notices
-    and license terms. Your use of the source code for these subcomponents is
-    subject to the terms and conditions of the following licenses.
-
-
-========================================================================
-Third party MIT licenses
-========================================================================
-
-The following components are provided under the MIT License. See project link for details.
-
-
-    SIKE
-      -> s2n/pq-crypto/sike_r1/LICENSE.txt
-
-   
-   

Makefile

@@ -47,7 +47,6 @@ include s2n.mk
 
 .PHONY : libs
 libs:
-	$(MAKE) -C pq-crypto
 	$(MAKE) -C utils
 	$(MAKE) -C error
 	$(MAKE) -C stuffer
@@ -101,12 +100,11 @@ run-lcov:
 	$(MAKE) -C bin lcov
 	$(MAKE) -C crypto lcov
 	$(MAKE) -C error lcov
-	$(MAKE) -C pq-crypto run-lcov
 	$(MAKE) -C stuffer lcov
 	$(MAKE) -C tests lcov
 	$(MAKE) -C tls run-lcov
 	$(MAKE) -C utils lcov
-	lcov -a crypto/coverage.info -a error/coverage.info -a pq-crypto/coverage.info -a pq-crypto/kyber_r3/coverage.info -a stuffer/coverage.info -a tls/coverage.info -a $(wildcard tls/*/coverage.info) -a utils/coverage.info --output ${COVERAGE_DIR}/all_coverage.info
+	lcov -a crypto/coverage.info -a error/coverage.info -a stuffer/coverage.info -a tls/coverage.info -a $(wildcard tls/*/coverage.info) -a utils/coverage.info --output ${COVERAGE_DIR}/all_coverage.info
 
 .PHONY : run-genhtml
 run-genhtml:
@@ -115,7 +113,6 @@ run-genhtml:
 
 .PHONY : indent
 indent:
-	$(MAKE) -C pq-crypto indentsource
 	$(MAKE) -C tests indentsource
 	$(MAKE) -C stuffer indentsource
 	$(MAKE) -C crypto indentsource
@@ -147,7 +144,6 @@ uninstall:
 
 .PHONY : clean
 clean:
-	$(MAKE) -C pq-crypto clean
 	$(MAKE) -C tests clean
 	$(MAKE) -C stuffer decruft
 	$(MAKE) -C crypto decruft

bindings/rust/generate.sh

@@ -19,7 +19,6 @@ cp -r \
   ../../api \
   ../../crypto \
   ../../error \
-  ../../pq-crypto \
   ../../stuffer \
   ../../tls \
   ../../utils \

bindings/rust/s2n-tls-sys/build.rs

@@ -93,6 +93,8 @@ fn build_vendored() {
 
     let mut build = builder(&libcrypto);
 
+    // TODO: update rust bindings to handle no pq-crypto dir
+
     let pq = option_env("CARGO_FEATURE_PQ").is_some();
 
     // TODO each pq section needs to be built separately since it

codebuild/bin/grep_simple_mistakes.sh

@@ -18,7 +18,7 @@ FAILED=0
 # Grep for any instances of raw memcpy() function. s2n code should instead be
 # using one of the *_ENSURE_MEMCPY macros.
 #############################################
-S2N_FILES_ASSERT_NOT_USING_MEMCPY=$(find "$PWD" -type f -name "s2n*.[ch]" -not -path "*/tests/*"  -not -path "*/pq-crypto/*")
+S2N_FILES_ASSERT_NOT_USING_MEMCPY=$(find "$PWD" -type f -name "s2n*.[ch]" -not -path "*/tests/*")
 for file in $S2N_FILES_ASSERT_NOT_USING_MEMCPY; do
   RESULT_NUM_LINES=`grep 'memcpy(' $file | wc -l`
   if [ "${RESULT_NUM_LINES}" != 0 ]; then
@@ -180,7 +180,7 @@ done
 ## Assert that there are no new uses of S2N_ERROR_IF
 # TODO add crypto, tls (see https://github.com/aws/s2n-tls/issues/2635)
 #############################################
-S2N_ERROR_IF_FREE="bin error pq-crypto scram stuffer utils tests"
+S2N_ERROR_IF_FREE="bin error scram stuffer utils tests"
 for dir in $S2N_ERROR_IF_FREE; do
   files=$(find "$dir" -type f -name "*.c" -path "*")
   for file in $files; do

compliance/generate_report.sh

@@ -15,7 +15,6 @@ duvet \
   --source-pattern '(*=,*#)bin/**/*.[ch]' \
   --source-pattern '(*=,*#)crypto/**/*.[ch]' \
   --source-pattern '(*=,*#)error/**/*.[ch]' \
-  --source-pattern '(*=,*#)pq-crypto/**/*.[ch]' \
   --source-pattern '(*=,*#)stuffer/**/*.[ch]' \
   --source-pattern '(*=,*#)tests/**/*.[ch]' \
   --source-pattern '(*=,*#)tls/**/*.[ch]' \

pq-crypto/s2n_kyber_evp.c crypto/s2n_kyber_evp.c

@@ -16,21 +16,20 @@
 #include <openssl/evp.h>
 #include <stddef.h>
 
+#include "crypto/s2n_pq.h"
 #include "error/s2n_errno.h"
-#include "pq-crypto/s2n_pq.h"
 #include "tls/s2n_kem.h"
 #include "utils/s2n_safety.h"
 #include "utils/s2n_safety_macros.h"
 
-#if defined(S2N_LIBCRYPTO_SUPPORTS_KYBER) && !defined(S2N_NO_PQ)
+#if defined(S2N_LIBCRYPTO_SUPPORTS_KYBER)
 
 DEFINE_POINTER_CLEANUP_FUNC(EVP_PKEY *, EVP_PKEY_free);
 DEFINE_POINTER_CLEANUP_FUNC(EVP_PKEY_CTX *, EVP_PKEY_CTX_free);
 
 int s2n_kyber_evp_generate_keypair(IN const struct s2n_kem *kem, OUT uint8_t *public_key,
         OUT uint8_t *secret_key)
 {
-    POSIX_ENSURE(s2n_pq_is_enabled(), S2N_ERR_PQ_DISABLED);
     DEFER_CLEANUP(EVP_PKEY_CTX *kyber_pkey_ctx = EVP_PKEY_CTX_new_id(EVP_PKEY_KEM, NULL), EVP_PKEY_CTX_free_pointer);
     POSIX_GUARD_PTR(kyber_pkey_ctx);
     POSIX_GUARD_OSSL(EVP_PKEY_CTX_kem_set_params(kyber_pkey_ctx, kem->kem_nid), S2N_ERR_PQ_CRYPTO);
@@ -53,7 +52,6 @@ int s2n_kyber_evp_generate_keypair(IN const struct s2n_kem *kem, OUT uint8_t *pu
 int s2n_kyber_evp_encapsulate(IN const struct s2n_kem *kem, OUT uint8_t *ciphertext, OUT uint8_t *shared_secret,
         IN const uint8_t *public_key)
 {
-    POSIX_ENSURE(s2n_pq_is_enabled(), S2N_ERR_PQ_DISABLED);
     DEFER_CLEANUP(EVP_PKEY *kyber_pkey = EVP_PKEY_kem_new_raw_public_key(kem->kem_nid, public_key, kem->public_key_length), EVP_PKEY_free_pointer);
     POSIX_GUARD_PTR(kyber_pkey);
 
@@ -74,7 +72,6 @@ int s2n_kyber_evp_encapsulate(IN const struct s2n_kem *kem, OUT uint8_t *ciphert
 int s2n_kyber_evp_decapsulate(IN const struct s2n_kem *kem, OUT uint8_t *shared_secret, IN const uint8_t *ciphertext,
         IN const uint8_t *private_key)
 {
-    POSIX_ENSURE(s2n_pq_is_enabled(), S2N_ERR_PQ_DISABLED);
     DEFER_CLEANUP(EVP_PKEY *kyber_pkey = EVP_PKEY_kem_new_raw_secret_key(kem->kem_nid, private_key, kem->private_key_length), EVP_PKEY_free_pointer);
     POSIX_GUARD_PTR(kyber_pkey);
 
@@ -90,36 +87,24 @@ int s2n_kyber_evp_decapsulate(IN const struct s2n_kem *kem, OUT uint8_t *shared_
     return S2N_SUCCESS;
 }
 
-#elif !defined(S2N_NO_PQ) /* Use interned Kyber512 implementation, otherwise bail. */
+#else /* If !S2N_LIBCRYPTO_SUPPORTS_KYBER, we won't have a Kyber impl so define relevant stubs here. */
 
 int s2n_kyber_evp_generate_keypair(IN const struct s2n_kem *kem, OUT uint8_t *public_key,
         OUT uint8_t *secret_key)
 {
-    POSIX_ENSURE(s2n_pq_is_enabled(), S2N_ERR_PQ_DISABLED);
-    if (kem == &s2n_kyber_512_r3) {
-        return s2n_kyber_512_r3_crypto_kem_keypair(kem, public_key, secret_key);
-    }
-    POSIX_BAIL(S2N_ERR_UNIMPLEMENTED);
+    POSIX_BAIL(S2N_ERR_NO_SUPPORTED_LIBCRYPTO_API);
 }
 
 int s2n_kyber_evp_encapsulate(IN const struct s2n_kem *kem, OUT uint8_t *ciphertext, OUT uint8_t *shared_secret,
         IN const uint8_t *public_key)
 {
-    POSIX_ENSURE(s2n_pq_is_enabled(), S2N_ERR_PQ_DISABLED);
-    if (kem == &s2n_kyber_512_r3) {
-        return s2n_kyber_512_r3_crypto_kem_enc(kem, ciphertext, shared_secret, public_key);
-    }
-    POSIX_BAIL(S2N_ERR_UNIMPLEMENTED);
+    POSIX_BAIL(S2N_ERR_NO_SUPPORTED_LIBCRYPTO_API);
 }
 
 int s2n_kyber_evp_decapsulate(IN const struct s2n_kem *kem, OUT uint8_t *shared_secret, IN const uint8_t *ciphertext,
         IN const uint8_t *secret_key)
 {
-    POSIX_ENSURE(s2n_pq_is_enabled(), S2N_ERR_PQ_DISABLED);
-    if (kem == &s2n_kyber_512_r3) {
-        return s2n_kyber_512_r3_crypto_kem_dec(kem, shared_secret, ciphertext, secret_key);
-    }
-    POSIX_BAIL(S2N_ERR_UNIMPLEMENTED);
+    POSIX_BAIL(S2N_ERR_NO_SUPPORTED_LIBCRYPTO_API);
 }
 
 #endif

pq-crypto/s2n_kyber_evp.h crypto/s2n_kyber_evp.h

@@ -13,11 +13,23 @@
  * permissions and limitations under the License.
  */
 
-#pragma once
+#include "s2n_pq.h"
 
-#include "utils/s2n_result.h"
+#include "crypto/s2n_openssl.h"
 
-typedef S2N_RESULT (*s2n_get_random_bytes_callback)(uint8_t *buffer, uint32_t num_bytes);
+bool s2n_libcrypto_supports_kyber()
+{
+    /* S2N_LIBCRYPTO_SUPPORTS_KYBER will be auto-detected and #defined if
+     * ./tests/features/S2N_LIBCRYPTO_SUPPORTS_KYBER.c successfully compiles
+     */
+#if defined(S2N_LIBCRYPTO_SUPPORTS_KYBER)
+    return true;
+#else
+    return false;
+#endif
+}
 
-S2N_RESULT s2n_get_random_bytes(uint8_t *buffer, uint32_t num_bytes);
-S2N_RESULT s2n_set_rand_bytes_callback_for_testing(s2n_get_random_bytes_callback rand_bytes_callback);
+bool s2n_pq_is_enabled()
+{
+    return s2n_libcrypto_supports_kyber();
+}

pq-crypto/s2n_pq_random.h crypto/s2n_pq.c

@@ -18,14 +18,8 @@
 #include <stdbool.h>
 
 #include "crypto/s2n_fips.h"
-#include "pq-crypto/s2n_pq_asm.h"
 #include "utils/s2n_result.h"
 #include "utils/s2n_safety.h"
 
-bool s2n_kyber512r3_is_avx2_bmi2_enabled(void);
-S2N_RESULT s2n_try_enable_kyber512r3_opt_avx2_bmi2(void);
-S2N_RESULT s2n_disable_kyber512r3_opt_avx2_bmi2(void);
-
 bool s2n_pq_is_enabled(void);
 bool s2n_libcrypto_supports_kyber(void);
-S2N_RESULT s2n_pq_init(void);

pq-crypto/s2n_pq.h crypto/s2n_pq.h

@@ -263,7 +263,6 @@ static const char *no_such_error = "Internal s2n error";
     ERR_ENTRY(S2N_ERR_INVALID_STATE, "Invalid state, this is the result of invalid use of an API. Check the API documentation for the function that raised this error for more info") \
     ERR_ENTRY(S2N_ERR_UNSUPPORTED_WITH_QUIC, "Functionality not supported when running with QUIC support enabled") \
     ERR_ENTRY(S2N_ERR_PQ_CRYPTO, "An error occurred in a post-quantum crypto function") \
-    ERR_ENTRY(S2N_ERR_PQ_DISABLED, "Post-quantum crypto is disabled") \
     ERR_ENTRY(S2N_ERR_DUPLICATE_PSK_IDENTITIES, "The list of pre-shared keys provided contains duplicate psk identities") \
     ERR_ENTRY(S2N_ERR_OFFERED_PSKS_TOO_LONG, "The total pre-shared key data is too long to send over the wire") \
     ERR_ENTRY(S2N_ERR_INVALID_SESSION_TICKET, "Session ticket data is not valid") \

error/s2n_errno.c

@@ -222,7 +222,6 @@ typedef enum {
     S2N_ERR_ASYNC_CALLBACK_FAILED,
     S2N_ERR_ASYNC_MORE_THAN_ONE,
     S2N_ERR_PQ_CRYPTO,
-    S2N_ERR_PQ_DISABLED,
     S2N_ERR_INVALID_CERT_STATE,
     S2N_ERR_INVALID_EARLY_DATA_STATE,
     S2N_ERR_PKEY_CTX_INIT,

error/s2n_errno.h

@@ -13,7 +13,7 @@
 # permissions and limitations under the License.
 #
 
-OBJS = $(wildcard ../utils/*.o ../stuffer/*.o ../tls/*.o ../tls/*/*.o ../iana/*.o ../crypto/*.o ../error/*.o ../pq-crypto/*.o  ../pq-crypto/kyber_r3/*.o)
+OBJS = $(wildcard ../utils/*.o ../stuffer/*.o ../tls/*.o ../tls/*/*.o ../iana/*.o ../crypto/*.o ../error/*.o)
 
 .PHONY : all
 all: libs2n.a libs2n.so libs2n.dylib