Commit graph

13739 commits

Author SHA1 Message Date
Manuel Pégourié-Gonnard ca8287cbaf Use test_set_step() in loop in cf_hmac test
We only have a single integer available for two nested loops, but the loop
sizes are small enough compared to the integer's range that we can encode both
indexes. Since the integer is displayed in decimal in case of errors, use a
power of 10 to pack the two indexes together.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-22 11:31:19 +02:00
Manuel Pégourié-Gonnard ed0e86428d Factor repeated condition to its own macro
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-22 11:31:19 +02:00
Manuel Pégourié-Gonnard 7a8b1e6b71 Implement cf_hmac() actually with constant flow
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-22 11:31:18 +02:00
Manuel Pégourié-Gonnard 9670a59230 Start testing cf_hmac() for constant flow
Currently this breaks all.sh component test_memsan_constant_flow, just as
expected, as the current implementation is not constant flow.

This will be fixed in the next commit.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-22 11:31:18 +02:00
Gilles Peskine 6d9c8d7b2d Minor documentation improvements
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-07-22 03:17:25 +02:00
Gilles Peskine a3de08d0b5 Reorder curve enumeration like mbedtls_ecp_group_id
No semantic change.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-07-22 03:17:24 +02:00
Gilles Peskine d3beca9e38 Test Everest with only Curve25519 enabled
tests/scripts/curves.pl tests the library with a single curve enabled.
This uses the legacy ECDH context and the default ECDH implementation.
For Curve25519, there is an alternative implementation, which is
Everest. Test this. This also tests the new ECDH context, which
Everest requires.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-07-22 03:17:24 +02:00
Gilles Peskine 0478c2f77e Add ChangeLog entry for single-curve build fixes
Fix #941, #1412, #1147, #2017

Signed-off-by: Gilles Peskine <gilles.peskine@arm.com>
2020-07-22 03:17:24 +02:00
Gilles Peskine 5997005611 Fix unused variables in Montgomery-only configuration
Signed-off-by: Gilles Peskine <gilles.peskine@arm.com>
2020-07-22 03:17:24 +02:00
Gilles Peskine a2611604d4 curves.pl: test with each elliptic curve enabled
Previously curves.pl tested with all elliptic curves enabled except
one, for each curve. This catches tests that are missing dependencies
on one of the curve that they use, but does not catch misplaced
conditional directives around parts of the library.

Now, we additionally test with a single curve, for each curve. This
catches missing or extraneous guards around code that is specific to
one particular curve or to a class of curves.

Signed-off-by: Gilles Peskine <gilles.peskine@arm.com>
2020-07-22 03:17:24 +02:00
Gilles Peskine a088c81fcb Adjust ECP self-test to support Curve448
Adjust the Montgomery self-test to use Curve448 in builds without
Curve25519.

Signed-off-by: Gilles Peskine <gilles.peskine@arm.com>
2020-07-22 03:17:24 +02:00
Gilles Peskine 24666795e4 ECP self test: add self-test step for Montgomery curves
Run some self-test both for a short Weierstrass curve and for a
Montgomery curve, if the build-time configuration includes a curve of
both types. Run both because there are significant differences in the
implementation.

The test data is suitable for Curve25519.

Signed-off-by: Gilles Peskine <gilles.peskine@arm.com>
2020-07-22 03:17:24 +02:00
Gilles Peskine c95696fec4 Factor common code in mbedtls_ecp_self_test
No intended behavior change.

Signed-off-by: Gilles Peskine <gilles.peskine@arm.com>
2020-07-22 03:17:24 +02:00
Gilles Peskine d9767a5799 Tweak ECP self-test to work with secp192k1
The constants used in the test worked with every supported curve
except secp192k1. For secp192k1, the "N-1" exponent was too large.

Signed-off-by: Gilles Peskine <gilles.peskine@arm.com>
2020-07-22 03:17:24 +02:00
Gilles Peskine 7ab66a6bf1 Add missing dependencies for ECDH_xxx key exchanges
ECDH_ECDSA requires ECDSA and ECDH_RSA requires RSA.

Signed-off-by: Gilles Peskine <gilles.peskine@arm.com>
2020-07-22 03:17:23 +02:00
Gilles Peskine 963a207678 Document what needs to be done when adding a new curve
Signed-off-by: Gilles Peskine <gilles.peskine@arm.com>
2020-07-22 03:17:23 +02:00
Gilles Peskine aa9493a411 Add guards around code that is specific to dynamically-loaded groups
For some curves (semi-coincidentally, short Weierstrass curves), the
ECP module calculates some group parameters dynamically. Build the
code to calculate the parameters only if a relevant curve is enabled.
This fixes an unused function warning when building with only
Montgomery curves.

Signed-off-by: Gilles Peskine <gilles.peskine@arm.com>
2020-07-22 03:17:23 +02:00
Gilles Peskine e8c04fed51 Replace ECP_xxx by MBEDTLS_ECP__xxx_ENABLED
Replace the now-redundant internal curve type macros ECP_xxx by the
macros MBEDTLS_ECP__xxx_ENABLED which are declared in ecp.h.

Signed-off-by: Gilles Peskine <gilles.peskine@arm.com>
2020-07-22 03:17:22 +02:00
Gilles Peskine 9b99a8942f mbedtls_ecp_muladd is only for short Weierstrass curves
Document that mbedtls_ecp_muladd and mbedtls_ecp_muladd_restartable
are only implemented on short Weierstrass curves.

Exclude these functions at build time if no short Weierstrass curve
is included in the build. Before, these functions failed to compile in
such a configuration.

Signed-off-by: Gilles Peskine <gilles.peskine@arm.com>
2020-07-22 02:48:31 +02:00
Gilles Peskine 799e57612a ECDSA requires a short Weierstrass curve
Document in config.h, and enforce in check_config.h, that
MBEDTLS_ECDSA_C requires at least one short Weierstrass curve to be
enabled. A Montgomery curve is not enough.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-07-22 02:48:31 +02:00
Manuel Pégourié-Gonnard d00f99772f Merge branch 'development' into development-restricted
* development:
  Update to renamed curve constant
  Remove superfluous argument to ecp_write_key
  Revise comments for x509write_csr_der_internal
  Avoid stack-allocation of large memory buffers
  Fix Curve25519 ecp_read_key vectors to match description
  Rewrite changelog for #3425 as requested
  Rework mbedtls_ecp_write_key to remove unnecessary output parameter
  Fix endianness and masking for Curve25519 keys handled by PSA
  Document masking of Montgomery private keys in psa_export_key
  Implement and test mbedtls_ecp_write_key
  Use local labels in padlock.c
  Add Changelog entry for PSA DH/ECC Macros rename
  Rename DH Family Macros According to PSA Spec
  Rename ECC Family Macros According to PSA Spec
2020-07-21 13:30:40 +02:00
Manuel Pégourié-Gonnard b51f04466f Fix misleading comment in test function
Everything works at the byte level, not bit level. Flipping the lsb is just
one convenient way to corrupt a byte, but don't really care about individual
bits.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-21 10:40:25 +02:00
Manuel Pégourié-Gonnard 864abbff4e Rework how lengths are expressed in CBC test
This is hopefully more readable in the .data file.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-21 10:37:14 +02:00
Gilles Peskine 5dd5a491da x509_crl_parse: fix 1-byte buffer overflow and entry->raw.tag
In the entries (mbedtls_x509_crl_entry values) on the list constructed
by mbedtls_x509_crl_parse_der(), set entry->raw.tag to
(SEQUENCE | CONSTRUCTED) rather than to the tag of the first ASN.1
element of the entry (which happens to be the tag of the serial
number, so INTEGER or INTEGER | CONTEXT_SPECIFIC). This is doesn't
really matter in practice (and in particular the value is never used
in Mbed TLS itself), and isn't documented, but at least it's
consistent with how mbedtls_x509_buf is normally used.

The primary importance of this change is that the old code tried to
access the tag of the first element of the entry even when the entry
happened to be empty. If the entry was empty and not followed by
anything else in the CRL, this could cause a read 1 byte after the end
of the buffer containing the CRL.

The test case "X509 CRL ASN1 (TBSCertList, single empty entry at end)"
hit the problematic buffer overflow, which is detected with ASan.

Credit to OSS-Fuzz for detecting the problem.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-07-16 18:40:17 +02:00
Gilles Peskine b2281e1cf0 x509parse_crl: more negative test cases
Add a few more negative test cases for mbedtls_x509_crl_parse.
The test data is manually adapted from the existing positive test case
"X509 CRL ASN1 (TBSCertList, sig present)" which decomposes as

305c
 3047                                   tbsCertList TBSCertList
  020100                                version INTEGER OPTIONAL
  300d                                  signatureAlgorithm AlgorithmIdentifier
   06092a864886f70d01010e
   0500
  300f                                  issuer Name
   310d300b0603550403130441424344
  170c303930313031303030303030          thisUpdate Time
  3014                                  revokedCertificates
   3012                                 entry 1
    8202abcd                            userCertificate CertificateSerialNumber
    170c303831323331323335393539        revocationDate Time
 300d                                   signatureAlgorithm AlgorithmIdentifier
  06092a864886f70d01010e
  0500
 03020001                               signatureValue BIT STRING

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-07-16 18:18:22 +02:00
Manuel Pégourié-Gonnard a80651c483 Add a pre-commit hook that checks generated files
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-16 10:54:38 +02:00
Manuel Pégourié-Gonnard 2774fc45ff Add -u option to check-generated-files.sh
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-16 10:54:38 +02:00
Manuel Pégourié-Gonnard 7868396e78 Clarify some comments
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-16 10:24:34 +02:00
gabor-mezei-arm 3c57ccd777
Add missing newline
Signed-off-by: gabor-mezei-arm <gabor.mezei@arm.com>
2020-07-16 10:19:18 +02:00
Manuel Pégourié-Gonnard 4adc04a8a3 Give a constant a name in test function
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-16 10:00:48 +02:00
Manuel Pégourié-Gonnard e288ec0651 Fix memory leak on error path
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-16 09:23:30 +02:00
gabor-mezei-arm a321413807
Zeroising of plaintext buffers to erase unused application data from memory
Signed-off-by: gabor-mezei-arm <gabor.mezei@arm.com>
2020-07-15 14:14:41 +02:00
Manuel Pégourié-Gonnard 8f4f9a8daf
Merge pull request #3425 from stevew817/montgomery-keys-clarification
Fix endianness handling of Curve25519 in PSA Crypto core
2020-07-15 13:33:46 +02:00
Manuel Pégourié-Gonnard 6240defd17 Add MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN
This option allows to test the constant-flow nature of selected code, using
MemSan and the fundamental observation behind ctgrind that the set of
operations allowed on undefined memory by dynamic analysers is the same as the
set of operations allowed on secret data to avoid leaking it to a local
attacker via side channels, namely, any operation except branching and
dereferencing.

(This isn't the full story, as on some CPUs some instructions have variable
execution depending on the inputs, most notably division and on some cores
multiplication. However, testing that no branch or memory access depends on
secret data is already a good start.)

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-15 12:26:22 +02:00
Manuel Pégourié-Gonnard 65a6fa3e26 Make cf_hmac() STATIC_TESTABLE
The test function now depends on MBEDTLS_TEST_HOOKS, which is enabled by
config.py full, and since there are already components in all.sh exercising
the full config, this test function is sill exercised even with this new
dependency.

Since this is the first time a test function depends on MBEDTLS_TEST_HOOKS,
fix a bug in check-names.sh that wasn't apparent so far: headers from
library/*.h were not considered when looking for macro definitions. This
became apparent because MBEDTLS_STATIC_TESTABLE is defined in library/common.h
and started being used in library/ssl_msg.c, so was flagged as a likely typo.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-15 12:26:21 +02:00
Manuel Pégourié-Gonnard 8aa29e382f Use existing implementation of cf_hmac()
Just move code from ssl_decrypt_buf() to the new cf_hmac() function and then
call cf_hmac() from there.

This makes the new cf_hmac() function used, opening the door for making it
static in the next commit. It also validates that its interface works for
using it in ssl_decrypt_buf().

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-15 12:26:21 +02:00
Manuel Pégourié-Gonnard 045f094c81 Add dummy constant-flow HMAC function with tests
The dummy implementation is not constant-flow at all for now, it's just
here as a starting point and a support for developing the tests and putting
the infrastructure in place.

Depending on the implementation strategy, there might be various corner cases
depending on where the lengths fall relative to block boundaries. So it seems
safer to just test all possible lengths in a given range than to use only a
few randomly-chosen values.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-15 12:25:52 +02:00
Steven Cooreman 5a3c210e15 Update to renamed curve constant
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
2020-07-13 17:26:26 +02:00
Gilles Peskine bcce2eff27 Transparent drivers may have init functions too
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-07-13 13:27:45 +02:00
Gilles Peskine 71db60bd11 Automatically define location/lifetime constants
PSA_KEY_LOCATION_acme, PSA_KEY_LIFETIME_acme

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-07-13 13:27:45 +02:00
Gilles Peskine 278e5ebf78 Start Mbed TLS guides to PSA drivers
Driver developer's guide: introduction on how to write a driver.

Driver integration guide: how to build Mbed TLS with drivers.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-07-13 13:27:45 +02:00
Gilles Peskine 2e66aca372 PSA unified driver interface
Working draft of the PSA cryptography unified interface specification.
Eventually this document will be under Arm PSA architecture ownership,
but for the time being this draft is maintained in Mbed TLS.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-07-13 13:27:45 +02:00
Gilles Peskine abeb58e814 Add a directory for proposed specifications
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-07-13 13:27:42 +02:00
Steven Cooreman aec44e9fe8 Merge remote-tracking branch 'mbedtls/development' into montgomery-keys-clarification 2020-07-13 11:48:21 +02:00
Steven Cooreman 0024df6b37 Remove superfluous argument to ecp_write_key
Removed after feedback from PR review.

Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
2020-07-13 10:59:40 +02:00
Manuel Pégourié-Gonnard d4d6ad0363
Merge pull request #3464 from CodeMonkeyLeet/csr_heap_alloc
Dynamically allocate requested CSR write buffer size
2020-07-10 10:57:22 +02:00
Manuel Pégourié-Gonnard 2df1f1f16f Factor repeated preprocessor condition to a macro
The condition is a complex and repeated a few times. There were already some
inconsistencies in the repetitions as some of them forgot about DES.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-09 12:13:31 +02:00
Manuel Pégourié-Gonnard 913a204a33 Fix undeclared dependency of test function
The ssl_tranform structure lacks some members accessed by this function when
CBC is not enabled.

This was found by test-ref-configs.pl and all.sh
test_when_no_ciphersuites_have_mac, so no need to add a new test.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-09 10:02:41 +02:00
Simon Leet 40ca54a9ac Revise comments for x509write_csr_der_internal
Address remaining PR comments for #2118
- Add ChangeLog.d/x509write_csr_heap_alloc.txt.
- Fix parameter alignment per Gille's recommendation.
- Update comments to more explicitly describe the manipulation of buf.
- Replace use of `MBEDTLS_MPI_MAX_SIZE` as `sig` buffer size for
  call to `x509write_csr_der_internal()` with more intuitive
  `MBEDTLS_PK_SIGNATURE_MAX_SIZE`.
- Update `mbedtls_x509write_csr_der()` to return
  `MBEDTLS_ERR_X509_ALLOC_FAILED` on mbedtls_calloc error.

Signed-off-by: Simon Leet <simon.leet@microsoft.com>
2020-07-08 18:32:47 +00:00
Doru Gucea 2957b35157 Avoid stack-allocation of large memory buffers
Using a stack-buffer with a size > 2K could easily produce a stack
overflow for an embedded device which has a limited stack size.
This commit dynamically allocates the large CSR buffer.

This commit avoids using a temporary buffer for storing the OIDs.
A single buffer is used:
a) OIDs are written backwards starting with the end of the buffer;
b) OIDs are memmove'd to the beginning of the buffer;
c) signature over this OIDs is computed and written backwards from the
end of the buffer;
d) the two memory regions are compacted.

Signed-off-by: Doru Gucea <doru-cristian.gucea@nxp.com>
2020-07-08 18:32:02 +00:00