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

CURLOPT_SSL_EC_CURVES and '--curves' ignored with BoringSSL #8553

Closed
lwthiker opened this issue Mar 7, 2022 · 2 comments
Closed

CURLOPT_SSL_EC_CURVES and '--curves' ignored with BoringSSL #8553

lwthiker opened this issue Mar 7, 2022 · 2 comments
Labels

Comments

@lwthiker
Copy link
Contributor

@lwthiker lwthiker commented Mar 7, 2022

I did this

  • Compiled curl with BoringSSL as per this guide
  • Ran curl with curl --curves X25519:P-256:P-384:P-521 ...

I expected the following

Expected the correct curves to be set in the TLS client hello message. However, the curves are not set. This is from Wireshark:

Extension: supported_groups (len=8)
    Type: supported_groups (10)
    Length: 8
    Supported Groups List Length: 6
    Supported Groups (3 groups)
        Supported Group: x25519 (0x001d)
        Supported Group: secp256r1 (0x0017)
        Supported Group: secp384r1 (0x0018)

(Notice that secp521r1 is missing).

The reason is this code in lib/vtls/openssl.c:

#if ((OPENSSL_VERSION_NUMBER >= 0x10101000L) && \
     !defined(LIBRESSL_VERSION_NUMBER) &&       \
     !defined(OPENSSL_IS_BORINGSSL))
#define HAVE_SSL_CTX_SET_CIPHERSUITES
#define HAVE_SSL_CTX_SET_POST_HANDSHAKE_AUTH
/* SET_EC_CURVES is available under the same preconditions: see
 * https://www.openssl.org/docs/manmaster/man3/SSL_CTX_set1_groups.html
 */
#define HAVE_SSL_CTX_SET_EC_CURVES
#endif

However, BoringSSL does supply the SSL_CTX_set1_curves_list() function, here's the documentation. It was added in this commit.

The fix is simple and I have a patch already. I was wondering whether there is a specific reason it was disabled with BoringSSL.

curl/libcurl version

curl 7.82.0 (x86_64-pc-linux-gnu) libcurl/7.82.0 BoringSSL zlib/1.2.11 brotli/1.0.7 nghttp2/1.40.0
Release-Date: 2022-03-05
Protocols: dict file ftp ftps gopher gophers http https imap imaps mqtt pop3 pop3s rtsp smb smbs smtp smtps telnet tftp 
Features: alt-svc AsynchDNS brotli HSTS HTTP2 HTTPS-proxy IPv6 Largefile libz NTLM NTLM_WB SSL UnixSockets

operating system

Linux ThinkPad 5.10.0-1055-oem #58-Ubuntu SMP Thu Jan 6 20:44:40 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

@bagder bagder added the TLS label Mar 7, 2022
@bagder
Copy link
Member

@bagder bagder commented Mar 7, 2022

I was wondering whether there is a specific reason it was disabled with BoringSSL.

I think just because nobody has tested/verified it for boringssl until you did now.

@lwthiker
Copy link
Contributor Author

@lwthiker lwthiker commented Mar 7, 2022

Alright, I will open a pull request.

lwthiker added a commit to lwthiker/curl that referenced this issue Mar 7, 2022
The CURLOPT_SSL_EC_CURVES option (used by the '--curves' flag)
in libcurl was ignored when compiling with BoringSSL because
HAVE_SSL_CTX_SET_EC_CURVES was explicitly disabled if BoringSSL was
detected.  However, this feature is supported in BoringSSL since
5fd1807d. This commit enables it, and also reduces the required minimal
OpenSSL version to 1.0.2 as per OpenSSL's official documentation.

Fixes curl#8553.
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

2 participants