Commit Graph

2775 Commits

Author SHA1 Message Date
Manuel Pégourié-Gonnard
1c678e0e06 Update doc about minimum max_ops value
Ok, so the original plan was to make mpi_inv_mod() the smallest block that
could not be divided. Updated plan is that the smallest block will be either:
- ecp_normalize_jac_many() (one mpi_inv_mod() + a number or mpi_mul_mpi()s)
- or the second loop in ecp_precompute_comb()

With default settings, the minimum non-restartable sequence is:
- for P-256: 222M
- for P-384: 341M

This is within a 2-3x factor of originally planned value of 120M. However,
that value can be approached, at the cost of some performance, by setting
ECP_WINDOW_SIZE (w below) lower than the default of 6. For example:
- w=4 -> 166M for any curve (perf. impact < 10%)
- w=2 -> 130M for any curve (perf. impact ~ 30%)

My opinion is that the current state with w=4 is a good compromise, and the
code complexity need to attain 120M is not warranted by the 1.4 factor between
that and the current minimum with w=4 (which is close to optimal perf).
2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
213541a548 Make the first precomp loop restartable 2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
ae55707b28 Turn double loop into single loop
In preparation for making the loop restartable
2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
e2d7cb3f10 Start splitting precompute_comb()
This is the easy part: with the current steps, all information between steps
is passed via T which is already saved. Next we'll need to split at least the
first loop, and maybe calls to normalize_jac_many() and/or the second loop.
2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
67c83fb871 Give a constant a name 2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
fc3e0beabf Separate auxiliary array in precompute_comb()
Separating main computation from filling of the auxiliary array makes things
clearer and easier to restart as we don't have to remember the in-progress
auxiliary array.
2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
07bf6f52c1 Tune T ownership code + comments
Don't miss the little code changes among all those comments change :)
2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
085b1dff40 Allow T to be computed in multiple steps
Previously there were only two states:
- T unallocated
- T allocated and valid

Now there are three:
- T unallocated
- T allocated and in progress
- T allocated and valid

Introduce new bool T_ok to distinguish the last two states.
2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
c9c0aa6306 Remember precomputed table
Free it as soon as it's no longer needed, but as a backup free it in
ecp_group_free(), in case ecp_mul() is not called again after returning
ECP_IN_PROGRESS.

So far we only remember it when it's fully computed, next step is to be able
to compute it in multiple steps.
2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
c5d844b999 Full restart support in ecp_mul_comb_core()
Still recomputing table every time, though.
2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
2fad7ae02a Start actually splitting computation
Temporary state is quite inefficient: pre-computed table is recomputed every
single time. This is WIP obviously.
2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
8962ddbb23 Don't write to destination until we're done 2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
78d564a841 Add check for changing arguments
In case of argument change, freeing everything is not the most efficient
(wastes one free()+calloc()) but makes the code simpler, which is probably
more important here
2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
77af79a324 Add proper allocation of restart context
We'll need to store MPIs and other things that allocate memory in this
context, so we need a place to free it. We can't rely on doing it before
returning from ecp_mul() as we might return MBEDTLS_ERR_ECP_IN_PROGRESS (thus
preserving the context) and never be called again (for example, TLS handshake
aborted for another reason). So, ecp_group_free() looks like a good place to
do this, if the restart context is part of struct ecp_group.

This means it's not possible to use the same ecp_group structure in different
threads concurrently, but:
- that's already the case (and documented) for other reasons
- this feature is precisely intended for environments that lack threading

An alternative option would be for the caller to have to allocate/free the
restart context and pass it explicitly, but this means creating new functions
that take a context argument, and putting a burden on the user.
2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
62738e9b17 Further restrict variable scope by moving code 2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
391f44153d Move more code to separate function
This reduces the scope of some variables (M, k), clarifying where they're
used.
2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
24be79588d Group related code together 2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
4b2336d7f6 Move some more code to new function 2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
22be635d13 Re-order some more code 2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
ec5606ad0c Extract code to separate function
ecp_mul_comb() is already 110 lines long and we're going to add complexity
with the early-return+restart code, so let's try to make it simpler first.
2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
7306dff01f Group related code together
This will be split to a new function next.
2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
510d5caece Add early return test + fake implementation 2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
054433c493 Add mbedtls_ecp_set_max_ops()
The plan is to count basic operations as follows:
- call to ecp_add_mixed()   -> 11
- call to ecp_double_jac()  -> 8
- call to mpi_mul_mpi()     -> 1
- call to mpi_inv_mod()     -> 120
- everything else           -> not counted

The counts for ecp_add_mixed() and ecp_double_jac() are based on the actual
number of calls to mpi_mul_mpi() they they make.

The count for mpi_inv_mod() is based on timing measurements on K64F and
LPC1768 boards, and are consistent with the usual very rough estimate of one
inversion = 100 multiplications. It could be useful to repeat that measurement
on a Cortex-M0 board as those have smaller divider and multipliers, so the
result could be a bit different but should be the same order of magnitude.

The documented limitation of 120 basic ops is due to the calls to mpi_inv_mod()
which are currently not interruptible nor planned to be so far.
2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
5e3c62fd1d Add MBEDTLS_ERR_ECP_IN_PROGRESS 2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
c3a3bc7636 Add config flag MBEDTLS_ECP_EARLY_RETURN 2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
225b37a543 Fix typos in comment 2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
24611f9383 Remove redundant variable
path_cnt was always chain_len - 1 in the loop body
2017-08-09 10:28:07 +02:00
Manuel Pégourié-Gonnard
562df401d3 Improve some comments, fix some typos+whitespace 2017-08-08 18:17:53 +02:00
Manuel Pégourié-Gonnard
66a36b03c6 Update comments 2017-08-08 11:06:51 +02:00
Manuel Pégourié-Gonnard
505c3953c7 Make the ver_chain length explicit 2017-08-08 11:06:51 +02:00
Manuel Pégourié-Gonnard
a707e1d1ef Extract code to separate function for readablity 2017-08-08 11:06:51 +02:00
Manuel Pégourié-Gonnard
ce6e52ff42 Make verify_chain() iterative 2017-08-08 11:06:51 +02:00
Manuel Pégourié-Gonnard
f86f491f25 Rm unneeded function arguments & update comments 2017-08-08 11:06:51 +02:00
Manuel Pégourié-Gonnard
c547d1ab1f Start using an explicit stack for callback info
This is the first step towards making verify_chain() iterative. While from a
readability point of view the current recursive version is fine, one of the
goals of this refactoring is to prepare for restartable ECC integration, which
will need the explicit stack anyway.
2017-08-08 11:06:51 +02:00
Manuel Pégourié-Gonnard
a468eb1764 verify_name(): factor duplicated code to function 2017-08-08 11:06:51 +02:00
Manuel Pégourié-Gonnard
1300e99eb1 Extract name checking to separate function
Just copy-paste and unindent
2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard
6368612a8f Move code to separate function for readability 2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard
27e94797aa Simplify handling of locally trusted EE certs
Though this might require one more walk of the list in some cases,
this avoid having a check for that deep inside check_parent().
2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard
bdc5440232 Update comments 2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard
cb39610093 Finally merge the remains of top() into child() 2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard
63642776b1 Let verify_top() handle only the parent
It felt wrong for it to call the vrfy callback on two certs.
2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard
6e786747fb Move top()'s checks on child to child() 2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard
784aee3366 Move other special case from top() to child() 2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard
b9983be73a Move one special case from verify_top() to child() 2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard
66fac75f8b Merge duplicated checks between child() and top() 2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard
58dcd2d9b2 Get rid of unused variables/arguments 2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard
8f8c282de9 Merge near-duplicated (grand)parent finding code
Besides avoiding near-duplication, this avoids having three generations of
certificate (child, parent, grandparent) in one function, with all the
off-by-one opportunities that come with it.

This also allows to simplify the signature of verify_child(), which will be
done in next commit.
2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard
f82a4d5aba Factor duplicated code into function 2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard
32fdc60c7b Unnest code in verify_top()
We now know that trust_ca != NULL till the end of the function
2017-08-08 11:06:50 +02:00
Manuel Pégourié-Gonnard
6038cb6909 Remove duplicate parent-searching in verify_top() 2017-08-08 11:06:50 +02:00