Commit Graph

9065 Commits

Author SHA1 Message Date
Manuel Pégourié-Gonnard
a771160799 Introduce new config.h flag for TLS
For now the option has no effect.

Adapted existing example config files. The fact that I needed to do this
highlights that this is a slightly incompatible change: existing users need to
update their existing custom configs (if standalone as opposed to based on the
default config) in order to still get the same behaviour.

The alternative would be to have a negative config option (eg NO_TLS or
DTLS_ONLY) but this doesn't fit as nicely with the existing options, so
hopefully the minor incompatibility is acceptable.

I don't think it's worth adding a new component to all.sh:
- builds with both DTLS and TLS are done in the default (and full) config
- TLS-only builds are done with eg config-suite-b.h in test-ref-configs
- a DTLS-only build is done with config-thread.h in test-ref-configs
- builds with none of them (and SSL_TLS_C enabled) are forbidden
2019-06-06 12:59:48 +02:00
Hanno Becker
68b856d0ac Fix style issue and wording 2019-06-05 14:25:28 +01:00
Hanno Becker
fe87027512 Fix memory leak 2019-06-05 14:25:28 +01:00
Hanno Becker
fd39919aa3 Improve formatting of ssl_parse_certificate_chain() 2019-06-05 14:25:28 +01:00
Hanno Becker
285ff0c362 Add compile-time guards around helper routine 2019-06-05 14:25:28 +01:00
Hanno Becker
1f2e466843 Adapt ChangeLog 2019-06-05 14:25:28 +01:00
Hanno Becker
33c3dc8591 Don't store the peer CRT chain twice during renegotiation
Context: During a handshake, the SSL/TLS handshake logic constructs
an instance of ::mbedtls_ssl_session representing the SSL session
being established. This structure contains information such as the
session's master secret, the peer certificate, or the session ticket
issues by the server (if applicable).
During a renegotiation, the new session is constructed aside the existing
one and destroys and replaces the latter only when the renegotiation is
complete. While conceptually clear, this means that during the renegotiation,
large pieces of information such as the peer's CRT or the session ticket
exist twice in memory, even though the original versions are removed
eventually.

This commit removes the simultaneous presence of two peer CRT chains
in memory during renegotiation, in the following way:
- Unlike in the case of SessionTickets handled in the previous commit,
  we cannot simply free the peer's CRT chain from the previous handshake
  before parsing the new one, as we need to verify that the peer's end-CRT
  hasn't changed to mitigate the 'Triple Handshake Attack'.
- Instead, we perform a binary comparison of the original peer end-CRT
  with the one presented during renegotiation, and if it succeeds, we
  avoid re-parsing CRT by moving the corresponding CRT pointer from the
  old to the new session structure.
- The remaining CRTs in the peer's chain are not affected by the triple
  handshake attack protection, and for them we may employ the canonical
  approach of freeing them before parsing the remainder of the new chain.

Note that this commit intends to not change any observable behavior
of the stack. In particular:
- The peer's CRT chain is still verified during renegotiation.
- The tail of the peer's CRT chain may change during renegotiation.
2019-06-05 14:25:28 +01:00
Hanno Becker
3d699e43ea SSL/TLS client: Remove old session ticket on renegotiation
Context: During a handshake, the SSL/TLS handshake logic constructs
an instance of ::mbedtls_ssl_session representing the SSL session
being established. This structure contains information such as the
session's master secret, the peer certificate, or the session ticket
issues by the server (if applicable).

During a renegotiation, the new session is constructed aside the existing
one and destroys and replaces the latter only when the renegotiation is
complete. While conceptually clear, this means that during the renegotiation,
large pieces of information such as the peer's CRT or the session ticket
exist twice in memory, even though the original versions are removed
eventually.

This commit starts removing this memory inefficiency by freeing the old
session's SessionTicket before the one for the new session is allocated.
2019-06-05 14:25:28 +01:00
Hanno Becker
80e92ad43a Adapt ChangeLog 2019-06-05 13:18:05 +01:00
Hanno Becker
2fa5e73d8d Modify existing X.509 test for also test new copyless API
The existing test `x509parse_crt()` for X.509 CRT parsing
so far used the generic parsing API `mbedtls_x509_crt_parse()`
capable of parsing both PEM encoded and DER encoded certficates,
but was actually only used with DER encoded input data. Moreover,
as the purpose of the test is the testing of the core DER X.509 parsing
functionality, not the PEM vs. DER dispatch (which is now already tested
in the various `x509_crt_info()` tests), the call can be replaced with a
direct call to `mbedtls_x509_parse_crt_der()`.

This commit does that, and further adds to the test an analogous
call to the new API `mbedtls_x509_parse_crt_der_nocopy()` to test
copyless parsing of X.509 certificates.
2019-06-05 13:16:55 +01:00
Hanno Becker
d58b133276 Add test for mbedtls_x509_parse_file() with DER encoded CRT 2019-06-05 13:16:55 +01:00
Hanno Becker
aa8665ac26 Add a new X.509 API call for copy-less parsing of CRTs
Context:
The existing API `mbedtls_x509_parse_crt_der()` for parsing DER
encoded X.509 CRTs unconditionally makes creates a copy of the
input buffer in RAM. While this comes at the benefit of easy use,
-- specifically: allowing the user to free or re-use the input
buffer right after the call -- it creates a significant memory
overhead, as the CRT is duplicated in memory (at least temporarily).
This might not be tolerable a resource constrained device.

As a remedy, this commit adds a new X.509 API call

   `mbedtls_x509_parse_crt_der_nocopy()`

which has the same signature as `mbedtls_x509_parse_crt_der()`
and almost the same semantics, with one difference: The input
buffer must persist and be unmodified for the lifetime of the
established instance of `mbedtls_x509_crt`, that is, until
`mbedtls_x509_crt_free()` is called.
2019-06-05 13:16:55 +01:00
Darryl Green
0ae1a53e32 Show removed symbols in abi check 2019-06-05 12:58:40 +01:00
Simon Butcher
21d1c32b2b Merge remote-tracking branch 'origin/pr/574' into baremetal 2019-06-04 15:08:32 +01:00
Simon Butcher
d2cd0b8041 Merge remote-tracking branch 'origin/pr/567' into baremetal 2019-06-04 15:06:20 +01:00
Simon Butcher
2b2cbb5c0f Merge remote-tracking branch 'origin/pr/570' into baremetal 2019-06-04 15:06:07 +01:00
Simon Butcher
786ff7911c Merge remote-tracking branch 'origin/pr/568' into baremetal 2019-06-04 15:05:42 +01:00
Hanno Becker
5549f46373 Correct placement of ChangeLog entry 2019-06-04 14:02:35 +01:00
Hanno Becker
c74ce446b9 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 14:01:10 +01:00
Hanno Becker
a49d4a6607 Adapt ChangeLog 2019-06-04 14:01:10 +01:00
Hanno Becker
2f472140f9 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 14:01:10 +01:00
Hanno Becker
4e1bfc19cc 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 14:01:10 +01:00
Hanno Becker
08c78a2e32 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-04 13:04:28 +01:00
Hanno Becker
7bf7710f40 Remove reference to outdated compile-time option 2019-06-04 09:44:25 +01:00
Manuel Pégourié-Gonnard
f3c43dde54 Merge branch 'mbedtls-2.16' into baremetal
* mbedtls-2.16:
  test: Always use `make clean` by itself
  list-symbols.sh: if the build fails, print the build transcript
  Document "check-names.sh -v"
  all.sh: invoke check-names.sh in print-trace-on-exit mode
  Print a command trace if the check-names.sh exits unexpectedly
  Only use submodule if present
  Update change log
  Reword ssl_conf_max_frag_len documentation for clarity
  Ignore more generated files: seedfile, apidoc
  Improve .gitignore grouping and documentation
  Generate tags for Vi, for Emacs and with Global
2019-06-04 09:39:51 +02:00
Hanno Becker
d0f2d8100c Document support for MD2 and MD4 in programs/x509/cert_write 2019-06-03 16:20:10 +01:00
Hanno Becker
bdf2035710 Correct name of X.509 parsing test for well-formed, ill-signed CRT 2019-06-03 16:20:05 +01:00
Hanno Becker
cfa341844f Add test cases exercising successful verification of MD2/MD4/MD5 CRT 2019-06-03 16:20:02 +01:00
Hanno Becker
4d7210c23c Add test case exercising verification of valid MD2 CRT
The X.509 parsing test suite test_suite_x509parse contains a test
exercising X.509 verification for a valid MD4/MD5 certificate in a
profile which doesn't allow MD4/MD5. This commit adds an analogous
test for MD2.
2019-06-03 16:19:59 +01:00
Hanno Becker
8d59f250bc Add MD[245] test CRTs to tree 2019-06-03 16:19:46 +01:00
Hanno Becker
928f617747 Add instructions for MD[245] test CRTs to tests/data_files/Makefile 2019-06-03 16:19:39 +01:00
Hanno Becker
8a0193e619 Add suppport for MD2 to CSR and CRT writing example programs
The example programs programs/x509/cert_req and programs/x509/cert_write
(demonstrating the use of X.509 CSR and CRT writing functionality)
previously didn't support MD2 signatures.

For testing purposes, this commit adds support for MD2 to cert_req,
and support for MD2 and MD4 to cert_write.
2019-06-03 16:19:34 +01:00
Hanno Becker
8b3bcbab79 Convert further x509parse tests to use lower-case hex data
With emacs: `replace-regexp "\([a-fA-F0-9]*\)" "\,(downcase \1)"`
2019-06-03 16:17:58 +01:00
Hanno Becker
5dbcc9f441 Introduce specific error for ver/cfg mismatch on deserialization
This commit introduces a new SSL error code

  `MBEDTLS_ERR_SSL_VERSION_MISMATCH`

which can be used to indicate operation failure due to a
mismatch of version or configuration.

It is put to use in the implementation of `mbedtls_ssl_session_load()`
to signal the attempt to de-serialize a session which has been serialized
in a build of Mbed TLS using a different version or configuration.
2019-06-03 13:01:21 +01:00
Hanno Becker
f78af3779a Improve test for detection of ver/cfg corruption in serialized data
This commit improves the test exercising the behaviour of
session deserialization when facing an unexpected version
or config, by testing ver/cfg corruption at any bit in the
ver/cfg header of the serialized data; previously, it had
only tested the first bit of each byte.
2019-06-03 12:49:36 +01:00
Hanno Becker
08ec129dd8 Use US spelling 'serialize' instead of UK spelling 'serialise' 2019-06-03 12:49:36 +01:00
Hanno Becker
baf968cf69 Use def'n consts for bits in config-identifier of serialized data 2019-06-03 12:49:09 +01:00
Hanno Becker
b36db4f368 Note that ver+fmt bytes in serialized data must not be removed 2019-06-03 12:49:09 +01:00
Hanno Becker
26829e99b2 Improve doc'n of config-identifying bitfield in serialized session 2019-06-03 12:48:50 +01:00
Hanno Becker
1d8b6d7b12 Session serialization: Fail with BAD_INPUT_DATA if buffer too small 2019-06-03 12:48:31 +01:00
Hanno Becker
cb9ba0f43c Use consistent spelling of 'serialise/serialize' in SSL test suite 2019-06-03 12:48:16 +01:00
Hanno Becker
f99ec2618d Add negative tests for unexpected ver/cfg in session deserialization 2019-06-03 12:48:16 +01:00
Hanno Becker
41527624f6 Encode relevant parts of the config in serialized session header
This commit makes use of the added space in the session header to
encode the state of those parts of the compile-time configuration
which influence the structure of the serialized session in the
present version of Mbed TLS. Specifically, these are
- the options which influence the presence/omission of fields
  from mbedtls_ssl_session (which is currently shallow-copied
  into the serialized session)
- the setting of MBEDTLS_X509_CRT_PARSE_C, which determines whether
  the serialized session contains a CRT-length + CRT-value pair after
  the shallow-copied mbedtls_ssl_session instance.
- the setting of MBEDTLS_SSL_SESSION_TICKETS, which determines whether
  the serialized session contains a session ticket.
2019-06-03 12:48:16 +01:00
Hanno Becker
557fe9ffde Add configuration identifier to serialized SSL sessions
This commit adds space for two bytes in the header of serizlied
SSL sessions which can be used to determine the structure of the
remaining serialized session in the respective version of Mbed TLS.

Specifically, if parts of the session depend on whether specific
compile-time options are set or not, the setting of these options
can be encoded in the added space.

This commit doesn't yet make use of the fields.
2019-06-03 12:46:39 +01:00
Hanno Becker
b5352f0489 Add Mbed TLS version to SSL sessions
The format of serialized SSL sessions depends on the version and the
configuration of Mbed TLS; attempts to restore sessions established
in different versions and/or configurations lead to undefined behaviour.

This commit adds an 3-byte version header to the serialized session
generated and cleanly fails ticket parsing in case a session from a
non-matching version of Mbed TLS is presented.
2019-06-03 12:46:23 +01:00
Jaeden Amero
08f363baa9 Merge remote-tracking branch 'origin/pr/2666' into mbedtls-2.16
* origin/pr/2666:
  test: Always use `make clean` by itself
2019-06-03 09:56:44 +01:00
Manuel Pégourié-Gonnard
c4f5080b34 Re-enable test that now works with new format
Previously the test didn't work because of embedded pointer values that
are not predictable. Now it works as we no longer serialize such values.
2019-06-03 10:53:47 +02:00
Manuel Pégourié-Gonnard
f8c355a012 Adapt buffering test to new ticket size
The size of the ticket used in this test dropped from 192 to 143 bytes, so
move all sizes used in this test down 50 bytes. Also, we now need to adapt the
server response size as the default size would otherwise collide with the new
mtu value.
2019-06-03 10:15:07 +02:00
Manuel Pégourié-Gonnard
60a4299bbf Add new ABI-independent format for serialization 2019-06-03 10:15:07 +02:00
Manuel Pégourié-Gonnard
35ccdbb636 Normalize spelling to serialiZation
We have explicit recommendations to use US spelling for technical writing, so
let's apply this to code as well for uniformity. (My fingers tend to prefer UK
spelling, so this needs to be fixed in many places.)

sed -i 's/\([Ss]eriali\)s/\1z/g' **/*.[ch] **/*.function **/*.data ChangeLog
2019-06-03 09:55:16 +02:00