diff --git a/aks-node-controller/app.go b/aks-node-controller/app.go index c44c513c660..df7d1bd239a 100644 --- a/aks-node-controller/app.go +++ b/aks-node-controller/app.go @@ -33,7 +33,8 @@ func isExpectedDiffCSEVar(key string) bool { "MCR_REPOSITORY_BASE", "BLOCK_OUTBOUND_NETWORK", "REPO_DEPOT_ENDPOINT", - "SKIP_WAAGENT_HOLD": + "SKIP_WAAGENT_HOLD", + "PROXY_VARS": return true } return false diff --git a/aks-node-controller/app_test.go b/aks-node-controller/app_test.go index 1b11938f58b..56379883629 100644 --- a/aks-node-controller/app_test.go +++ b/aks-node-controller/app_test.go @@ -780,3 +780,16 @@ func TestCompareEnvs_MultipleDifferences(t *testing.T) { } assert.True(t, found, "expected CompareEnvs guest agent event") } + +func TestDiffEnvMaps_IgnoresProxyVarsCompatibilityDifference(t *testing.T) { + pcEnv := map[string]string{ + "PROXY_VARS": `if [ -n "${HTTP_PROXY_URLS}" ]; then export HTTP_PROXY="${HTTP_PROXY_URLS}"; fi`, + "VM_TYPE": "vmss", + } + nbcEnv := map[string]string{ + "PROXY_VARS": `export http_proxy="http://proxy.example:8080";`, + "VM_TYPE": "standard", + } + + assert.Equal(t, []string{"differs: VM_TYPE"}, diffEnvMaps(pcEnv, nbcEnv)) +} diff --git a/parts/linux/cloud-init/artifacts/cse_cmd.sh b/parts/linux/cloud-init/artifacts/cse_cmd.sh index 9da86ec448e..65386dc7b70 100644 --- a/parts/linux/cloud-init/artifacts/cse_cmd.sh +++ b/parts/linux/cloud-init/artifacts/cse_cmd.sh @@ -131,10 +131,10 @@ CSE_CONFIG_FILEPATH="{{GetCSEConfigScriptFilepath}}" AZURE_PRIVATE_REGISTRY_SERVER="{{GetPrivateAzureRegistryServer}}" HAS_CUSTOM_SEARCH_DOMAIN="{{HasCustomSearchDomain}}" CUSTOM_SEARCH_DOMAIN_FILEPATH="{{GetCustomSearchDomainsCSEScriptFilepath}}" -HTTP_PROXY_URLS="{{GetHTTPProxy}}" -HTTPS_PROXY_URLS="{{GetHTTPSProxy}}" -NO_PROXY_URLS="{{GetNoProxy}}" -PROXY_VARS="{{GetProxyVariables}}" +HTTP_PROXY_URLS={{GetVariable "httpProxyShellQuoted"}} +HTTPS_PROXY_URLS={{GetVariable "httpsProxyShellQuoted"}} +NO_PROXY_URLS={{GetVariable "noProxyShellQuoted"}} +PROXY_VARS='{{GetProxyVariables}}' ENABLE_SECURE_TLS_BOOTSTRAPPING="{{EnableSecureTLSBootstrapping}}" SECURE_TLS_BOOTSTRAPPING_AAD_RESOURCE="{{GetSecureTLSBootstrappingAADResource}}" SECURE_TLS_BOOTSTRAPPING_USER_ASSIGNED_IDENTITY_ID="{{GetSecureTLSBootstrappingUserAssignedIdentityID}}" diff --git a/parts/linux/cloud-init/artifacts/cse_main.sh b/parts/linux/cloud-init/artifacts/cse_main.sh index df0e2a148af..a4666f51b73 100755 --- a/parts/linux/cloud-init/artifacts/cse_main.sh +++ b/parts/linux/cloud-init/artifacts/cse_main.sh @@ -46,6 +46,20 @@ source "${CSE_INSTALL_FILEPATH}" source "${CSE_DISTRO_INSTALL_FILEPATH}" source "${CSE_CONFIG_FILEPATH}" +# configureEtcEnvironment persists these values, but the current CSE process needs them immediately. +if [ -n "${HTTP_PROXY_URLS}" ]; then + export HTTP_PROXY="${HTTP_PROXY_URLS}" + export http_proxy="${HTTP_PROXY_URLS}" +fi +if [ -n "${HTTPS_PROXY_URLS}" ]; then + export HTTPS_PROXY="${HTTPS_PROXY_URLS}" + export https_proxy="${HTTPS_PROXY_URLS}" +fi +if [ -n "${NO_PROXY_URLS}" ]; then + export NO_PROXY="${NO_PROXY_URLS}" + export no_proxy="${NO_PROXY_URLS}" +fi + # Disable a single kernel module with a known LPE vulnerability. # Writes a modprobe blacklist rule and unloads the module if loaded. # Safe to run repeatedly during VHD build or provisioning; idempotent (overwrites with same content if already present). @@ -180,13 +194,6 @@ function basePrep { systemctl restart systemd-timesyncd fi - # Eval proxy vars to ensure curl commands use proxy if configured. - # e.g. PROXY_VARS=`export HTTPS_PROXY="https://proxy.example.com:8080"; export http_proxy="http://proxy.example.com:8080"; export NO_PROXY="127.0.0.1,localhost";` - # Setting vars in etc environment (configureEtcEnvironment) won't take effect in current shell session. - if [ -n "${PROXY_VARS}" ]; then - eval $PROXY_VARS - fi - resolve_packages_source_url logs_to_events "AKS.CSE.setPackagesBaseURL" "echo $PACKAGE_DOWNLOAD_BASE_URL" @@ -446,9 +453,6 @@ function nodePrep { fi if [ -n "${OUTBOUND_COMMAND}" ]; then - if [ -n "${PROXY_VARS}" ]; then - eval $PROXY_VARS - fi retrycmd_if_failure 20 1 15 $OUTBOUND_COMMAND >> /var/log/azure/cluster-provision-cse-output.log 2>&1 || exit $ERR_OUTBOUND_CONN_FAIL; fi if [ -n "${BOOTSTRAP_PROFILE_CONTAINER_REGISTRY_SERVER}" ]; then diff --git a/pkg/agent/baker_test.go b/pkg/agent/baker_test.go index d54ef64979b..7f5fca2538c 100644 --- a/pkg/agent/baker_test.go +++ b/pkg/agent/baker_test.go @@ -9,6 +9,9 @@ import ( "errors" "fmt" "io" + "os" + "os/exec" + "path/filepath" "regexp" "strings" @@ -1172,6 +1175,59 @@ var _ = Describe("getLinuxNodeCSECommand", func() { Expect(cseCmd).To(ContainSubstring("bash")) }) + It("should safely preserve proxy values for older VHD scripts in scriptless mode", func() { + tempDir, err := os.MkdirTemp("", "agentbaker-proxy-test") + Expect(err).NotTo(HaveOccurred()) + defer os.RemoveAll(tempDir) + + httpMarker := filepath.Join(tempDir, "http-injected") + httpsMarker := filepath.Join(tempDir, "https-injected") + noProxyMarker := filepath.Join(tempDir, "no-proxy-injected") + httpProxy := `http://user:p'ass"word/$(touch ` + httpMarker + ");`touch " + httpMarker + "`/*?[x]\\value" + httpsProxy := `https://proxy.example/$(touch ` + httpsMarker + ")" + noProxyValues := []string{"localhost", `$(touch ` + noProxyMarker + ")", ".svc"} + baseConfig.HTTPProxyConfig = &datamodel.HTTPProxyConfig{ + HTTPProxy: &httpProxy, + HTTPSProxy: &httpsProxy, + NoProxy: &noProxyValues, + } + + var encodedNBCCmd string + for _, file := range templateGenerator.getScriptlessConfiguration(baseConfig) { + if file.path == aksNbcCmdFilepath { + encodedNBCCmd = file.content + break + } + } + Expect(encodedNBCCmd).NotTo(BeEmpty()) + compressedNBCCmd, err := base64.StdEncoding.DecodeString(encodedNBCCmd) + Expect(err).NotTo(HaveOccurred()) + cseCmdBytes, err := getGzipDecodedValue(compressedNBCCmd) + Expect(err).NotTo(HaveOccurred()) + cseCmd := string(cseCmdBytes) + start := strings.Index(cseCmd, "HTTP_PROXY_URLS=") + Expect(start).To(BeNumerically(">=", 0)) + end := strings.Index(cseCmd[start:], " ENABLE_SECURE_TLS_BOOTSTRAPPING=") + Expect(end).To(BeNumerically(">", 0)) + proxyAssignments := cseCmd[start : start+end] + command := proxyAssignments + ` /bin/bash -c 'eval $PROXY_VARS; printf "%s\n" "$HTTP_PROXY" "$http_proxy" "$HTTPS_PROXY" "$https_proxy" "$NO_PROXY" "$no_proxy"'` + + output, err := exec.Command("/bin/bash", "-c", command).CombinedOutput() + Expect(err).NotTo(HaveOccurred(), string(output)) + Expect(strings.Split(strings.TrimSuffix(string(output), "\n"), "\n")).To(Equal([]string{ + httpProxy, + httpProxy, + httpsProxy, + httpsProxy, + strings.Join(noProxyValues, ","), + strings.Join(noProxyValues, ","), + })) + Expect(httpMarker).NotTo(BeAnExistingFile()) + Expect(httpsMarker).NotTo(BeAnExistingFile()) + Expect(noProxyMarker).NotTo(BeAnExistingFile()) + Expect(getProxyVariables(baseConfig)).NotTo(ContainSubstring(tempDir)) + }) + It("should embed cloud-init status checks when custom data is enabled", func() { Expect(baseConfig.DisableCustomData).To(BeFalse()) diff --git a/pkg/agent/variables.go b/pkg/agent/variables.go index ca307c0649c..8215933b8a0 100644 --- a/pkg/agent/variables.go +++ b/pkg/agent/variables.go @@ -4,7 +4,6 @@ package agent import ( - "fmt" "strconv" "strings" @@ -87,6 +86,18 @@ func getWindowsCustomDataVariables(config *datamodel.NodeBootstrappingConfigurat func getCSECommandVariables(config *datamodel.NodeBootstrappingConfiguration) paramsMap { cs := config.ContainerService profile := config.AgentPoolProfile + httpProxy, httpsProxy, noProxy := "", "", "" + if config.HTTPProxyConfig != nil { + if config.HTTPProxyConfig.HTTPProxy != nil { + httpProxy = *config.HTTPProxyConfig.HTTPProxy + } + if config.HTTPProxyConfig.HTTPSProxy != nil { + httpsProxy = *config.HTTPProxyConfig.HTTPSProxy + } + if config.HTTPProxyConfig.NoProxy != nil { + noProxy = strings.Join(*config.HTTPProxyConfig.NoProxy, ",") + } + } // this method is called for both windows and linux. If there's no windows profile, then let's just // use a blank one. @@ -145,6 +156,9 @@ func getCSECommandVariables(config *datamodel.NodeBootstrappingConfiguration) pa "serviceAccountImagePullDefaultClientID": getServiceAccountImagePullDefaultClientID(cs), "serviceAccountImagePullDefaultTenantID": getServiceAccountImagePullDefaultTenantID(cs), "identityBindingsLocalAuthoritySNI": getServiceAccountImagePullLocalAuthoritySNI(cs), + "httpProxyShellQuoted": shellQuote(httpProxy), + "httpsProxyShellQuoted": shellQuote(httpsProxy), + "noProxyShellQuoted": shellQuote(noProxy), } } @@ -230,20 +244,19 @@ func getOutBoundCmd(nbc *datamodel.NodeBootstrappingConfiguration, cloudSpecConf return connectivityCheckCommand } +func shellQuote(value string) string { + return "'" + strings.ReplaceAll(value, "'", `'"'"'`) + "'" +} + func getProxyVariables(nbc *datamodel.NodeBootstrappingConfiguration) string { - // only use https proxy, if user doesn't specify httpsProxy we autofill it with value from httpProxy. - proxyVars := "" - if nbc.HTTPProxyConfig != nil { - if nbc.HTTPProxyConfig.HTTPProxy != nil { - // from https://curl.se/docs/manual.html, curl uses http_proxy but uppercase for others? - proxyVars = fmt.Sprintf("export http_proxy=\"%s\";", *nbc.HTTPProxyConfig.HTTPProxy) - } - if nbc.HTTPProxyConfig.HTTPSProxy != nil { - proxyVars = fmt.Sprintf("export HTTPS_PROXY=\"%s\"; %s", *nbc.HTTPProxyConfig.HTTPSProxy, proxyVars) - } - if nbc.HTTPProxyConfig.NoProxy != nil { - proxyVars = fmt.Sprintf("export NO_PROXY=\"%s\"; %s", strings.Join(*nbc.HTTPProxyConfig.NoProxy, ","), proxyVars) - } + if nbc.HTTPProxyConfig == nil || + (nbc.HTTPProxyConfig.HTTPProxy == nil && nbc.HTTPProxyConfig.HTTPSProxy == nil && nbc.HTTPProxyConfig.NoProxy == nil) { + return "" } - return proxyVars + + // Older VHDs evaluate PROXY_VARS. Keep this payload free of customer-controlled values; + // those values are shell-quoted separately and referenced only through variables here. + return `if [ -n "${HTTP_PROXY_URLS}" ]; then export HTTP_PROXY="${HTTP_PROXY_URLS}" http_proxy="${HTTP_PROXY_URLS}"; fi; ` + + `if [ -n "${HTTPS_PROXY_URLS}" ]; then export HTTPS_PROXY="${HTTPS_PROXY_URLS}" https_proxy="${HTTPS_PROXY_URLS}"; fi; ` + + `if [ -n "${NO_PROXY_URLS}" ]; then export NO_PROXY="${NO_PROXY_URLS}" no_proxy="${NO_PROXY_URLS}"; fi` } diff --git a/spec/parts/linux/cloud-init/artifacts/cse_main_spec.sh b/spec/parts/linux/cloud-init/artifacts/cse_main_spec.sh index 880111e392d..d21a4eaaa05 100644 --- a/spec/parts/linux/cloud-init/artifacts/cse_main_spec.sh +++ b/spec/parts/linux/cloud-init/artifacts/cse_main_spec.sh @@ -127,7 +127,118 @@ Describe 'select_localdns_corefile()' End End +Describe 'proxy environment exports' + setup() { + unset HTTP_PROXY http_proxy HTTPS_PROXY https_proxy NO_PROXY no_proxy + HTTP_PROXY_URLS="" + HTTPS_PROXY_URLS="" + NO_PROXY_URLS="" + proxy_exports="$(sed -n '/^# configureEtcEnvironment persists/,/^# Disable a single kernel module/p' parts/linux/cloud-init/artifacts/cse_main.sh)" + } + + cleanup() { + unset HTTP_PROXY http_proxy HTTPS_PROXY https_proxy NO_PROXY no_proxy + unset HTTP_PROXY_URLS HTTPS_PROXY_URLS NO_PROXY_URLS + } + + exported_proxy_environment() { + eval "${proxy_exports}" + /bin/bash -c 'printf "%s\n" "${HTTP_PROXY-}" "${http_proxy-}" "${HTTPS_PROXY-}" "${https_proxy-}" "${NO_PROXY-}" "${no_proxy-}"' + } + + proxy_environment_matches() { + actual="$(exported_proxy_environment)" + expected="$(printf '%s\n' "$@")" + [ "${actual}" = "${expected}" ] + } + + BeforeEach 'setup' + AfterEach 'cleanup' + + It 'exports HTTP proxy values only' + HTTP_PROXY_URLS="http://proxy.example.com:8080" + + When call proxy_environment_matches \ + "http://proxy.example.com:8080" "http://proxy.example.com:8080" \ + "" "" "" "" + The status should be success + End + + It 'exports HTTPS proxy values only' + HTTPS_PROXY_URLS="https://proxy.example.com:8443" + + When call proxy_environment_matches \ + "" "" \ + "https://proxy.example.com:8443" "https://proxy.example.com:8443" \ + "" "" + The status should be success + End + + It 'exports no-proxy values only' + NO_PROXY_URLS="127.0.0.1,localhost,.svc" + + When call proxy_environment_matches \ + "" "" "" "" \ + "127.0.0.1,localhost,.svc" "127.0.0.1,localhost,.svc" + The status should be success + End + + It 'exports all proxy values simultaneously' + HTTP_PROXY_URLS="http://proxy.example.com:8080" + HTTPS_PROXY_URLS="https://proxy.example.com:8443" + NO_PROXY_URLS="127.0.0.1,localhost,.svc" + + When call proxy_environment_matches \ + "http://proxy.example.com:8080" "http://proxy.example.com:8080" \ + "https://proxy.example.com:8443" "https://proxy.example.com:8443" \ + "127.0.0.1,localhost,.svc" "127.0.0.1,localhost,.svc" + The status should be success + End + + It 'leaves existing values unchanged when proxy URLs are empty' + export HTTP_PROXY="existing-http-upper" + export http_proxy="existing-http-lower" + export HTTPS_PROXY="existing-https-upper" + export https_proxy="existing-https-lower" + export NO_PROXY="existing-no-proxy-upper" + export no_proxy="existing-no-proxy-lower" + + When call proxy_environment_matches \ + "existing-http-upper" "existing-http-lower" \ + "existing-https-upper" "existing-https-lower" \ + "existing-no-proxy-upper" "existing-no-proxy-lower" + The status should be success + End +End + Describe 'connectivity preflight timeouts' + It 'exports proxy values before package resolution and the outbound check' + proxy_consumer_order() { + awk ' + /export HTTP_PROXY=/ && !proxy_exports { proxy_exports = NR } + /^[[:space:]]*resolve_packages_source_url$/ { package_resolution = NR } + /retrycmd_if_failure 20 1 15 \$OUTBOUND_COMMAND/ { outbound_check = NR } + END { + if (proxy_exports > 0 && package_resolution > 0 && outbound_check > 0 && + proxy_exports < package_resolution && proxy_exports < outbound_check) { + print "true" + } else { + print "false" + } + } + ' parts/linux/cloud-init/artifacts/cse_main.sh + } + + When call proxy_consumer_order + The output should equal "true" + End + + It 'does not evaluate PROXY_VARS' + When run grep -F 'eval $PROXY_VARS' parts/linux/cloud-init/artifacts/cse_main.sh + The status should be failure + The output should equal "" + End + It 'allows DNS failover during the outbound check' When run awk '/retrycmd_if_failure [0-9]+ [0-9]+ [0-9]+ \$OUTBOUND_COMMAND/ { print $2, $3, $4 }' parts/linux/cloud-init/artifacts/cse_main.sh The output should eq "20 1 15"