Commit graph

102 commits

Author SHA1 Message Date
Chris Jones 4d01c5b5c3 Remove dead code from pk_parse_key_pkcs8_unencrypted_der
pk_get_pk_alg will either return 0 or a pk error code. This means that
the error code will always be a high level module ID and so we just
return ret.

Signed-off-by: Chris Jones <christopher.jones@arm.com>
2021-04-28 14:12:07 +01:00
Chris Jones fdb588b3a7 Fix an incorrect error code addition in pk_parse_key_pkcs8_unencrypted_der
An incorrect error code addition was spotted by the new invasive testing
infrastructure whereby pk_get_pk_alg will always return a high level
error or zero and pk_parse_key_pkcs8_unencrypted_der will try to add
another high level error, resulting in a garbage error code.

Apply the same fix from ae3741e8a to fix the bug.

Signed-off-by: Chris Jones <christopher.jones@arm.com>
2021-04-15 11:19:56 +01:00
Chris Jones 9f7a693f2c Apply MBEDTLS_ERROR_ADD to library
Replace all occurences of error code addition in the library with the new
MBEDTLS_ERROR_ADD macro.

Signed-off-by: Chris Jones <christopher.jones@arm.com>
2021-04-15 11:19:47 +01:00
Jens Reimann 9ad4a33a54 fix return code
Signed-off-by: Jens Reimann <jreimann@redhat.com>
2020-09-22 11:57:16 +02:00
Bence Szépkúti 1e14827beb Update copyright notices to use Linux Foundation guidance
As a result, the copyright of contributors other than Arm is now
acknowledged, and the years of publishing are no longer tracked in the
source files.

Also remove the now-redundant lines declaring that the files are part of
MbedTLS.

This commit was generated using the following script:

# ========================
#!/bin/sh

# Find files
find '(' -path './.git' -o -path './3rdparty' ')' -prune -o -type f -print | xargs sed -bi '

# Replace copyright attribution line
s/Copyright.*Arm.*/Copyright The Mbed TLS Contributors/I

# Remove redundant declaration and the preceding line
$!N
/This file is part of Mbed TLS/Id
P
D
'
# ========================

Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>
2020-08-19 10:35:41 +02:00
Gilles Peskine db09ef6d22 Include common.h instead of config.h in library source files
In library source files, include "common.h", which takes care of
including "mbedtls/config.h" (or the alternative MBEDTLS_CONFIG_FILE)
and other things that are used throughout the library.

FROM=$'#if !defined(MBEDTLS_CONFIG_FILE)\n#include "mbedtls/config.h"\n#else\n#include MBEDTLS_CONFIG_FILE\n#endif' perl -i -0777 -pe 's~\Q$ENV{FROM}~#include "common.h"~' library/*.c 3rdparty/*/library/*.c scripts/data_files/error.fmt scripts/data_files/version_features.fmt

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-07-02 11:26:57 +02:00
Manuel Pégourié-Gonnard bbb5a0a94a Fix pkparse bug wrt MBEDTLS_RSA_ALT
Some code paths want to access members of the mbedtls_rsa_context structure.
We can only do that when using our own implementation, as otherwise we don't
know anything about that structure.
2020-02-18 10:31:29 +01:00
Manuel Pégourié-Gonnard c42267920c Check public part when parsing private RSA key 2020-02-18 10:18:43 +01:00
Manuel Pégourié-Gonnard a04a2c3ef1 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:18:43 +01:00
Jack Lloyd 2e9eef4f7b Final review comments 2020-01-28 14:43:52 -05:00
Jack Lloyd 60239753d2 Avoid memory leak when RSA-CRT is not enabled in build 2020-01-27 17:53:36 -05:00
Jack Lloyd 8c2631b6d3 Address review comments 2020-01-23 17:23:52 -05:00
Jack Lloyd 80cc811039 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
2020-01-22 17:34:29 -05:00
Janos Follath 24eed8d2d2 Initialise return values to an error
Initialising the return values to and error is best practice and makes
the library more robust.
2019-12-03 16:07:18 +00:00
Andrzej Kurek c470b6b021 Merge development commit 8e76332 into development-psa
Additional changes to temporarily enable running tests:
ssl_srv.c and test_suite_ecdh use mbedtls_ecp_group_load instead of
mbedtls_ecdh_setup
test_suite_ctr_drbg uses mbedtls_ctr_drbg_update instead of 
mbedtls_ctr_drbg_update_ret
2019-01-31 08:20:20 -05:00
Hanno Becker 780f0a4cc1 Reinitialize PK ctx in mbedtls_pk_parse_key before reuse are free
Context: This commit makes a change to mbedtls_pk_parse_key() which
is responsible for parsing of private keys. The function doesn't know
the key format in advance (PEM vs. DER, encrypted vs. unencrypted) and
tries them one by one, resetting the PK context in between.

Issue: The previous code resets the PK context through a call to
mbedtls_pk_free() along, lacking the accompanying mbedtls_pk_init()
call. Practically, this is not an issue because functionally
mbedtls_pk_free() + mbedtls_pk_init() is equivalent to mbedtls_pk_free()
with the current implementation of these functions, but strictly
speaking it's nonetheless a violation of the API semantics according
to which xxx_free() functions leave a context in uninitialized state.
(yet not entirely random, because xxx_free() functions must be idempotent,
so they cannot just fill the context they operate on with garbage).

Change: The commit adds calls to mbedtls_pk_init() after those calls
to mbedtls_pk_free() within mbedtls_pk_parse_key() after which the
PK context might still be used.
2018-10-11 11:31:15 +01:00
Dawid Drozd 0e2c07e83e
Remove unnecessary mark as unused #1098
`ret` is used always at line 1305 in statement:
`if( ( ret = pk_parse_key_pkcs8_unencrypted_der( pk, key, keylen ) ) == 0 )`
2018-07-11 15:16:53 +02:00
Andres Amaya Garcia 1f6301b3c8 Rename mbedtls_zeroize to mbedtls_platform_zeroize 2018-04-17 10:00:21 -05:00
Andres Amaya Garcia e32df087fb Remove individual copies of mbedtls_zeroize()
This commit removes all the static occurrencies of the function
mbedtls_zeroize() in each of the individual .c modules. Instead the
function has been moved to utils.h that is included in each of the
modules.
2018-04-17 09:19:05 -05:00
Jethro Beekman d2df936e67 Fix parsing of PKCS#8 encoded Elliptic Curve keys.
The relevant ASN.1 definitions for a PKCS#8 encoded Elliptic Curve key are:

PrivateKeyInfo ::= SEQUENCE {
  version                   Version,
  privateKeyAlgorithm       PrivateKeyAlgorithmIdentifier,
  privateKey                PrivateKey,
  attributes           [0]  IMPLICIT Attributes OPTIONAL
}

AlgorithmIdentifier  ::=  SEQUENCE  {
  algorithm   OBJECT IDENTIFIER,
  parameters  ANY DEFINED BY algorithm OPTIONAL
}

ECParameters ::= CHOICE {
  namedCurve         OBJECT IDENTIFIER
  -- implicitCurve   NULL
  -- specifiedCurve  SpecifiedECDomain
}

ECPrivateKey ::= SEQUENCE {
  version        INTEGER { ecPrivkeyVer1(1) } (ecPrivkeyVer1),
  privateKey     OCTET STRING,
  parameters [0] ECParameters {{ NamedCurve }} OPTIONAL,
  publicKey  [1] BIT STRING OPTIONAL
}

Because of the two optional fields, there are 4 possible variants that need to
be parsed: no optional fields, only parameters, only public key, and both
optional fields. Previously mbedTLS was unable to parse keys with "only
parameters". Also, only "only public key" was tested. There was a test for "no
optional fields", but it was labelled incorrectly as SEC.1 and not run because
of a great renaming mixup.
2018-03-22 18:01:18 -07:00
Jaeden Amero 24b2d6fb6d Merge remote-tracking branch 'upstream-restricted/pr/459' into development-restricted-proposed 2018-03-15 08:24:44 +00:00
Gilles Peskine a31d8206b1 Merge remote-tracking branch 'upstream-public/pr/778' into development-proposed 2018-03-12 23:45:08 +01:00
Sanne Wouda 7b2e85dd7c Use both applicable error codes and a proper coding style 2018-03-06 23:28:46 +01:00
Sanne Wouda b2b29d5259 Add end-of-buffer check to prevent heap-buffer-overflow
Dereference of *p should not happen when it points past the end of the
buffer.

Internal reference: IOTSSL-1663
2018-03-06 23:28:46 +01:00
Gilles Peskine e6844ccf2b Merge branch 'pr_1135' into development-proposed 2018-02-14 17:20:42 +01:00
Ron Eldor 9566ff7913 Fix minor issues raised in PR review
1. Style issues fixes - remove redundant spacing.
2. Remove depency of `MBEDTLS_RSA_C` in `pk_parse_public_keyfile_rsa()`
tests, as the function itself is dependent on it.
2018-02-07 18:59:41 +02:00
Andres Amaya Garcia e9124b943d Ensure that mbedtls_pk_parse_key() does not allocate 0 bytes 2018-01-23 20:03:52 +00:00
Jaeden Amero f342cb791b Merge branch 'development' into development-restricted 2018-01-09 13:16:37 +00:00
Hanno Becker 895c5ab88e Preserve old behavior by checking public key in RSA parsing function
The function `pk_get_rsapubkey` originally performed some basic
sanity checks (e.g. on the size of public exponent) on the parsed
RSA public key by a call to `mbedtls_rsa_check_pubkey`.
This check was dropped because it is not possible to thoroughly
check full parameter sanity (i.e. that (-)^E is a bijection on Z/NZ).

Still, for the sake of not silently changing existing behavior,
this commit puts back the call to `mbedtls_rsa_check_pubkey`.
2018-01-05 08:08:09 +00:00
Hanno Becker 32297e8314 Merge branch 'development' into iotssl-1619 2017-12-22 10:24:32 +00:00
Manuel Pégourié-Gonnard 6774fa6e87 Merge branch 'development' into development-restricted
* development:
  Add --no-yotta option to all.sh
  Fix build without MBEDTLS_FS_IO
2017-12-18 11:43:35 +01:00
Gilles Peskine 832f349f93 Fix build without MBEDTLS_FS_IO
Fix missing definition of mbedtls_zeroize when MBEDTLS_FS_IO is
disabled in the configuration.

Introduced by e7707228b4
    Merge remote-tracking branch 'upstream-public/pr/1062' into development
2017-11-30 12:03:27 +01:00
Gilles Peskine c753f5daf4 Merge remote-tracking branch 'upstream-restricted/pr/369' into development-restricted 2017-11-28 14:16:47 +01:00
Ron Eldor 3f2da84bca Resolve PR review comments
1) Fix style comments
2) Fix typo in Makefile
3) Remove the `MBEDTLS_MD5_C` dependency from test data file,
as the used keys are not encrypted
2017-10-17 15:53:32 +03:00
Ron Eldor 5472d43ffb Fix issues when MBEDTLS_PEM_PARSE_C not defined
1) Fix compilatoin issues when `MBEDTLS_PEM_PARSE_C` not defined
2) remove dependency for `MBEDTLS_PEM_PARSE_C` in DER tests
2017-10-17 09:50:39 +03:00
Ron Eldor 40b14a894b change order of parsing public key
First parse PEM, and if fails, parse DER. Use some convention as
in parsing the private key (`mbedtls_pk_parse_key`)
2017-10-17 09:40:33 +03:00
Ron Eldor 84df1aeeaf use internal pk_get_rsapubkey function
1) use `pk_get_rsapubkey` function instead of `pk_parse_key_pkcs1_der`
2) revert changes in `pk_parse_key_pkcs1_der`
2017-10-16 17:14:46 +03:00
Ron Eldor b006518289 Resolve PR review comments
1) use `pk_get_rsapubkey` instead of reimplementing the parsing
2) rename the key files, according to their type and key size
3) comment in the data_files/Makefile hoe the keys were generated
4) Fix issue of failure parsing pkcs#1 DER format parsing, missed in previous commit
2017-10-16 12:40:27 +03:00
Hanno Becker efa14e8b0c Reduce number of MPI's used in pk_parse_key_pkcs1_der
As the optional RSA parameters DP, DQ and QP are effectively discarded (they are only considered for their length to
ensure that the key fills the entire buffer), it is not necessary to read them into separate MPI's.
2017-10-11 19:45:19 +01:00
Hanno Becker 7f25f850ac Adapt uses of mbedtls_rsa_complete to removed PRNG argument 2017-10-10 16:56:22 +01:00
Ron Eldor d0c56de934 Add support for public keys encoded with PKCS#1
1) Add support for public keys encoded with PKCS#1
2) Add tests for PKCS#1 PEM and DER, and PKCS#8 DER
2017-10-10 17:12:07 +03:00
Hanno Becker c6fc878eda Remove mbedtls_rsa_check_crt
This is no longer needed after the decision to not exhaustively validate private key material.
2017-10-02 13:20:15 +01:00
Hanno Becker b4274210a4 Improve documentation in pkparse.c
State explicitly that `pk_parse_pkcs8_undencrypted_der` and `pk_parse_key_pkcs8_encrypted_der` are not responsible for
zeroizing and freeing the provided key buffer.
2017-09-29 19:18:51 +01:00
Hanno Becker f04111f5c5 Fix typo 2017-09-29 19:18:42 +01:00
Hanno Becker 9be1926b69 Correct parsing checks in mbedtls_pk_parse_key
Two code-paths in `mbedtls_pk_parse_key` returned success on a failure in `mbedtls_pk_setup`.
2017-09-08 12:39:44 +01:00
Hanno Becker 66a0f83d58 Remove unreachable branches in pkparse.c 2017-09-08 12:39:21 +01:00
Hanno Becker b8d1657148 Mention in-place decryption in pk_parse_key_pkcs8_encrypted_der
Also fixes a typo.
2017-09-07 15:29:01 +01:00
Hanno Becker 2aa80a706f Remove unnecessary cast 2017-09-07 15:28:45 +01:00
Hanno Becker 9c6cb38ba8 Fix typo in pkparse.c 2017-09-05 10:08:01 +01:00
Hanno Becker fab3569963 Use in-place decryption in pk_parse_pkcs8_encrypted_der
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.
2017-08-25 13:57:21 +01:00