Commit Graph

3373 Commits

Author SHA1 Message Date
Manuel Pégourié-Gonnard
869e9668dd Check public part when parsing private RSA key 2020-02-18 10:53:13 +01:00
Manuel Pégourié-Gonnard
8cc0491966 Don't pass zero to rsa_complete() as a param
When parsing a PKCS#1 RSAPrivateKey structure, all parameters are always
present. After importing them, we need to call rsa_complete() for the sake of
alternative implementations. That function interprets zero as a signal for
"this parameter was not provided". As that's never the case, we mustn't pass
any zero value to that function, so we need to explicitly check for it.
2020-02-18 10:53:13 +01:00
Manuel Pégourié-Gonnard
6cf5931f1d
Merge pull request #3028 from gilles-peskine-arm/mpi_copy_shrink-2.7
Backport 2.7: Improve robustness and testing of mbedtls_mpi_copy
2020-02-06 09:52:18 +01:00
Janos Follath
b40d60f096 Revert "Merge pull request #3011 from Patater/dev/jp-bennett/development-2.7"
This reverts commit 130e136439, reversing
changes made to 071b3e170e.

stat() will never return S_IFLNK as the file type, as stat() explicitly
follows symlinks.

Fixes #3005.
2020-02-04 14:47:45 +00:00
Janos Follath
5d1171268c
Merge pull request #3020 from mpg/fix-ssl-opt-gnutls-no-sha1-2.7
[backport 2.7] Fix ssl-opt.sh for GnuTLS versions rejecting SHA-1
2020-02-04 11:19:18 +00:00
Manuel Pégourié-Gonnard
ac3c80673f Fix comment to match reality
We can't fix the code to match the comment, so have it the other way round.
2020-02-04 09:52:27 +01:00
Manuel Pégourié-Gonnard
918b25d8fd Revert "Fix certs.c to match the content of the files"
This reverts commit 205e88cb20.
2020-02-04 09:48:08 +01:00
Gilles Peskine
774c163eae Minor comment improvement 2020-02-03 16:34:53 +01:00
Gilles Peskine
6a26967382 Improve comments in mpi_shrink 2020-02-03 16:34:53 +01:00
Gilles Peskine
2aeab87cf7 mpi_copy: make the 0 case slightly more robust
If Y was constructed through functions in this module, then Y->n == 0
iff Y->p == NULL. However we do not prevent filling mpi structures
manually, and zero may be represented with n=0 and p a valid pointer.
Most of the code can cope with such a representation, but for the
source of mbedtls_mpi_copy, this would cause an integer underflow.
Changing the test for zero from Y->p==NULL to Y->n==0 causes this case
to work at no extra cost.
2020-02-03 16:34:53 +01:00
Manuel Pégourié-Gonnard
205e88cb20 Fix certs.c to match the content of the files
The comment on TEST_SRV_CRT_RSA_SHA256 that it was
tests/data_files/server2-sha256.crt was a lie, the contents were actually
those of the mbedtls-2.16 version of the same file.

While it didn't have a noticeable impact on its own, it was confusing and
distracting while investigating an issue that cause gnutls-cli to not trust
the default RSA-SHA256 cert given test-ca.crt as a root, so worth fixing.
2020-02-03 15:54:11 +01:00
Jack Lloyd
100e147c71 Parse RSA parameters DP, DQ and QP from PKCS1 private keys
Otherwise these values are recomputed in mbedtls_rsa_deduce_crt, which
currently suffers from side channel issues in the computation of QP
(see https://eprint.iacr.org/2020/055). By loading the pre-computed
values not only is the side channel avoided, but runtime overhead of
loading RSA keys is reduced.

Discussion in https://github.com/ARMmbed/mbed-crypto/issues/347

Backport of https://github.com/ARMmbed/mbed-crypto/pull/352
2020-01-29 13:13:04 -05:00
Janos Follath
a67508e066 Merge pull request #3002 from gilles-peskine-arm/coverity-20200115-2.7 into mbedtls-2.7 2020-01-29 14:53:48 +00:00
Jaeden Amero
130e136439
Merge pull request #3011 from Patater/dev/jp-bennett/development-2.7
Backport 2.7: Allow loading symlinked certificates
2020-01-28 15:55:41 +00:00
Jonathan Bennett
791babcbb9 Allow loading symlinked certificates
When mbedtls_x509_crt_parse_path() checks each object in the supplied path, it only processes regular files. This change makes it also accept a symlink to a file. Fixes #3005.

This was observed to be a problem on Fedora/CentOS/RHEL systems, where the ca-bundle in the default location is actually a symlink.
2020-01-28 11:26:47 +00:00
Gilles Peskine
0f595f714a Remove redundant block_size validity check
Check the value only once, as soon as we've obtained it.
2020-01-22 19:09:05 +01:00
Gilles Peskine
d22a7933d2 Add missing return code check on call to mbedtls_md() 2020-01-22 19:06:32 +01:00
Janos Follath
ee88f8145d Bump version to Mbed TLS 2.7.13 2020-01-20 14:28:41 +00:00
Jaeden Amero
d8180f8d84 Merge remote-tracking branch 'origin/mbedtls-2.7' into mbedtls-2.7-restricted
* origin/mbedtls-2.7:
  Enable more test cases without MBEDTLS_MEMORY_DEBUG
  More accurate test case description
  Clarify that the "FATAL" message is expected
  Note that mbedtls_ctr_drbg_seed() must not be called twice
  Fix CTR_DRBG benchmark
  Changelog entry for xxx_drbg_set_entropy_len before xxx_drbg_seed
  CTR_DRBG: support set_entropy_len() before seed()
  CTR_DRBG: Don't use functions before they're defined
  HMAC_DRBG: support set_entropy_len() before seed()
2020-01-15 16:59:10 +00:00
Gilles Peskine
b2be1fca2c Catch AES failure in mbedtls_ctr_drbg_random
The functions mbedtls_ctr_drbg_random() and
mbedtls_ctr_drbg_random_with_add() could return 0 if an AES function
failed. This could only happen with alternative AES
implementations (the built-in implementation of the AES functions
involved never fail), typically due to a failure in a hardware
accelerator.

Bug reported and fix proposed by Johan Uppman Bruce and Christoffer
Lauri, Sectra.
2019-11-28 09:55:25 +01:00
Jaeden Amero
c5a016dde1 Merge remote-tracking branch 'restricted/pr/666' into mbedtls-2.7-restricted
* restricted/pr/666: (24 commits)
  Add ChangeLog entry
  mpi_lt_mpi_ct: fix condition handling
  mpi_lt_mpi_ct: Add further tests
  mpi_lt_mpi_ct: Fix test numbering
  mpi_lt_mpi_ct perform tests for both limb size
  ct_lt_mpi_uint: cast the return value explicitely
  mbedtls_mpi_lt_mpi_ct: add tests for 32 bit limbs
  mbedtls_mpi_lt_mpi_ct: simplify condition
  Rename variable for better readability
  mbedtls_mpi_lt_mpi_ct: Improve documentation
  Make mbedtls_mpi_lt_mpi_ct more portable
  Bignum: Document assumptions about the sign field
  Add more tests for mbedtls_mpi_lt_mpi_ct
  mpi_lt_mpi_ct test: hardcode base 16
  Document ct_lt_mpi_uint
  mpi_lt_mpi_ct: make use of unsigned consistent
  ct_lt_mpi_uint: make use of biL
  Change mbedtls_mpi_cmp_mpi_ct to check less than
  mbedtls_mpi_cmp_mpi_ct: remove multiplications
  Remove excess vertical space
  ...
2019-11-12 10:47:55 +00:00
Jaeden Amero
e70059df85 Merge remote-tracking branch 'restricted/pr/668' into mbedtls-2.7-restricted
* restricted/pr/668:
  Zeroize local AES variables before exiting the function
2019-11-12 10:42:45 +00:00
Andrzej Kurek
07597365cd Zeroize local AES variables before exiting the function
This issue has been reported by Tuba Yavuz, Farhaan Fowze, Ken (Yihang) Bai,
Grant Hernandez, and Kevin Butler (University of Florida) and
Dave Tian (Purdue University).

In AES encrypt and decrypt some variables were left on the stack. The value
of these variables can be used to recover the last round key. To follow best
practice and to limit the impact of buffer overread vulnerabilities (like
Heartbleed) we need to zeroize them before exiting the function.
2019-11-12 03:23:51 -05:00
Janos Follath
b4edac5616 mpi_lt_mpi_ct: fix condition handling
The code previously only set the done flag if the return value was one.
This led to overriding the correct return value later on.
2019-11-11 12:27:36 +00:00
Janos Follath
5823961558 ct_lt_mpi_uint: cast the return value explicitely
The return value is always either one or zero and therefore there is no
risk of losing precision. Some compilers can't deduce this and complain.
2019-11-11 12:27:36 +00:00
Janos Follath
cff9e6e03d mbedtls_mpi_lt_mpi_ct: simplify condition
In the case of *ret we might need to preserve a 0 value throughout the
loop and therefore we need an extra condition to protect it from being
overwritten.

The value of done is always 1 after *ret has been set and does not need
to be protected from overwriting. Therefore in this case the extra
condition can be removed.
2019-11-11 12:27:36 +00:00
Janos Follath
8ec2a953af Rename variable for better readability 2019-11-11 12:27:36 +00:00
Janos Follath
a2b9a96fb8 mbedtls_mpi_lt_mpi_ct: Improve documentation 2019-11-11 12:27:36 +00:00
Janos Follath
51ed14e20f Make mbedtls_mpi_lt_mpi_ct more portable
The code relied on the assumptions that CHAR_BIT is 8 and that unsigned
does not have padding bits.

In the Bignum module we already assume that the sign of an MPI is either
-1 or 1. Using this, we eliminate the above mentioned dependency.
2019-11-11 12:27:36 +00:00
Janos Follath
3173a53fe9 Document ct_lt_mpi_uint 2019-11-11 12:27:36 +00:00
Janos Follath
782cbe592d mpi_lt_mpi_ct: make use of unsigned consistent 2019-11-11 12:27:36 +00:00
Janos Follath
db9f449409 ct_lt_mpi_uint: make use of biL 2019-11-11 12:27:36 +00:00
Janos Follath
c3b376e2f2 Change mbedtls_mpi_cmp_mpi_ct to check less than
The signature of mbedtls_mpi_cmp_mpi_ct() meant to support using it in
place of mbedtls_mpi_cmp_mpi(). This meant full comparison functionality
and a signed result.

To make the function more universal and friendly to constant time
coding, we change the result type to unsigned. Theoretically, we could
encode the comparison result in an unsigned value, but it would be less
intuitive.

Therefore we won't be able to represent the result as unsigned anymore
and the functionality will be constrained to checking if the first
operand is less than the second. This is sufficient to support the
current use case and to check any relationship between MPIs.

The only drawback is that we need to call the function twice when
checking for equality, but this can be optimised later if an when it is
needed.
2019-11-11 12:27:36 +00:00
Janos Follath
8461c0e2a8 mbedtls_mpi_cmp_mpi_ct: remove multiplications
Multiplication is known to have measurable timing variations based on
the operands. For example it typically is much faster if one of the
operands is zero. Remove them from constant time code.
2019-11-11 12:27:36 +00:00
Janos Follath
8de2d45cd7 Remove excess vertical space 2019-11-11 12:27:36 +00:00
Janos Follath
c587a32a9c Remove declaration after statement
Visual Studio 2013 does not like it for some reason.
2019-11-11 12:27:36 +00:00
Janos Follath
5f3019b298 Fix side channel vulnerability in ECDSA 2019-11-11 12:27:36 +00:00
Janos Follath
e0187b95f0 Add new, constant time mpi comparison 2019-11-11 12:27:27 +00:00
Janos Follath
82debf8332 ECDSA: Fix side channel vulnerability
The blinding applied to the scalar before modular inversion is
inadequate. Bignum is not constant time/constant trace, side channel
attacks can retrieve the blinded value, factor it (it is smaller than
RSA keys and not guaranteed to have only large prime factors). Then the
key can be recovered by brute force.

Reducing the blinded value makes factoring useless because the adversary
can only recover pk*t+z*N instead of pk*t.
2019-10-25 09:01:34 +01:00
Gilles Peskine
b729e1b9ba CTR_DRBG: support set_entropy_len() before seed()
mbedtls_ctr_drbg_seed() always set the entropy length to the default,
so a call to mbedtls_ctr_drbg_set_entropy_len() before seed() had no
effect. Change this to the more intuitive behavior that
set_entropy_len() sets the entropy length and seed() respects that and
only uses the default entropy length if there was no call to
set_entropy_len().

The former test-only function mbedtls_ctr_drbg_seed_entropy_len() is
no longer used, but keep it for strict ABI compatibility.
2019-10-23 18:01:25 +02:00
Gilles Peskine
845ac103a9 CTR_DRBG: Don't use functions before they're defined
Move the definitions of mbedtls_ctr_drbg_seed_entropy_len() and
mbedtls_ctr_drbg_seed() to after they are used. This makes the code
easier to read and to maintain.
2019-10-23 18:01:25 +02:00
Gilles Peskine
9c742249cf HMAC_DRBG: support set_entropy_len() before seed()
mbedtls_hmac_drbg_seed() always set the entropy length to the default,
so a call to mbedtls_hmac_drbg_set_entropy_len() before seed() had no
effect. Change this to the more intuitive behavior that
set_entropy_len() sets the entropy length and seed() respects that and
only uses the default entropy length if there was no call to
set_entropy_len().
2019-10-23 18:01:25 +02:00
Jaeden Amero
d7bd10dc89 Bump version to Mbed TLS 2.7.12 2019-09-06 13:28:28 +01:00
Jaeden Amero
20b77ecb4a Merge remote-tracking branch 'origin/mbedtls-2.7' into mbedtls-2.7-restricted
* origin/mbedtls-2.7:
  Add ChangeLog entry
  fix memory leak in mpi_miller_rabin()
2019-09-03 19:42:50 +01:00
Jaeden Amero
68cfefee34 Merge remote-tracking branch 'origin/pr/2399' into mbedtls-2.7
* origin/pr/2399:
  Add ChangeLog entry
  fix memory leak in mpi_miller_rabin()
2019-09-03 16:32:06 +01:00
Jaeden Amero
dfe95aefce Merge remote-tracking branch 'origin/mbedtls-2.7' into mbedtls-2.7-restricted
* origin/mbedtls-2.7:
  HMAC DRBG: Split entropy-gathering requests to reduce request sizes
2019-08-30 14:31:21 +01:00
Hanno Becker
b98e326455 HMAC DRBG: Split entropy-gathering requests to reduce request sizes
According to SP800-90A, the DRBG seeding process should use a nonce
of length `security_strength / 2` bits as part of the DRBG seed. It
further notes that this nonce may be drawn from the same source of
entropy that is used for the first `security_strength` bits of the
DRBG seed. The present HMAC DRBG implementation does that, requesting
`security_strength * 3 / 2` bits of entropy from the configured entropy
source in total to form the initial part of the DRBG seed.

However, some entropy sources may have thresholds in terms of how much
entropy they can provide in a single call to their entropy gathering
function which may be exceeded by the present HMAC DRBG implementation
even if the threshold is not smaller than `security_strength` bits.
Specifically, this is the case for our own entropy module implementation
which only allows requesting at most 32 Bytes of entropy at a time
in configurations disabling SHA-512, and this leads to runtime failure
of HMAC DRBG when used with Mbed Crypto' own entropy callbacks in such
configurations.

This commit fixes this by splitting the seed entropy acquisition into
two calls, one requesting `security_strength` bits first, and another
one requesting `security_strength / 2` bits for the nonce.

Fixes #237.
2019-08-30 12:16:55 +01:00
Gilles Peskine
3b8cf47004 Merge remote-tracking branch 'upstream-restricted/pr/508' into mbedtls-2.7-restricted 2019-08-14 16:25:10 +02:00
Gilles Peskine
298a43a77e Merge remote-tracking branch 'upstream-restricted/pr/549' into mbedtls-2.7-restricted 2019-08-14 16:24:51 +02:00
Gilles Peskine
ab327dfec7 Merge remote-tracking branch 'upstream-restricted/pr/614' into mbedtls-2.7-restricted 2019-08-14 16:24:08 +02:00