Commit graph

9432 commits

Author SHA1 Message Date
Jaeden Amero 7af080a9f9 Merge remote-tracking branch 'origin/pr/2442' into development
* origin/pr/2442:
  Correct placement of ChangeLog entry
  Improve documentation of mbedtls_x509_get_ext()
  Adapt ChangeLog
  Always return a high-level error code from X.509 module
  Obey bounds of ASN.1 substructures
2019-06-14 15:27:42 +01:00
Jaeden Amero 3acb5eb306 Merge remote-tracking branch 'origin/pr/2694' into development
* origin/pr/2694:
  UDP proxy: Don't attempt to dissect dgram into records when dropping
2019-06-14 08:55:11 +01:00
Jaeden Amero d042198419 Merge remote-tracking branch 'origin/pr/2675' into development
* origin/pr/2675:
  Show removed symbols in abi check
2019-06-14 08:54:21 +01:00
Jaeden Amero 39a8a26cff Merge remote-tracking branch 'origin/pr/2661' into development
* origin/pr/2661:
  Remove all abi dumps, not just ones shared between versions
2019-06-14 08:51:38 +01:00
Jaeden Amero 6b5dc689f7 Merge remote-tracking branch 'origin/pr/2430' into development
* origin/pr/2430:
  Document support for MD2 and MD4 in programs/x509/cert_write
  Correct name of X.509 parsing test for well-formed, ill-signed CRT
  Add test cases exercising successful verification of MD2/MD4/MD5 CRT
  Add test case exercising verification of valid MD2 CRT
  Add MD[245] test CRTs to tree
  Add instructions for MD[245] test CRTs to tests/data_files/Makefile
  Add suppport for MD2 to CSR and CRT writing example programs
  Remove use of MD2 in further x509parse tests
  Convert further x509parse tests to use lower-case hex data
  Correct placement of ChangeLog entry
  Adapt ChangeLog
  Use SHA-256 instead of MD2 in X.509 CRT parsing tests
  Consistently use lower case hex data in X.509 parsing tests
2019-06-14 08:49:31 +01:00
Jaeden Amero e1b02df515 Merge remote-tracking branch 'origin/pr/2260' into development
* origin/pr/2260:
  Update crypto submodule
  Remove heading spaces in tests/data_files/Makefile
  Re-generate library/certs.c from script
  Add new line at the end of test-ca2.key.enc
  Use strict syntax to annotate origin of test data in certs.c
  Add run to all.sh exercising !MBEDTLS_PEM_PARSE_C + !MBEDTLS_FS_IO
  Allow DHM self test to run without MBEDTLS_PEM_PARSE_C
  ssl-opt.sh: Auto-skip tests that use files if MBEDTLS_FS_IO unset
  Document origin of hardcoded certificates in library/certs.c
  Adapt ChangeLog
  Rename server1.der to server1.crt.der
  Add DER encoded files to git tree
  Add build instructions to generate DER versions of CRTs and keys
  Document "none" value for ca_path/ca_file in ssl_client2/ssl_server2
  ssl_server2: Skip CA setup if `ca_path` or `ca_file` argument "none"
  ssl_client2: Skip CA setup if `ca_path` or `ca_file` argument "none"
  Correct white spaces in ssl_server2 and ssl_client2
  Adapt ssl_client2 to parse DER encoded test CRTs if PEM is disabled
  Adapt ssl_server2 to parse DER encoded test CRTs if PEM is disabled
2019-06-14 08:46:48 +01:00
Hanno Becker 961e677fe3 UDP proxy: Don't attempt to dissect dgram into records when dropping
To prevent dropping the same message over and over again, the UDP proxy
test application programs/test/udp_proxy _logically_ maintains a mapping
from records to the number of times the record has already been dropped,
and stops dropping once a configurable threshold (currently 2) is passed.

However, the actual implementation deviates from this logical view
in two crucial respects:
- To keep the implementation simple and independent of
  implementations of suitable map interfaces, it only counts how
  many times a record of a given _size_ has been dropped, and
  stops dropping further records of that size once the configurable
  threshold is passed. Of course, this is not fail-proof, but a
  good enough approximation for the proxy, and it allows to use
  an inefficient but simple array for the required map.
- The implementation mixes datagram lengths and record lengths:
  When deciding whether it is allowed to drop a datagram, it
  uses the total datagram size as a lookup index into the map
  counting the number of times a package has been dropped. However,
  when updating this map, the UDP proxy traverses the datagram
  record by record, and updates the mapping at the level of record
  lengths.

Apart from this inconsistency, the introduction of the Connection ID
feature leads to yet another problem: The CID length is not part of
the record header but dynamically negotiated during (potentially
encrypted!) handshakes, and it is hence impossible for a passive traffic
analyzer (in this case our UDP proxy) to reliably parse record headers;
especially, it isn't possible to reliably infer the length of a record,
nor to dissect a datagram into records.

The previous implementation of the UDP proxy was not CID-aware and
assumed that the record length would always reside at offsets 11, 12
in the DTLS record header, which would allow it to iterate through
the datagram record by record. As mentioned, this is no longer possible
for CID-based records, and the current implementation can run into
a buffer overflow in this case (because it doesn't validate that
the record length is not larger than what remains in the datagram).

This commit removes the inconsistency in datagram vs. record length
and resolves the buffer overflow issue by not attempting any dissection
of datagrams into records, and instead only counting how often _datagrams_
of a particular size have been dropped.

There is only one practical situation where this makes a difference:
If datagram packing is used by default but disabled on retransmission
(which OpenSSL has been seen to do), it can happen that we drop a
datagram in its initial transmission, then also drop some of its records
when they retransmitted one-by-one afterwards, yet still keeping the
drop-counter at 1 instead of 2. However, even in this situation, we'll
correctly count the number of droppings from that point on and eventually
stop dropping, because the peer will not fall back to using packing
and hence use stable record lengths.
2019-06-13 11:09:35 +01:00
Jaeden Amero a3daa21d8c Merge remote-tracking branch 'origin/pr/2678' into development
* origin/pr/2678:
  Update crypto submodule to 1.1.0d2
  all.sh: Perform targeted EtM tests for MAC-less configs
  ssl: Don't access non-existent encrypt_then_mac field
2019-06-10 11:00:14 +01:00
Jaeden Amero e1ae731efa Update crypto submodule to 1.1.0d2 2019-06-07 13:51:36 +01:00
Jaeden Amero 048df33d62 Merge remote-tracking branch 'origin/pr/2679' into development
* origin/pr/2679:
  test: Remove redundant 0-byte decryption test
  test: Check empty buffer decryption for chachapoly
2019-06-06 14:19:23 +01:00
Jaeden Amero bb16d0c956 Merge remote-tracking branch 'origin/pr/2654' into development
* origin/pr/2654:
  Create link to include/mbedtls only when testing is enabled
2019-06-06 14:18:23 +01:00
Jaeden Amero 2353b542d9 test: Remove redundant 0-byte decryption test
Remove the "Decrypt empty buffer" test, as ChaCha20 is a stream cipher
and 0 bytes encrypted is identical to a 0 length buffer. The "ChaCha20
Encrypt and decrypt 0 bytes" test will test decryption of a 0 length
buffer.
2019-06-06 11:56:54 +01:00
Jaeden Amero ab11889958 test: Check empty buffer decryption for chachapoly
Previously, even in the Chacha20 and Chacha20-Poly1305 tests, we would
test that decryption of an empty buffer would work with
MBEDTLS_CIPHER_AES_128_CBC.

Make the cipher used with the dec_empty_buf() test configurable, so that
Chacha20 and Chacha20-Poly1305 empty buffer tests can use ciphers other
than AES CBC. Then, make the Chacha20 and Chacha20-Poly1305 empty buffer
tests use the MBEDTLS_CIPHER_CHACHA20 and
MBEDTLS_CIPHER_CHACHA20_POLY1305 cipher suites.
2019-06-06 11:56:54 +01:00
Jaeden Amero 6b1683dd5d all.sh: Perform targeted EtM tests for MAC-less configs
When testing a configuration where no ciphersuites have MAC, via
component_test_when_no_ciphersuites_have_mac(), perform a targeted test
of only encrypt-then-MAC tests within ssl-opt.sh.
2019-06-05 14:42:50 +01:00
Jaeden Amero 2de07f1dd1 ssl: Don't access non-existent encrypt_then_mac field
When MBEDTLS_SSL_ENCRYPT_THEN_MAC is enabled, but not
MBEDTLS_SSL_SOME_MODES_USE_MAC, mbedtls_ssl_derive_keys() and
build_transforms() will attempt to use a non-existent `encrypt_then_mac`
field in the ssl_transform.

    Compile [ 93.7%]: ssl_tls.c
    [Error] ssl_tls.c@865,14: 'mbedtls_ssl_transform {aka struct mbedtls_ssl_transform}' ha
s no member named 'encrypt_then_mac'
    [ERROR] ./mbed-os/features/mbedtls/src/ssl_tls.c: In function 'mbedtls_ssl_derive_keys'
:
    ./mbed-os/features/mbedtls/src/ssl_tls.c:865:14: error: 'mbedtls_ssl_transform {aka str
uct mbedtls_ssl_transform}' has no member named 'encrypt_then_mac'
         transform->encrypt_then_mac = session->encrypt_then_mac;
                  ^~

Change mbedtls_ssl_derive_keys() and build_transforms() to only access
`encrypt_then_mac` if `encrypt_then_mac` is actually present.

Add a regression test to detect when we have regressions with
configurations that do not include any MAC ciphersuites.

Fixes d56ed2491b ("Reduce size of `ssl_transform` if no MAC ciphersuite is enabled")
2019-06-05 14:09:29 +01:00
Darryl Green c6f874bfea Show removed symbols in abi check 2019-06-05 12:57:50 +01:00
Simon Butcher a1491fe74f Merge remote-tracking branch 'public/pr/2651' into HEAD 2019-06-04 16:09:30 +01:00
Simon Butcher fe20bea3c7 Merge remote-tracking branch 'public/pr/2643' into HEAD 2019-06-04 16:09:20 +01:00
Simon Butcher 150deca7b9 Merge remote-tracking branch 'public/pr/2642' into HEAD 2019-06-04 16:09:14 +01:00
Simon Butcher ca6aee4a45 Merge remote-tracking branch 'public/pr/2641' into HEAD 2019-06-04 16:09:07 +01:00
Hanno Becker 3c03a881eb Correct placement of ChangeLog entry 2019-06-04 13:59:55 +01:00
Hanno Becker 3cddba887a Improve documentation of mbedtls_x509_get_ext()
- Explain the use of explicit ASN.1 tagging for the extensions structuree
- Remove misleading comment which suggests that mbedtls_x509_get_ext()
  also parsed the header of the first extension, which is not the case.
2019-06-04 13:59:55 +01:00
Hanno Becker d57a3a6a4f Adapt ChangeLog 2019-06-04 13:59:55 +01:00
Hanno Becker 6ccfb18ab1 Always return a high-level error code from X.509 module
Some functions within the X.509 module return an ASN.1 low level
error code where instead this error code should be wrapped by a
high-level X.509 error code as in the bulk of the module.

Specifically, the following functions are affected:
- mbedtls_x509_get_ext()
- x509_get_version()
- x509_get_uid()

This commit modifies these functions to always return an
X.509 high level error code.

Care has to be taken when adapting `mbetls_x509_get_ext()`:
Currently, the callers `mbedtls_x509_crt_ext()` treat the
return code `MBEDTLS_ERR_ASN1_UNEXPECTED_TAG` specially to
gracefully detect and continue if the extension structure is not
present. Wrapping the ASN.1 error with
`MBEDTLS_ERR_X509_INVALID_EXTENSIONS` and adapting the check
accordingly would mean that an unexpected tag somewhere
down the extension parsing would be ignored by the caller.

The way out of this is the following: Luckily, the extension
structure is always the last field in the surrounding structure,
so if there is some data remaining, it must be an Extension
structure, so we don't need to deal with a tag mismatch gracefully
in the first place.

We may therefore wrap the return code from the initial call to
`mbedtls_asn1_get_tag()` in `mbedtls_x509_get_ext()` by
`MBEDTLS_ERR_X509_INVALID_EXTENSIONS` and simply remove
the special treatment of `MBEDTLS_ERR_ASN1_UNEXPECTED_TAG`
in the callers `x509_crl_get_ext()` and `x509_crt_get_ext()`.

This renders `mbedtls_x509_get_ext()` unsuitable if it ever
happened that an Extension structure is optional and does not
occur at the end of its surrounding structure, but for CRTs
and CRLs, it's fine.

The following tests need to be adapted:
- "TBSCertificate v3, issuerID wrong tag"
  The issuerID is optional, so if we look for its presence
  but find a different tag, we silently continue and try
  parsing the subjectID, and then the extensions. The tag '00'
  used in this test doesn't match either of these, and the
  previous code would hence return LENGTH_MISMATCH after
  unsucessfully trying issuerID, subjectID and Extensions.
  With the new code, any data remaining after issuerID and
  subjectID _must_ be Extension data, so we fail with
  UNEXPECTED_TAG when trying to parse the Extension data.
- "TBSCertificate v3, UIDs, invalid length"
  The test hardcodes the expectation of
  MBEDTLS_ERR_ASN1_INVALID_LENGTH, which needs to be
  wrapped in MBEDTLS_ERR_X509_INVALID_FORMAT now.

Fixes #2431.
2019-06-04 13:59:48 +01:00
Hanno Becker 12f62fb82c Obey bounds of ASN.1 substructures
When parsing a substructure of an ASN.1 structure, no field within
the substructure must exceed the bounds of the substructure.
Concretely, the `end` pointer passed to the ASN.1 parsing routines
must be updated to point to the end of the substructure while parsing
the latter.

This was previously not the case for the routines
- x509_get_attr_type_and_value(),
- mbedtls_x509_get_crt_ext(),
- mbedtls_x509_get_crl_ext().
These functions kept using the end of the parent structure as the
`end` pointer and would hence allow substructure fields to cross
the substructure boundary. This could lead to successful parsing
of ill-formed X.509 CRTs.

This commit fixes this.

Care has to be taken when adapting `mbedtls_x509_get_crt_ext()`
and `mbedtls_x509_get_crl_ext()`, as the underlying function
`mbedtls_x509_get_ext()` returns `0` if no extensions are present
but doesn't set the variable which holds the bounds of the Extensions
structure in case the latter is present. This commit addresses
this by returning early from `mbedtls_x509_get_crt_ext()` and
`mbedtls_x509_get_crl_ext()` if parsing has reached the end of
the input buffer.

The following X.509 parsing tests need to be adapted:
- "TBSCertificate, issuer two inner set datas"
  This test exercises the X.509 CRT parser with a Subject name
  which has two empty `AttributeTypeAndValue` structures.
  This is supposed to fail with `MBEDTLS_ERR_ASN1_OUT_OF_DATA`
  because the parser should attempt to parse the first structure
  and fail because of a lack of data. Previously, it failed to
  obey the (0-length) bounds of the first AttributeTypeAndValue
  structure and would try to interpret the beginning of the second
  AttributeTypeAndValue structure as the first field of the first
  AttributeTypeAndValue structure, returning an UNEXPECTED_TAG error.
- "TBSCertificate, issuer, no full following string"
  This test exercises the parser's behaviour on an AttributeTypeAndValue
  structure which contains more data than expected; it should therefore
  fail with MBEDTLS_ERR_ASN1_LENGTH_MISMATCH. Because of the missing bounds
  check, it previously failed with UNEXPECTED_TAG because it interpreted
  the remaining byte in the first AttributeTypeAndValue structure as the
  first byte in the second AttributeTypeAndValue structure.
- "SubjectAltName repeated"
  This test should exercise two SubjectAltNames extensions in succession,
  but a wrong length values makes the second SubjectAltNames extension appear
  outside of the Extensions structure. With the new bounds in place, this
  therefore fails with a LENGTH_MISMATCH error. This commit adapts the test
  data to put the 2nd SubjectAltNames extension inside the Extensions
  structure, too.
2019-06-04 10:15:09 +01:00
Hanno Becker e35f02e1c4 Update crypto submodule
This commit updates the Crypto submodule to the merge commit of

  https://github.com/ARMmbed/mbed-crypto/pull/133

which mirrors the change of the DHM selftest, making the latter
succeed in builds with !MBEDTLS_PEM_PARSE_C.
2019-06-03 17:46:56 +01:00
Hanno Becker 4cbea4b07e Remove heading spaces in tests/data_files/Makefile 2019-06-03 17:46:56 +01:00
Hanno Becker ff552f7d56 Re-generate library/certs.c from script 2019-06-03 17:46:56 +01:00
Hanno Becker 471ad477bb Add new line at the end of test-ca2.key.enc 2019-06-03 17:46:56 +01:00
Hanno Becker 92b4f8131c Use strict syntax to annotate origin of test data in certs.c
This allows to auto-generate them from scripts.
2019-06-03 17:46:56 +01:00
Hanno Becker a545be2622 Add run to all.sh exercising !MBEDTLS_PEM_PARSE_C + !MBEDTLS_FS_IO 2019-06-03 17:46:56 +01:00
Hanno Becker 3217c8d7e8 Allow DHM self test to run without MBEDTLS_PEM_PARSE_C 2019-06-03 17:46:56 +01:00
Hanno Becker 91e72c3121 ssl-opt.sh: Auto-skip tests that use files if MBEDTLS_FS_IO unset
This should allow to run ssl-opt.sh successfully in the default
configuration minus MBEDTLS_PEM_PARSE_C minus MBEDTLS_FS_IO.
2019-06-03 17:46:56 +01:00
Hanno Becker 960e588278 Document origin of hardcoded certificates in library/certs.c
All of them are copied from (former) CRT and key files in `tests/data_files`.
For files which have been regenerated since they've been copied to `certs.c`,
update the copy.

Add declarations for DER encoded test CRTs to certs.h

Add DER encoded versions of CRTs to certs.c

fix comment in certs.c

Don't use (signed) char for DER encoded certificates

Consistently use `const char *` for test CRTs regardless of encoding

Remove non-sensical and unused PW variable for DER encoded key

Provide test CRTs in PEM and DER fmt, + pick suitable per config

This commit modifies `certs.h` and `certs.c` to start following the
following pattern for the provided test certificates and files:

- Raw test data is named `NAME_ATTR1_ATTR2_..._ATTRn`

  For example, there are
     `TEST_CA_CRT_{RSA|EC}_{PEM|DER}_{SHA1|SHA256}`.

- Derived test data with fewer attributes, iteratively defined as one
  of the raw test data instances which suits the current configuration.

  For example,
     `TEST_CA_CRT_RSA_PEM`
  is one of `TEST_CA_CRT_RSA_PEM_SHA1` or `TEST_CA_CRT_RSA_PEM_SHA256`,
  depending on whether SHA-1 and/or SHA-256 are defined in the current
  config.

Add missing public declaration of test key password

Fix signedness and naming mismatches

Further improve structure of certs.h and certs.c

Fix definition of mbedtls_test_cas test CRTs depending on config

Remove semicolon after macro string constant in certs.c
2019-06-03 17:46:56 +01:00
Hanno Becker 666f60dd80 Adapt ChangeLog 2019-06-03 17:46:56 +01:00
Hanno Becker 58fc28ce1a Rename server1.der to server1.crt.der 2019-06-03 17:46:56 +01:00
Hanno Becker 8843c250c0 Add DER encoded files to git tree 2019-06-03 17:46:56 +01:00
Hanno Becker e21387e014 Add build instructions to generate DER versions of CRTs and keys 2019-06-03 17:46:56 +01:00
Hanno Becker 422d1993d2 Document "none" value for ca_path/ca_file in ssl_client2/ssl_server2 2019-06-03 17:46:56 +01:00
Hanno Becker 8174bdfb94 ssl_server2: Skip CA setup if ca_path or ca_file argument "none"
This allows to test PSK-based ciphersuites via ssl_server2 in builds
which have MBEDTLS_X509_CRT_PARSE_C enabled but both MBEDTLS_FS_IO and
MBEDTLS_CERTS_C disabled.
2019-06-03 17:46:56 +01:00
Hanno Becker 623e7b4422 ssl_client2: Skip CA setup if ca_path or ca_file argument "none"
This allows to test PSK-based ciphersuites via ssl_client2 in builds
which have MBEDTLS_X509_CRT_PARSE_C enabled but both MBEDTLS_FS_IO and
MBEDTLS_CERTS_C disabled.

A similar change is applied to the `crt_file` and `key_file` arguments.
2019-06-03 17:46:56 +01:00
Hanno Becker a0c5ceb2af Correct white spaces in ssl_server2 and ssl_client2 2019-06-03 17:46:56 +01:00
Hanno Becker 2900b14b11 Adapt ssl_client2 to parse DER encoded test CRTs if PEM is disabled 2019-06-03 17:46:56 +01:00
Hanno Becker 09b8cae006 Adapt ssl_server2 to parse DER encoded test CRTs if PEM is disabled 2019-06-03 17:46:56 +01:00
Hanno Becker b9355b77d2 Add description of CID feature to ChangeLog 2019-06-03 16:07:50 +01:00
Hanno Becker d0ac5faa72 Insert records with unexpected CID in CID tests in ssl-opt.sh 2019-06-03 16:07:50 +01:00
Hanno Becker e8d6afd627 Add debug line witnessing receipt of unexpected CID 2019-06-03 16:07:50 +01:00
Hanno Becker 98aaf25686 Add bad_cid option UDP proxy to insert unexpected CID records
This commit adds the command line option 'bad_cid' to the UDP proxy
`./programs/test/udp_proxy`. It takes a non-negative integral value N,
which if not 0 has the effect of duplicating every 1:N CID records
and modifying the CID in the first copy sent.

This is to exercise the stacks documented behaviour on receipt
of unexpected CIDs.

It is important to send the record with the unexpected CID first,
because otherwise the packet would be dropped already during
replay protection (the same holds for the implementation of the
existing 'bad_ad' option).
2019-06-03 16:07:50 +01:00
Hanno Becker 92d30f5bcf Fix indentation in debug message in ssl_tls.c 2019-06-03 16:07:50 +01:00
Hanno Becker 8e55b0f852 Improve comment in ssl_parse_record_header() 2019-06-03 16:07:50 +01:00