Commit graph

5506 commits

Author SHA1 Message Date
Manuel Pégourié-Gonnard
45fd0164dd Rename multiplication-specific restart context
It's going to be convenient for each function that can generate a
MBEDTLS_ERR_ECP_IN_PROGRESS on its own (as opposed to just passing it around)
to have its own restart context that they can allocate and free as needed
independently of the restart context of other functions.

For example ecp_muladd() is going to have its own restart_muladd context that
in can managed, then when it calls ecp_mul() this will manage a restart_mul
context without interfering with the caller's context.

So, things need to be renames to avoid future name clashes.
2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
e58f65a04b Expand documentation with notes and warnings 2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
e685449004 Scale ops count for larger curves
From a user's perspective, you want a "basic operation" to take approximately
the same amount of time regardless of the curve size, especially since max_ops
is a global setting: otherwise if you pick a limit suitable for P-384 then
when you do an operation on P-256 it will return way more often than needed.

Said otherwise, a user is actually interested in actual running time, and we
do the API in terms of "basic ops" for practical reasons (no timers) but then
we should make sure it's a good proxy for running time.
2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
d3a0ca8500 Add tests about number of restarts 2017-08-09 11:44:53 +02:00
Manuel Pégourié-Gonnard
9c5c78ff5c Fix indicative values of ops counts
Previous measurements were wrong due to counting multiplication by a small
constant as a full multiplication, which it is not.
2017-08-09 11:44:53 +02:00
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
a54f6cc874 Unify name of default profile in X.509 tests 2017-08-09 10:41:42 +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
7ff243a87c Add missing dependency in test-certs Makefile 2017-08-08 18:54:13 +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
ea2dc14c0c Fix some whitespace 2017-08-08 11:10:37 +02:00
Manuel Pégourié-Gonnard
a7c4c8a46c Make some perl scripts usable with git bisect run
For that they need to return between 0 and 124 on error, while die returns
255, causing bisect-run to abort.
2017-08-08 11:06:51 +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