"--tls1.2" flag causes connection to fail in macOS starting with curl 7.54.0

[nmiyake]

The command curl --tlsv1.2 https://tlstest.paypal.com/ works in curl 7.53.1 but does not work in curl 7.54.0 or later when built with SecureTransport. Tested on macOS Sierra 10.12.6 (16G29).

• The macOS Sierra 10.12.6 update updates the curl version from 7.51.0 to 7.54.0, so this behavior regresses when updating from macOS 10.12.5 to 10.12.6
• Omitting the --tlsv1.2 flag and calling curl https://tlstest.paypal.com/ works in all cases
• Flag works when built with OpenSSL rather than SecureTransport

7.53.1-DEV (built locally; works)

curl -V && curl --tlsv1.2 https://tlstest.paypal.com/:

curl 7.53.1-DEV (x86_64-apple-darwin16.7.0) libcurl/7.53.1-DEV SecureTransport zlib/1.2.8
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtsp smb smbs smtp smtps telnet tftp 
Features: Debug TrackMemory IPv6 Largefile NTLM NTLM_WB SSL libz UnixSockets 
PayPal_Connection_OK%

7.54.0 (included in macOS Sierra 10.12.6; does not work)

curl -V && curl --tlsv1.2 https://tlstest.paypal.com/:

curl 7.54.0 (x86_64-apple-darwin16.0) libcurl/7.54.0 SecureTransport zlib/1.2.8
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtsp smb smbs smtp smtps telnet tftp 
Features: AsynchDNS IPv6 Largefile GSS-API Kerberos SPNEGO NTLM NTLM_WB SSL libz UnixSockets 
curl: (35) Server aborted the SSL handshake

7.54.1-DEV (built locally; does not work)

curl -V && curl --tlsv1.2 https://tlstest.paypal.com/:

curl 7.54.1-DEV (x86_64-apple-darwin16.7.0) libcurl/7.54.1-DEV SecureTransport zlib/1.2.8
Release-Date: [unreleased]
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtsp smb smbs smtp smtps telnet tftp 
Features: Debug TrackMemory IPv6 Largefile NTLM NTLM_WB SSL libz UnixSockets 
curl: (35) unsupported max version passed via CURLOPT_SSLVERSION

7.54.1 (installed using brew; works)

curl -V && curl --tlsv1.2 https://tlstest.paypal.com/:

curl 7.54.1 (x86_64-apple-darwin16.7.0) libcurl/7.54.1 OpenSSL/1.0.2l zlib/1.2.8
Release-Date: 2017-06-14
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtsp smb smbs smtp smtps telnet tftp 
Features: IPv6 Largefile NTLM NTLM_WB SSL libz TLS-SRP UnixSockets HTTPS-proxy 
PayPal_Connection_OK% 

[jay]

how are you building curl with darwin ssl can you show me the actual config line and make line

[nmiyake]

Built and installed as follows:

./configure --disable-shared --enable-debug --enable-maintainer-mode --with-darwinssl
make && make install

[jay]

ok. would you mind running an automated bisect? it should take very little of your time. here is the bisect script:

#!/bin/bash

# Sample script to run an automated bisect.
#
# Refer to https://github.com/curl/curl/wiki/how-to-git-bisect
#
# Put this file in the curl repo as a.sh
#
# git bisect start master curl-7_53_1
# git bisect run ./a.sh

# if git clean fails then bisect skip
# warning: this removes all untracked directories and files except a.sh
# enable if you experience unexplained autotools build problems
#git clean -fdx -e a.sh || exit 125

# if buildconf fails then bisect skip
./buildconf || exit 125

# if configure fails then bisect skip
./configure --disable-shared \
            --enable-debug \
            --enable-maintainer-mode \
            --with-darwinssl \
            || exit 125

# if make fails then bisect skip
make || exit 125

# if curl version fails then bisect skip
src/curl -V || exit 125

# Test for issue 1703:
# "--tls1.2" flag causes connection to fail in macOS starting with curl 7.54.0
# https://github.com/curl/curl/issues/1703
#
# if curl fails due to error 35 (unsupported max version) then bisect bad
# if curl fails due to a different error then bisect skip
src/curl --tlsv1.2 https://tlstest.paypal.com/
rc=$?
if (( $rc == 35 )); then exit 1; elif (( $rc != 0 )); then exit 125; fi

# bisect good
exit 0

[nmiyake]

I ran the bisect, but unfortunately there were a chunk of commits that couldn't be tested because of the following compilation error:

vtls/darwinssl.c:1050:10: error: use of undeclared identifier 'ssl_version'; did you mean 'curl_version'?
  switch(ssl_version) {
         ^~~~~~~~~~~
         curl_version

Here's the final output:

The first bad commit could be any of:
e08c0cd3275727978b69a1087bcebe0e38a5c9c9
8d105209933e27293cfc4f224614cea57ddd8372
ec1d0ed1c14d1b2ed06d8914c19b3df2da575005
d2bcf1e3e247d116dc96bd3ea32056e3f089449c
e84a863dc37f81bca066838a76e1b461c3cabeaf
0afbcfd800c45e766e225e4ce273b128ee6a8c25
1890d59905414ab84a35892b2e45833654aa5c13
5278462c32a70cd972a8cc824a38f164151d6c6d
75398731e40dad2daee74981742882e4ef9f8a49
381aff03857544b05c34f3f095089cd170023978
3863de57f2fdb9a487d7f92c0bd64d740dc7a96f
03c5223ee7fa8dcdb5c54ea58c1fb2cca67f35e6
db900ab7328706b7ad8ce18ea8c459ffcd7bb216
1e54c5fb095c71084e1d27583f124067c7a56fa9
c25e0761d0fc49c415417c4bc527fb1c69300825
7c847ab9d522c4ee08060f93726f22ea6037db06
bfb2a0ad77daaec5ced53ddce80ca01f2d0cc685
12f04142c1923069cbf60bdb3789a95c464daf6d
db87bcfcf21f8c3b8188d0c5ab82faf804ffd5ea
26e9fc27adbc9dd3095eaf77c0e95bf3dffd7b5b
66de563482c0fd4324e1eae19809d2499e3c4fa8
3627769d140eede69a2241c35e8eadae950e7123
e1960fef129b864fdbdf5771c0d938b605667b3b
a2eb08607c1bcc16d9804f4f94b96570fc2d832b
b1dc45a82202f6e106a6ea2d834f001d46b8fc82
280e8c6e371ebd0f2e0a907e016ab44d6c9549af
9f3bc40f7f0c5a9b809bd8870c5d63c98cc45777
7d62502d6c123d08411a0be8a9a38d90861833d8
c5357b7b992ac6bda05a605815f43a13c78eca17
28a3453ac74b23f5d2b5271a531157c3e7643d69
49f7b13536c4f5e0cab3d93a30ab3f5ad5a7b91f
6733acc9033d96b52d387f757ac39614b45c1913
f7b3914c558d9d2995208171bea42ca58b73237f
6448f98c1857de521fb2dd3f9d4e5659845b5474
98afec033e425b56cce00b9ef5b44828df0276f0
9873431c6f73f2e87f35e1b95e8f4a5b2e1f9791
We cannot bisect more!
bisect run cannot continue any more

If I can figure out how to fix the compilation error then I can re-run the bisect.

[bagder]

I believe this little fix is all that's needed:

--- a/lib/vtls/darwinssl.c
+++ b/lib/vtls/darwinssl.c
@@ -1073,11 +1073,12 @@ CF_INLINE bool is_file(const char *filename)
     return S_ISREG(st.st_mode);
   return false;
 }
 
 #if CURL_BUILD_MAC_10_8 || CURL_BUILD_IOS
-static CURLcode darwinssl_version_from_curl(long *darwinver, long ssl_version)
+static CURLcode darwinssl_version_from_curl(SSLProtocol *darwinver,
+                                            long ssl_version)
 {
   switch(ssl_version) {
     case CURL_SSLVERSION_TLSv1_0:
       *darwinver = kTLSProtocol1;
       return CURLE_OK;

[nmiyake]

Thanks, this patch allowed me to run the bisect to completion!

Here's the result of the bisect:

9873431c6f73f2e87f35e1b95e8f4a5b2e1f9791 is the first bad commit
commit 9873431c6f73f2e87f35e1b95e8f4a5b2e1f9791
Author: Palo Markovic <pavol.markovic@kompiler.info>
Date:   Sun Mar 19 00:37:51 2017 +1300

    darwinssl: fix typo in variable name
    
    Broken a week ago in 6448f98.
    
    Closes https://github.com/curl/curl/pull/1337

:040000 040000 a54e0fbf7ee8ef79b465afc6f9d63b7395126c4d c6e9f46d64bb1f0f2b5c248f447076e80c9aba9b M	lib
bisect run success

I also confirmed manually that the commit immediately before this (with the patch applied) does work correctly.

For future reference, I had to modify the patch slightly to get it to apply using git apply in the bisect script:

diff --git a/lib/vtls/darwinssl.c b/lib/vtls/darwinssl.c
index 26ea04c..d7c44f9 100644
--- a/lib/vtls/darwinssl.c
+++ b/lib/vtls/darwinssl.c
@@ -1045,7 +1045,8 @@ CF_INLINE bool is_file(const char *filename)
 }
 
 #if CURL_BUILD_MAC_10_8 || CURL_BUILD_IOS
-static CURLcode darwinssl_version_from_curl(long *darwinver, long version)
+static CURLcode darwinssl_version_from_curl(SSLProtocol *darwinver,
+                                            long ssl_version)
 {
   switch(ssl_version) {
     case CURL_SSLVERSION_TLSv1_0:

[jay]

i'm confused , does @bagder's patch fix the issue, I thought the problem was someone had it deref to a long instead of an int

[nmiyake]

Sorry, a few different things going on here...

• Commit 9873431 builds as-is and exhibits incorrect behavior (connection does not work)
• The commit prior to that commit does not compile due to a bug
  • Applying the patch to this commit makes it compile, and exhibits correct behavior (SSL connection works)
• First prior commit that compiles without patching exhibits correct behavior

I did not try applying the patch to the latest commit(s) that compile but produce incorrect behavior.

[bagder]

Without the patch, the code blanks the ssl_version_max variable on the first invoke of the darwinssl_version_from_curl() function. That then makes the second invoke return the error code.

[bagder]

since @nmiyake has confirmed this patch fixes the problem and it has been pushed already, I suppose we can close this now?

[nmiyake]

I never applied the patch on top of any versions that compiled but didn't work -- I can try that now to verify.

[bagder]

Ah, yes please do. I personally tried on the current git master and it works for me with this applied.

[nmiyake]

Confirmed that issue is resolved in latest master branch with this patch applied, so I consider this resolved -- thanks!