In a hypothetical build with no curves, or in the future when we add a
new curve type and possibly forget updating this function with a new
block for the new type, we write to `ret` at the beginning or the
function then immediately overwrite it with MPI_CHK(check_privkey),
which static analyzers understandably find questionable.
Use `ret` here and check the key only if it was actually set.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
I went for "may be" as I was thinking just checking the tag technically
does not guarantee that what follows is correct, but I was wrong:
according to ASN.1, when there are variants, the tag does distinguish
unambiguously between variants, so we can be more positive here.
(Whether the thing inside that variant is correct is a different
question.)
As a welcome side effect, this makes the name more standard hence more
readable.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Using return here is only correct if we know that group_load() is atomic
(either succeeds, or allocates no ressources). I'm not sure it is, and
even if it were, goto exit is more obviously correct, so let's use that.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
The recent changes in pkparse made it so ECDSA (deterministic or not) is
set as the secondary alg and ECDH the first one. This broke the wrapper
in pk_wrap as it was only checking the first alg when deciding whether
to use deterministic or not. The wrapper should not have unnecessary
requirements on how algs are set up, so make the check more flexible.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
One of the calling site needs to distinguish between "the format is
potentially valid but not supported" vs "other errors", and it uses
MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE for that.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
- Fix the logic around format: we were just assuming that if the format
was not compressed, it was uncompressed, but it could also have been
just invalid.
- Remove redundant length check: the fallback does its own checks.
- Remove set_algorithm() that's not needed and introduced a depencency
on ECDSA.
- Some style / naming / scope reduction.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Also new name, for consistency, and documentation.
The signature **p, *end is mostly for parsing functions that may not
consume everything, and need to update the "current" pointer to reflect
what has been consumed. This is not the case here.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
- new semantic: sets the pubkey directly in the PK context
- new name to reflect that semantic and obey the naming scheme
- trivial case first
- documentation and better parameter names
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
- avoid useless use of ret in PSA code, keep only status
- improve variable names
- keep declarations closer to use
- a few internal comments
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
The general order is low-level first, top-level last, for the sake of
static function and avoiding forward declarations.
The obvious exception was functions that parse files that were at the
beginning; move them to the end.
Also start defining sections in the file; this is incomplete as I don't
have a clear view of the beginning of the file yet. Will continue in
further commits.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
- new name starting with pk_ecc for consistency
- re-order params to match the PSA convention: buf, len, &size
- add comment about input consumption
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
- centralizes decision making about which version to use when
- avoids nested #ifs in pk_ecc_set_key()
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Just moving code around. The two blocks do morally the same thing: load
the key, and grouping them makes the #if #else structure clearer.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
- add a comment explain potentially surprising parameters
- avoid nesting #if guards: I find the linear structure #if #elif #else
makes the three cases clearer.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
There were two places that were calling either pk_update_ecparams() or
mbedtls_ecp_group_load() depending on the same guard. Factor this into a
single function, that works in both configs, so that callers don't have
to worry about guards.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
- we don't use any ECDSA function here
- we only need to include ecp.h when supporting ECC keys
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
It's also included later, guarded by support for ECC keys, and actually
that's the only case where we need it.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
This changes local variable name RCON to round_constants.
RCON being definition in xc32 compiler headers for some PIC32 register.
Without this change, mynewt project for PIC32 platform fails to build due to
macro redefinition.
This does not changes behavior of library in any way.
Signed-off-by: Jerzy Kasenberg <jerzy.kasenberg@codecoup.pl>
MinGW provides both kinds of implementations of `__cpuid`,
but since `cpuid.h` is provided by GNUC, so we should choose
the implementation by the compiler type instead of OS type.
Signed-off-by: Pengyu Lv <pengyu.lv@arm.com>
Change the type of array that stores the cpuinfo
data to int[4] to match the signature of `__cpuinfo`
in `intrin.h` header file.
Signed-off-by: Pengyu Lv <pengyu.lv@arm.com>
`__cpuid` has two kinds of signatures in different
headers depending on the target OS. We make it
consistent between the usages ang the included header.
Signed-off-by: Pengyu Lv <pengyu.lv@arm.com>
In PSA headers include psa/build_info.h instead
of mbedtls/build_info.h. In Mbed TLS, both are
equivalent but not in TF-PSA-Crypto where
psa/build_info.h is the correct one.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
GCC warns about comparing uint8_t to a size that may be >255.
Strangely, casting the uint8_t to a size_t in the comparison expression
doesn't avoid the warning. So change the type of the variable.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
On a parsing error in TLS, return MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE, not a
crypto error code.
On error paths, emit a level-1 debug message. Report the offending sizes.
Downgrade an informational message's level to 3.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
In client-side code with MBEDTLS_USE_PSA_CRYPTO, use the buffer size to
validate what is written in handshake->xxdh_psa_peerkey. The previous code
was correct, but a little fragile to misconfiguration or maintenance.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Fix a buffer overflow in TLS 1.2 ClientKeyExchange parsing. When
MBEDTLS_USE_PSA_CRYPTO is enabled, the length of the public key in an ECDH
or ECDHE key exchange was not validated. This could result in an overflow of
handshake->xxdh_psa_peerkey, overwriting further data in the handshake
structure or further on the heap.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Fix a buffer overflow in TLS 1.3 ServerHello and ClientHello parsing. The
length of the public key in an ECDH- or FFDH-based key exchange was not
validated. This could result in an overflow of handshake->xxdh_psa_peerkey,
overwriting further data in the handshake structure or further on the heap.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
MBEDTLS_TARGET_PREFIX is prepended to the CMake targets for Mbed TLS
except for targets in 3rdparty. Change this so that 3rdparty targets use
the prefix as well.
This allows multiple copies of Mbed TLS to be used in the same CMake
tree when using code in the 3rdparty directory.
Signed-off-by: David Horstmann <david.horstmann@arm.com>
Restore guards from the previous release, instead of the new, more
permissive guards.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Now that we are generating psa_crypto_driver_wrappers.h, we need to pass
build/library as an include directory.
Signed-off-by: David Horstmann <david.horstmann@arm.com>
Move get_key_buf_size/get_builtin_key out of
the psa wrapper auto generated file
Slot_management.c include the head file instead of the source file
Signed-off-by: Xiaokang Qian <xiaokang.qian@arm.com>
Separate the fits-in-buffer check (*data_length <= data_size) from the
we-think-it's-a-sensible-size check (*data_length <=
MBEDTLS_X509_MAX_DN_NAME_SIZE).
This requires using an intermediate buffer for the DER data, since its
maximum sensible size has to be larger than the maximum sensible size for
the payload, due to the overhead of the ASN.1 tag+length.
Remove test cases focusing on the DER length since the implementation no
longer has a threshold for it.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Rename the function from parse_attribute_value_der_encoded: the hex aspect
seems important.
There was a buffer overflow due to not validating that the intermediate data
fit in the stack buffer. The rewrite doesn't use this buffer, and takes care
not to overflow the buffer that it does use.
Document all that's going on.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Conflicts:
* `include/mbedtls/build_info.h`: a new fragment to auto-enable
`MBEDTLS_CIPHER_PADDING_PKCS7` was added in
c9f4040f7f in `development-restricted`.
In `development`, this section of the file has moved to
`include/mbedtls/config_adjust_legacy_crypto.h`.
* `library/bignum.c`: function name change in `development-restricted` vs
comment change in development. The comment change in `development` is not
really relevant, so just take the line from `development-restricted`.
Variable had the very Windows name of lengthAsInt, which is fine for C# but
doesn't match the Mbed TLS coding standards.
Signed-off-by: Simon Butcher <simon.butcher@arm.com>
Changes to the build to add the new Win32 Crypto API's inadvertently broke
the build for mingw and Visual Studio builds when generated by CMake.
Signed-off-by: Simon Butcher <simon.butcher@arm.com>
CryptGenRandom and lstrlenW are not permitted in Windows Store apps,
meaning apps that use mbedTLS can't ship in the Windows Store.
Instead, use BCryptGenRandom and wcslen, respectively, which are
permitted.
Also make sure conversions between size_t, ULONG, and int are
always done safely; on a 64-bit platform, these types are different
sizes.
Also suppress macro redefinition warning for intsafe.h:
Visual Studio 2010 and earlier generates C4005 when including both
<intsafe.h> and <stdint.h> because a number of <TYPE>_MAX constants
are redefined. This is fixed in later versions of Visual Studio.
The constants are guaranteed to be the same between both files,
however, so we can safely suppress the warning when including
intsafe.h.
Signed-off-by: Kevin Kane <kkane@microsoft.com>
