Since a is not a pointer, it is passed by value and declaring it const
doesn’t make any sense and on the first read can make me miss the fact
that a is not a pointer.
Signed-off-by: Janos Follath <janos.follath@arm.com>
Co-authored-by: Tom Cosgrove <81633263+tom-cosgrove-arm@users.noreply.github.com>
Co-authored-by: Werner Lewis <Werner.Lewis@arm.com>
Signed-off-by: Janos Follath <janos.follath@arm.com>
Selected ciphersuite MUST be same with ciphsersuite of PSK.
Overwrite the old ciphersuite with the one of PSK.
Signed-off-by: Jerry Yu <jerry.h.yu@arm.com>
- wrong typo in comments
- replace psk null check with key_exchange_mode check
- set psk NULL when error return in export hs psk
Signed-off-by: Jerry Yu <jerry.h.yu@arm.com>
Avoid the shorthand practice of the form 'x = func(foo)->bar' which
exposes the code to NULL pointer de-referencing when the 'func()'
returns a NULL pointer.
The first chunk is for when the curve group code is not recognized by
the library, and is cleanly rejected if offered.
The second chunk addresses the unlikely case of an internal error:
if 'mbedtls_pk_can_do()' returns TRUE, it should rule out
'mbedtls_pk_ec()' returning a NULL, unless there is a regression.
Signed-off-by: Leonid Rozenboim <leonid.rozenboim@oracle.com>
Co-authored-by: Tom Cosgrove <81633263+tom-cosgrove-arm@users.noreply.github.com>
Co-authored-by: Werner Lewis <werner.wmlewis@gmail.com>
Signed-off-by: Janos Follath <janos.follath@arm.com>
Numbers:
- A, B for mbedtls_mpi_uint* operands
- a, b for mbedtls_mpi_uint operands
- X or x for result
- HAC references where applicable
Lengths:
- Reserve size or length for length/size in bytes or byte buffers.
- For length of mbedtls_mpi_uint* buffers use limbs
- Length parameters are qualified if possible (eg. input_length or
a_limbs)
Setup functions:
- The parameters match the corresponding structure member's name
- The structure to set up is a standard lower case name even if in other
functions different naming conventions would apply
Scope of changes/conventions:
- bignum_core
- bignum_mod
- bignum_mod_raw
Signed-off-by: Janos Follath <janos.follath@arm.com>
This is done to be able to bild test_psa_crypto_config_accel_hash component where MD5 is only available accelerated (PSA_WANT_ALG_MD5 is enabled and MBEDTLS_MD5_C is disabled) but MBEDTLS_USE_PSA_CRYPTO is disabled.
So the build should not attempt to enable pem_pbkdf1.
Signed-off-by: Przemek Stekiel <przemyslaw.stekiel@mobica.com>
In the new bignum files (bignum_core.c, bignum_mod_raw.c and
bignum_mod.c) the loop variables are declared in the loop head wherever
this change is beneficial.
There are loops where the loop variable is used after the end of the
loop (this might not be good practice, but that is out of scope for this
commit) and others where there are several loop variables and declaring
them there would hurt readability.
Signed-off-by: Janos Follath <janos.follath@arm.com>
The modulus value won't change during normal operations, make this clear
in the struct and the function signatures.
This won't prevent the caller from modifying the passed buffer, but
might give a hint and reinforces the message of the documentation.
Signed-off-by: Janos Follath <janos.follath@arm.com>
Skip reading if output pointer is NULL even if the length of the input buffer is 0.
The memory sanitizer will mark this as an error.
Signed-off-by: Gabor Mezei <gabor.mezei@arm.com>
These functions have full documentation in the header. Maintaing two
copies does not worth the effort and having an out of sync reduced
duplicate is not helpful.
Signed-off-by: Janos Follath <janos.follath@arm.com>
This used to resize MPIs in the legacy interface, which is not
needed/possible as the new interface has fixed size MPIs.
Inlining this function makes the code easier to read and maintain, while
there is no obvious drawback to it.
Signed-off-by: Janos Follath <janos.follath@arm.com>
A null pointer dereference, or null pointer plus small offset, is a
clean runtime error in most environments. So it's not particularly
useful to protect against this.
While at it make a null pointer check that is actually necessary more
robust.
Signed-off-by: Janos Follath <janos.follath@arm.com>
- Made use of enums in struct and function declaration
- All enums are handled by switch case now
- If the switch does nothing on default, omit the default case to make
compiler warnings more powerful
- The two enums are now disjoint and the value 1 is skipped to make
mistakes easier to detect
Signed-off-by: Janos Follath <janos.follath@arm.com>
with matched identity and mismatch binder, should check next psk key.
Exit with error will break multi-psk cases.
Signed-off-by: Jerry Yu <jerry.h.yu@arm.com>
With OpenSSL and GnuTLS client, if the MAC of ciphersuite
does not match selected binder, client will reject connection.
This change is to select ciphersuite base on algo of psk binder.
Signed-off-by: Jerry Yu <jerry.h.yu@arm.com>
The code will make the decision based on availability of MD, not of
MD+this_hash. The later would only be possible at runtime (the hash
isn't known until then, that's the whole point of MD), so we'd need to
have both MD-based and PSA-based code paths in a single build, which
would have a very negative impact on code size. So, instead, we choose
based on the presence of MD, which is know at compile time, so we only
have one of the two code paths in each build.
Adjust the macros so that they match the logic of the code using them.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Test coverage not there yet, as the entire test_suite_pkcs1_v21 is
skipped so far - dependencies to be adjusted in a future commit.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
This allows callers not to worry with md_info and makes it easier to
provide a PSA version for when MD_C is not available.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
This will also make it easier to provide a PSA-based version for when MD
is not available.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Some of them no longer need md_ctx, some of those no longer need the
exit dance that was used to free it, or need it on a smaller scope.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
This is a first step towards making a version of this function that
uses PSA when MD is not available.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Since 11e9310 add mbedtls_aes_init call in mbedtls_ctr_drbg_init, it
should not init aes_ctx again in mbedtls_ctr_drbg_seed.
Signed-off-by: kXuan <kxuanobj@gmail.com>
Application may enabled AES_ALT and define mbedtls_aes_context by its own.
The initial state of user-defined mbedtls_aes_context may not all byte zero.
In mbedtls_ctr_drbg_init, the code set all byte to zero, including the AES
context nested in the ctr_drbg context.
And in mbedtls_ctr_drbg_free, the code calls mbedtls_aes_free on an AES
context without calling mbedtls_aes_init.
If user-defined AES context requires an non-zero init, the mbedtls_aes_free
call in mbedtls_ctr_drbg_free is illegal.
This patch fix this issue by add mbedtls_aes_init in mbedtls_ctr_drbg_init.
So aes context will always be initialized to correct state.
Signed-off-by: kXuan <kxuanobj@gmail.com>
Looking at the bigger picture it is clear that if `ssl->session` is NULL,
there will be a failure much earlier, and that is well protected from,
however, the practice of dereferencing a pointer which has not been
verified in prior for validity goes against secure coding practices.
Signed-off-by: Leonid Rozenboim <leonid.rozenboim@oracle.com>
Extract functions declared in bignum_mod.h into a source file with a
matching name.
We are doing this because:
- This is a general best practice/convention
- We hope that this will make resolving merge conflicts in the future
easier
- Having them in a unified source file is a premature optimisation at
this point
This makes library/bignum_new.c empty and therefore it is deleted.
Signed-off-by: Janos Follath <janos.follath@arm.com>
Extract functions declared in bignum_mod_raw.h into a source file with a
matching name.
We are doing this because:
- This is a general best practice/convention
- We hope that this will make resolving merge conflicts in the future
easier
- Having them in a unified source file is a premature optimisation at
this point
Signed-off-by: Janos Follath <janos.follath@arm.com>
Extract functions declared in bignum_core.h into a source file with a
matching name.
We are doing this because:
- This is a general best practice/convention
- We hope that this will make resolving merge conflicts in the future
easier
- Having them in a unified source file is a premature optimisation at
this point
Signed-off-by: Janos Follath <janos.follath@arm.com>
This reverts commit 62c5901f0a5061a8825e19c77f88c91fea235078.
Reverting commit due the macros are meant to be local and not following the
naming convention.
Signed-off-by: Gabor Mezei <gabor.mezei@arm.com>
Unfortunately reusing the new function from the signed constant time
comparison is not trivial.
One option would be to do temporary conditional swaps which would prevent
qualifying input to const. Another way would be to add an additional
flag for the sign and make it an integral part of the computation, which
would defeat the purpose of having an unsigned core comparison.
Going with two separate function for now and the signed version can be
retired/compiled out with the legacy API eventually.
The new function in theory could be placed into either
`library/constant_time.c` or `library/bignum_new.c`. Going with the
first as the other functions in the second are not constant time yet and
this distinction seems more valuable for new (as opposed to belonging to
the `_core` functions.
Signed-off-by: Janos Follath <janos.follath@arm.com>
- We don't check for NULL pointers this deep in the library
- Accessing a NULL pointer when the limb number is 0 as a mistake is the
very similar to any other out of bounds access
- We could potentially mandate at least 1 limb representation for 0 but
we either would need to enforce it or the implementation would be less
robust.
- Allowing zero limb representation - (NULL, 0) in particular - for zero
is present in the legacy interface, if we disallow it, the
compatibility code will need to deal with this (more code size and
opportunities for mistakes)
In summary, interpreting (NULL, 0) as the number zero in the core
interface is the least of the two evils.
Signed-off-by: Janos Follath <janos.follath@arm.com>
The test case where there were extra limbs in the MPI failed and this
commit contains the corresponding fix as well. (We used to use the
minimum required limbs instead of the actual limbs present.)
Signed-off-by: Janos Follath <janos.follath@arm.com>
Fix usage with sed:
s/MBEDTLS_OR_PSA_WANT_\([A-Z_0-9]*\)/MBEDTLS_HAS_\1_VIA_LOWLEVEL_OR_PSA/
s/MBEDTLS_USE_PSA_WANT_\([A-Z_0-9]*\)/MBEDTLS_HAS_\1_VIA_MD_OR_PSA_BASED_ON_USE_PSA/
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
If there is a PSK involved in the key exchange
and thus no certificate we do not go through the
MBEDTLS_SSL_CERTIFICATE_REQUEST state thus there
is no reason to check that in the coordination
function of that state.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
ECDHE operations have to be done in
ephemeral and PSK-ephemeral key exchange
mode, not just ephemeral key exhange mode.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
lstat is not available on some platforms (e.g. Ubuntu 16.04). In this
particular case stat is sufficient.
Signed-off-by: Dave Rodgman <dave.rodgman@arm.com>
On non-windows environments, when loading certificates from a given
path through mbedtls_x509_crt_parse_path() function, if a symbolic
link is found and is broken (meaning the target file don't exists),
the function is returning MBEDTLS_ERR_X509_FILE_IO_ERROR which is
not honoring the default behavior of just skip the bad certificate file
and increase the counter of wrong files.
The problem have been raised many times in our open source project
called Fluent Bit which depends on MbedTLS:
https://github.com/fluent/fluent-bit/issues/843#issuecomment-486388209
The expected behavior is that if a simple certificate cannot be processed,
it should just be skipped.
This patch implements a workaround with lstat(2) and stat(2) to determinate
first if the entry found in the directory is a symbolic link or not, if is
a simbolic link, do a proper stat(2) for the target file, otherwise process
normally. Upon find a broken symbolic link it will increase the counter of
not processed certificates.
Signed-off-by: Eduardo Silva <eduardo@treaure-data.com>
Again, the guards are slightly different, let's see in the CI if this
causes any trouble.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
There is a small difference: the global function only works for hashes
that are included in the build, while the old one worked for all hashes
regardless of whether they were enabled or not.
We'll see in the CI is this causes any issues.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
This is mostly:
sed -i 's/mbedtls_psa_translate_md/mbedtls_hash_info_psa_from_md/' \
library/*.c tests/suites/*.function
This should be good for code size as the old inline function was used
from 10 translation units inside the library, so we have 10 copies at
least.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Using static inline functions is bad for code size; the function from
md_internal.h was already used from 3 different C files, so already was
copied at least 3 times in the library, and this would only get worse
over time.
Use actual functions, and also share the actual data between them.
Provide a consistent set of operations. Conversion to/from
human-readable string was omitted for now but could be added later if
needed.
In the future, this can be used to replace other similar (inline)
functions that are currently scattered, including (but perhaps not
limited to):
- mbedtls_psa_translate_md() from psa_util.h
- mbedtls_md_info_from_psa() (indirectly) from psa_crypto_hash.h
- get_md_alg_from_psa() from psa_crypto_rsa.c
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Fixes#1910
With ebx added to the MULADDC_STOP clobber list to fix#1550, the inline
assembly fails to build with GCC < 5 in PIC mode with the following error:
include/mbedtls/bn_mul.h:46:13: error: PIC register clobbered by ‘ebx’ in ‘asm’
This is because older GCC versions treated the x86 ebx register (which is
used for the GOT) as a fixed reserved register when building as PIC.
This is fixed by an improved register allocator in GCC 5+. From the release
notes:
Register allocation improvements: Reuse of the PIC hard register, instead of
using a fixed register, was implemented on x86/x86-64 targets. This
improves generated PIC code performance as more hard registers can be used.
https://www.gnu.org/software/gcc/gcc-5/changes.html
As a workaround, detect this situation and disable the inline assembly,
similar to the MULADDC_CANNOT_USE_R7 logic.
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
It was only used in test_suite_pk which was fixed to no longer compute
hashes in a temporary buffer.
Also, it's not entirely clear is this macro was really a good idea:
perhaps it's better for each user to have an explicit #if
defined(MBEDTSL_USE_PSA_CRYPTO) and use either MBEDTLS_MD_MAX_SIZE or
PSA_HASH_MAX_SIZE in each branch of that #if.
So, removing it for the time being.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Within the M-profile of the Arm architecture, some instructions
admit both a 16-bit and a 32-bit encoding. For those instructions,
some assemblers support the use of the .n (narrow) and .w (wide)
suffixes to force a choice of instruction encoding width.
Forcing the size of encodings may be useful to ensure alignment
of code, which can have a significant performance impact on some
microarchitectures.
It is for this reason that a previous commit introduced explicit
.w suffixes into what was believed to be M-profile only assembly
in library/bn_mul.h.
This change, however, introduced two issues:
- First, the assembly block in question is used also for Armv7-A
systems, on which the .n/.w distinction is not meaningful
(all instructions are 32-bit).
- Second, compiler support for .n/.w suffixes appears patchy,
leading to compilation failures even when building for M-profile
targets.
This commit removes the .w annotations in order to restore working
code, deferring controlled re-introduction for the sake of performance.
Fixes#6089.
Signed-off-by: Hanno Becker <hanno.becker@arm.com>
Currently just replacing existing uses, but the real point of having
these conditions as a single macro is that we'll be able to use them in
tests case dependencies, see next commit.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
While at it, also fix buffer size for functions that already depend on
USE_PSA: it should be PSA_HASH_MAX_SIZE for functions that always use
PSA, and the new macro MBEDTLS_USE_PSA_MD_MAX_SIZE for functions that
use it or not depending on USE_PSA.
The only case where MBEDTLS_MD_MAX_SIZE is OK is when the function
always uses MD - currently this is the case with
pk_sign_verify_restart() as it is incompatible with USE_PSA anyway.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
For now in an internal header as it's the safest option and that way we
can change whenever we want.
Later on if we think the macros can be useful to applications as well then we
can move them to a public location.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
The previous commit made the PKCS#1v1.5 part of rsa.c independent from
md.c, but there was still a dependency in the corresponding part in PSA.
This commit removes it.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
This is a step towards building with RSA PKCS#1v1.5 without MD.
Also loosen guards around oid data: the OID definitions clearly don't
depend on our software implementation.
We could simply have no dependency as this is just data. But for the
sake of code size, let's have some guards so that people who don't use
MD5, SHA1 or RIPEMD160 don't have to pay the price for them.
Note: this is used for RSA (PKCS#v1.5) signatures among other things, an
area that is not influenced by USE_PSA, so the guards should not depend
on it either.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
PKCS#1 v1.5 mostly does not need hash operations. This is a first step
towards allowing builds with PKCS#1 v1.5 only (no v2.1) without MD.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Modules / tests that only need to get the size of a hash from its type,
without actually computing a hash, need not depend on MD_C.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Use a more straightforward condition to note that session resumption
is happening.
Co-authored-by: Ronald Cron <ronald.cron@arm.com>
Signed-off-by: Andrzej Kurek <andrzej.kurek@arm.com>
After opening a file containing sensitive data, call mbedtls_setbuf() to
disable buffering. This way, we don't expose sensitive data to a memory
disclosure vulnerability in a buffer outside our control.
This commit adds a call to mbedtls_setbuf() after each call to fopen(),
except:
* In ctr_drbg.c, in load_file(), because this is only used for DH parameters
and they are not confidential data.
* In psa_its_file.c, in psa_its_remove(), because the file is only opened
to check its existence, we don't read data from it.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Add a platform function mbedtls_setbuf(), defaulting to setbuf().
The intent is to allow disabling stdio buffering when reading or writing
files with sensitive data, because this exposes the sensitive data to a
subsequent memory disclosure vulnerability.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
