Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Client Hello and Server Hello should include ECC extensions only when negotiating an elliptic curve ciphersuite #1157

Closed
milenamil opened this issue Oct 27, 2017 · 3 comments
Labels

Comments

@milenamil
Copy link

Description

  • Type: Bug
  • Priority: Major

Bug

OS
NA

mbed TLS build:
2.6.0
OS version: NA
Configuration: Any config that supports both EC and non-EC ciphersuites

Peer device TLS stack and version
BouncyCastle, others

Expected behavior
Client Hello and Server Hello should respect the RFC 4492.

The relevant Client Hello behavior is described in section 4:
"The client MUST NOT include these extensions in the ClientHello message if it does not propose any ECC cipher suites."

The relevant Server Hello behavior is described in section 5.2:
"When this extension is sent:
The Supported Point Formats Extension is included in a ServerHello
message in response to a ClientHello message containing the Supported
Point Formats Extension when negotiating an ECC cipher suite."

Actual behavior
The client incorrectly sends ECC extensions in the Client Hello, under the following conditions:

  • elliptic curve support is compiled in (e.g., MBEDTLS_ECDH_C)
  • the client SSL config includes only non-ECC ciphersuites

The server incorrectly sends the ECC extension in the Server Hello, under the following conditions:

  • elliptic curve support is compiled in (e.g., MBEDTLS_ECDH_C)
  • the client peer sends the ECC extensions in its Hello
  • the negotiated ciphersuite does not use ECC
    The server behavior results in a terminated handshake with a BouncyCastle client (Fatal Alert, illegal parameter), since BouncyCastle has fairly strict checks for the Server Hello extensions.

On the client side, the extensions are sent unconditionally if any ECC key exchange is compiled in (MBEDTLS_ECDH_C, MBEDTLS_ECDSA_C, or MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED). They should be sent only if one of the proposed ciphersuites uses ECC.

The situation is similar on the server side - the ECC extension is sent if any ECC key exchange is compiled in and the client sent ECC extensions. The extension should be sent only if the negotiated ciphersuite uses ECC.

Steps to reproduce
See above.

@RonEld
Copy link
Contributor

RonEld commented Oct 29, 2017

Hi @milenamil Thank you for reporting this!
I believe you are correct, and we will look into this

@ciarmcom
Copy link

ARM Internal Ref: IOTSSL-1857

RonEld pushed a commit to RonEld/mbedtls that referenced this issue Feb 16, 2018
Fix compliancy to RFC4492. ECC extensions should be included
only if ec ciphersuites are used. Interoperability issue with
bouncy castle. Mbed-TLS#1157
RonEld pushed a commit to RonEld/mbedtls that referenced this issue Jun 21, 2018
Fix compliancy to RFC4492. ECC extensions should be included
only if ec ciphersuites are used. Interoperability issue with
bouncy castle. Mbed-TLS#1157
@RonEld
Copy link
Contributor

RonEld commented Jun 21, 2018

@milenamil Could you confirm if #1378 fixes your issue?

RonEld pushed a commit to RonEld/mbedtls that referenced this issue Jun 28, 2018
Fix compliancy to RFC4492. ECC extensions should be included
only if ec ciphersuites are used. Interoperability issue with
bouncy castle. Mbed-TLS#1157
RonEld pushed a commit to RonEld/mbedtls that referenced this issue Jun 28, 2018
Fix compliancy to RFC4492. ECC extensions should be included
only if ec ciphersuites are used. Interoperability issue with
bouncy castle. Mbed-TLS#1157
RonEld pushed a commit to RonEld/mbedtls that referenced this issue Jun 28, 2018
Fix compliancy to RFC4492. ECC extensions should be included
only if ec ciphersuites are used. Interoperability issue with
bouncy castle. Mbed-TLS#1157
minosgalanakis pushed a commit that referenced this issue Mar 28, 2024
…protection-backport

[Backport] Key management buffer protection
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants