Commit graph

192 commits

Author SHA1 Message Date
Raoul Strackx 2a8e9587a7 Always revoke certificate on CRL
RFC5280 does not state that the `revocationDate` should be checked.

In addition, when no time source is available (i.e., when MBEDTLS_HAVE_TIME_DATE is not defined), `mbedtls_x509_time_is_past` always returns 0. This results in the CRL not being checked at all.

https://tools.ietf.org/html/rfc5280
Signed-off-by: Raoul Strackx <raoul.strackx@fortanix.com>
2020-08-26 11:38:41 +02:00
Bence Szépkúti 44bfbe3b95 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 16:54:51 +02:00
Bence Szépkúti 4e9f71227a Update license headers to Apache-2.0 OR GPL-2.0-or-later
This will allow us to ship the LTS branches in a single archive

This commit was generated using the following script:

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

header1='\ *  SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later\
 *\
 *  This file is provided under the Apache License 2.0, or the\
 *  GNU General Public License v2.0 or later.\
 *\
 *  **********\
 *  Apache License 2.0:\
 *\
 *  Licensed under the Apache License, Version 2.0 (the "License"); you may\
 *  not use this file except in compliance with the License.\
 *  You may obtain a copy of the License at\
 *\
 *  http://www.apache.org/licenses/LICENSE-2.0\
 *\
 *  Unless required by applicable law or agreed to in writing, software\
 *  distributed under the License is distributed on an "AS IS" BASIS, WITHOUT\
 *  WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\
 *  See the License for the specific language governing permissions and\
 *  limitations under the License.\
 *\
 *  **********\
 *\
 *  **********\
 *  GNU General Public License v2.0 or later:\
 *\
 *  This program is free software; you can redistribute it and/or modify\
 *  it under the terms of the GNU General Public License as published by\
 *  the Free Software Foundation; either version 2 of the License, or\
 *  (at your option) any later version.\
 *\
 *  This program is distributed in the hope that it will be useful,\
 *  but WITHOUT ANY WARRANTY; without even the implied warranty of\
 *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the\
 *  GNU General Public License for more details.\
 *\
 *  You should have received a copy of the GNU General Public License along\
 *  with this program; if not, write to the Free Software Foundation, Inc.,\
 *  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.\
 *\
 *  **********'

find -path './.git' -prune -o '(' -name '*.c' -o -name '*.cpp' -o -name '*.fmt' -o -name '*.h' ')' -print | xargs sed -i "
# Normalize the first line of the copyright headers (no text on the first line of a block comment)
/^\/\*.*Copyright.*Arm/I s/\/\*/&\n */

# Insert new copyright header
/SPDX-License-Identifier/ i\
$header1

# Delete old copyright header
/SPDX-License-Identifier/,$ {
  # Delete lines until the one preceding the mbedtls declaration
  N
  1,/This file is part of/ {
    /This file is part of/! D
  }
}
"

# Format copyright header for inclusion into scripts
header2=$(echo "$header1" | sed 's/^\\\? \* \?/#/')

find -path './.git' -prune -o '(' -name '*.gdb' -o -name '*.pl' -o -name '*.py' -o -name '*.sh' ')' -print | xargs sed -i "
# Insert new copyright header
/SPDX-License-Identifier/ i\
$header2

# Delete old copyright header
/SPDX-License-Identifier/,$ {
  # Delete lines until the one preceding the mbedtls declaration
  N
  1,/This file is part of/ {
    /This file is part of/! D
  }
}
"
# ========================

Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>
2020-06-15 12:56:41 +02:00
Andrzej Kurek 3fd9297658 Guard from undefined behaviour in case of an INT_MAX max_pathlen
When parsing a certificate with the basic constraints extension
the max_pathlen that was read from it was incremented regardless
of its value. However, if the max_pathlen is equal to INT_MAX (which
is highly unlikely), an undefined behaviour would occur.
This commit adds a check to ensure that such value is not accepted
as valid. Relevant tests for INT_MAX and INT_MAX-1 are also introduced.
Certificates added in this commit were generated using the
test_suite_x509write, function test_x509_crt_check. Input data taken
from the "Certificate write check Server1 SHA1" test case, so the generated
files are like the "server1.crt", but with the "is_ca" field set to 1 and
max_pathlen as described by the file name.

Signed-off-by: Andrzej Kurek <andrzej.kurek@arm.com>
Signed-off-by: Piotr Nowicki <piotr.nowicki@arm.com>
2020-04-17 11:30:21 +02: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
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
Hanno Becker 19557c2078 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:03:27 +01:00
Hanno Becker 1de13dbc49 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:03:27 +01:00
Simon Butcher 9136dab9c3 Merge remote-tracking branch 'public/pr/2138' into mbedtls-2.7-restricted-proposed 2018-11-07 13:34:42 +00:00
Hanno Becker 18a4cbfac3 Don't perform binary comparison of CRL issuer and CA subject
Previously, when checking whether a CRT was revoked through
one of the configured CRLs, the library would only consider
those CRLs whose `issuer` field binary-matches the `subject`
field of the CA that has issued the CRT in question. If those
fields were not binary equivalent, the corresponding CRL was
discarded.

This is not in line with RFC 5280, which demands that the
comparison should be format- and case-insensitive. For example:

- If the same string is once encoded as a `PrintableString` and
  another time as a `UTF8String`, they should compare equal.
- If two strings differ only in their choice of upper and lower case
  letters, they should compare equal.

This commit fixes this by using the dedicated x509_name_cmp()
function to compare the CRL issuer with the CA subject.

Fixes #1784.
2018-11-06 13:18:40 +00:00
Hanno Becker 52da7ee9e0 Move static x509_name_cmp() in library/x509_crt.c
A subsequent change will need this function earlier
within the file.
2018-11-06 13:18:23 +00:00
Simon Butcher e5eb2581f8 Make inclusion of stdio.h conditional in x509_crt.c
stdio.h was being included both conditionally if MBEDTLS_FS_IO was
defined, and also unconditionally, which made at least one of them
redundant.

This change removes the unconditional inclusion of stdio.h and makes it
conditional on MBEDTLS_PLATFORM_C.
2018-10-25 18:23:07 +01:00
Simon Butcher 242169bdc3 Merge remote-tracking branch 'restricted/pr/498' into mbedtls-2.7-restricted 2018-08-28 15:29:55 +01:00
Philippe Antoine 84cc74e82b Fix undefined shifts
- in x509_profile_check_pk_alg
- in x509_profile_check_md_alg
- in x509_profile_check_key

and in ssl_cli.c : unsigned char gets promoted to signed integer
2018-07-26 22:49:42 +01:00
k-stachowiak f4a668870f Fix code formatting 2018-07-24 12:54:39 +02:00
k-stachowiak 6978949cd0 Prevent buffer overread by one byte 2018-07-16 12:30:39 +02:00
Gilles Peskine 2cfeb887b4 Merge tag 'mbedtls-2.7.2' into iotssl-1381-x509-verify-refactor-2.7-restricted
Conflict resolution:

* ChangeLog
* tests/data_files/Makefile: concurrent additions, order irrelevant
* tests/data_files/test-ca.opensslconf: concurrent additions, order irrelevant
* tests/scripts/all.sh: one comment change conflicted with a code
  addition. In addition some of the additions in the
  iotssl-1381-x509-verify-refactor-restricted branch need support for
  keep-going mode, this will be added in a subsequent commit.
2018-03-23 02:12:44 +01:00
Manuel Pégourié-Gonnard e786a7ecdb x509: fix remaining unchecked call to mbedtls_md()
The other two calls have been fixed already, fix that one too for consistency.
2018-03-07 09:41:20 +01:00
Manuel Pégourié-Gonnard 71df3733d0 Clarify mutual references in comments 2018-03-07 09:36:30 +01:00
Andres Amaya Garcia 6451909160 Fix x509_get_subject_alt_name to drop invalid tag
Fix the x509_get_subject_alt_name() function to not accept invalid
tags. The problem was that the ASN.1 class for tags consists of two
bits. Simply doing bit-wise and of the CONTEXT_SPECIFIC macro with the
input tag has the potential of accepting tag values 0x10 (private)
which would indicate that the certificate has an incorrect format.
2018-03-06 19:24:19 +00:00
Manuel Pégourié-Gonnard 35eb39a924 Fix some issues in comments 2018-03-06 10:34:11 +01:00
Hanno Becker 2e7fee09da Fix bug in X.509 CRT verification code 2018-03-05 13:25:45 +01:00
Manuel Pégourié-Gonnard 75d35600de Fix some comment typos 2018-03-05 13:22:59 +01:00
Manuel Pégourié-Gonnard c08e906f3a Add comments on chain verification cases
This is the beginning of a series of commits refactoring the chain
building/verification functions in order to:
- make it simpler to understand and work with
- prepare integration of restartable ECC
2018-03-05 13:05:25 +01:00
Manuel Pégourié-Gonnard b9c72eb733 Improve handling of md errors in X.509
md() already checks for md_info == NULL. Also, in the future it might also
return other errors (eg hardware errors if acceleration is used), so it make
more sense to check its return value than to check for NULL ourselves and then
assume no other error can occur.

Also, currently, md_info == NULL can never happen except if the MD and OID modules
get out of sync, or if the user messes with members of the x509_crt structure
directly.

This commit does not change the current behaviour, which is to treat MD errors
the same way as a bad signature or no trusted root.
2018-03-05 13:00:31 +01:00
Ron Eldor 85e1dcff6a Fix handshake failure in suite B
Fix handshake failure where PK key is translated as `MBEDTLS_ECKEY`
instead of `MBEDTLS_ECDSA`
2018-02-06 15:59:38 +02:00
Andres AG 80164741e1 Fix potential integer overflow parsing DER CRT
This patch prevents a potential signed integer overflow during the
certificate version verification checks.
2017-07-27 21:44:34 +01:00
Ron Eldor 3e19df5c95 Resource leak fix on windows platform
Fix a resource leak on windows platform, in mbedtls_x509_crt_parse_path,
in case a failure. when an error occurs, goto cleanup, and free the
resource, instead of returning error code immediately.
2017-07-27 21:44:33 +01:00
Ron Eldor 8ab0595538 Wrong preproccessor condition fix
Fix for issue #696
Change #if defined(MBEDTLS_THREADING_PTHREAD)
to #if defined(MBEDTLS_THREADING_C)
2017-07-27 21:44:33 +01:00
Manuel Pégourié-Gonnard 9107b5fdd3 Improve comments 2017-07-06 12:16:25 +02:00
Manuel Pégourié-Gonnard 31458a1878 Only return VERIFY_FAILED from a single point
Everything else is a fatal error. Also improve documentation about that for
the vrfy callback.
2017-07-06 11:58:41 +02:00
Manuel Pégourié-Gonnard d15795acd5 Improve behaviour on fatal errors
If we didn't walk the whole chain, then there may be any kind of errors in the
part of the chain we didn't check, so setting all flags looks like the safe
thing to do.
2017-07-06 11:58:41 +02:00
Gilles Peskine 5d2511c4d4 SHA-1 deprecation: allow it in key exchange
By default, keep allowing SHA-1 in key exchange signatures. Disabling
it causes compatibility issues, especially with clients that use
TLS1.2 but don't send the signature_algorithms extension.

SHA-1 is forbidden in certificates by default, since it's vulnerable
to offline collision-based attacks.
2017-06-06 18:44:14 +02:00
Gilles Peskine 5e79cb3662 Remove SHA-1 in TLS by default
Default to forbidding the use of SHA-1 in TLS where it is unsafe: for
certificate signing, and as the signature hash algorithm for the TLS
1.2 handshake signature. SHA-1 remains allowed in HMAC-SHA-1 in the
XXX_SHA ciphersuites and in the PRF for TLS <= 1.1.

For easy backward compatibility for use in controlled environments,
turn on the MBEDTLS_TLS_DEFAULT_ALLOW_SHA1 compiled-time option.
2017-06-06 18:44:13 +02:00
Simon Butcher fc794ff2b7 Merge branch 'iotssl-1071-ca-flags'
Fixes a regression introduced by an earlier commit that modified
x509_crt_verify_top() to ensure that valid certificates that are after past or
future valid in the chain are processed. However the change introduced a change
in behaviour that caused the verification flags MBEDTLS_X509_BADCERT_EXPIRED and
MBEDTLS_BADCERT_FUTURE to always be set whenever there is a failure in the
verification regardless of the cause.

The fix maintains both behaviours:

 * Ensure that valid certificates after future and past are verified
 * Ensure that the correct verification flags are set.
2017-02-27 19:06:05 +00:00
Andres AG c0db511820 Fix unused variable/function compilation warnings
This PR fixes a number of unused variable/function compilation warnings
that arise when using a config.h that does not define the macro
MBEDTLS_PEM_PARSE_C.
2017-02-15 22:54:42 +02:00
Simon B 3c6b18df3a Fix various compiler warnings with MSVC
Fixes various compiler warnings found with Microsoft Visual Studio 2015
(and earlier versions).
2017-02-15 09:08:26 +00:00
Andres AG d16506624a Fix verify out flags from x509_crt_verify_top()
This change fixes a regression introduced by an earlier commit that
modified x509_crt_verify_top() to ensure that valid certificates
that are after past or future valid in the chain are processed. However
the change introduced a change in behaviour that caused the
verification flags MBEDTLS_X509_BADCERT_EXPIRED and
MBEDTLS_BADCERT_FUTURE to always be set whenever there is a failure in
the verification regardless of the cause.

The fix maintains both behaviours:
  * Ensure that valid certificates after future and past are verified
  * Ensure that the correct verification flags are set.

To do so, a temporary pointer to the first future or past valid
certificate is maintained while traversing the chain. If a truly valid
certificate is found then that one is used, otherwise if no valid
certificate is found and the end of the chain is reached, the program
reverts back to using the future or past valid certificate.
2017-01-20 13:52:01 +00:00
Simon B 3249cb780b Fix various compiler warnings with MSVC
Fixes various compiler warnings found with Microsoft Visual Studio 2015
(and earlier versions).
2016-11-10 15:37:33 +00:00
Andres AG f9113194af Allow the entry_name size to be set in config.h
Allow the size of the entry_name character array in x509_crt.c to be
configurable through a macro in config.h. entry_name holds a
path/filename string. The macro introduced in
MBEDTLS_X509_MAX_FILE_PATH_LEN.
2016-09-16 11:42:35 +01:00
Janos Follath 98e28a74e3 Address user reported coverity issues. 2016-06-09 13:41:28 +01:00
Simon Butcher 4b852db299 Merge branch 'iotssl-629-der-trailing-bytes'
Fixes bug in mbedtls_x509_crt_parse that caused trailing extra data in the
buffer following DER certificates to be included in the raw representation.
2016-03-12 23:28:26 +00:00
Janos Follath b437b4b125 X509: Fix bug triggered by future CA among trusted
Fix an issue that caused valid certificates being rejected whenever an
expired or not yet valid version of the trusted certificate was before the
valid version in the trusted certificate list.
2016-03-09 19:32:10 +00:00
Manuel Pégourié-Gonnard 0c6aad90f2 x509: remove obsolete TODO comment
- basicContraints checks are done during verification
- there is no need to set extensions that are not present to default values,
  as the code using the extension will check if it was present using
ext_types. (And default values would not make sense anyway.)
2016-03-09 19:32:09 +00:00
Manuel Pégourié-Gonnard eeef947040 Clarify documentation about missing CRLs
Also tune up some working while at it.
2016-03-09 19:32:08 +00:00
Janos Follath cc0e49ddde x509: trailing bytes in DER: fix bug
Fix bug in mbedtls_x509_crt_parse that caused trailing extra data in the
buffer after DER certificates to be included in the raw representation. #377
2016-02-17 14:41:36 +00:00
Manuel Pégourié-Gonnard f4569b14c4 Fix bug checking pathlen on first intermediate
Remove check on the pathLenConstraint value when looking for a parent to the
EE cert, as the constraint is on the number of intermediate certs below the
parent, and that number is always 0 at that point, so the constraint is always
satisfied.

The check was actually off-by-one, which caused valid chains to be rejected
under the following conditions:
- the parent certificate is not a trusted root, and
- it has pathLenConstraint == 0 (max_pathlen == 1 in our representation)

fixes #280
2015-11-19 11:10:38 +01:00
Manuel Pégourié-Gonnard 537e2a9b58 Merge branch 'iotssl-518-winpathlen-restricted' into development-restricted
* iotssl-518-winpathlen-restricted:
  Fix potential heap corruption on Windows
2015-11-02 11:04:59 +09:00
Manuel Pégourié-Gonnard 568f1e7cb3 Merge branch 'iotssl-515-max-pathlen' into development
* iotssl-515-max-pathlen:
  Add Changelog entries for this branch
  Fix a style issue
  Fix whitespace at EOL issues
  Use symbolic constants in test data
  Fixed pathlen contraint enforcement.
  Additional corner cases for testing pathlen constrains. Just in case.
  Added test case for pathlen constrains in intermediate certificates
2015-11-02 05:49:08 +09:00
Manuel Pégourié-Gonnard e670f90e48 Fix whitespace at EOL issues 2015-10-30 09:23:19 +01:00