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 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.
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.
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.
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.
There is only one length byte but for some reason we skipped two, resulting in
reading one byte past the end of the extension. Fortunately, even if that
extension is at the very end of the ClientHello, it can't be at the end of the
buffer since the ClientHello length is at most SSL_MAX_CONTENT_LEN and the
buffer has some more room after that for MAC and so on. So there is no
buffer overread.
Possible consequences are:
- nothing, if the next byte is 0x00, which is a comment first byte for other
extensions, which is why the bug remained unnoticed
- using a point format that was not offered by the peer if next byte is 0x01.
In that case the peer will reject our ServerKeyExchange message and the
handshake will fail.
- thinking that we don't have a common point format even if we do, which will
cause us to immediately abort the handshake.
None of these are a security issue.
The same bug was fixed client-side in fd35af15
May happen with a faulty configuration (eg no allowed curve but trying to use
ECDHE key exchange), but not trigger able remotely.
(Found with Clang's scan-build.)
While at it, fix the following:
- on server with RSA_PSK, we don't want to set flags (client auth happens via
the PSK, no cert is expected).
- use safer tests (eg == OPTIONAL vs != REQUIRED)
