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

Require TLS when using curl #1277

Merged
merged 8 commits into from
Jan 20, 2022
Merged

Require TLS when using curl #1277

merged 8 commits into from
Jan 20, 2022

Conversation

olix0r
Copy link
Member

@olix0r olix0r commented Jan 12, 2022

Our usage of curl could be vulnerable to protocol downgrade attacks. This
change updates most of our usage of curl--especially when fetching executable
resources--with the following command-line flags:

  • --proto '=https' forces use of HTTPS. This ensures that dropping https://
    from a URL will cause the command to fail instead of reverting to use
    unsecured HTTP.
  • --tlsv1.2 disables the use of older TLS versions.
  • -f|--fail ensures that curl does not output anything to stdout when a
    non-2xx response is received.
  • -S|--show-error causes errors to be printed to stderr (when -s|--silent
    is used).

Related to linkerd/linkerd2#7593

Signed-off-by: Oliver Gould [email protected]

Our usage of `curl` could be vulnerable to protocol downgrade attacks. This
change updates most of our usage of curl--especially when fetching executable
resources--with the following command-line flags:

* `--proto '=https'` forces use of HTTPS. This ensures that dropping `https://`
  from a URL will cause the command to fail instead of reverting to use
  unsecured HTTP.
* `--tlsv1.3` disables the use of older TLS versions. This flag was added to
  curl in 2016, which now seems old enough to be a reasonable default.
* `-f|--fail` ensures that curl does not output anything to stdout when a
  non-2xx response is received.
* `-S|--show-error` causes errors to be printed to stderr (when `-s|--silent`
  is used).

Related to linkerd/linkerd2#7593

Signed-off-by: Oliver Gould <[email protected]>
@olix0r olix0r requested a review from wmorgan January 14, 2022 19:55
Copy link
Member

@alpeb alpeb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks that you mostly targeted 2.10 docs and not 2.11?

Copy link
Contributor

@cpretzer cpretzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On macOS Big Sur, these commands fail because the default version of LibreSSL doesn't have TLS 1.3 support built in.

--tlsv1.2 works, though. Do we need to have two sets of instructions like we do in other places?

@olix0r
Copy link
Member Author

olix0r commented Jan 18, 2022

We should update this to be --tlsv1.2; and we should make sure the 2.11 docs are updated... I think I missed them via merge or something

Copy link
Member

@wmorgan wmorgan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edit: oops I see @cpretzer beat me to the scoop.

On my 2020 M1 Mac running Big Sur (i.e. reasonably up-to-date), I get:

❯ curl --proto '=https' --tlsv1.3 -sSfL https://buoyant.cloud/install 
curl: (4) LibreSSL was built without TLS 1.3 support

@olix0r olix0r changed the title Require TLS v1.3 when using curl Require TLS when using curl Jan 18, 2022
@olix0r
Copy link
Member Author

olix0r commented Jan 19, 2022

@cpretzer does this look okay now?

Copy link
Contributor

@cpretzer cpretzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚢

@olix0r olix0r merged commit 6f6ae6e into main Jan 20, 2022
@klingerf
Copy link
Contributor

klingerf commented Mar 15, 2022

Apologies for the late review here. The updated curl command from this branch fails for me locally:

$ curl --proto '=https' --tlsv1.2 -sSfL buoyant.cloud/install
curl: (1) Protocol "http" not supported or disabled in libcurl
$ curl --version
curl 7.77.0 (x86_64-apple-darwin21.0) libcurl/7.77.0 (SecureTransport) LibreSSL/2.8.3 zlib/1.2.11 nghttp2/1.42.0
Release-Date: 2021-05-26
Protocols: dict file ftp ftps gopher gophers http https imap imaps ldap ldaps mqtt pop3 pop3s rtsp smb smbs smtp smtps telnet tftp 
Features: alt-svc AsynchDNS GSS-API HSTS HTTP2 HTTPS-proxy IPv6 Kerberos Largefile libz MultiSSL NTLM NTLM_WB SPNEGO SSL UnixSockets

It's likely I could install a compatible version of curl via homebrew, but I wanted to mention it since I suspect other osx users may run into similar issues with the default curl install.

@olix0r
Copy link
Member Author

olix0r commented Mar 17, 2022

@klingerf this is flagging that this isn't using https! So it's working as intended :) we should update that link to be https://buoyant.cloud...

@olix0r olix0r deleted the ver/curl-hard branch March 17, 2022 13:44
olix0r added a commit that referenced this pull request Mar 17, 2022
#1277 missed one URL. This fixes it.

Signed-off-by: Oliver Gould <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants