Goals include:
- reducing the number of local variables in the main function (so that we
don't have to worry about saving/restoring them)
- reducing the number exit points in the main function, making it easier to
update ssl->state only right before we return
- more consistent naming with ecrs prefix for everything
- always check it enabled before touching the rest
- rm duplicated code in parse_server_hello()
For selection of test cases, see comments added in the commit.
It makes the most sense to test with chains using ECC only, so for the chain
of length 2 we use server10 -> int-ca3 -> int-ca2 and trust int-ca2 directly.
Note: server10.crt was created by copying server10_int3_int-ca2.crt and
manually truncating it to remove the intermediates. That base can now be used
to create derived certs (without or with a chain) in a programmatic way.
This is mainly for the benefit of SSL modules, which only supports restart in
a limited number of cases. In the other cases (ECDHE_PSK) it would currently
return ERR_ECP_IN_PROGRESS and the user would thus call ssl_handshake() again,
but the SSL code wouldn't handle state properly and things would go wrong in
possibly unexpected ways. This is undesirable, so it should be possible for
the SSL module to choose if ECDHE should behave the old or the new way.
Not that it also brings ECDHE more in line with the other modules which
already have that choice available (by passing a NULL or valid restart
context).
For RSA, we could either have the function return an error code like
NOT_IMPLEMENTED or just run while disregarding ecp_max_ops. IMO the second
option makes more sense, as otherwise the caller would need to check whether
the key is EC or RSA before deciding to call either sign() or
sign_restartable(), and having to do this kind of check feels contrary to the
goal of the PK layer.
Two different changes:
- the first one will allow us to store k in the restart context while
restarting the following ecp_mul() operation
- the second one is an simplification, unrelated to restartability, made
possible by the fact that ecp_gen_privkey() is now public
(Unrelated to restartable work, just noticed while staring at the code.)
Checking at the end is inefficient as we might give up when we just generated
a valid signature or key.
Test relies on deterministic signature as this uses plain sig internally, so
if deterministic works, then so does non-deterministic, while the reciprocal
is false. (Also, deterministic is enabled by default in config.h.)
Test case is taken from a RFC 6979 test vector, just manually converting (r,s)
to the encoded signature.
Otherwise code that uses these functions in other modules will have to do:
#if defined(MBEDTLS_ECP_RESTARTABLE)
ret = do_stuff( there, may, be, many, args );
#else
ret = do_stuff( their, may, be, namy, args, rs_ctx );
#fi
and there is a risk that the arg list will differ when code is updated, and
this might not be caught immediately by tests because this depends on a
config.h compile-time option which are harder to test.
Always declaring the restartable variants of the API functions avoids this
problem; the cost in ROM size should be negligible.
This will be useful for restartable ECDH and ECDSA. Currently they call
mbedtls_ecp_gen_keypair(); one could make that one restartable, but that means
adding its own sub-context, while ECDH and ECDSA (will) have their own
contexts already, so switching to this saves one extra context.
This should only be done in the top-level function.
Also, we need to know if we indeed are the top-level function or not: for
example, when mbedtls_ecp_muladd() calls mbedtls_ecp_mul(), the later should
not reset ops_done. This is handled by the "depth" parameter in the restart
context.
