fix(security): protect credentials in deployment scripts - #520
Open
carlosmmatos-cs wants to merge 3 commits into
Open
fix(security): protect credentials in deployment scripts#520carlosmmatos-cs wants to merge 3 commits into
carlosmmatos-cs wants to merge 3 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.
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
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. That keeps them out of the process list and out of process-creation logs. Alongside that, every request and redirect is restricted to HTTPS, shell and PowerShell tracing is disabled 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. cURL 7.29.0 does not know the
oauth2-beareroption, and rather than rejecting it, it ignores the key and sends the request with no credential at all — a silent failure. Each script now detects the capability and falls back to a rawAuthorizationheader, still delivered on the configuration input. The version gate moves from 7.55 to 7.33, the release that addedoauth2-bearer, so cURL 7.33 through 7.54 is no longer blocked for no reason, and the gate now exists in all four scripts instead of two.ALLOW_LEGACY_CURLand--allow-legacy-curlwork as before, and their warning text now names the one thing that is genuinely unverified on very old cURL, which is redirect handling, plus the HTTPS restriction that bounds it. This is not a breaking change: anyone on EL7 already sets one of those flags to get past the old 7.55 gate.Two smaller fixes fell out of this.
aws_ssm_parameterprinted its whole response body on failure, and since the request asks for decryption that body can carry the client secret, so it now reports the parameter name and the AWS error message instead. Separately, thecommand -v curlcheck sat inside a command substitution, wheredieonly exits the subshell and leaves the result empty; infalcon-linux-uninstall.shit also ran 39 lines beforediewas defined, so a host without cURL gotdie: command not foundrather than the intended message.On the CI side, the container compatibility matrix passed its setup command through a variable that the container shell expanded after parsing, so
&&and>arrived as literal arguments and all five legs errored out. A small script that detects the package manager replaces it, and Oracle Linux 7 and CentOS 7 legs are added — the oldest cURL exercised before this was 7.81, so nothing anywhere near the version gate was covered. The regression suite itself could not fail: it calledrg, which is not installed onubuntu-latest, soif rg ...; then fail; firead exit 127 as "no match" and three of its checks quietly did nothing. It now usesgrepbehind a helper that inspects the exit status explicitly, so a missing or broken tool fails instead of passing, and its mode loop exercises both credential mechanisms rather than a variable nothing read.Last, the release job swept the whole tree for
*.shand*.ps1, so the new test and CI helper scripts would have shipped as release assets and appeared inchecksum.txt. It is now restricted to the directories holding shipped artifacts, which brings the asset list back to the same 8 scripts and 3 zips as before.