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

Be more paranoid when using curl #7593

Closed
olix0r opened this issue Jan 12, 2022 · 0 comments · Fixed by #7658
Closed

Be more paranoid when using curl #7593

olix0r opened this issue Jan 12, 2022 · 0 comments · Fixed by #7658

Comments

@olix0r
Copy link
Member

olix0r commented Jan 12, 2022

In a variety of places (especially in our development scripts), we fetch resources using curl. We're not super consistent in the flags we pass to curl. It would be good to start using a standard set of flags (and probably to encode these in a reusable function somewhere).

I recommend:

  • --proto='=https' -- forces to curl to use https, avoiding being downgraded to HTTP on redirect.
  • --tls1.3 -- forces curl to use TLS v1.3, which avoids a variety of downgrade attacks.
  • --fail|-f -- prevents curl from emitting a payload when it encounters a non-2XX response
  • --show-error|-S -- causes errors to be logged to stderr (necessary when using --silent|-s)

Perhaps these should be encoded in a bin/scurl utility? Ideally we'd have a check in CI to prevent new calls to curl outside of this script, but I'm not sure if there's a good tool for auditing this. (Can shellcheck do it?)

@olix0r olix0r added this to the stable-2.12.0 milestone Jan 12, 2022
olix0r added a commit to linkerd/website that referenced this issue 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.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 added a commit to linkerd/website that referenced this issue 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.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 added a commit to linkerd/website that referenced this issue Jan 20, 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]>
olix0r pushed a commit that referenced this issue Jan 24, 2022
Introduce `bin/scurl` which invokes `curl` with a standard set of flags to enforce best practices.
All `curl` invocations have been replaced with `scurl`.

Fixes #7593

Signed-off-by: Alex Leong <[email protected]>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 24, 2022
olix0r pushed a commit that referenced this issue Mar 31, 2022
Introduce `bin/scurl` which invokes `curl` with a standard set of flags to enforce best practices.
All `curl` invocations have been replaced with `scurl`.

Fixes #7593

Signed-off-by: Alex Leong <[email protected]>
(cherry picked from commit 2c6db25)
Signed-off-by: Oliver Gould <[email protected]>
olix0r pushed a commit that referenced this issue Apr 2, 2022
Introduce `bin/scurl` which invokes `curl` with a standard set of flags to enforce best practices.
All `curl` invocations have been replaced with `scurl`.

Fixes #7593

Signed-off-by: Alex Leong <[email protected]>
(cherry picked from commit 2c6db25)
Signed-off-by: Oliver Gould <[email protected]>
olix0r pushed a commit that referenced this issue Apr 7, 2022
Introduce `bin/scurl` which invokes `curl` with a standard set of flags to enforce best practices.
All `curl` invocations have been replaced with `scurl`.

Fixes #7593

Signed-off-by: Alex Leong <[email protected]>
(cherry picked from commit 2c6db25)
Signed-off-by: Oliver Gould <[email protected]>
olix0r pushed a commit that referenced this issue Apr 8, 2022
Introduce `bin/scurl` which invokes `curl` with a standard set of flags to enforce best practices.
All `curl` invocations have been replaced with `scurl`.

Fixes #7593

Signed-off-by: Alex Leong <[email protected]>
(cherry picked from commit 2c6db25)
Signed-off-by: Oliver Gould <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant