In asn1_write tests, when there's a parsing function corresponding to the
write function, call it and check that it can parse what we wrote.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Test with the output buffer size up to *and including* the expected output
size plus one. `... < expected->len + 1` was evidently a mistake.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
mbedtls_asn1_write_mpi() correctly handles the sign bit, so there's no
reason not to test that it's handled correctly.
Fix copypasta in test data that was commented out.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
If the key agreement or the public key export in
ssl_write_client_key_exchange() fails, the handshake enters a failed state.
The only valid thing you can do in a failed handshake is to abort it, which
calls mbedtls_ssl_handshake_free(), which destroys ecdh_psa_privey. While
it's good hygiene to destroy the key in the function that creates it, it
would have been cleaned up a little later in the normal course of things
anyway, so there wasn't an actual bug.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Fix a null pointer dereference when performing some operations on zero
represented with 0 limbs: mbedtls_mpi_mod_int() dividing by 2, or
mbedtls_mpi_write_string() in base 2.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
GCC 12 emits a warning because it thinks `buffer1` is used after having been
freed. The code is correct C because we're only using the value of
`(uintptr_t)buffer1`, not `buffer1`. However, we aren't using the value for
anything useful: it doesn't really matter if an alloc-free-alloc sequence
returns the same address twice. So don't print that bit of information, and
this way we don't need to save the old address.
Fixes#5974.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
This is an external function, so in the absence of link-time
optimisation (LTO) the compiler can't know anything about it and has to
call it the number of times it's called in the source code.
This only matters for pk_ec, but change pk_rsa as well for the sake of
uniformity.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
ssl_client2.c used to check that we force a ciphersuite that worked;
that would have prevented testing so I removed it. The library should be
robust even when the application tries something that doesn't work.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
That's actually the only non-PSK key exchange that needs to be
negative-tested: all the other key exchanges are either positive-tested
or use RSA, for which we can't even create opaque keys in this branch.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
ECDSA keys work with mbedtls_pk_ec() too, but we don't want to accept
them here, so the comment should reflect that the check is not just
about ensuring pk_ec() works.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Trusting the caller to perform the appropriate check is both risky, and
a bit user-unfriendly. Returning NULL on error seems both safer
(dereferencing a NULL pointer is more likely to result in a clean crash,
while mis-casting a pointer might have deeper, less predictable
consequences) and friendlier (the caller can just check the return
value for NULL, which is a common idiom).
Only add that as an additional way of using the function, for the sake
of backwards compatibility. Calls where we know the type of the context
for sure (for example because we just set it up) were legal and safe, so
they should remain legal without checking the result for NULL, which
would be redundant.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
The previous wording "ensure it holds an XXX" context did not mean
anything without looking at the source.
Looking at the source, the criterion is:
- for mbedtls_pk_rsa(), that the info structure uses rsa_alloc_wrap;
- for mbedtls_pk_ec(), that it uses eckey_alloc_wrap or
ecdsa_alloc_wrap, since mbedtls_ecdsa_context is a typedef for
mbedtls_ecp_keypair. (Note that our test code uses mbedtls_pk_ec() on
contexts of type MBEDTLS_PK_ECDSA.)
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Using opaque keys for static ECDH is not supported in this branch (will
be introduced in 3.2). In case we reach that point, error out cleanly
instead of miscasting a pointer. Since opaque keys were introduced,
mbedtls_pk_can_do() was no longer a precise enough check.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
