Add Montgomery conversion and high level I/O #6213

minosgalanakis · 2022-08-18T17:25:14Z

Description

This pr will be rebased on top of PR 6095 and PR 6083

Because it brings all the changes forward please disregard all the files, aside from the following

git diff df6170759..HEAD --name-status
M       library/bignum_mod.c
M       library/bignum_mod.h
M       library/bignum_mod_raw.c
M       library/bignum_mod_raw.h

Status

*IN DEVELOPMENT

Requires Backporting

NO

Additional comments

Any additional information that could be of interest

Todos

Rebase as
Tests

This patch adds the Montgomery constants to the `mbedtls_mpi_mont_struct`. Signed-off-by: Minos Galanakis <[email protected]>

At the current state, those fields are initialised to 0,NULL. Signed-off-by: Minos Galanakis <[email protected]>

…m prototype Signed-off-by: Minos Galanakis <[email protected]>

This patch modifies `mbedtls_mpi_get_montgomery_constant_unsafe()` to include intermediate mpi management inside the method and simplify invocation parameters. Signed-off-by: Minos Galanakis <[email protected]>

This patch extends the setup method to pre-calculate mm and RR when setting up a modulus structure Signed-off-by: Minos Galanakis <[email protected]>

…resentation Signed-off-by: Minos Galanakis <[email protected]>

This patch updates the the method names as: `mbedtls_mpi_core_mod_conv_fwd` -> `mbedtls_mpi_mod_raw_to_mont_rep` `mbedtls_mpi_core_mod_conv_inv` -> `mbedtls_mpi_mod_raw_from_mont_rep` `mbedtls_mpi_mod_modulus *modulus` is also renamed to `*m` for consistency. Signed-off-by: Minos Galanakis <[email protected]>

This patch adds input and ouput fucntions in the `bignum_mod` layer. The data will be automatically converted between Cannonical and Montgomery representation if required. Signed-off-by: Minos Galanakis <[email protected]>

This patch adds a test for `MBEDTLS_MPI_MOD_REP_MONTGOMERY`, and will not perform operations where input structure is using cannonical representation. Signed-off-by: Minos Galanakis <[email protected]>

This patch updates `mbedtls_mpi_set_montgomery_constant_unsafe()` and `mbedtls_mpi_mod_modulus_setup()` to change the memory allocation hierarchy used for `m->rep.mont.rr`. The memory is now allocated by `mbedtls_mpi_set_montgomery_constant_unsafe()` and freed by `mbedtls_mpi_mod_modulus_free()` Signed-off-by: Minos Galanakis <[email protected]>

…t_struct.rr` Signed-off-by: Minos Galanakis <[email protected]>

Signed-off-by: Minos Galanakis <[email protected]>

…e()` This patch updates input parameters of the method, to eliminate the dependency to a partially initialised structure and to simplify testing. The error codes have also been simplified to 0 when successful and `MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED` when not. Signed-off-by: Minos Galanakis <[email protected]>

yanesca

Had a quick look and left a couple of comments that I thought could be helpful when writing the tests.

yanesca · 2022-10-06T13:08:10Z

library/bignum_mod_raw.c

+                                       const mbedtls_mpi_mod_modulus *m )
+{
+    mbedtls_mpi_uint one = 1;
+    mbedtls_mpi T;


We shouldn't be using the legacy mbedtls_mpi structure in the new bignum modules. (The function mbedtls_mpi_set_montgomery_constant_unsafe() is an exception that we are accepting for now, but mbedtls_mpis will eventually will be removed from there too.)

We should be using raw pointers (mbedtls_mpi_uint *) here instead.

mbedtls_mpi T is only used as temporary storage. The method has been updated to initialise and zeroise that space. The required space shold be limbs_m *2 +1

The only argument for using mpi strucutres, is that the same could achieved by mbedtls_mpi_init( &T ); && mbedtls_mpi_grow( &T, m->limbs* 2 + 1 ) without the need to replicate the size/boundary checks.

Should add an minimal inline checking method in the new api?

yanesca · 2022-10-06T13:13:33Z

library/bignum_mod_raw.c

+{
+    mbedtls_mpi_uint one = 1;
+    mbedtls_mpi T;
+    mbedtls_mpi_init( &T );


mbedtls_mpi_init() doesn't allocate any memory, it just zeroes out the structure. The old API automatically grows initialised MPIs to the size needed, but bignum_core functions don't do that.

This has been addressed as part of the work for the previous item.

yanesca · 2022-10-06T13:14:31Z

library/bignum_mod_raw.c

+{
+    mbedtls_mpi T;
+    mbedtls_mpi_init( &T );
+    mbedtls_mpi_core_montmul( X, X, m->rep.mont.rr, 1, m->p, m->limbs,


I think that the length of m->rep.mont.rr is m->limbs and not 1.

You are indeed correct that this 1 needs to be updated.

Since we brought this subject, the actual limb size needed to represent RR is not always the same always m->limbs We can have an 8 limbs modulo and RR == 0x1 which would need just one limb.

The implicit assumption is that limbs_RR <= limbs_N. It shouldn't affect our operations, but it may be worth to look into for optimisation?

In theory yes, however unless the modulus has some peculiar structure, I would expect RR to need limbs_N in most cases.

This patch updates the method to fail when limb size is invalid. The size limitation of `MBEDTLS_MPI_MAX_LIMBS / 2) - 2` is imposed by R^2 and modular division requirements. Signed-off-by: Minos Galanakis <[email protected]>

This patch removes the dependency on the legaly bignum interface. `mbedtls_mpi` is no longer used for temporary buffer allocation. Signed-off-by: Minos Galanakis <[email protected]>

This patch adds the test for the method calculating the RR. The input/expected data are generated manually using the following Python3 snippet: ~~~~~ def limb_no(number): return int(int.bit_length(number)/64) + 1 def calc_rr(number): return '{:x}'.format(pow(pow(2, limb_no(number) * 64), 2, number)) ~~~~~ Signed-off-by: Minos Galanakis <[email protected]>

This patch separates the arithmetic functionality of the method to the memory allocation and lifecycle logic. `mbedtls_mpi_get_montgomery_constant_unsafe()` has been reverted to the original protype form, and moved to bignum_core. A new static method `set_mont_const_square()` is used for the lifecycle management in the `bignum_mod` layer. Tests have been updated accordingly. Signed-off-by: Minos Galanakis <[email protected]>

minosgalanakis · 2022-12-06T10:23:55Z

This PR has been deprecated and was used as a refference point. The issue #6017 has been resolved so I am closing it

minosgalanakis added enhancement component-crypto Crypto primitives and low-level interfaces needs-preceding-pr Requires another PR to be merged first priority-high High priority - will be reviewed soon labels Aug 18, 2022

minosgalanakis self-assigned this Aug 18, 2022

minosgalanakis linked an issue Aug 18, 2022 that may be closed by this pull request

Bignum: Add Montgomery conversion and high level I/O #6017

Closed

7 tasks

minosgalanakis force-pushed the minos/6017_add_Montgomery_conversion_high_lv_IO branch from d70742f to 9903195 Compare August 19, 2022 08:09

minosgalanakis added the needs-work label Aug 19, 2022

minosgalanakis force-pushed the minos/6017_add_Montgomery_conversion_high_lv_IO branch from 9903195 to e18cf06 Compare September 12, 2022 14:33

minosgalanakis force-pushed the minos/6017_add_Montgomery_conversion_high_lv_IO branch from 89e3d16 to 651dfcd Compare September 21, 2022 22:14

Hanno Becker and others added 13 commits October 4, 2022 10:07

bignum_mod: Added Montgomery constants

63fa4c0

This patch adds the Montgomery constants to the `mbedtls_mpi_mont_struct`. Signed-off-by: Minos Galanakis <[email protected]>

bignum_mod: Updated mbedtls_mpi_mod_modulus_setup/free with new fields

7d50dd9

At the current state, those fields are initialised to 0,NULL. Signed-off-by: Minos Galanakis <[email protected]>

bignum_mod_raw: Ported mbedtls_mpi_get_montgomery_constant_unsafe fro…

6feb0c7

…m prototype Signed-off-by: Minos Galanakis <[email protected]>

binum_mod: Refactored mbedtls_mpi_get_montgomery_constant_unsafe()

0cda524

This patch modifies `mbedtls_mpi_get_montgomery_constant_unsafe()` to include intermediate mpi management inside the method and simplify invocation parameters. Signed-off-by: Minos Galanakis <[email protected]>

binum_mod: Updated mbedtls_mpi_mod_modulus_setup()

82db083

This patch extends the setup method to pre-calculate mm and RR when setting up a modulus structure Signed-off-by: Minos Galanakis <[email protected]>

bignum_mod_raw: Added conversion methods for internal/public data rep…

433cce1

…resentation Signed-off-by: Minos Galanakis <[email protected]>

bignum_mod: Added mbedtls_mpi_mod_read/write() IO functions

da22ee5

This patch adds input and ouput fucntions in the `bignum_mod` layer. The data will be automatically converted between Cannonical and Montgomery representation if required. Signed-off-by: Minos Galanakis <[email protected]>

bignum_mod_raw: Updated bad input data checks

892f4b0

This patch adds a test for `MBEDTLS_MPI_MOD_REP_MONTGOMERY`, and will not perform operations where input structure is using cannonical representation. Signed-off-by: Minos Galanakis <[email protected]>

bignum_mod: Added const qualifier to data pointed by `mbedtls_mpi_mon…

4fafa66

…t_struct.rr` Signed-off-by: Minos Galanakis <[email protected]>

bignum_mod: Cleaned up mbedtls_mpi_mod_read/write()

965280d

Signed-off-by: Minos Galanakis <[email protected]>

minosgalanakis force-pushed the minos/6017_add_Montgomery_conversion_high_lv_IO branch from 651dfcd to 8456beb Compare October 4, 2022 09:09

yanesca reviewed Oct 6, 2022

View reviewed changes

minosgalanakis added 3 commits October 10, 2022 14:55

bignum_mod_raw: Updated mod_raw_from/to_mont_rep()

095c8fe

This patch removes the dependency on the legaly bignum interface. `mbedtls_mpi` is no longer used for temporary buffer allocation. Signed-off-by: Minos Galanakis <[email protected]>

minosgalanakis force-pushed the minos/6017_add_Montgomery_conversion_high_lv_IO branch from 897d619 to eb8aa40 Compare October 10, 2022 13:56

minosgalanakis closed this Dec 6, 2022

gilles-peskine-arm mentioned this pull request Dec 6, 2022

Make Montgomery reduction available to applications #2537

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Montgomery conversion and high level I/O #6213

Add Montgomery conversion and high level I/O #6213

minosgalanakis commented Aug 18, 2022

yanesca left a comment

yanesca Oct 6, 2022

minosgalanakis Oct 10, 2022

yanesca Oct 6, 2022

minosgalanakis Oct 10, 2022

yanesca Oct 6, 2022

minosgalanakis Oct 10, 2022

yanesca Oct 10, 2022

minosgalanakis commented Dec 6, 2022

Add Montgomery conversion and high level I/O #6213

Add Montgomery conversion and high level I/O #6213

Conversation

minosgalanakis commented Aug 18, 2022

Description

Status

Requires Backporting

Additional comments

Todos

yanesca left a comment

Choose a reason for hiding this comment

yanesca Oct 6, 2022

Choose a reason for hiding this comment

minosgalanakis Oct 10, 2022

Choose a reason for hiding this comment

yanesca Oct 6, 2022

Choose a reason for hiding this comment

minosgalanakis Oct 10, 2022

Choose a reason for hiding this comment

yanesca Oct 6, 2022

Choose a reason for hiding this comment

minosgalanakis Oct 10, 2022

Choose a reason for hiding this comment

yanesca Oct 10, 2022

Choose a reason for hiding this comment

minosgalanakis commented Dec 6, 2022