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

install.sh: Invoke curl with secure defaults #999

Merged
merged 3 commits into from
Mar 15, 2022

Conversation

olix0r
Copy link
Contributor

@olix0r olix0r commented Mar 4, 2022

When invoking the install script, the installation process can fail
silently so that the k3d binary includes only the ASCII string 'Not
found'. This can be avoided by passing the --fail flag so that the
command fails if a 4XX or 5XX response is received.

This change adds a scurl function that invokes curl with this flag,
as well as flags that ensure that at least TLS v1.2 is used (this helps
mitigate protocol downgrade attacks).

This is based on the defaults we use in Linkerd. See: https://github.com/linkerd/linkerd2/blob/a0b112471eef7e3975fbc8564df52d83894c3fa3/bin/scurl

When invoking the install script, the installation process can fail
silently so that the `k3d` binary includes only the ASCII string 'Not
found'. This can be avoided by passing the `--fail` flag so that the
command fails if a 4XX or 5XX response is received.

This change adds a `scurl` function that invokes `curl` with this flag,
as well as flags that ensure that at least TLS v1.2 is used (this helps
mitigate protocol downgrade attacks).

This is based on the defaults we use in Linkerd. See: https://github.com/linkerd/linkerd2/blob/a0b112471eef7e3975fbc8564df52d83894c3fa3/bin/scurl

Signed-off-by: Oliver Gould <[email protected]>
@olix0r
Copy link
Contributor Author

olix0r commented Mar 4, 2022

When invoking the install script, the installation process can fail
silently so that the k3d binary includes only the ASCII string 'Not
found'.

In my case, I was calling the script when the TAG environment was already set to something invalid. With this change, the script will fail with a more obvious error like:

curl: (22) The requested URL returned error: 404 

Copy link
Member

@iwilltry42 iwilltry42 left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@iwilltry42 iwilltry42 left a comment

Choose a reason for hiding this comment

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

LGTM! Just one suggestion that I will commit 👍

install.sh Outdated Show resolved Hide resolved
@iwilltry42 iwilltry42 merged commit 1ec9db2 into k3d-io:main Mar 15, 2022
@iwilltry42
Copy link
Member

Good catch @olix0r ! Thanks for providing this addition :)

@iwilltry42 iwilltry42 added this to the v5.4.0 milestone Mar 15, 2022
@olix0r olix0r deleted the ver/scurl branch March 17, 2022 13:29
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.

2 participants