SDK-485 fix: install.sh should use TLS 1.3 if available#213
Merged
Conversation
It was using 1.2 before which is not as secure. I reversed the order of evaluation of help flags; instead of checking if TLS is _not_ available, I check which version is available first, and default to the insecure behaviour. This is less surprising when reading the code. Fixes SDK-485.
hansl
force-pushed
the
hansl/SDK-485-tls-1-3
branch
from
5a3d8a0 to
acdb0b0
Compare
eftychis
reviewed
Dec 2, 2019
public/install/200_downloader.sh
Outdated
| if check_help_for curl --proto --tlsv1.3; then | ||
| curl --proto '=https' --tls-max=1.3 --silent --show-error --fail --location "$1" --output "$2" | ||
| elif check_help_for curl --proto --tlsv1.2; then | ||
| warn "Not forcing TLS v1.3, using TLS v1.2, this is potentially less secure" |
Contributor
There was a problem hiding this comment.
I think the standard would be to ask the users if they want to downgrade (the usual answer should be no).
eftychis
reviewed
Dec 2, 2019
public/install/200_downloader.sh
Outdated
| warn "Not forcing TLS v1.3, using TLS v1.2, this is potentially less secure" | ||
| curl --proto '=https' --tls-max=1.3 --silent --show-error --fail --location "$1" --output "$2" | ||
| else | ||
| warn "Not forcing TLS v1.3 or v1.2, this is potentially less secure" |
Contributor
There was a problem hiding this comment.
this is great for testing but not sure if we should allow it without an explicit flag.
Contributor
Author
There was a problem hiding this comment.
Good idea. We don't have any easy way to pass in flags though.
Contributor
Author
There was a problem hiding this comment.
Added functions to define boolean flags.
eftychis
reviewed
Dec 2, 2019
| # Arguments: | ||
| # $1 - The long name of the boolean. This will be used on the command line. The name of the | ||
| # environment variable will be `flag_NAME` where NAME is this argument, capitalized. | ||
| # The value of this argument is empty string if not specified, and "1" if it is. |
Contributor
There was a problem hiding this comment.
So if I am reading this right we have to call flag_INSECURE ./install.sh?
Contributor
Author
There was a problem hiding this comment.
Not quite. That will not work as the value of flag_INSECURE is reset to "", but ./install.sh --insecure will set up the internal var flag_INSECURE. So in the shell script you need to use $flag_INSECURE.
eftychis
approved these changes
Dec 2, 2019
dfinity-bot
added a commit
that referenced
this pull request
Jul 8, 2020
## Changelog for common: Branch: master Commits: [dfinity-lab/common@05c44edd...0030194f](https://github.com/dfinity-lab/common/compare/05c44eddb864464c06c27295460eabcf923bc2e9...0030194fc2454c8a4e42bff87aa82f7f8ed96433) * [`2a5d4183`](https://github.com/dfinity-lab/common/commit/2a5d4183065f7811edce500e9d20fd5a4fcb10df) niv nixpkgs: update e79d1e83 -> 5a462920 * [`413ce2b3`](https://github.com/dfinity-lab/common/commit/413ce2b3b176863ff33c4fdf276842d7f94bbb5b) rust: 1.41.1 -> 1.43 * [`899366be`](https://github.com/dfinity-lab/common/commit/899366be97d4c79fdcf4230164a7ffca9ed4e309) niv nixpkgs: update 5a462920 -> 05a32d8e * [`f2c71849`](https://github.com/dfinity-lab/common/commit/f2c7184973a5bd3d4dacfb0c1245d07ef8e39139) Add support for incremental Rust workspaces ([dfinity-lab/common#199](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/common/issues/199)) * [`f484c11e`](https://github.com/dfinity-lab/common/commit/f484c11e040c2a91204cfddc8ddaca1c5e51979d) Revert "Add support for incremental Rust workspaces ([dfinity-lab/common#199](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/common/issues/199))" ([dfinity-lab/common#204](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/common/issues/204)) * [`74c64048`](https://github.com/dfinity-lab/common/commit/74c640486d2e34d3cc63df1a20e4346e07a21bbf) gitSource.nix: Use gitMinimal * [`fa568e26`](https://github.com/dfinity-lab/common/commit/fa568e267a189ee51721af6d7d08eb2af8d54de4) disable parallel rustc build ([dfinity-lab/common#206](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/common/issues/206)) * [`90627152`](https://github.com/dfinity-lab/common/commit/9062715209261439648898b03f7652cd3f138399) [INF-1316] Support for overriding cargoBuild ([dfinity-lab/common#207](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/common/issues/207)) * [`e2d48d84`](https://github.com/dfinity-lab/common/commit/e2d48d8404ccce8252d9a78d76282fc9f1bd026b) nixpkgs: Backports patches for static builds of Haskell ([dfinity-lab/common#208](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/common/issues/208)) * [`45bbce1c`](https://github.com/dfinity-lab/common/commit/45bbce1cfb34c6b723bbe3f89466a63ae66bfb77) rust: Add release build with symbols ([dfinity-lab/common#209](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/common/issues/209)) * [`c9a4327d`](https://github.com/dfinity-lab/common/commit/c9a4327df3040921fa27a7b48c6dc924b13a423b) ci: introduce the all-systems-go aggregate job * [`e8689e50`](https://github.com/dfinity-lab/common/commit/e8689e50dfff33663f498df2baf994eeee39bb7d) Disable selectors2 tests on Darwin * [`eb8225ea`](https://github.com/dfinity-lab/common/commit/eb8225ea65dfa4bcc688a3d58f7c9633affd9234) nix/overlays/ci.nix: increase schedulingPriority of all-systems-go * [`dac9442c`](https://github.com/dfinity-lab/common/commit/dac9442c1110253f3386208790757ec11ff4605c) [INF-589] Incremental build support ([dfinity-lab/common#213](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/common/issues/213)) * [`ebaf4cf7`](https://github.com/dfinity-lab/common/commit/ebaf4cf726e75f2037570f162f79b103a3931bbd) make rustWorkspace overridable * [`a1fb28aa`](https://github.com/dfinity-lab/common/commit/a1fb28aa2d8ddb6ac730264dc4273c4643b818f7) allow crates-io references for existing naersk builds * [`724330e8`](https://github.com/dfinity-lab/common/commit/724330e8e6d8db0fdc6d8b835ed7b107333fed44) Update niv-updater-action to v6
dfinity-bot
added a commit
that referenced
this pull request
Jul 15, 2020
## Changelog for common: Branch: master Commits: [dfinity-lab/common@05c44edd...9162ebde](https://github.com/dfinity-lab/common/compare/05c44eddb864464c06c27295460eabcf923bc2e9...9162ebdee5e914c0a7d22c71596d1760230156df) * [`2a5d4183`](https://github.com/dfinity-lab/common/commit/2a5d4183065f7811edce500e9d20fd5a4fcb10df) niv nixpkgs: update e79d1e83 -> 5a462920 * [`413ce2b3`](https://github.com/dfinity-lab/common/commit/413ce2b3b176863ff33c4fdf276842d7f94bbb5b) rust: 1.41.1 -> 1.43 * [`899366be`](https://github.com/dfinity-lab/common/commit/899366be97d4c79fdcf4230164a7ffca9ed4e309) niv nixpkgs: update 5a462920 -> 05a32d8e * [`f2c71849`](https://github.com/dfinity-lab/common/commit/f2c7184973a5bd3d4dacfb0c1245d07ef8e39139) Add support for incremental Rust workspaces ([dfinity-lab/common#199](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/common/issues/199)) * [`f484c11e`](https://github.com/dfinity-lab/common/commit/f484c11e040c2a91204cfddc8ddaca1c5e51979d) Revert "Add support for incremental Rust workspaces ([dfinity-lab/common#199](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/common/issues/199))" ([dfinity-lab/common#204](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/common/issues/204)) * [`74c64048`](https://github.com/dfinity-lab/common/commit/74c640486d2e34d3cc63df1a20e4346e07a21bbf) gitSource.nix: Use gitMinimal * [`fa568e26`](https://github.com/dfinity-lab/common/commit/fa568e267a189ee51721af6d7d08eb2af8d54de4) disable parallel rustc build ([dfinity-lab/common#206](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/common/issues/206)) * [`90627152`](https://github.com/dfinity-lab/common/commit/9062715209261439648898b03f7652cd3f138399) [INF-1316] Support for overriding cargoBuild ([dfinity-lab/common#207](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/common/issues/207)) * [`e2d48d84`](https://github.com/dfinity-lab/common/commit/e2d48d8404ccce8252d9a78d76282fc9f1bd026b) nixpkgs: Backports patches for static builds of Haskell ([dfinity-lab/common#208](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/common/issues/208)) * [`45bbce1c`](https://github.com/dfinity-lab/common/commit/45bbce1cfb34c6b723bbe3f89466a63ae66bfb77) rust: Add release build with symbols ([dfinity-lab/common#209](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/common/issues/209)) * [`c9a4327d`](https://github.com/dfinity-lab/common/commit/c9a4327df3040921fa27a7b48c6dc924b13a423b) ci: introduce the all-systems-go aggregate job * [`e8689e50`](https://github.com/dfinity-lab/common/commit/e8689e50dfff33663f498df2baf994eeee39bb7d) Disable selectors2 tests on Darwin * [`eb8225ea`](https://github.com/dfinity-lab/common/commit/eb8225ea65dfa4bcc688a3d58f7c9633affd9234) nix/overlays/ci.nix: increase schedulingPriority of all-systems-go * [`dac9442c`](https://github.com/dfinity-lab/common/commit/dac9442c1110253f3386208790757ec11ff4605c) [INF-589] Incremental build support ([dfinity-lab/common#213](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/common/issues/213)) * [`ebaf4cf7`](https://github.com/dfinity-lab/common/commit/ebaf4cf726e75f2037570f162f79b103a3931bbd) make rustWorkspace overridable * [`a1fb28aa`](https://github.com/dfinity-lab/common/commit/a1fb28aa2d8ddb6ac730264dc4273c4643b818f7) allow crates-io references for existing naersk builds * [`724330e8`](https://github.com/dfinity-lab/common/commit/724330e8e6d8db0fdc6d8b835ed7b107333fed44) Update niv-updater-action to v6 * [`b48fa5d6`](https://github.com/dfinity-lab/common/commit/b48fa5d64ae8aecd4b900f4e04054c942f839e38) Fix typo in README * [`a63e292f`](https://github.com/dfinity-lab/common/commit/a63e292f45cf2d5238b04fb2a2bc173ea44ba21b) Add a replaceStdenv function ([dfinity-lab/common#202](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/common/issues/202)) * [`7d3742a2`](https://github.com/dfinity-lab/common/commit/7d3742a2efb70206dac5dcfba78f7102cc2e0b2a) niv cargo2nix: update b9ef68fe -> 31d34998 * [`8431c1e6`](https://github.com/dfinity-lab/common/commit/8431c1e66239f7bc14a0f113c95692648bb211d2) patch ruamel * [`9162ebde`](https://github.com/dfinity-lab/common/commit/9162ebdee5e914c0a7d22c71596d1760230156df) Migrate back to OpenSSL ([dfinity-lab/common#219](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/common/issues/219))
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
It was using 1.2 before which is not as secure.
I reversed the order of evaluation of help flags; instead of checking
if TLS is not available, I check which version is available first,
and default to the insecure behaviour. This is less surprising when
reading the code.
Fixes SDK-485.