Merge pull request #9 from aido/1.7.4-rc.2

v1.7.4-rc.2 : Ensure result does not overlap with operands in calls to `cx_bn_gf2_n_mul()`
aido · Jun 11, 2024 · 6a841cf · 6a841cf
2 parents 92bd8b7 + 0686dbc
commit 6a841cf
Show file tree

Hide file tree

Showing 4 changed files with 67 additions and 54 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,10 +5,16 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [1.7.4-rc.2] - 2024-06-03
+
+### Fixed
+- Use `cx_bn_gf2_n_mul()` syscall instead of internal `bn_gf2_n_mul()` on LNSP and LNX
+- Ensure result does not overlap with operands in calls to `cx_bn_gf2_n_mul()`
+
 ## [1.7.4-rc.1] - 2024-05-31
 
 ### Fixed
-- Use internal `bn_gf2_n_mul()` instead of `cx_bn_gf2_n_mul()` syscall
+- Use internal `bn_gf2_n_mul()` instead of `cx_bn_gf2_n_mul()` syscall on all devices
 
 ## [1.7.3] - 2024-05-29
 

diff --git a/Makefile b/Makefile
@@ -26,12 +26,12 @@ all: default
 # Main app configuration
 
 APPNAME = "Seed Tool"
-APPVERSION_M  = 1
-APPVERSION_N  = 7
-APPVERSION_P  = 4
-APPVERSION_RC = 1
-APPVERSION    = "$(APPVERSION_M).$(APPVERSION_N).$(APPVERSION_P)"
+APPVERSION_M = 1
+APPVERSION_N = 7
+APPVERSION_P = 4
+APPVERSION   = "$(APPVERSION_M).$(APPVERSION_N).$(APPVERSION_P)"
 
+APPVERSION_RC = 2
 ifdef APPVERSION_RC
     APPVERSION := $(APPVERSION)-rc.$(APPVERSION_RC)
 endif

diff --git a/src/sskr/sss/interpolate.c b/src/sskr/sss/interpolate.c
@@ -23,9 +23,12 @@
 // Minimal required bytes for BN storing a GF(256) value
 #define GF2_8_MPI_BYTES 16
 
+#if defined(TARGET_NANOS) && !defined API_LEVEL
 /**
  * @brief Performs a multiplication over GF(2^n).
  *
+ * @details *bn_r* must be distinct from *bn_a* and *bn_b*.
+ *
  * @param[out] bn_r BN index for the result.
  *
  * @param[in]  bn_a BN index of the first operand.
@@ -44,14 +47,20 @@
  *                  - CX_INVALID_PARAMETER
  *                  - CX_MEMORY_FULL
  */
-cx_err_t bn_gf2_n_mul(cx_bn_t bn_r,
-                      const cx_bn_t bn_a,
-                      const cx_bn_t bn_b,
-                      const cx_bn_t bn_n,
-                      const cx_bn_t bn_h __attribute__((unused))) {
+cx_err_t cx_bn_gf2_n_mul(cx_bn_t bn_r,
+                         const cx_bn_t bn_a,
+                         const cx_bn_t bn_b,
+                         const cx_bn_t bn_n,
+                         const cx_bn_t bn_h __attribute__((unused))) {
     cx_err_t error = CX_OK;
     uint32_t degree, nbits_a, nbits_b;
 
+    // Ensure bn_r is distinct from bn_a and bn_b
+    if (bn_r == bn_a || bn_r == bn_b) {
+        error = CX_INVALID_PARAMETER;
+        goto end;
+    }
+
     // Calculate the degree of the modulus polynomial
     CX_CHECK(cx_bn_cnt_bits(bn_n, &degree));
     degree--;
@@ -66,35 +75,33 @@ cx_err_t bn_gf2_n_mul(cx_bn_t bn_r,
     }
 
     // Preliminaries
-    cx_bn_t bn_tempa, bn_tempb, bn_tempx;
+    cx_bn_t bn_tempa, bn_copy;
     uint32_t bit_indexb = 0;
     size_t nbytes;
     bool bit_set = 0;
 
     CX_CHECK(cx_bn_nbytes(bn_n, &nbytes));
     CX_CHECK(cx_bn_alloc(&bn_tempa, nbytes));
-    CX_CHECK(cx_bn_alloc(&bn_tempb, nbytes));
-    CX_CHECK(cx_bn_alloc(&bn_tempx, nbytes));
+    CX_CHECK(cx_bn_alloc(&bn_copy, nbytes));
 
     CX_CHECK(cx_bn_copy(bn_tempa, bn_a));
-    CX_CHECK(cx_bn_copy(bn_tempb, bn_b));
     CX_CHECK(cx_bn_set_u32(bn_r, (uint32_t) 0));
 
     // Main loop for multiplication
     if (nbits_a) {
         while (nbits_b > bit_indexb) {
-            CX_CHECK(cx_bn_tst_bit(bn_tempb, bit_indexb, &bit_set));
+            CX_CHECK(cx_bn_tst_bit(bn_b, bit_indexb, &bit_set));
             if (bit_set) {
-                CX_CHECK(cx_bn_copy(bn_tempx, bn_r));
-                CX_CHECK(cx_bn_xor(bn_r, bn_tempa, bn_tempx));
+                CX_CHECK(cx_bn_copy(bn_copy, bn_r));
+                CX_CHECK(cx_bn_xor(bn_r, bn_tempa, bn_copy));
             }
 
             CX_CHECK(cx_bn_shl(bn_tempa, 1));
             CX_CHECK(cx_bn_tst_bit(bn_tempa, degree, &bit_set));
 
             if (bit_set) {
-                CX_CHECK(cx_bn_copy(bn_tempx, bn_tempa));
-                CX_CHECK(cx_bn_xor(bn_tempa, bn_n, bn_tempx));
+                CX_CHECK(cx_bn_copy(bn_copy, bn_tempa));
+                CX_CHECK(cx_bn_xor(bn_tempa, bn_n, bn_copy));
             }
 
             bit_indexb++;
@@ -103,12 +110,12 @@ cx_err_t bn_gf2_n_mul(cx_bn_t bn_r,
 
     // Clean up
     CX_CHECK(cx_bn_destroy(&bn_tempa));
-    CX_CHECK(cx_bn_destroy(&bn_tempb));
-    CX_CHECK(cx_bn_destroy(&bn_tempx));
+    CX_CHECK(cx_bn_destroy(&bn_copy));
 
 end:
     return error;
 }
+#endif
 
 cx_err_t interpolate(uint8_t n,
                      const uint8_t* xi,
@@ -121,19 +128,19 @@ cx_err_t interpolate(uint8_t n,
 
     cx_err_t error = CX_OK;  // By default, until some error occurs
     cx_bn_t bn_x, bn_xc_i;
-    cx_bn_t bn_numerator, bn_denominator, bn_lagrange;
-    cx_bn_t bn_result, bn_tempa, bn_tempb, bn_n, bn_r2;
+    cx_bn_t bn_numerator, bn_lagrange;
+    cx_bn_t bn_result, bn_tempa, bn_tempb, bn_tempc, bn_n, bn_r2;
     uint32_t result_u32;
 
     CX_CHECK(cx_bn_lock(GF2_8_MPI_BYTES, 0));
     CX_CHECK(cx_bn_alloc(&bn_x, GF2_8_MPI_BYTES));
     CX_CHECK(cx_bn_alloc(&bn_xc_i, GF2_8_MPI_BYTES));
     CX_CHECK(cx_bn_alloc(&bn_numerator, GF2_8_MPI_BYTES));
-    CX_CHECK(cx_bn_alloc(&bn_denominator, GF2_8_MPI_BYTES));
     CX_CHECK(cx_bn_alloc(&bn_lagrange, GF2_8_MPI_BYTES));
     CX_CHECK(cx_bn_alloc(&bn_result, GF2_8_MPI_BYTES));
     CX_CHECK(cx_bn_alloc(&bn_tempa, GF2_8_MPI_BYTES));
     CX_CHECK(cx_bn_alloc(&bn_tempb, GF2_8_MPI_BYTES));
+    CX_CHECK(cx_bn_alloc(&bn_tempc, GF2_8_MPI_BYTES));
     CX_CHECK(cx_bn_alloc_init(&bn_n, GF2_8_MPI_BYTES, N, sizeof(N)));
     CX_CHECK(cx_bn_alloc_init(&bn_r2, GF2_8_MPI_BYTES, R2, sizeof(R2)));
 
@@ -160,45 +167,45 @@ cx_err_t interpolate(uint8_t n,
                 CX_CHECK(cx_bn_xor(bn_numerator, bn_x, bn_tempa));
 
                 // Calculate the denominator (xc[i] - xc[j])
-                CX_CHECK(cx_bn_xor(bn_denominator, bn_xc_i, bn_tempa));
+                CX_CHECK(cx_bn_xor(bn_tempb, bn_xc_i, bn_tempa));
 
                 // Calculate the inverse of the denominator
                 // In GF(2^8) the inverse of x = x^254
-                // bn_result = bn_denominator^2
-                CX_CHECK(bn_gf2_n_mul(bn_result, bn_denominator, bn_denominator, bn_n, bn_r2));
-                // bn_result = bn_denominator^4
-                CX_CHECK(bn_gf2_n_mul(bn_result, bn_result, bn_result, bn_n, bn_r2));
-                // bn_tempa = bn_denominator^8
-                CX_CHECK(bn_gf2_n_mul(bn_tempa, bn_result, bn_result, bn_n, bn_r2));
-                // bn_tempb = bn_denominator^9
-                CX_CHECK(bn_gf2_n_mul(bn_tempb, bn_tempa, bn_denominator, bn_n, bn_r2));
-                // bn_tempa = bn_denominator^16
-                CX_CHECK(bn_gf2_n_mul(bn_tempa, bn_tempa, bn_tempa, bn_n, bn_r2));
-                // bn_tempa = bn_denominator^25
-                CX_CHECK(bn_gf2_n_mul(bn_tempa, bn_tempa, bn_tempb, bn_n, bn_r2));
-                // bn_tempa = bn_denominator^50
-                CX_CHECK(bn_gf2_n_mul(bn_tempa, bn_tempa, bn_tempa, bn_n, bn_r2));
-                // bn_tempb = bn_denominator^100
-                CX_CHECK(bn_gf2_n_mul(bn_tempb, bn_tempa, bn_tempa, bn_n, bn_r2));
-                // bn_tempb = bn_denominator^200
-                CX_CHECK(bn_gf2_n_mul(bn_tempb, bn_tempb, bn_tempb, bn_n, bn_r2));
-                // bn_tempa = bn_denominator^250
-                CX_CHECK(bn_gf2_n_mul(bn_tempa, bn_tempa, bn_tempb, bn_n, bn_r2));
-                // bn_denominator = bn_denominator^254
-                CX_CHECK(bn_gf2_n_mul(bn_denominator, bn_result, bn_tempa, bn_n, bn_r2));
+                // bn_tempa = denominator^2
+                CX_CHECK(cx_bn_gf2_n_mul(bn_tempa, bn_tempb, bn_tempb, bn_n, bn_r2));
+                // bn_result = denominator^4
+                CX_CHECK(cx_bn_gf2_n_mul(bn_result, bn_tempa, bn_tempa, bn_n, bn_r2));
+                // bn_tempa = denominator^8
+                CX_CHECK(cx_bn_gf2_n_mul(bn_tempa, bn_result, bn_result, bn_n, bn_r2));
+                // bn_tempc = denominator^9
+                CX_CHECK(cx_bn_gf2_n_mul(bn_tempc, bn_tempa, bn_tempb, bn_n, bn_r2));
+                // bn_tempb = denominator^16
+                CX_CHECK(cx_bn_gf2_n_mul(bn_tempb, bn_tempa, bn_tempa, bn_n, bn_r2));
+                // bn_tempa = denominator^25
+                CX_CHECK(cx_bn_gf2_n_mul(bn_tempa, bn_tempb, bn_tempc, bn_n, bn_r2));
+                // bn_tempb = denominator^50
+                CX_CHECK(cx_bn_gf2_n_mul(bn_tempb, bn_tempa, bn_tempa, bn_n, bn_r2));
+                // bn_tempc = denominator^100
+                CX_CHECK(cx_bn_gf2_n_mul(bn_tempc, bn_tempb, bn_tempb, bn_n, bn_r2));
+                // bn_tempa = denominator^200
+                CX_CHECK(cx_bn_gf2_n_mul(bn_tempa, bn_tempc, bn_tempc, bn_n, bn_r2));
+                // bn_tempc = denominator^250
+                CX_CHECK(cx_bn_gf2_n_mul(bn_tempc, bn_tempa, bn_tempb, bn_n, bn_r2));
+                // bn_tempb = denominator^254
+                CX_CHECK(cx_bn_gf2_n_mul(bn_tempb, bn_result, bn_tempc, bn_n, bn_r2));
 
                 // Calculate the lagrange basis coefficient
-                CX_CHECK(bn_gf2_n_mul(bn_lagrange, bn_numerator, bn_lagrange, bn_n, bn_r2));
-                CX_CHECK(bn_gf2_n_mul(bn_lagrange, bn_denominator, bn_lagrange, bn_n, bn_r2));
+                CX_CHECK(cx_bn_gf2_n_mul(bn_tempa, bn_numerator, bn_lagrange, bn_n, bn_r2));
+                CX_CHECK(cx_bn_gf2_n_mul(bn_lagrange, bn_tempa, bn_tempb, bn_n, bn_r2));
             }
         }
 
         for (uint8_t j = 0; j < yl; j++) {
             CX_CHECK(cx_bn_set_u32(bn_tempa, (uint32_t) yij[i][j]));
             CX_CHECK(cx_bn_set_u32(bn_tempb, (uint32_t) result[j]));
 
-            CX_CHECK(bn_gf2_n_mul(bn_tempa, bn_lagrange, bn_tempa, bn_n, bn_r2));
-            CX_CHECK(cx_bn_xor(bn_result, bn_tempa, bn_tempb));
+            CX_CHECK(cx_bn_gf2_n_mul(bn_tempc, bn_lagrange, bn_tempa, bn_n, bn_r2));
+            CX_CHECK(cx_bn_xor(bn_result, bn_tempb, bn_tempc));
             CX_CHECK(cx_bn_get_u32(bn_result, &result_u32));
             result[j] = (uint8_t) result_u32;
             result_u32 = 0;
@@ -209,11 +216,11 @@ cx_err_t interpolate(uint8_t n,
     CX_CHECK(cx_bn_destroy(&bn_x));
     CX_CHECK(cx_bn_destroy(&bn_xc_i));
     CX_CHECK(cx_bn_destroy(&bn_numerator));
-    CX_CHECK(cx_bn_destroy(&bn_denominator));
     CX_CHECK(cx_bn_destroy(&bn_lagrange));
     CX_CHECK(cx_bn_destroy(&bn_result));
     CX_CHECK(cx_bn_destroy(&bn_tempa));
     CX_CHECK(cx_bn_destroy(&bn_tempb));
+    CX_CHECK(cx_bn_destroy(&bn_tempc));
     CX_CHECK(cx_bn_destroy(&bn_n));
     CX_CHECK(cx_bn_destroy(&bn_r2));
 

diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt
@@ -110,7 +110,7 @@ set(PICKY_DEVELOPER OFF CACHE BOOL "CMocka: Build with picky developer flags" FO
 FetchContent_MakeAvailable(cmocka)
 
 add_compile_definitions(TEST DEBUG=0 SKIP_FOR_CMOCKA)
-add_compile_definitions(HAVE_HASH HAVE_HMAC HAVE_SHA224 HAVE_SHA256 HAVE_SHA512 HAVE_PBKDF2 HAVE_ECC HAVE_CRC HAVE_RNG IO_HID_EP_LENGTH=64)
+add_compile_definitions(TARGET_NANOS HAVE_HASH HAVE_HMAC HAVE_SHA224 HAVE_SHA256 HAVE_SHA512 HAVE_PBKDF2 HAVE_ECC HAVE_CRC HAVE_RNG IO_HID_EP_LENGTH=64)
 
 include_directories(${CMAKE_CURRENT_SOURCE_DIR}/lib $ENV{LEDGER_SECURE_SDK}/include $ENV{LEDGER_SECURE_SDK}/lib_cxng/src $ENV{LEDGER_SECURE_SDK}/lib_cxng/include $ENV{LEDGER_SECURE_SDK}/lib_ux/include $ENV{LEDGER_SECURE_SDK}/lib_bagl/include)