This commit returns to using constant macros instead of global variables for the DHM group constants. Further, macros
providing the binary encoding of the primes from RFC 3526 and RFC 7919 are added. The hex-string macros are deprecated.
This commit modifies the PKCS1 v1.5 signature verification function `mbedtls_rsa_rsassa_pkcs1_v15_verify` to prepare the
expected PKCS1-v1.5-encoded hash using the function also used by the signing routine `mbedtls_rsa_rsassa_pkcs1_v15_sign`
and comparing it to the provided byte-string afterwards. This comes at the benefits of (1) avoiding any error-prone
parsing, (2) removing the dependency of the RSA module on the ASN.1 parsing module, and (3) reducing code size.
This commit moves the code preparing PKCS1 v1.5 encoded hashes from `mbedtls_rsa_rsassa_pkcs1_v15_sign` to a separate
non-public function `rsa_rsassa_pkcs1_v15_encode`. This code-path will then be re-used by the signature verification function
`mbetls_rsa_rsassa_pkcs1_v15_verify` in a later commit.
Original intention was to be allowed to perform in-place operations like changing the byte-order before importing
parameters into an HSM. Now a copy is needed in this case, but there's no more danger of a user expecting the arguments
to be left untouched.
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.
This commit changes the implementation of `mbedtls_rsa_get_len` to return
`ctx->len` instead of always re-computing the modulus' byte-size via
`mbedtls_mpi_size`.
Although the variable ret was initialised to an error, the
MBEDTLS_MPI_CHK macro was overwriting it. Therefore it ended up being
0 whenewer the bignum computation was successfull and stayed 0
independently of the actual check.
This commit renames the test-only flag MBEDTLS_ENTROPY_HAVE_STRONG to ENTROPY_HAVE_STRONG to make it more transparent
that it's an internal flag, and also to content the testscript tests/scripts/check-names.pl which previously complained
about the macro occurring in a comment in `entropy.c` without being defined in a library file.
This commit removes extension-writing code for X.509 non-v3 certificates from
mbedtls_x509write_crt_der. Previously, even if no extensions were present an
empty sequence would have been added.
Fix compilation error on Mingw32 when `_TRUNCATE` is defined. Use
`_TRUNCATE` only if `__MINGW32__` not defined. Fix suggested by
Thomas Glanzmann and Nick Wilson on issue #355
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.
This commit adds the function mbedtls_rsa_validate_crt for validating a set of CRT parameters. The function
mbedtls_rsa_check_crt is simplified accordingly.
Primality testing is guarded by the configuration flag MBEDTLS_GENPRIME and used in the new RSA helper functions. This
commit adds a corresponding preprocessor directive.
Alternative RSA implementations can be provided by defining MBEDTLS_RSA_ALT in
config.h, defining an mbedtls_rsa_context struct in a new file rsa_alt.h and
re-implementing the RSA interface specified in rsa.h.
Through the previous reworkings, the adherence to the interface is the only
implementation obligation - in particular, implementors are free to use a
different layout for the RSA context structure.
There were preprocessor directives in pk.c and pk_wrap.c that cheked
whether the bit length of size_t was greater than that of unsigned int.
However, the check relied on the MBEDTLS_HAVE_INT64 macro being defined
which is not directly related to size_t. This might result in errors in
some platforms. This change modifies the check to use the macros
SIZE_MAX and UINT_MAX instead making the code more robust.
As noted in #557, several functions use 'index' resp. 'time'
as parameter names in their declaration and/or definition, causing name
conflicts with the functions in the C standard library of the same
name some compilers warn about.
This commit renames the arguments accordingly.
Modify the function mbedtls_x509_csr_parse_der() so that it checks the
parsed CSR version integer before it increments the value. This prevents
a potential signed integer overflow, as these have undefined behaviour
in the C standard.
Rename the macro MBEDTLS_PLATFORM_SETUP_ALT to
MBEDTLS_PLATFORM_SETUP_TEARDOWN_ALT to make the name more descriptive
as this macro enables/disables both functions.
Add the following two functions to allow platform setup and teardown
operations for the full library to be hooked in:
* mbedtls_platform_setup()
* mbedtls_platform_teardown()
An mbedtls_platform_context C structure is also added and two internal
functions that are called by the corresponding setup and teardown
functions above:
* mbedtls_internal_platform_setup()
* mbedtls_internal_plartform_teardown()
Finally, the macro MBEDTLS_PLATFORM_SETUP_ALT is also added to allow
mbedtls_platform_context and internal function to be overriden by the
user as needed for a platform.
The previous commit bd5ceee484f201b90a384636ba12de86bd330cba removed
the definition of the global constants
- mbedtls_test_ca_crt_rsa_len,
- mbedtls_test_cli_crt_rsa_len,
- mbedtls_test_ca_crt_rsa, and
- mbedtls_test_cli_crt_rsa.
This commit restores these to maintain ABI compatibility.
Further, it was noticed that without SHA256_C being enabled the
previous code failed to compile because because the SHA1 resp. SHA256
certificates were only defined when the respective SHAXXX_C options
were set, but the emission of the global variable mbedtls_test_ca_crt
was unconditionally defined through the SHA256
certificate. Previously, the RSA SHA1 certificate was unconditionally
defined and used for that.
As a remedy, this commit makes sure some RSA certificate is defined
and exported through the following rule:
1. If SHA256_C is active, define an RSA SHA256 certificate and export
it as mbedtls_test_ca_crt. Also, define SHA1 certificates only if
SHA1_C is set.
2. If SHA256_C is not set, always define SHA1 certificate and export
it as mbedtls_test_ca_crt.
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.
Protecting the ECP hardware acceleratior with mutexes is inconsistent with the
philosophy of the library. Pre-existing hardware accelerator interfaces
leave concurrency support to the underlying platform.
Fixes#863
Modify the function mbedtls_x509_csr_parse_der() so that it checks the
parsed CSR version integer before it increments the value. This prevents
a potential signed integer overflow, as these have undefined behaviour
in the C standard.
The RSA key generation test needs strong entropy to succeed. This commit captures the presence of a strong entropy
source in a preprocessor flag and only runs the key generation test if that flag is set.
Rename the macro MBEDTLS_PLATFORM_SETUP_ALT to
MBEDTLS_PLATFORM_SETUP_TEARDOWN_ALT to make the name more descriptive
as this macro enables/disables both functions.
Add the following two functions to allow platform setup and teardown
operations for the full library to be hooked in:
* mbedtls_platform_setup()
* mbedtls_platform_teardown()
An mbedtls_platform_context C structure is also added and two internal
functions that are called by the corresponding setup and teardown
functions above:
* mbedtls_internal_platform_setup()
* mbedtls_internal_plartform_teardown()
Finally, the macro MBEDTLS_PLATFORM_SETUP_ALT is also added to allow
mbedtls_platform_context and internal function to be overriden by the
user as needed for a platform.
The previous commit bd5ceee484f201b90a384636ba12de86bd330cba removed
the definition of the global constants
- mbedtls_test_ca_crt_rsa_len,
- mbedtls_test_cli_crt_rsa_len,
- mbedtls_test_ca_crt_rsa, and
- mbedtls_test_cli_crt_rsa.
This commit restores these to maintain ABI compatibility.
Further, it was noticed that without SHA256_C being enabled the
previous code failed to compile because because the SHA1 resp. SHA256
certificates were only defined when the respective SHAXXX_C options
were set, but the emission of the global variable mbedtls_test_ca_crt
was unconditionally defined through the SHA256
certificate. Previously, the RSA SHA1 certificate was unconditionally
defined and used for that.
As a remedy, this commit makes sure some RSA certificate is defined
and exported through the following rule:
1. If SHA256_C is active, define an RSA SHA256 certificate and export
it as mbedtls_test_ca_crt. Also, define SHA1 certificates only if
SHA1_C is set.
2. If SHA256_C is not set, always define SHA1 certificate and export
it as mbedtls_test_ca_crt.
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.
Protecting the ECP hardware acceleratior with mutexes is inconsistent with the
philosophy of the library. Pre-existing hardware accelerator interfaces
leave concurrency support to the underlying platform.
Fixes#863
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.
This change moves the calls to mbedtls_sha256_starts() and
mbedtls_sha512_starts() out of the mbedtls_entropy_init() function as
these now have return codes which need to be checked.
This patch modifies the entropy.c module to ensure that the sha256 and
sha512 contexts are correctly initialised and freed instead of skipping
these calls or simply zeroizing with memset() or mbedtls_zeroize().
This is important as the sha contexts might otherwise leak memory or
other resources, and even more so in the context of hardware
accelerators where the configuration of the device might be done in the
init and free calls.
This patch modifies the internal md context structure in md_wrap.c to
add return values to the function pointers. This enables us to use the
new API in the corresponding MD modules so that failures can be
found at any point in an MD computation.
The following function calls are being deprecated to introduce int
return values.
* mbedtls_sha512()
* mbedtls_sha512_starts()
* mbedtls_sha512_update()
* mbedtls_sha512_finish()
* mbedtls_sha512_process()
The return codes can be used to return error values. This is important
when using hardware accelerators.
The following function calls are being deprecated to introduce int
return values.
* mbedtls_sha256()
* mbedtls_sha256_starts()
* mbedtls_sha256_update()
* mbedtls_sha256_finish()
* mbedtls_sha256_process()
The return codes can be used to return error values. This is important
when using hardware accelerators.
The following function calls are being deprecated to introduce int
return values.
* mbedtls_ripemd160()
* mbedtls_ripemd160_starts()
* mbedtls_ripemd160_update()
* mbedtls_ripemd160_finish()
* mbedtls_ripemd160_process()
The return codes can be used to return error values. This is important
when using hardware accelerators.
The following function calls are being deprecated to introduce int
return values.
* mbedtls_md5()
* mbedtls_md5_starts()
* mbedtls_md5_update()
* mbedtls_md5_finish()
* mbedtls_md5_process()
The return codes can be used to return error values. This is important
when using hardware accelerators.
The following function calls are being deprecated to introduce int
return values.
* mbedtls_md4()
* mbedtls_md4_starts()
* mbedtls_md4_update()
* mbedtls_md4_finish()
* mbedtls_md4_process()
The return codes can be used to return error values. This is important
when using hardware accelerators.
The following function calls are being deprecated to introduce int
return values.
* mbedtls_md2()
* mbedtls_md2_starts()
* mbedtls_md2_update()
* mbedtls_md2_finish()
* mbedtls_md2_process()
The return codes can be used to return error values. This is important
when using hardware accelerators.
The following function calls are being deprecated to introduce int
return values.
* mbedtls_sha1()
* mbedtls_sha1_starts()
* mbedtls_sha1_update()
* mbedtls_sha1_finish()
* mbedtls_sha1_process()
The return codes can be used to return error values. This is important
when using hardware accelerators.
The check `if( *p + n > end )` in `ssl_parse_client_psk_identity` is
unsafe because `*p + n` might overflow, thus bypassing the check. As
`n` is a user-specified value up to 65K, this is relevant if the
library happens to be located in the last 65K of virtual memory.
This commit replaces the check by a safe version.
This patch modifies the function mbedtls_gcm_self_test() function to
ensure that AES-GCM-192 tests are only run if the key size is supported
by the available implementation. This is useful when using
MBEDTLS_AES_ALT as some hardware crypto accelerators might not support
AES-192.
This patch modifies the function mbedtls_aes_selftest() function to
ensure that AES-192 tests are only run if the key size is supported by
the available implementation. This is useful when using MBEDTLS_AES_ALT
as some hardware crypto accelerators might not support AES-192.
* restricted/iotssl-1398:
Add ChangeLog entry
Ensure application data records are not kept when fully processed
Add hard assertion to mbedtls_ssl_read_record_layer
Fix mbedtls_ssl_read
Simplify retaining of messages for future processing
This commit fixes the following case: If a client is both expecting a
SERVER_HELLO and has an application data record that's partially
processed in flight (that's the situation the client gets into after
receiving a ServerHelloRequest followed by ApplicationData), a
subsequent call to mbedtls_ssl_read will set keep_current_message = 1
when seeing the unexpected application data, but not reset it to 0
after the application data has been processed. This commit fixes this.
It also documents and suggests how the problem might be solved in a
more structural way on the long run.
This commit adds a hard assertion to mbedtls_ssl_read_record_layer
triggering if both ssl->in_hslen and ssl->in_offt are not 0. This
should never happen, and if it does, there's no sensible way of
telling whether the previous message was a handshake or an application
data message.
There are situations in which it is not clear what message to expect
next. For example, the message following the ServerHello might be
either a Certificate, a ServerKeyExchange or a CertificateRequest. We
deal with this situation in the following way: Initially, the message
processing function for one of the allowed message types is called,
which fetches and decodes a new message. If that message is not the
expected one, the function returns successfully (instead of throwing
an error as usual for unexpected messages), and the handshake
continues to the processing function for the next possible message. To
not have this function fetch a new message, a flag in the SSL context
structure is used to indicate that the last message was retained for
further processing, and if that's set, the following processing
function will not fetch a new record.
This commit simplifies the usage of this message-retaining parameter
by doing the check within the record-fetching routine instead of the
specific message-processing routines. The code gets cleaner this way
and allows retaining messages to be used in other situations as well
without much effort. This will be used in the next commits.
This commit adds four tests to tests/ssl-opt.sh:
(1) & (2): Check behaviour of optional/required verification when the
trusted CA chain is empty.
(3) & (4): Check behaviour of optional/required verification when the
client receives a server certificate with an unsupported curve.
This commit changes the behaviour of mbedtls_ssl_parse_certificate
to make the two authentication modes MBEDTLS_SSL_VERIFY_REQUIRED and
MBEDTLS_SSL_VERIFY_OPTIONAL be in the following relationship:
Mode == MBEDTLS_SSL_VERIFY_REQUIRED
<=> Mode == MBEDTLS_SSL_VERIFY_OPTIONAL + check verify result
Also, it changes the behaviour to perform the certificate chain
verification even if the trusted CA chain is empty. Previously, the
function failed in this case, even when using optional verification,
which was brought up in #864.
* gilles/IOTSSL-1330/development:
Changelog entry for the bug fixes
SSLv3: when refusing renegotiation, stop processing
Ignore failures when sending fatal alerts
Cleaned up double variable declaration
Code portability fix
Added changelog entry
Send TLS alerts in many more cases
Skip all non-executables in run-test-suites.pl
SSL tests: server requires auth, client has no certificate
Balanced braces across preprocessor conditionals
Support setting the ports on the command line
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.
In the TLS test client, allow SHA-1 as a signature hash algorithm.
Without this, the renegotation tests failed.
A previous commit had allowed SHA-1 via the certificate profile but
that only applied before the initial negotiation which includes the
signature_algorithms extension.
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.
* hanno/sig_hash_compatibility:
Improve documentation
Split long lines
Remember suitable hash function for any signature algorithm.
Introduce macros and functions to characterize certain ciphersuites.
Add missing return code checks in the functions pem_des_decrypt(),
pem_3des_decrypt() and pem_aes_decrypt() so that the calling function
mbedtls_pem_read_buffer() is notified of errors reported by the crypto
primitives AES, DES and 3DES.
Fixed a bug in ssl_srv.c when parsing TLS_FALLBACK_SCSV in the
ciphersuite list that caused it to miss it sometimes. Reported by Hugo
Leisink as issue #810. Fix initially by @andreasag01; this commit
isolates the bug fix and adds a non-regression test.
This patch modifies the following 2 functions in the AES module to
change the return type from void to int:
* mbedtls_aes_encrypt() -> mbedtls_internal_aes_encrypt()
* mbedtls_aes_decrypt() -> mbedtls_internal_aes_decrypt()
This change is necessary to allow users of MBEDTLS_AES_ALT,
MBEDTLS_AES_DECRYPT_ALT and MBEDTLS_AES_ENCRYPT_ALT to return an error
code when replacing the default with their own implementation, e.g.
a hardware crypto accelerator.
The RSA private key functions rsa_rsaes_pkcs1_v15_decrypt and
rsa_rsaes_oaep_decrypt put sensitive data (decryption results) on the
stack. Wipe it before returning.
Thanks to Laurent Simon for reporting this issue.
The sliding window exponentiation algorithm is vulnerable to
side-channel attacks. As a countermeasure we add exponent blinding in
order to prevent combining the results of different measurements.
This commit handles the case when the Chinese Remainder Theorem is used
to accelerate the computation.
The sliding window exponentiation algorithm is vulnerable to
side-channel attacks. As a countermeasure we add exponent blinding in
order to prevent combining the results of fifferent measurements.
This commits handles the case when the Chinese Remainder Theorem is NOT
used to accelerate computations.
According to RFC5246 the server can indicate the known Certificate
Authorities or can constrain the aurhorisation space by sending a
certificate list. This part of the message is optional and if omitted,
the client may send any certificate in the response.
The previous behaviour of mbed TLS was to always send the name of all the
CAs that are configured as root CAs. In certain cases this might cause
usability and privacy issues for example:
- If the list of the CA names is longer than the peers input buffer then
the handshake will fail
- If the configured CAs belong to third parties, this message gives away
information on the relations to these third parties
Therefore we introduce an option to suppress the CA list in the
Certificate Request message.
Providing this feature as a runtime option comes with a little cost in
code size and advantages in maintenance and flexibility.
This commit changes `ssl_parse_signature_algorithms_ext` to remember
one suitable ( := supported by client and by our config ) hash
algorithm per signature algorithm.
It also modifies the ciphersuite checking function
`ssl_ciphersuite_match` to refuse a suite if there
is no suitable hash algorithm.
Finally, it adds the corresponding entry to the ChangeLog.
The routine `mbedtls_ssl_write_server_key_exchange` heavily depends on
what kind of cipher suite is active: some don't need a
ServerKeyExchange at all, some need (EC)DH parameters but no server
signature, some require both. Each time we want to restrict a certain
piece of code to some class of ciphersuites, it is guarded by a
lengthy concatentation of configuration checks determining whether at
least one of the relevant cipher suites is enabled in the config; on
the code level, it is guarded by the check whether one of these
cipher suites is the active one.
To ease readability of the code, this commit introduces several helper
macros and helper functions that can be used to determine whether a
certain class of ciphersuites (a) is active in the config, and
(b) contains the currently present ciphersuite.
With this commit the Elliptic Curve Point interface is rewised. Two
compile time options has been removed to simplify the interface and
the function names got a new prefix that indicates that these functions
are for internal use and not part of the public interface.
The intended use of the abstraction layer for Elliptic Curve Point
arithmetic is to enable using hardware cryptographic accelerators.
These devices are a shared resource and the driver code rarely provides
thread safety.
This commit adds mutexes to the abstraction layer to protect the device
in a multi-threaded environment.
The primary use case behind providing an abstraction layer to enable
alternative Elliptic Curve Point arithmetic implementation, is making
use of cryptographic acceleration hardware if it is present.
To provide thread safety for the hardware accelerator we need a mutex
to guard it.
The compile time macros enabling the initialisation and deinitialisation
in the alternative Elliptic Curve Point arithmetic implementation had
names that did not end with '_ALT' as required by check-names.sh.
This patch introduces some additional checks in the PK module for 64-bit
systems only. The problem is that the API functions in the PK
abstraction accept a size_t value for the hashlen, while the RSA module
accepts an unsigned int for the hashlen. Instead of silently casting
size_t to unsigned int, this change checks whether the hashlen overflows
an unsigned int and returns an error.
The test case was generated by modifying our signature code so that it
produces a 7-byte long padding (which also means garbage at the end, so it is
essential in to check that the error that is detected first is indeed the
padding rather than the final length check).
In many places in TLS handling, some code detects a fatal error, sends
a fatal alert message, and returns to the caller. If sending the alert
fails, then return the error that triggered the alert, rather than
overriding the return status. This effectively causes alert sending
failures to be ignored. Formerly the code was inconsistently sometimes
doing one, sometimes the other.
In general ignoring the alert is the right thing: what matters to the
caller is the original error. A typical alert failure is that the
connection is already closed.
One case which remains not handled correctly is if the alert remains
in the output buffer (WANT_WRITE). Then it won't be sent, or will be
truncated. We'd need to either delay the application error or record
the write buffering notice; to be done later.
When provided with an empty line, mpi_read_file causes a numeric
underflow resulting in a stack underflow. This commit fixes this and
adds some documentation to mpi_read_file.
The modular inversion function hangs when provided with the modulus 1. This commit refuses this modulus with a BAD_INPUT error code. It also adds a test for this case.
The TLS client and server code was usually closing the connection in
case of a fatal error without sending an alert. This commit adds
alerts in many cases.
Added one test case to detect that we send the alert, where a server
complains that the client's certificate is from an unknown CA (case
tracked internally as IOTSSL-1330).
Fix a buffer overflow when writting a string representation of an MPI
number to a buffer in hexadecimal. The problem occurs because hex
digits are written in pairs and this is not accounted for in the
calculation of the required buffer size when the number of digits is
odd.
When using ssl_cookie with MBEDTLS_THREADING_C, fix a resource leak caused by
initiating a mutex in mbedtls_ssl_cookie_free instead of freeing it.
Raised and fix suggested by lan Gillingham in the mbed TLS forum
Tracked in #771
The function ecp_mod_koblitz computed the space for the result of a
multiplication optimally for that specific case, but unfortunately
the function mbedtls_mpi_mul_mpi performs a generic, suboptimal
calculation and needs one more limb for the result. Since the result's
buffer is on the stack, the best case scenario is that the program
stops.
This only happened on 64 bit platforms.
Fixes#569
A heap overread might happen when parsing malformed certificates.
Reported by Peng Li and Yueh-Hsun Lin.
Refactoring the parsing fixes the problem. This commit applies the
relevant part of the OpenVPN contribution applied to mbed TLS 1.3
in commit 17da9dd829.
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.
Modifies the function mbedtls_x509_crl_parse() to ensure that a CRL in PEM
format with trailing characters after the footer does not result in the
execution of an infinite loop.
Fix potential integer overflows in the function mbedtls_base64_decode().
This overflow would mainly be exploitable in 32-bit systems and could
cause buffer bound checks to be bypassed.
Fix potential integer overflows in the following functions:
* mbedtls_md2_update() to be bypassed and cause
* mbedtls_cipher_update()
* mbedtls_ctr_drbg_reseed()
This overflows would mainly be exploitable in 32-bit systems and could
cause buffer bound checks to be bypassed.
Fix an incorrect condition in ssl_check_ctr_renegotiate() that compared
64 bits of record counter instead of 48 bits as described in RFC 6347
Section 4.3.1. This would cause the function's return value to be
occasionally incorrect and the renegotiation routines to be triggered
at unexpected times.
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.
Fixes many typos, and errors in comments.
* Clarifies many comments
* Grammar correction in config.pl help text
* Removed comment about MBEDTLS_X509_EXT_NS_CERT_TYPE.
* Comment typo fix (Dont => Don't)
* Comment typo fix (assure => ensure)
* Comment typo fix (byes => bytes)
* Added citation for quoted standard
* Comment typo fix (one complement => 1's complement)
The is some debate about whether to prefer "one's complement", "ones'
complement", or "1's complement". The more recent RFCs related to TLS
(RFC 6347, RFC 4347, etc) use " 1's complement", so I followed that
convention.
* Added missing ")" in comment
* Comment alignment
* Incorrect comment after #endif
Fix an incorrect condition in ssl_check_ctr_renegotiate() that compared
64 bits of record counter instead of 48 bits as described in RFC 6347
Section 4.3.1. This would cause the function's return value to be
occasionally incorrect and the renegotiation routines to be triggered
at unexpected times.
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.
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.
This patch modifies the function mbedtls_x509_crl_parse() to ensure
that a CRL in PEM format with trailing characters after the footer does
not result in the execution of an infinite loop.
Fix an incorrect condition in ssl_check_ctr_renegotiate() that compared
64 bits of record counter instead of 48 bits as described in RFC 6347
Section 4.3.1. This would cause the function's return value to be
occasionally incorrect and the renegotiation routines to be triggered
at unexpected times.
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.
Fixes many typos, and errors in comments.
* Clarifies many comments
* Grammar correction in config.pl help text
* Removed comment about MBEDTLS_X509_EXT_NS_CERT_TYPE.
* Comment typo fix (Dont => Don't)
* Comment typo fix (assure => ensure)
* Comment typo fix (byes => bytes)
* Added citation for quoted standard
* Comment typo fix (one complement => 1's complement)
The is some debate about whether to prefer "one's complement", "ones'
complement", or "1's complement". The more recent RFCs related to TLS
(RFC 6347, RFC 4347, etc) use " 1's complement", so I followed that
convention.
* Added missing ")" in comment
* Comment alignment
* Incorrect comment after #endif
The PKCS#1 standard says nothing about the relation between P and Q
but many libraries guarantee P>Q and mbed TLS did so too in earlier
versions.
This commit restores this behaviour.
Fix implementation and documentation missmatch for the function
arguments to mbedtls_gcm_finish(). Also, removed redundant if condition
that always evaluates to true.
The bracketing in some expressions where an assignment was being made in an if statement in cmac.c had been accidentally broken and was causing compiler warnings with armcc.
Minor fixes following review including:
* formatting changes including indentation and code style
* corrections
* removal of debug code
* clarification of code through variable renaming
* memory leak
* compiler warnings
Change the CMAC interface to match the mbedtls_md_hmac_xxxx() interface. This
changes the overall design of the CMAC interface to make it more consistent with
the existing HMAC interface, and will allow incremental updates of input data
rather than requiring all data to be presented at once, which is what the
current interface requires.
- use one less temporary buffer
- pedantic: in_len + 15 was a potential overflow
- use a more explicit name instead of 'flag'
- Mn was a bit misleading
The previous version had secret-dependent memory accesses. While it was
probably not an issue in practice cause the two bytes of the array are
probably on the same cache line anyway, as a matter of principle this should
be avoided.
Due to inconsistent freeing strategy in pkparse.c the sample mutex
implementation in threading.c could lead to undefined behaviour by
destroying the same mutex several times.
This fix prevents mutexes from being destroyed several times in the
sample threading implementation.
The library/net.c and its corresponding include/mbedtls/net.h file are
renamed to library/net_sockets.c and include/mbedtls/net_sockets.h
respectively. This is to avoid naming collisions in projects which also
have files with the common name 'net'.
The PKCS#1 standard says nothing about the relation between P and Q
but many libraries guarantee P>Q and mbed TLS did so too in earlier
versions.
This commit restores this behaviour.
Fix implementation and documentation missmatch for the function
arguments to mbedtls_gcm_finish(). Also, removed redundant if condition
that always evaluates to true.
Certificates with unsupported algorithms in the certificate chain
prevented verification even if a certificate before the unsupported
ones was already trusted.
We change the behaviour to ignoring every certificate with unknown
(unsupported) signature algorithm oid when parsing the certificate
chain received from the peer.
The bracketing in some expressions where an assignment was being made in an if statement in cmac.c had been accidentally broken and was causing compiler warnings with armcc.
Minor fixes following review including:
* formatting changes including indentation and code style
* corrections
* removal of debug code
* clarification of code through variable renaming
* memory leak
* compiler warnings
Change the CMAC interface to match the mbedtls_md_hmac_xxxx() interface. This
changes the overall design of the CMAC interface to make it more consistent with
the existing HMAC interface, and will allow incremental updates of input data
rather than requiring all data to be presented at once, which is what the
current interface requires.
- use one less temporary buffer
- pedantic: in_len + 15 was a potential overflow
- use a more explicit name instead of 'flag'
- Mn was a bit misleading
The previous version had secret-dependent memory accesses. While it was
probably not an issue in practice cause the two bytes of the array are
probably on the same cache line anyway, as a matter of principle this should
be avoided.
Due to inconsistent freeing strategy in pkparse.c the sample mutex
implementation in threading.c could lead to undefined behaviour by
destroying the same mutex several times.
This fix prevents mutexes from being destroyed several times in the
sample threading implementation.
The library/net.c and its corresponding include/mbedtls/net.h file are
renamed to library/net_sockets.c and include/mbedtls/net_sockets.h
respectively. This is to avoid naming collisions in projects which also
have files with the common name 'net'.
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.
Ensure that the entropy self test always fails whenever
MBEDTLS_TEST_NULL_ENTROPY is defined. This is because the option is
meant to be for testing and development purposes rather than production
quality software. Also, this patch enhances the documentation for
mbedtls_entropy_source_self_test() and mbedtls_entropy_self_test().
Instead of polling the hardware entropy source a single time and
comparing the output with itself, the source is polled at least twice
and make sure that the separate outputs are different.
The self test is a quick way to check at startup whether the entropy
sources are functioning correctly. The self test only polls 8 bytes
from the default entropy source and performs the following checks:
- The bytes are not all 0x00 or 0xFF.
- The hardware does not return an error when polled.
- The entropy does not provide data in a patter. Only check pattern
at byte, word and long word sizes.
In platform.c, made the time functions dependent on the configuration
MBEDTLS_HAVE_TIME to fix a build break where the functions could be
built but the mbedtls_time_t was not defined.
Separates platform time abstraction into it's own header from the
general platform abstraction as both depend on different build options.
(MBEDTLS_PLATFORM_C vs MBEDTLS_HAVE_TIME)
This commit fixes following warning:
> CC: aes.c
> aes.c: In function 'mbedtls_aes_self_test':
> aes.c:1225:19: error: unused variable 'iv' [-Werror=unused-variable]
> unsigned char iv[16];
> ^
> cc1: all warnings being treated as errors
If the option MBEDTLS_TEST_NULL_ENTROPY is enabled, the cmake generated
makefile will generate an error unless a UNSAFE_BUILD switch is also enabled.
Equally, a similar warning will always be generated if the Makefile is built,
and another warning is generated on every compilation of entropy.c.
This is to ensure the user is aware of what they're doing when they enable the
null entropy option.
Update the NV entropy seed before generating any entropy for outside
use. The reason this is triggered here and not in mbedtls_entropy_init(),
is that not all entropy sources mights have been added at that time.
Introduces mbedtls_nv_seed_read() and mbedtls_nv_seed_write().
The platform-layer functions are only available when
MBEDTLS_ENTROPY_NV_SEED is enabled.
Add a switch that turns entropy collecting off entirely, but enables
mbed TLS to run in an entirely unsafe mode. Enables to test mbed TLS
on platforms that don't have their entropy sources integrated yet.
Commit daf534d from PR #457 breaks the build. This may reintroduce a
clang-analyse warning, but this is the wrong fix for that.
The fix removed a call to mbedtls_ecp_curve_info_from_grp_id() to find
the curve info. This fix adds that back in.
The check is already effectively performed later in the function, but
implicitly, so Clang's analysis fail to notice the functions are in
fact safe. Pulling the check up to the top helps Clang to verify the
behaviour.
Since the buffer is used in a few places, it seems Clang isn't clever
enough to realise that the first byte is never touched. So, even though
the function has a correct null check for ssl->handshake, Clang
complains. Pulling the handshake type out into its own variable is
enough for Clang's analysis to kick in though.
The function appears to be safe, since grow() is called with sensible
arguments in previous functions. Ideally Clang would be clever enough to
realise this. Even if N has size MBEDTLS_MPI_MAX_LIMBS, which will
cause the grow to fail, the affected lines in montmul won't be reached.
Having this sanity check can hardly hurt though.
It is used only by `mbedtls_sha512_process()`, and in case `MBEDTLS_SHA512_PROCESS_ALT` is defined, it still cannot be reused because of `static` declaration.
On x32 systems, pointers are 4-bytes wide and are therefore stored in %e?x
registers (instead of %r?x registers). These registers must be accessed using
"addl" instead of "addq", however the GNU assembler will acccept the generic
"add" instruction and determine the correct opcode based on the registers
passed to it.
The server code parses the client hello extensions even when the
protocol is SSLv3 and this behaviour is non compliant with rfc6101.
Also the server sends extensions in the server hello and omitting
them may prevent interoperability problems.
Commit daf534d from PR #457 breaks the build. This may reintroduce a
clang-analyse warning, but this is the wrong fix for that.
The fix removed a call to mbedtls_ecp_curve_info_from_grp_id() to find
the curve info. This fix adds that back in.
The check is already effectively performed later in the function, but
implicitly, so Clang's analysis fail to notice the functions are in
fact safe. Pulling the check up to the top helps Clang to verify the
behaviour.
Since the buffer is used in a few places, it seems Clang isn't clever
enough to realise that the first byte is never touched. So, even though
the function has a correct null check for ssl->handshake, Clang
complains. Pulling the handshake type out into its own variable is
enough for Clang's analysis to kick in though.
The function appears to be safe, since grow() is called with sensible
arguments in previous functions. Ideally Clang would be clever enough to
realise this. Even if N has size MBEDTLS_MPI_MAX_LIMBS, which will
cause the grow to fail, the affected lines in montmul won't be reached.
Having this sanity check can hardly hurt though.
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.
The callback typedefs defined for mbedtls_ssl_set_bio() and
mbedtls_ssl_set_timer_cb() were not used consistently where the callbacks were
referenced in structures or in code.
- 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.)
- document why we made that choice
- remove the two TODOs about checking hash and CA
- remove the code that parsed certificate_type: it did nothing except store
the selected type in handshake->cert_type, but that field was never accessed
afterwards. Since handshake_params is now an internal type, we can remove that
field without breaking the ABI.
We don't implement anonymous key exchanges, and we don't intend to, so it can
never happen that an unauthenticated server requests a certificate from us.
After the record contents are decompressed, in_len is no longer
accessed directly, only in_msglen is accessed. in_len is only read by
ssl_parse_record_header() which happens before ssl_prepare_record_contents().
This is also made clear by the fact that in_len is not touched after
decrypting anyway, so if it was accessed after that it would be wrong unless
decryption is used - as this is not the case, it show in_len is not accessed.
Previously it was failing with errors about headers not found, which is
suboptimal in terms of clarity. Now give a clean error with pointer to the
documentation.
Do the checks in the .c files rather than check_config.h as it keeps them
closer to the platform-specific implementations.
It is used only by `mbedtls_sha512_process()`, and in case `MBEDTLS_SHA512_PROCESS_ALT` is defined, it still cannot be reused because of `static` declaration.
armar doesn't understand the syntax without dash. OTOH, the syntax with dash
is the only one specified by POSIX, and it's accepted by GNU ar, BSD ar (as
bundled with OS X) and armar, so it looks like the most portable syntax.
fixes#386
* yanesca/iss309:
Improved on the previous fix and added a test case to cover both types of carries.
Removed recursion from fix#309.
Improved on the fix of #309 and extended the test to cover subroutines.
Tests and fix added for #309 (inplace mpi doubling).
On x32 systems, pointers are 4-bytes wide and are therefore stored in %e?x
registers (instead of %r?x registers). These registers must be accessed using
"addl" instead of "addq", however the GNU assembler will acccept the generic
"add" instruction and determine the correct opcode based on the registers
passed to it.
See for example page 8 of
http://csrc.nist.gov/publications/nistpubs/800-38D/SP-800-38D.pdf
The previous constant probably came from a typo as it was 2^26 - 2^5 instead
of 2^36 - 2^5. Clearly the intention was to allow for a constant bigger than
2^32 as the ull suffix and cast to uint64_t show.
fixes#362
By looking just at that test, it looks like 2 + dn_size could overflow. In
fact that can't happen as that would mean we've read a CA cert of size is too
big to be represented by a size_t.
However, it's best for code to be more obviously free of overflow without
having to reason about the bigger picture.
In case an entry with the given OID already exists in the list passed to
mbedtls_asn1_store_named_data() and there is not enough memory to allocate
room for the new value, the existing entry will be freed but the preceding
entry in the list will sill hold a pointer to it. (And the following entries
in the list are no longer reachable.) This results in memory leak or a double
free.
The issue is we want to leave the list in a consistent state on allocation
failure. (We could add a warning that the list is left in inconsistent state
when the function returns NULL, but behaviour changes that require more care
from the user are undesirable, especially in a stable branch.)
The chosen solution is a bit inefficient in that there is a time where both
blocks are allocated, but at least it's safe and this should trump efficiency
here: this code is only used for generating certificates, which is unlikely to
be done on very constrained devices, or to be in the critical loop of
anything. Also, the sizes involved should be fairly small anyway.
fixes#367
When the peer retransmits a flight with many record in the same datagram, and
we already saw one of the records in that datagram, we used to drop the whole
datagram, resulting in interoperability failure (spurious handshake timeouts,
due to ignoring record retransmitted by the peer) with some implementations
(issues with Chrome were reported).
So in those cases, we want to only drop the current record, and look at the
following records (if any) in the same datagram. OTOH, this is not something
we always want to do, as sometime the header of the current record is not
reliable enough.
This commit introduces a new return code for ssl_parse_header() that allows to
distinguish if we should drop only the current record or the whole datagram,
and uses it in mbedtls_ssl_read_record()
fixes#345
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
* iotssl-519-asn1write-overflows-restricted:
Fix other int casts in bounds checking
Fix other occurrences of same bounds check issue
Fix potential buffer overflow in asn1write
* 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
fixes#310
Actually all key exchanges that use a certificate use signatures too, and
there is no key exchange that uses signatures but no cert, so merge those two
flags.
Not a security issue as here we know the buffer is large enough (unless
something else if badly wrong in the code), and the value cast to int is less
than 2^16 (again, unless issues elsewhere).
Still changing to a more correct check as a matter of principle
Two causes:
- the buffer is too short (missing 4 bytes for encoding id_len)
- the test was wrong
Would only happen when MBEDTLS_ECP_MAX_BITS == the bitsize of the curve
actually used (does not happen in the default config).
Could not be triggered remotely.
* development: (73 commits)
Bump yotta dependencies version
Fix typo in documentation
Corrected misleading fn description in ssl_cache.h
Corrected URL/reference to MPI library
Fix yotta dependencies
Fix minor spelling mistake in programs/pkey/gen_key.c
Bump version to 2.1.2
Fix CVE number in ChangeLog
Add 'inline' workaround where needed
Fix references to non-standard SIZE_T_MAX
Fix yotta version dependencies again
Upgrade yotta dependency versions
Fix compile error in net.c with musl libc
Add missing warning in doc
Remove inline workaround when not useful
Fix macroization of inline in C++
Changed attribution for Guido Vranken
Merge of IOTSSL-476 - Random malloc in pem_read()
Fix for IOTSSL-473 Double free error
Fix potential overflow in CertificateRequest
...
Conflicts:
include/mbedtls/ssl_internal.h
library/ssl_cli.c
- "master secret" is the usual name
- move key block arg closer to the related lengths
- document lengths
Also fix some trailing whitespace while at it
In BER encoding, any boolean with a non-zero value is considered as
TRUE. However, DER encoding require a value of 255 (0xFF) for TRUE.
This commit makes `mbedtls_asn1_write_bool` function uses `255` instead
of `1` for BOOLEAN values.
With this fix, boolean values are now reconized by OS X keychain (tested
on OS X 10.11).
Fixes#318.
Two possible integer overflows (during << 2 or addition in BITS_TO_LIMB())
could result in far too few memory to be allocated, then overflowing the
buffer in the subsequent for loop.
Both integer overflows happen when slen is close to or greater than
SIZE_T_MAX >> 2 (ie 2^30 on a 32 bit system).
Note: one could also avoid those overflows by changing BITS_TO_LIMB(s << 2) to
CHARS_TO_LIMB(s >> 1) but the solution implemented looks more robust with
respect to future code changes.
