diff --git a/dag/extdeps/bmc/http.dag b/dag/extdeps/bmc/http.dag index 0de891226ad..60385c67b6a 100644 --- a/dag/extdeps/bmc/http.dag +++ b/dag/extdeps/bmc/http.dag @@ -5,7 +5,7 @@ import extdeps.external_authority { ExternalAuthority } import extdeps.uri { Uri, Https } import std.decl_ref { DeclarationRef, WholeDeclaration } import std.disposition { Disposition, RealizationDispatch, Scaffold } -import std.types { Bool, NonEmptyStr, Secret, String } +import std.types { Bool, NonEmptyStr, String } data extdeps_external_authority_anchor: ExternalAuthority = ExternalAuthority { uri: Uri { scheme: Https @@ -26,6 +26,34 @@ data redfish_http_hardwired_transport_note: String = "§3(b) declared debt: ever data redfish_http_success_semantics_note: String = "LOAD-BEARING: success:Bool derives from exit_success, so curl MUST fail-close on HTTP status. Every argv carries --fail-with-body (curl >=7.76). WITHOUT it curl exits 0 on 401/403/404/5xx and `success` reads a rejected auth or a missing resource as TRUE — a §5 fail-open: a factory-401 misreads as FactoryCredentialActive (so an already-rotated BMC gets re-rotated), and a rotation PATCH that 401s reports rotated while changing nothing (the 'secret populated, server never rotated' state). --fail-with-body (not bare --fail) keeps the OpenBMC error body in `body` for diagnosis (e.g. PropertyValueFormatError on a rejected password). Do not drop the flag; a new redfish op without it re-opens the arm. Found by execution against srv4 2026-07-23." +// EVERY REDFISH OPERATION AUTHENTICATES BY FILE, AND THE ABSENCE OF `Secret` IN THIS MODULE'S +// IMPORTS IS THE RECEIPT. GetSystem, GetPower and GetThermal each declared `password: Secret` and +// then interpolated it into an argv token -- `-u {username}:{password}` -- so the credential became +// a word in curl's command line. On the fleet hosts that is not a theoretical exposure: /proc is +// mounted with no hidepid, /proc/PID/cmdline is mode 0444, and the CI runner slots are ordinary host +// systemd services sharing the host pid namespace, so any job payload reads it with an unprivileged +// ps. bmc_onboard called GetSystem with the FRESHLY ROTATED password, which put the new credential +// in argv at the exact moment the old one was being retired. +// +// THIS WAS A HALF-FINISHED REPLACEMENT MIGRATION, NOT A NEW DISCOVERY, and that is the part worth +// recording. Every MUTATING operation here already used --netrc-file, and a GetSystemWithNetrc sat +// directly beside the leaking GetSystem answering the identical question. So the correct shape was +// already present and already understood; what kept the leak alive was that the old form was left +// standing next to the new one instead of being cut at the root. While X stands, callers keep +// reaching for it -- five call sites did. +// +// The cut is therefore at the root rather than at the leaves: the three argv-borne arms are DELETED, +// not deprecated, and GetSystemWithNetrc is deleted too because once GetSystem no longer takes a +// password the `WithNetrc` suffix distinguishes nothing and is a second name for one concept. One +// operation per resource, authenticating one way. A future operation cannot reintroduce the class by +// copying a neighbour, because no neighbour spells it any more. +// +// WHAT THIS DOES NOT CLAIM: the module is clean by CONSTRUCTION nowhere. Nothing refuses a new +// `password: Secret` interpolated into an argv token -- v2.std.operation_argv's binding wall refuses +// undeclared names and a non-literal executable position, but ShellTransportOperationRow carries +// declared_inputs as a List, names without types, so the binder cannot see that an input is +// Secret and could not refuse it. That missing declared-input TYPE on the row is this class's +// next-rung trigger; until it lands the class sits at mitigatable and this note is diligence. service redfish.Http { operation GetServiceRoot { @@ -42,19 +70,6 @@ service redfish.Http { } operation GetSystem { - input { bmc_host: NonEmptyStr, username: NonEmptyStr, password: Secret } - output { body: String from "stdout", success: Bool from "exit_success" } - readonly - transport shell { - argv: ["curl", "--fail-with-body", "-sS", "-k", "-u", "{username}:{password}", "https://{bmc_host}/redfish/v1/Systems/system"] - } - exit { - 0 => Unit - nonzero => String "redfish system GET failed" - } - } - - operation GetSystemWithNetrc { input { bmc_host: NonEmptyStr, netrc_file: NonEmptyStr } output { body: String from "stdout", success: Bool from "exit_success" } readonly @@ -63,7 +78,7 @@ service redfish.Http { } exit { 0 => Unit - nonzero => String "redfish system GET (netrc) failed" + nonzero => String "redfish system GET failed" } } @@ -81,11 +96,11 @@ service redfish.Http { } operation GetPower { - input { bmc_host: NonEmptyStr, username: NonEmptyStr, password: Secret } + input { bmc_host: NonEmptyStr, netrc_file: NonEmptyStr } output { body: String from "stdout", success: Bool from "exit_success" } readonly transport shell { - argv: ["curl", "--fail-with-body", "-sS", "-k", "-u", "{username}:{password}", "https://{bmc_host}/redfish/v1/Chassis/Self/Power"] + argv: ["curl", "--fail-with-body", "-sS", "-k", "--netrc-file", "{netrc_file}", "https://{bmc_host}/redfish/v1/Chassis/Self/Power"] } exit { 0 => Unit @@ -94,11 +109,11 @@ service redfish.Http { } operation GetThermal { - input { bmc_host: NonEmptyStr, username: NonEmptyStr, password: Secret } + input { bmc_host: NonEmptyStr, netrc_file: NonEmptyStr } output { body: String from "stdout", success: Bool from "exit_success" } readonly transport shell { - argv: ["curl", "--fail-with-body", "-sS", "-k", "-u", "{username}:{password}", "https://{bmc_host}/redfish/v1/Chassis/Self/Thermal"] + argv: ["curl", "--fail-with-body", "-sS", "-k", "--netrc-file", "{netrc_file}", "https://{bmc_host}/redfish/v1/Chassis/Self/Thermal"] } exit { 0 => Unit diff --git a/dag/extdeps/bmc/mock_corpus.dag b/dag/extdeps/bmc/mock_corpus.dag index 491abbdee8d..671afc6c60f 100644 --- a/dag/extdeps/bmc/mock_corpus.dag +++ b/dag/extdeps/bmc/mock_corpus.dag @@ -9,13 +9,6 @@ data redfish_http_mock_case_get_system: PublishedMockCase = PublishedMockCase { case_id: 0 } -data redfish_http_mock_case_get_system_with_netrc: PublishedMockCase = PublishedMockCase { - operation_key: "redfish.Http.GetSystemWithNetrc", - service: "redfish.Http", - operation: "GetSystemWithNetrc", - case_id: 0 -} - data redfish_http_mock_case_get_chassis_sensors: PublishedMockCase = PublishedMockCase { operation_key: "redfish.Http.GetChassisSensors", service: "redfish.Http", @@ -60,7 +53,6 @@ data bmc_ipmi_tool_mock_case_sel_list: PublishedMockCase = PublishedMockCase { data bmc_published_mock_corpus: List = [ redfish_http_mock_case_get_system, - redfish_http_mock_case_get_system_with_netrc, redfish_http_mock_case_get_chassis_sensors, redfish_http_mock_case_get_power, redfish_http_mock_case_get_thermal, diff --git a/dag/extdeps/diagnostic_mock_corpus.dag b/dag/extdeps/diagnostic_mock_corpus.dag index e5b79b7e281..0a300017a1e 100644 --- a/dag/extdeps/diagnostic_mock_corpus.dag +++ b/dag/extdeps/diagnostic_mock_corpus.dag @@ -11,7 +11,6 @@ import extdeps.ssh.mock_corpus { import extdeps.linux.mock_corpus { linux_edac_rasmcctl_mock_case_correctable_counts } import extdeps.bmc.mock_corpus { redfish_http_mock_case_get_system, - redfish_http_mock_case_get_system_with_netrc, redfish_http_mock_case_get_chassis_sensors, redfish_http_mock_case_get_power, redfish_http_mock_case_get_thermal, @@ -27,7 +26,6 @@ data diagnostic_published_mock_corpus: List = [ ssh_session_mock_case_exec_portable_words_with_stdin, ssh_session_mock_case_reachability, redfish_http_mock_case_get_system, - redfish_http_mock_case_get_system_with_netrc, redfish_http_mock_case_get_chassis_sensors, redfish_http_mock_case_get_power, redfish_http_mock_case_get_thermal, diff --git a/dag/gunbc/ci_spec.dag b/dag/gunbc/ci_spec.dag index 466bec0c5f4..d2a724c5335 100644 --- a/dag/gunbc/ci_spec.dag +++ b/dag/gunbc/ci_spec.dag @@ -737,22 +737,69 @@ fn gunbc_ci_multi_principal_probe_invoke() -> String { // fulfilled by this same change (gunbc_ci_fleet_key_agent_prelude below, emitted through // v2.workflow.gunbc_invoke_step_emit gunbc_invoke_step_emit_pipeline), so a corrected row // would be created and discharged in the same diff — ceremony, not a standing obligation. +// THE TOKEN IS WRITTEN BY A SHELL BUILTIN, AND THAT IS THE WHOLE MECHANISM. Both preludes below used +// `curl -H "Authorization: Bearer $WIF_ACCESS_TOKEN"`, and the shell expands that BEFORE exec, so the +// GCP access token became a word in curl's argv -- readable by any unprivileged `ps` for the life of +// the request. On these runners that is not a hypothetical audience: /proc carries no hidepid, +// /proc/PID/cmdline is mode 0444, and the runner slots are ordinary host systemd services in the host +// pid namespace, so a job payload sees every process on the machine. +// +// curl reads a header from a file with `-H @FILE` (>=7.55; the runners carry 7.88), which moves the +// value out of curl's argv. That alone would not be enough: writing the file with any EXTERNAL +// command would simply relocate the token from curl's argv into that command's argv. `printf` is a +// bash BUILTIN, so it forks no process and creates no command line -- the token exists in the shell's +// own memory and in the file, and never in any process table. +// +// The mode is set BEFORE the content exists. Both preludes already run `umask 077`, so the redirect +// creates the file 0600 at creation rather than writing world-readable bytes and narrowing them +// afterwards; there is no window where the token sits at a wider mode. Each prelude's existing trap +// removes HDR_FILE on exit alongside the other credential material it already cleans up. +// +// WHAT IS NOT CLAIMED: this is a repair at two call sites, not a wall. Nothing refuses the next +// prelude that interpolates a credential into a command string -- these bodies are hand-authored +// shell text, not declared operation argv, so v2.std.operation_argv's binding wall cannot see them +// at all. The class stays mitigatable here and the trigger is the same one extdeps.bmc.http names: +// credentials modeled as declared inputs whose type the binder can refuse. +// WHY THIS PAYLOAD IS SHELL TEXT AND WHAT WOULD CHANGE THAT (review 55857 asked for a bash-emit +// scaffold marker here; recorded rather than marked, because the marker would assert the wrong +// thing). This prelude is NOT the unmigrated medium-as-string form that the two scaffolds in this +// module carry a dissolution trigger for -- those mark *_script functions that concat-build a whole +// script, and their trigger text says so. gunbc_ci_fleet_key_agent_script builds a typed Pipeline of +// PipelineStep and emits it through v2.workflow.gunbc_invoke_step_emit, which IS the orchestration +// emit those triggers name as their terminal state; gunbc.fleet_ssh_credential_verify +// fleet_ssh_credential_shell_verification_supersession_note records that migration. Marking one +// command payload as a scaffold while its twenty peers in the same function carry none would claim +// this line is unmigrated debt when it sits in the migrated construction. +// +// The residue the review is right about: a Run's `command` is still shell text, corpus-wide, tracked +// by bash-emit rather than per line. And the reason no modeled actuator was available HERE is a +// boundary rather than a preference -- this prelude executes as raw shell in a FOREIGN EXECUTOR +// (GitHub Actions `run:`) before gunbc runs at all, so Filesystem.WriteOwnerOnly, which this same +// change uses for the netrc on the BMC path, is unreachable at this point in a job. Choosing printf +// is not declining a modeled route; there is no modeled route on that side of the boundary yet, +// which is exactly what bash-emit would change. +fn gunbc_ci_auth_header_file_write_command() -> String { + "printf 'Authorization: Bearer %s\\n' \"$WIF_ACCESS_TOKEN\" > \"$HDR_FILE\"" +} + fn gunbc_ci_fleet_key_agent_prelude() -> List { [ Do { run: ci_retry_body_run(command: "set -euo pipefail") }, Do { run: ci_retry_body_run(command: "umask 077") }, Do { run: ci_retry_body_run(command: "KEY_FILE=\"$RUNNER_TEMP/fleet-automation-key\"") }, + Do { run: ci_retry_body_run(command: "HDR_FILE=\"$RUNNER_TEMP/fleet-automation-auth-header\"") }, Do { run: ci_retry_body_run(command: "AGENT_STARTED=0") }, Do { run: ci_retry_body_run( - command: "cleanup() { rm -f \"$KEY_FILE\"; if [ \"$AGENT_STARTED\" = 1 ] && [ -n \"$\{SSH_AGENT_PID:-}\" ]; then ssh-agent -k >/dev/null 2>&1 || true; fi; }" + command: "cleanup() { rm -f \"$KEY_FILE\" \"$HDR_FILE\"; if [ \"$AGENT_STARTED\" = 1 ] && [ -n \"$\{SSH_AGENT_PID:-}\" ]; then ssh-agent -k >/dev/null 2>&1 || true; fi; }" ) }, Do { run: ci_retry_body_run(command: "trap cleanup EXIT") }, + Do { run: ci_retry_body_run(command: gunbc_ci_auth_header_file_write_command()) }, Do { run: ci_retry_body_run( command: join([ - "curl -sSf -H \"Authorization: Bearer $WIF_ACCESS_TOKEN\" \"", + "curl -sSf -H @\"$HDR_FILE\" \"", fleet_automation_privkey_sm_access_url(), "\" | python3 -c 'import sys,json,base64; sys.stdout.buffer.write(base64.b64decode(json.load(sys.stdin)[\"payload\"][\"data\"]))' > \"$KEY_FILE\"" ], "") @@ -843,11 +890,13 @@ fn gunbc_ci_spark_grant_install_credential_prelude() -> List { Do { run: ci_retry_body_run(command: "umask 077") }, Do { run: ci_retry_body_run(command: "CRED_FILE=\"$RUNNER_TEMP/spark-administrator-credential\"") }, Do { run: ci_retry_body_run(command: "RESP_FILE=\"$RUNNER_TEMP/spark-administrator-response.json\"") }, - Do { run: ci_retry_body_run(command: "trap 'rm -f \"$CRED_FILE\" \"$RESP_FILE\"' EXIT") }, + Do { run: ci_retry_body_run(command: "HDR_FILE=\"$RUNNER_TEMP/spark-administrator-auth-header\"") }, + Do { run: ci_retry_body_run(command: "trap 'rm -f \"$CRED_FILE\" \"$RESP_FILE\" \"$HDR_FILE\"' EXIT") }, + Do { run: ci_retry_body_run(command: gunbc_ci_auth_header_file_write_command()) }, Do { run: ci_retry_body_run( command: join([ - "HTTP_CODE=\"$(curl -sS -o \"$RESP_FILE\" -w '%\{http_code}' -H \"Authorization: Bearer $WIF_ACCESS_TOKEN\" \"", + "HTTP_CODE=\"$(curl -sS -o \"$RESP_FILE\" -w '%\{http_code}' -H @\"$HDR_FILE\" \"", secret_ref_access_url(ref: spark_administrator_password_secret_ref), "\" || echo 000)\"" ], "") diff --git a/dag/gunbc/credential_argv_exposure.dag b/dag/gunbc/credential_argv_exposure.dag new file mode 100644 index 00000000000..3e450d102ab --- /dev/null +++ b/dag/gunbc/credential_argv_exposure.dag @@ -0,0 +1,169 @@ +module gunbc.credential_argv_exposure + +import std.types { List, NonEmptyStr, String } +import std.decl_ref { DeclarationRef, WholeDeclaration } + +// A CREDENTIAL IN ARGV IS READ BY EVERYONE ON THE HOST, AND ON THIS FLEET THAT SENTENCE IS MEASURED +// RATHER THAN ASSUMED. The three facts that decide it were taken on srv1 on 2026-08-25: /proc is +// mounted with no hidepid, /proc/PID/cmdline is mode 0444, and the CI runner slots are ordinary host +// systemd services -- actions-runner@srv1-01 under system.slice, User=root dropping to ghrunner via +// setpriv -- running in the HOST pid namespace rather than a container. So the audience for an argv +// word is not "other processes owned by the same user"; it is any job payload executing on a runner +// slot, in a repository whose workflows a pull request can influence. +// +// That is why this class is recorded as an incident carrier rather than as hygiene. The usual reason +// argv exposure gets deprioritized is an implicit assumption that the reader must already be +// privileged. Here the reader is unprivileged and arrives through the front door. +data credential_argv_exposure_measured_surface_note: String = "srv1, 2026-08-25: /proc mounted rw,nosuid,nodev,noexec,relatime with NO hidepid; /proc/PID/cmdline mode 0444; actions-runner@srv1-01 is a host systemd service in the host pid namespace, not containerized. A host-level ps enumerated 22 session-container tmux processes whose argv carried live forwarded credentials. Measurement performed by session warm-tern-755 with fleet SSH; this module records the reading, it does not re-derive it." + +// THE ROOT CAUSE OF THE REPORTED INCIDENT IS NOT IN THIS REPOSITORY, and saying so precisely is the +// point of this row. The observation that opened the incident -- GITHUB_TOKEN and BUILDBUDDY_API_KEY +// visible in ps -- comes from the session container entrypoint, which forwards a roster of names into +// a tmux invocation as `-e NAME=VALUE` arguments. Those become the argv of the long-lived tmux server +// and stay there for the life of the session. That file is baked into the session-dashboard container +// image; no checkout of it exists in this repository, so it cannot be repaired from here and this +// module does not pretend to have repaired it. +// +// The fix was established by execution rather than by reasoning, and is recorded here so the owning +// repository does not have to re-derive it: exporting the variables in the parent shell and dropping +// the `-e` arguments entirely leaves the value absent from the tmux server argv while the pane still +// receives it, because the server captures its own environment as the global environment at start. +// A planted canary is present in the argv under the `-e` form and absent under the export form, with +// the pane reading the correct value in both -- so the detector is discriminating rather than merely +// quiet. `tmux setenv` is NOT the fix: it relocates the value from a server argv to a client argv. +data credential_argv_exposure_out_of_repo_root_cause_note: String = "The reported instance lives in the session container entrypoint (session-dashboard infrastructure), which projects a forwarded credential roster into `tmux new-session -e NAME=VALUE` arguments. Out of scope for this repository by ownership, not by severity. Verified repair shape: export in the parent shell, drop -e; the tmux server inherits its environment as the global environment and panes inherit it, with the value absent from every argv. Routing to the owning repository is an operator decision recorded on the incident, not a change this module can make." + +type CredentialArgvInstance { + subject: NonEmptyStr + mechanism: NonEmptyStr + disposition: NonEmptyStr +} + +// THE THREE INSTANCES THIS REPOSITORY OWNS, and they are listed together because the incident's +// value is the CLASS rather than the one process someone happened to run ps against. Each was found +// by asking the same question of our own tree that the reporter asked of the host: which credential +// becomes a command-line word before exec. +data redfish_basic_auth_instance: CredentialArgvInstance = CredentialArgvInstance { + subject: "extdeps.bmc.http GetSystem / GetPower / GetThermal" as NonEmptyStr, + mechanism: "declared `password: Secret` interpolated into the argv token `-u \{username}:\{password}`; bmc_onboard called GetSystem with the freshly ROTATED credential, so the new password entered argv at the moment the old one was retired" as NonEmptyStr, + disposition: "REPAIRED. The three argv-borne arms are deleted at the root rather than deprecated, and every caller authenticates through a 0600 netrc. `Secret` no longer appears anywhere in that module." as NonEmptyStr +} + +data secret_manager_bearer_instance: CredentialArgvInstance = CredentialArgvInstance { + subject: "gunbc.ci_spec fleet-key and spark-grant preludes" as NonEmptyStr, + mechanism: "`curl -H \"Authorization: Bearer $WIF_ACCESS_TOKEN\"` -- the shell expands before exec, so the GCP access token is a curl argv word on the runner, readable by exactly the audience measured above" as NonEmptyStr, + disposition: "REPAIRED. The header is written to a 0600 file by the `printf` BUILTIN, which forks no process and so creates no second command line, and curl reads it with `-H @FILE`." as NonEmptyStr +} + +// THE UNREPAIRED ONE IS NAMED HERE RATHER THAN LEFT FOR REDISCOVERY, because a partial repair that +// is silent about its remainder is how a class gets marked closed while an instance survives. This +// one is the worst of the three by blast radius and the most expensive to fix, which is exactly the +// combination that gets quietly dropped from a security change. +// +// It is also the only one whose repair is NOT the same move as the other two. websocat has no +// `@file` header form, and the argv is handed to `systemd-run --unit`, which performs no shell +// expansion -- that is precisely why an earlier change replaced a `$BMCWEB_SESSION_TOKEN` reference +// with the materialized value, and it was right about the mechanism. The consequence is that the +// token reaches the transient unit's recorded ExecStart and the journal, not merely the process +// table. The repair shape is a systemd EnvironmentFile at 0600 with the argv carrying an unexpanded +// reference resolved by an `sh -c` wrapper, which moves the value into /proc/PID/environ (mode 0400, +// owner-only) rather than cmdline (0444, world-readable). +// +// TWO WITNESSES CURRENTLY ASSERT THE TOKEN IS IN THE ARGV and would have to be inverted with it, so +// this is not a one-line change dressed up as a large one: a probe written to prove the value was +// MATERIALIZED rather than left as a dead shell reference has, as a side effect, frozen the argv +// placement as a contract. That is worth recording on its own -- an assertion can pin the defect it +// was never asked about. +data bmcweb_session_token_instance: CredentialArgvInstance = CredentialArgvInstance { + subject: "extdeps.bmc.webui.nbd_proxy_serve nbd_proxy_serve_websocat_argv" as NonEmptyStr, + mechanism: "the live BMCWEB session token is concatenated into a `--header=` argv word and handed to systemd_run RunTransient, whose own argv is `systemd-run --unit=\{unit} --collect -- \{command_argv}`. So the token occupies THREE positions, not one: the systemd-run process's own argv while it runs, the websocat process's argv for the whole lifetime of the served ISO, and systemd's recorded ExecStart for the transient unit -- which is what reaches the journal and `systemctl show`. The middle one is long-lived and is the identical world-readable cmdline surface as the incident that opened this carrier" as NonEmptyStr, + disposition: "NOT REPAIRED in this change, and deliberately so. websocat has no @file header form and systemd-run does not expand shell references, so the repair is a systemd EnvironmentFile at 0600 plus an sh -c wrapper -- a different mechanism from the other two instances, on the srv3 OS-install path, requiring two witnesses that currently ASSERT the token's presence in argv to be inverted. Rushing it inside a security change risks breaking an install path to close a leak that is already declared." as NonEmptyStr +} + +// THE JOURNAL HALF IS MEASURED, AND THE ANSWER IS "UNEXERCISED", WHICH IS NOT "CLEAN". Run on all +// four fleet hosts on 2026-08-25 against the derivable unit glob `nbd-proxy-*-ws` +// (nbd_proxy_serve_websocat_unit_name builds `nbd-proxy--ws`): units=0, journal_lines=0, +// grep_journal=0, execstart_now=0, live websocat processes=0, on every host. +// +// BOTH CONTROLS ARE ZERO, so the greps distinguish NOTHING. No such unit has ever existed and the +// journal holds no lines for that selection, which means the srv3 OS-install route has not run on +// this fleet. The correct reading is that the leak is LATENT rather than absent: the code path is +// real and would expose the token on all three surfaces the moment it runs. A zero measured against +// no history is not evidence of safety, and recording it as one would be the same non-result this +// lane has now produced repeatedly. +// +// THE DETECTOR RULE THAT PRODUCED THREE FALSE POSITIVES BEFORE THIS READING WAS TRUSTED, kept because +// none of the three was caught by reading the code and all three had one shape. A first pass reported +// live token-bearing processes on all four hosts; every hit was `grep` MATCHING ITS OWN COMMAND LINE, +// since the search pattern is itself an argv word that ps can see. Excluding the searcher's own +// process returns zero. The same artifact inflated an unrelated orphan census, and the same artifact +// made this module's own curl repair read as a non-repair until the probe was narrowed to the subject +// process's own cmdline. +// +// So: any ps- or /proc-based detector must exclude its own process, and the receipt for a ZERO must +// be a PLANTED POSITIVE rather than the absence of hits. That is not a nicety here -- it is the same +// defect this whole carrier is about, turned on the instrument: a value in a command line read by +// something that had no business reading it, which in that case was the measurement itself. +data bmcweb_token_journal_measurement_note: String = "MEASURED 2026-08-25, all four fleet hosts, unit glob nbd-proxy-*-ws: units=0 journal_lines=0 grep_journal=0 execstart_now=0 live_websocat=0. BOTH CONTROLS ZERO, so this is UNEXERCISED PATH, not clean -- the srv3 install route has not run on this fleet and the leak is LATENT, not absent. Queries: `systemctl list-units --all 'nbd-proxy-*'` and `journalctl -u 'nbd-proxy-*-ws' --output=cat | wc -l` as the controls; `grep -c -F 'Authorization: Token'` over the same journal selection and over `systemctl show 'nbd-proxy-*-ws' -p ExecStart` as the measurements. DETECTOR RULE, from three false positives in one day: a ps/proc detector MUST exclude its own process (the search pattern is an argv word ps can see), and a zero is only meaningful against a planted positive." + +// TWO AXES, AND A READER WHO COLLAPSES THEM WILL MIS-RANK THIS. Instance 3 is the most severe by +// SURFACE COUNT AND LIFETIME, and it is also the only one NOT CURRENTLY FIRING -- measured above as +// unexercised on all four hosts. Instances 1 and 2 were LIVE and are closed; instance 3 is LATENT and +// open. Both facts are needed: severity without liveness over-ranks it against work that is leaking +// today, and liveness without severity under-ranks a defect that exposes a credential on three +// surfaces the moment the install path runs. +// +// The unexercised finding also STRENGTHENS the decision to declare rather than repair. Nothing is +// leaking right now, so no emergency forces a risky change to the srv3 install path under time +// pressure, and the repair can be done properly -- with the two witnesses that currently assert the +// token's presence inverted alongside it, rather than bolted on afterwards. +data credential_argv_open_instance_is_the_most_severe_note: String = "RANKING ON TWO AXES, both required. SEVERITY: the open instance is the worst of the three -- argv PLUS a durable record. LIVENESS: it is the only one NOT currently firing (measured unexercised on all four hosts), while instances 1 and 2 were live and are now closed. Severity alone over-ranks it against work leaking today; liveness alone under-ranks a three-surface exposure that fires the moment the install path runs. Latency is also why declaring rather than repairing was right: no emergency forces a rushed change to the srv3 install path, so the repair can land properly with its two inverted witnesses. Original note follows -- RANKING, stated because the intuitive order is backwards: the two REPAIRED instances were argv-only on short-lived processes and are closed; the one still OPEN (nbd_proxy_serve) is argv PLUS a durable record across three surfaces -- systemd-run's transient argv, websocat's argv for the lifetime of the served ISO, and the recorded ExecStart which persists in the journal and systemctl show after the process exits. Revoking the token stops it working without removing it from the record. This instance outranks the binder trigger scoped below for whoever picks up next work." + +data credential_argv_exposure_instances: List = [ + redfish_basic_auth_instance, + secret_manager_bearer_instance, + bmcweb_session_token_instance, +] + +// THE RUNG, STATED AT THE HONEST GRAIN. What landed is two repairs and one declaration, not a wall. +// Nothing in this repository refuses the next author who interpolates a credential into a command +// string, and the reason is structural rather than an oversight: v2.std.operation_argv's binding wall +// already refuses undeclared names and a non-literal executable position, but ShellTransportOperationRow +// carries `declared_inputs` as a List -- NAMES WITHOUT TYPES -- so the binder cannot see that +// an input is `Secret` and could not refuse it if it wanted to. The hand-authored shell in ci_spec is +// further out still: it is not declared operation argv at all, so no binder sees it under any change. +// +// So the class sits at MITIGATABLE and its next-rung trigger is a specific missing capability, not a +// resolution to be careful: the declared input's TYPE reaching ShellTransportOperationRow, at which +// point `Secret` bound into an argv token becomes a typed refusal beside UndeclaredInputBound and the +// bmc instance above becomes unwritable rather than merely rewritten. Naming that trigger is what +// separates "cannot climb further" from "can climb after one grounding"; this is the second. +data credential_argv_exposure_rung_note: String = "RUNG: mitigatable. Two instances repaired by construction at their own call sites, one declared unrepaired with its mechanism and repair shape. NOT mechanically preventable: v2.std.operation_argv refuses undeclared names and a non-literal argv[0], but ShellTransportOperationRow carries declared_inputs as names without types, so a Secret-typed input bound into an argv token is invisible to the binder. NEXT-RUNG TRIGGER: carry the declared input TYPE on that row, then refuse Secret-into-argv-token as a typed cause alongside UndeclaredInputBound. Until that lands, this class is held by review diligence, which is strictly weaker and is recorded as such rather than counted as coverage." + +// THE NEXT ITEM, SCOPED HERE AND DELIBERATELY NOT STARTED IN THIS CHANGE. Everything above is two +// repairs and a declaration -- the class is held by review diligence, and review diligence is what +// failed for however long the three instances stood. The construction answer is one specific missing +// fact, and naming its shape is what makes it a queued item rather than an aspiration. +// +// WHAT IS MISSING: v2.std.operation_argv's ShellTransportOperationRow carries `declared_inputs` as a +// List. Names, not types. The binder therefore knows THAT an operation declares `password` +// and cannot know that `password` was declared `Secret`, so the refusal it would need to raise is not +// merely unwritten -- it is unexpressible against the row it would have to read. +// +// WHAT IT WOULD BUY: with the declared type on the row, `Secret` bound into an argv token becomes a +// typed cause beside UndeclaredInputBound, and instance 1 above stops being a thing an author +// rewrites and becomes a thing an author cannot write. That is the difference between this carrier +// being a record of three repairs and being the last time this class is found by reading. +// +// WHAT IT DOES NOT REACH, stated so the scope is not overclaimed when someone picks it up: the +// ci_spec instance is hand-authored shell text rather than declared operation argv, so no binder sees +// it under any version of this change. That half stays at review diligence plus the emission witness, +// and closing it would need shell command construction to become a modeled surface -- a much larger +// question that this trigger neither answers nor depends on. +data credential_argv_next_item_scope_note: String = "NEXT ITEM (scoped, not started): carry the declared input TYPE on v2.std.operation_argv ShellTransportOperationRow, which today holds declared_inputs as names only. That single fact is what makes a Secret-into-argv-token refusal expressible, turning the extdeps.bmc.http class from rewritten into unwritable. Explicitly out of its reach: gunbc.ci_spec's hand-authored shell is not declared operation argv and no binder change reaches it; that half stays at review diligence plus test.claim.credential_argv_exposure_witness until shell command construction is itself modeled." + +data credential_argv_binder_trigger_ref: DeclarationRef = DeclarationRef { + module_path: "v2.std.operation_argv", + decl_name: "ShellTransportOperationRow", + field: WholeDeclaration +} diff --git a/dag/gunbc/tools/bmc_first_contact.dag b/dag/gunbc/tools/bmc_first_contact.dag index 1debbbf8da4..109d4d5ccaf 100644 --- a/dag/gunbc/tools/bmc_first_contact.dag +++ b/dag/gunbc/tools/bmc_first_contact.dag @@ -1,5 +1,13 @@ module gunbc.tools.bmc_first_contact +// THE NETRC AUTHORITY IS IMPORTED RATHER THAN RE-SPELLED. This module previously declared no imports +// at all and resolved every name through corpus scope, which is survivable for a diagnostic tool and +// is NOT survivable for the one function that decides the mode of a file holding a credential: a +// second local netrc_content would be a fork of an authority whose whole point is that there is one +// of it, and bare resolution over a whole-corpus pool is not a stable binding to reason about when +// the question is "which write verb ran". +import gunbc.tools.bmc_onboard { bmc_onboard_netrc_path, write_netrc } + // `shell.Env.Get` answers `value: String?`, and this func reached it with `as NonEmptyStr`, which // does not eliminate the absence: with BMC_HOST unset the Redfish call was dialled at a host whose @@ -13,14 +21,24 @@ fn bmc_first_contact_host(value: String?) -> NonEmptyStr { } } +// THE FACTORY PASSWORD IS PUBLISHED, AND IT STILL DOES NOT GO IN ARGV. It is tempting to treat a +// vendor-published default as harmless in a command line, but the exception would be the wrong shape +// to teach: this func calls the same operation bmc_onboard uses to authenticate with a ROTATED +// secret, and an operation that accepts a credential by argv accepts every credential by argv. +// Keeping one authentication path means the leaky one has nowhere to survive, so first-contact +// writes the same 0600 netrc as every other Redfish caller and refuses loudly if it cannot. func bmc_first_contact() -> ProcessExit uses net: std.resources.Network { host = shell.Env.Get(name: "BMC_HOST") + let bmc = bmc_first_contact_host(value: host.value) + + if write_netrc(host: bmc as String, account: openbmc_factory_login.username as String, password: openbmc_factory_login.published_password) == false { + return exit_failure(reason: "first-contact: netrc write failed") + } resp = redfish.Http.GetSystem( - bmc_host: bmc_first_contact_host(value: host.value), - username: openbmc_factory_login.username, - password: openbmc_factory_login.published_password as Secret + bmc_host: bmc, + netrc_file: bmc_onboard_netrc_path as NonEmptyStr ) return if resp.success { ExitSuccess } else { exit_failure(reason: resp.body) } diff --git a/dag/gunbc/tools/bmc_onboard.dag b/dag/gunbc/tools/bmc_onboard.dag index ca0c8589c64..e305594adc7 100644 --- a/dag/gunbc/tools/bmc_onboard.dag +++ b/dag/gunbc/tools/bmc_onboard.dag @@ -1,6 +1,7 @@ module gunbc.tools.bmc_onboard import extdeps.bmc.http +import extdeps.shell import extdeps.filesystem.filesystem_io import extdeps.entropy import extdeps.cloud.gcp.secret_manager { @@ -36,7 +37,7 @@ import gunbc.auth.access_token_source { access_token_upsert_required_reason, ensure_access_token, } -import std.types { NonEmptyStr, Secret, String } +import std.types { FilePath, NonEmptyStr, Secret, String } import std.logic { Bool } import std.resources { Network } import std.bytes { utf8_encode_bytes } @@ -112,10 +113,30 @@ fn mint_bmc_credential() -> Secret { Urandom.ReadPassword(count: bmc_credential_length_chars).password as Secret } +// THE NETRC FILE IS THE CREDENTIAL'S ONLY CARRIER NOW, so its MODE is load-bearing rather than +// incidental. Every Redfish call in this tool authenticates through this path since the argv-borne +// `-u {username}:{password}` arms were deleted from extdeps.bmc.http, which means a world-readable +// netrc would simply move the exposure from the process table to the filesystem -- on hosts where +// the audience is identical, since the same unprivileged reader that could ps the argv can stat /tmp. +// Filesystem.Write creates with the ambient umask; WriteOwnerOnly creates 0600 at creation via +// O_EXCL rather than writing first and chmod-ing after, so there is no window in which the bytes +// exist at a wider mode. +// +// O_EXCL is why the shred precedes the write instead of following the read: WriteOwnerOnly REFUSES +// an existing path, and this tool legitimately rewrites the netrc within one run -- once for the +// factory login and again after rotation. Removing first makes the rewrite the ordinary case rather +// than a failure, and it also means a netrc left behind by an earlier interrupted run is destroyed +// before this one authenticates, instead of being silently reused. fn write_netrc(host: String, account: String, password: String) -> Bool { - Filesystem.Write(path: bmc_onboard_netrc_path, content: netrc_content(host: host, account: account, password: password)).success + let _ = shell.Remove.FileForce(path: bmc_onboard_netrc_path as FilePath) + Filesystem.WriteOwnerOnly(path: bmc_onboard_netrc_path, content: netrc_content(host: host, account: account, password: password)).success } +// THE POST-ROTATION REAUTH RE-WRITES THE NETRC, because the credential the BMC now expects is the +// one this func just installed. That write is CHECKED rather than assumed: a silently-failed netrc +// would leave the PREVIOUS credential's file in place, and the reauth would then appear to succeed +// while proving nothing about the rotation it exists to confirm -- a green that means the opposite +// of what it reads as. func bmc_rotate_credential(plan: BmcOnboardingPlan, new_credential: Secret) -> ProcessExit uses net: Network { @@ -144,7 +165,10 @@ func bmc_rotate_credential(plan: BmcOnboardingPlan, new_credential: Secret) -> P return exit_failure(reason: rot.body) } - verify = redfish.Http.GetSystem(bmc_host: host, username: account, password: new_credential) + if write_netrc(host: host, account: account, password: new_credential as String) == false { + return exit_failure(reason: "rotate: netrc write for post-rotation reauth failed") + } + verify = redfish.Http.GetSystem(bmc_host: host, netrc_file: bmc_onboard_netrc_path as NonEmptyStr) return if verify.success { ExitSuccess } else { @@ -213,10 +237,12 @@ func bmc_assimilate_credential(plan: BmcOnboardingPlan, token_source: AccessToke if root_resp.success == false { return exit_failure(reason: concat("acquire: service root unreachable: ", root_resp.body)) } + if write_netrc(host: host, account: account, password: plan.factory_login.published_password) == false { + return exit_failure(reason: "acquire: netrc write failed") + } acq = redfish.Http.GetSystem( bmc_host: host, - username: account, - password: plan.factory_login.published_password as Secret + netrc_file: bmc_onboard_netrc_path as NonEmptyStr ) if acq.success == false { return exit_failure(reason: concat("acquire: factory login failed: ", acq.body)) @@ -225,16 +251,23 @@ func bmc_assimilate_credential(plan: BmcOnboardingPlan, token_source: AccessToke return bmc_store_and_rotate(plan: plan, token_source: token_source, credential: mint_bmc_credential()) } +// A NETRC THAT COULD NOT BE WRITTEN IS NOT EVIDENCE ABOUT THE CREDENTIAL, and this probe keeps those +// apart. Each write failure returns CredentialStateUnknown rather than falling through to the next +// arm, because reporting `factory login failed` for a local filesystem fault would send an operator +// to the BMC to investigate a problem that is on this host. The same rule governs the stored- +// credential arm below it. func bmc_probe_credential_phase(plan: BmcOnboardingPlan, token_source: AccessTokenSource) -> BmcCredentialProbeResult uses net: Network { let host = plan.bmc.host let account = plan.factory_login.username + if write_netrc(host: host, account: account, password: plan.factory_login.published_password) == false { + return CredentialStateUnknown { reason: "netrc write for factory credential probe failed" } + } factory_check = redfish.Http.GetSystem( bmc_host: host, - username: account, - password: plan.factory_login.published_password as Secret + netrc_file: bmc_onboard_netrc_path as NonEmptyStr ) if factory_check.success { return FactoryCredentialActive @@ -254,10 +287,12 @@ func bmc_probe_credential_phase(plan: BmcOnboardingPlan, token_source: AccessTok ) match decode_sm_access_version_secret_wire(data_b64: secret_resp.data_b64) { SmAccessVersionSecretDecoded { credential: password } => { + if write_netrc(host: host, account: account, password: password as String) == false { + return CredentialStateUnknown { reason: "netrc write for stored credential probe failed" } + } rotated_verify = redfish.Http.GetSystem( bmc_host: host, - username: account, - password: password + netrc_file: bmc_onboard_netrc_path as NonEmptyStr ) return if rotated_verify.success { RotatedCredentialActive diff --git a/dag/gunbc/tools/bmc_read_telemetry.dag b/dag/gunbc/tools/bmc_read_telemetry.dag index e2db85d0112..828875dca7c 100644 --- a/dag/gunbc/tools/bmc_read_telemetry.dag +++ b/dag/gunbc/tools/bmc_read_telemetry.dag @@ -22,7 +22,7 @@ func bmc_read_telemetry() -> ProcessExit let netrc_file = netrc_v as NonEmptyStr let chassis = asrock_altrad8ud_redfish_chassis_id - system_resp = redfish.Http.GetSystemWithNetrc(bmc_host: bmc, netrc_file: netrc_file) + system_resp = redfish.Http.GetSystem(bmc_host: bmc, netrc_file: netrc_file) if system_resp.success == false { return exit_failure(reason: system_resp.body) } diff --git a/dag/test/claim/bmc_auth_runtime_secret_seam_test.dag b/dag/test/claim/bmc_auth_runtime_secret_seam_test.dag deleted file mode 100644 index fd0d4b5fdc9..00000000000 --- a/dag/test/claim/bmc_auth_runtime_secret_seam_test.dag +++ /dev/null @@ -1,18 +0,0 @@ -module test.claim.bmc_auth_runtime_secret_seam - - -data live_tree_disposition: LiveTreeDisposition = SubstrateInputsOnly - -fn redfish_basic_auth_password(password: Secret) -> Secret { - password -} - -test fn factory_published_password_promotes_to_basic_auth_secret() -> Bool { - redfish_basic_auth_password(password: openbmc_factory_login.published_password as Secret) == "0penBmc" -} - -test fn gcp_access_payload_produces_basic_auth_secret() -> Bool { - redfish_basic_auth_password( - password: utf8_secret_from_access_payload(payload: utf8_encode_bytes(s: "bmc-srv3-admin-rotated")) - ) == "bmc-srv3-admin-rotated" -} diff --git a/dag/test/claim/credential_argv_exposure_witness_test.dag b/dag/test/claim/credential_argv_exposure_witness_test.dag new file mode 100644 index 00000000000..2934b715ea7 --- /dev/null +++ b/dag/test/claim/credential_argv_exposure_witness_test.dag @@ -0,0 +1,63 @@ +module test.claim.credential_argv_exposure_witness + +import gunbc.ci_spec { + gunbc_ci_fleet_key_agent_script, + gunbc_ci_spark_grant_install_invoke, + gunbc_ci_auth_header_file_write_command, +} + +data live_tree_disposition: LiveTreeDisposition = SubstrateInputsOnly + +// THIS WITNESS ASSERTS TWO THINGS, AND ONE OF THEM ALONE WOULD BE A DECORATION. The absence of the +// leaking spelling is the safety property; the presence of the file-backed one is what makes the +// absence mean the credential still ARRIVES rather than that the step stopped authenticating. A +// prelude that had simply dropped the header would satisfy an absence-only check perfectly, which is +// the failure mode extdeps.llm.cursor_sdk already records under a different name: an argv that is +// clean because it is broken. +// +// The subject is the EMITTED SHELL rather than the committed workflow file, deliberately. The +// committed .github/workflows/fleet-converge.yml is a projection of these functions; asserting +// against it would test whether someone remembered to regenerate, while asserting against the +// emission tests the authority that decides what any future regeneration produces. + +data forbidden_bearer_interpolation: String = "Authorization: Bearer $WIF_ACCESS_TOKEN" + +// THE RED ARM IS AUTHORABLE, which is what distinguishes this from a check that is green by +// construction: restoring the old spelling in either prelude turns these two tests red, and that is +// exactly the edit a future author would make by copying a neighbouring curl invocation. +test fn fleet_key_prelude_does_not_interpolate_the_token_into_a_command() -> Bool { + !string_contains(s: gunbc_ci_fleet_key_agent_script(), pattern: forbidden_bearer_interpolation) + && string_contains(s: gunbc_ci_fleet_key_agent_script(), pattern: "-H @\"$HDR_FILE\"") +} + +test fn spark_grant_prelude_does_not_interpolate_the_token_into_a_command() -> Bool { + !string_contains(s: gunbc_ci_spark_grant_install_invoke(), pattern: forbidden_bearer_interpolation) + && string_contains(s: gunbc_ci_spark_grant_install_invoke(), pattern: "-H @\"$HDR_FILE\"") +} + +// THE WRITER MUST BE A BUILTIN, and this is the assertion most likely to be lost in a later edit +// because it looks like a formatting preference. It is not: `printf` runs inside the shell process +// and forks nothing, so the token exists in the shell's memory and in the file. Any EXTERNAL command +// writing the same bytes -- echo as a binary, a here-doc through tee, a python one-liner -- would +// leave the value in ITS OWN argv, relocating the exposure rather than removing it. A reviewer who +// swaps printf for tee has not made a cosmetic change, and this test is what says so. +test fn the_auth_header_is_written_by_a_shell_builtin_not_an_external_command() -> Bool { + string_contains(s: gunbc_ci_auth_header_file_write_command(), pattern: "printf ") + && !string_contains(s: gunbc_ci_auth_header_file_write_command(), pattern: "tee ") + && !string_contains(s: gunbc_ci_auth_header_file_write_command(), pattern: "python") +} + +// Both preludes must reach the writer, since a header file that is never written makes `-H @FILE` a +// curl error rather than a leak -- loud, but a different failure than the one under test, and one a +// reader would mistake for this repair having been abandoned. +test fn both_preludes_write_the_header_file_before_using_it() -> Bool { + string_contains(s: gunbc_ci_fleet_key_agent_script(), pattern: "printf 'Authorization: Bearer") + && string_contains(s: gunbc_ci_spark_grant_install_invoke(), pattern: "printf 'Authorization: Bearer") +} + +test fn credential_argv_exposure_witness() -> Bool { + fleet_key_prelude_does_not_interpolate_the_token_into_a_command() + && spark_grant_prelude_does_not_interpolate_the_token_into_a_command() + && the_auth_header_is_written_by_a_shell_builtin_not_an_external_command() + && both_preludes_write_the_header_file_before_using_it() +} diff --git a/src/v2/test/lens_mock_totality/diagnostic_mock_totality_test.dag b/src/v2/test/lens_mock_totality/diagnostic_mock_totality_test.dag index 13376324e82..2d342c2ab96 100644 --- a/src/v2/test/lens_mock_totality/diagnostic_mock_totality_test.dag +++ b/src/v2/test/lens_mock_totality/diagnostic_mock_totality_test.dag @@ -26,8 +26,6 @@ fn materialize_diagnostic_mock(case: PublishedMockCase) -> Outcome