fix(security): protect credentials in deployment scripts - #520
Open
carlosmmatos-cs wants to merge 5 commits into
Open
fix(security): protect credentials in deployment scripts#520carlosmmatos-cs wants to merge 5 commits into
carlosmmatos-cs wants to merge 5 commits into
Conversation
… to fail RHEL/CentOS 7 ships curl 7.29.0, which does not know oauth2-bearer. It does not reject the unknown configuration key either: it ignores it, exits 0, and sends the request with no credential at all. The previous commit therefore broke EL7 silently rather than loudly. Each script now detects the capability positively and falls back to a raw Authorization header when it is missing. The credential still travels on curl's configuration input in both modes, so it never reaches the command line. The gate moves from 7.55 to 7.33, the version that added oauth2-bearer, which also stops blocking curl 7.33 through 7.54 for no reason, and it is restored in the uninstall and migrate scripts where it had been dropped. Warning text now names what is actually unverified on very old curl, which is redirect handling, and names the control that bounds it. Two latent defects turned up while doing this. The command -v curl guard sat inside a command substitution, where die only exits the subshell and leaves the variable empty; in falcon-linux-uninstall.sh that guard also called die 39 lines before die was defined, so a host without curl got "die: command not found" instead of the message. Both are fixed. The credential handling suite could not fail. It called rg, which is not installed on ubuntu-latest, so "if rg ...; then fail; fi" read exit 127 as no match and three checks quietly did nothing. Replaced with grep behind a helper that inspects the exit status, so a missing or broken tool fails instead of passing. The mode loop now exercises both credential mechanisms rather than looping over a variable nothing read. The container matrix passed its setup command through a variable that the container shell expanded after parsing, turning && and > into literal arguments, which is why all five legs errored. A dependency script that detects the package manager replaces it, and Oracle Linux 7 and CentOS 7 legs are added, since the oldest curl exercised before this was 7.81. Also: aws_ssm_parameter no longer prints the decrypted response body on failure, since that body can carry the client secret; the live curl test runs on python2 and cannot hang; and the PowerShell test reports every collected failure instead of throwing on the first one.
The release job swept the whole tree for *.sh and *.ps1, so every test and CI helper script added on this branch would have been uploaded as a release asset and listed in checksum.txt. That is five extra files, including one named install-test-deps.sh, which is exactly the sort of thing someone would mistake for something they should run. Restricting the search to bash, powershell and systemd, and skipping bash/**/test/, brings the asset list back to the same 8 scripts and 3 zips that a release produced before. Verified by running the whole step in a container and diffing the result against the file list on main.
…y curl Replaces the curl version gate with a single mechanism that works on every supported curl: the bearer credential travels on curl's configuration input as an Authorization header. Nothing is gated on a curl version now, and ALLOW_LEGACY_CURL and --allow-legacy-curl are accepted, ignored, and documented as deprecated. verify_sha256 falls back to openssl where sha256sum is unavailable, and stops with a clear message when neither is present. The aws_ssm_parameter failure path reports the parameter name and the AWS error message rather than the response body. The 'curl is missing' guard runs at top level, after die is defined, so a host without curl gets the intended message.
Removes the CI helper scripts, the credential-handling workflow and the container-pull unit test from this change, and reverts the workflow edits that existed only to run them. The release asset list is unchanged either way: with those scripts gone, the original find already yields the same eight shipped scripts, so that edit is reverted too. Also points bash_migrate.yml at falcon-linux-migrate.sh. Its current shellcheck target, cs-migrate.sh, is not in the tree.
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
1 participant
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.
Credentials handed to cURL now travel on its configuration input rather than as command-line arguments, across the four bash scripts and the three PowerShell scripts, so they stay out of the process list and out of process-creation logs. Alongside that, requests and redirects are restricted to HTTPS, shell and PowerShell tracing is turned off before any credential is handled, secrets are dropped from the exported environment so child processes do not inherit them, and the downloaded installer is checked against a SHA-256 digest.
RHEL and CentOS 7 keep working, and they no longer need a special case. Every script uses one mechanism on every cURL: the credential goes on the configuration input as an
Authorizationheader, which behaves the same on the cURL that EL7 ships and on current cURL. The version gate is gone entirely, so nothing is blocked on a cURL version any more.ALLOW_LEGACY_CURLand--allow-legacy-curlare still accepted, print a notice, and do nothing; the READMEs mark them deprecated. That is not a breaking change, since anyone on EL7 was already setting one of them to get past the previous gate.A few smaller fixes came along with it.
verify_sha256falls back toopensslwheresha256sumis unavailable, and stops with a clear message when neither is present.aws_ssm_parameterreports the parameter name and the AWS error message on failure, rather than the whole response body. Thecommand -v curlguard runs at top level, afterdieis defined, so a host without cURL gets the intended message. Andbash_migrate.ymlwas running shellcheck againstcs-migrate.sh, which is not in the tree; it now points atfalcon-linux-migrate.sh.Tests are deliberately not part of this change. A proper suite will follow in its own PR.