The stack buffer used to hold the decrypted key in pk_parse_pkcs8_encrypted_der
was statically sized to 2048 bytes, which is not enough for DER encoded 4096bit
RSA keys.
This commit resolves the problem by performing the key-decryption in-place,
circumventing the introduction of another stack or heap copy of the key.
There are two situations where pk_parse_pkcs8_encrypted_der is invoked:
1. When processing a PEM-encoded encrypted key in mbedtls_pk_parse_key.
This does not need adaption since the PEM context used to hold the decoded
key is already constructed and owned by mbedtls_pk_parse_key.
2. When processing a DER-encoded encrypted key in mbedtls_pk_parse_key.
In this case, mbedtls_pk_parse_key calls pk_parse_pkcs8_encrypted_der with
the buffer provided by the user, which is declared const. The commit
therefore adds a small code paths making a copy of the keybuffer before
calling pk_parse_pkcs8_encrypted_der.
If CRT is not used, the helper fields CRT are not assumed to be present in the
RSA context structure, so do the verification directly in this case. If CRT is
used, verification could be done using CRT, but we're sticking to ordinary
verification for uniformity.
This commit adds the function mbedtls_rsa_validate_crt for validating a set of CRT parameters. The function
mbedtls_rsa_check_crt is simplified accordingly.
If rsm != NULL then the curve type has to be Short Weierstrass, as we don't
implement restartable Montgomery now. If and when we do, then it's better to
check for the subcontext only, and not for the curve type.
Exactly one of three ways will be used, so make that clear by using an
if 1 else if 2 else 3 structure.
While at it, don't initialize variables at declaration, just to make extra
sure they're properly initialized afterwards in all code paths.
As done by previous commits for ECC and ECDSA:
- use explicit state assignments rather than increment
- always place the state update right before the operation label
This will make it easier to add restart support for other operations later if
desired.
SSL-specific changes:
- remove useless states: when the last restartable operation on a message is
complete, ssl->state is incremented already, so we don't need any additional
state update: ecrs_state is only meant to complement ssl->state
- rename remaining states consistently as <message>_<operation>
- move some labels closer to the actual operation when possible (no assignment
to variables used after the label between its previous and current position)
Systematically assign state just before the next operation that may return,
rather that just after the previous one. This makes things more local. (For
example, previously precompute_comb() has to handle a state reset for
mul_comb_core(), a kind of coupling that's best avoided.)
Note that this change doesn't move the location of state updates relative
to any potential return point, which is all that matters.
Incrementing the state is error-prone as we can end up doing it too many times
(loops) or not enough (skipped branches), or just make programming mistakes
(eg. the state was incremented twice at the end, so it ended up with a value
not in the enum...)
This is the first step of the rework, the next one will rationalize where the
state assignments are done.
Primality testing is guarded by the configuration flag MBEDTLS_GENPRIME and used in the new RSA helper functions. This
commit adds a corresponding preprocessor directive.
The call would anyway check for pointer equality and return early, but it
doesn't hurt to save a function call, and also this follows more uniformly the
pattern that those two lines go together:
#if defined(MBEDTLS_ECP_RESTARTBLE)
if( rs_ctx != NULL && ...
Alternative RSA implementations can be provided by defining MBEDTLS_RSA_ALT in
config.h, defining an mbedtls_rsa_context struct in a new file rsa_alt.h and
re-implementing the RSA interface specified in rsa.h.
Through the previous reworkings, the adherence to the interface is the only
implementation obligation - in particular, implementors are free to use a
different layout for the RSA context structure.
Child was almost redundant as it's already saved in ver_chain, except it was
multiplexed to also indicate whether an operation is in progress. This commit
removes it and introduces an explicit state variable instead.
This state can be useful later if we start returning IN_PROGRESS at other
points than find_parent() (for example when checking CRL).
Note that the state goes none -> find_parent and stays there until the context
is free(), as it's only on the first call that nothing was in progress.
Some parts were already implicitly using this as the two ifdefs were nested,
and some others didn't, which resulted in compile errors in some configs. This
fixes those errors and saves a bit of code+RAM that was previously wasted when
ECP_RESTARTABLE was defined but ECDSA_C wasn't
Previously we kept the ecdsa context created by the PK layer for ECDSA
operations on ECKEY in the ecdsa_restart_ctx structure, which was wrong, and
caused by the fact that we didn't have a proper handling of restart
sub-contexts in the PK layer.
The fact that you needed to pass a pointer to mbedtls_ecdsa_restart_ctx (or
that you needed to know the key type of the PK context) was a breach of
abstraction.
Change the API (and callers) now, and the implementation will be changed in
the next commit.
