Skip to content

Commit a9dc616

Browse files
committed
orchestrator: reject wire text that would reach the terminal as an escape
A component chooses the text in three wire fields — a result's name, and a disposition's gate and msg — and all three are rendered, gate and msg into the hardening report an operator forwards. api.h said the parser at ingest was the gatekeeper for them; no such gatekeeper existed, so a control byte travelled from a component line to the terminal, where erase-line and cursor movement redraw what was already printed and a finding can be made to read as its opposite. Validate at ingest and reject the record whole, joining the existing parse discard rather than truncating or scrubbing: losing a record only widens a window, and a name that survives with its bytes replaced would silently change the identity merge_results groups on. The admissible set is per field. name and gate are identifiers whose every source is ASCII by its own specification — C identifiers, dtc-checked device-tree node names, ACPI signatures, a PCI BDF — so printable ASCII without the space, which the wire could never carry anyway. msg is quoted prose and admits the space. Both refuse 0x80 and above: nothing that fills these fields produces one, and the range cannot be split byte-wise, since 0x80..0x9F is both the C1 controls a terminal in an 8-bit locale acts on and the UTF-8 continuation range. --verbose echoes each component line before any parser sees it, so that echo strips control bytes independently, showing them as `?` rather than dropping them. Its range stops short of 0x80 on purpose: a diagnostic quotes arbitrary kernel text where a high byte can be legitimate. check-wire-text covers both paths with stub components, asserting that no control byte reaches any mode, that a rejected record never renders, and that a clean record still does — the last so a parser that rejected everything could not pass. check-render-color proves KASLD's own escapes strip to plain; it says nothing about escapes arriving in data.
1 parent 32cd635 commit a9dc616

5 files changed

Lines changed: 241 additions & 4 deletions

File tree

Makefile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -820,6 +820,7 @@ lint :
820820
$(TEST_DIR)/check-baseline \
821821
$(TEST_DIR)/check-render-parity \
822822
$(TEST_DIR)/check-render-color \
823+
$(TEST_DIR)/check-wire-text \
823824
$(TEST_DIR)/hardening-fixtures \
824825
$(TEST_DIR)/cli-flags
825826

docs/testing.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,7 @@ stays plain, and setting `KASLD_COLOR` non-empty or empty forces either.
208208
| `check-baseline` | the no-component baseline (`-s '*'`) renders in every output mode and exits with the no-results status, and a run that gathers evidence resolves a window *inside* it † |
209209
| `check-render-parity` | the text readout, the markdown report and JSON name the same set of resolved quantities for a given run † |
210210
| `check-render-color` | coloured output is byte-identical to plain output once the escape sequences are removed, and markdown, JSON and oneline carry no escapes at all † |
211+
| `check-wire-text` | a component cannot put an escape sequence on the terminal: a record whose `name`, or a disposition whose `gate` or `msg`, leaves printable ASCII is rejected, and the verbose echo of component output strips control bytes † |
211212
| `check-guard-docs` | this table lists exactly the guards `make lint` runs — the same parity check `check-manpages` applies to flags, applied to the guard list itself |
212213
| `check-matrix-summary` | the summary table in `docs/reproducibility.md` restates the full per-scenario matrix it precedes: same cells, same KASLR state, same `default` and `perf-open` results in both directions |
213214
| `check-readout-docs` | documented sample output uses the renderer's current vocabulary and fits 100 columns (live output is measured separately by `check-render-width`) † |

src/include/kasld/api.h

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2075,8 +2075,16 @@ kasld_scalar_fact_from_wire(const char *s) {
20752075
* Rejection happens for: CONF_UNKNOWN, invalid type, invalid region,
20762076
* helper-specific preconditions (e.g. _sized overflow).
20772077
* `name = NULL` or `""` means no specific instance (the wire form omits
2078-
* the `:name` suffix). Names with leading/trailing whitespace or unsupported
2079-
* chars are NOT validated here — the parser at ingest is the gatekeeper.
2078+
* the `:name` suffix).
2079+
*
2080+
* The character set is not checked here; the parser at ingest enforces it, and
2081+
* what it enforces is this: a name is made only of printable ASCII excluding
2082+
* space, 0x21..0x7E, and a record carrying anything else is rejected whole
2083+
* rather than truncated or scrubbed. The same rule governs a disposition's
2084+
* `gate`, while its quoted `msg` additionally admits the space. Every source
2085+
* these fields draw on is ASCII by its own specification, so a byte outside
2086+
* the set is an artefact; and since all three reach a terminal, a control byte
2087+
* among them is an escape sequence rather than a cosmetic flaw.
20802088
* ========================================================================= */
20812089

20822090
static inline int kasld__emit_check(enum kasld_addr_type t, enum kasld_region r,

src/orchestrator.c

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -780,6 +780,40 @@ struct parsed_tail {
780780
unsigned long lo, hi, sz, sample, base_align;
781781
};
782782

783+
/* Is a free-text wire field made only of characters the protocol admits?
784+
*
785+
* `name`, `gate` and `msg` are the three fields a component fills with text of
786+
* its own choosing, and all three reach the operator's terminal — `gate` and
787+
* `msg` through the hardening report. A control byte among them is an escape
788+
* sequence: cursor movement and erase-line can redraw a finding as its
789+
* opposite in the very report meant to expose it.
790+
*
791+
* The admissible set is per field rather than global. `name` and `gate` are
792+
* identifiers, and every source that fills them is ASCII by its own
793+
* specification — kernel symbols are C identifiers, device-tree node names are
794+
* checked against a fixed set by dtc, ACPI signatures and OEM ids are ASCII by
795+
* the ACPI spec, a PCI BDF is hex with `:` and `.`. `msg` is quoted prose and
796+
* so admits the space as well.
797+
*
798+
* Bytes at or above 0x80 are excluded with the controls. Nothing that fills
799+
* these fields produces one, and admitting them would admit both the C1
800+
* controls — which a terminal in an 8-bit locale acts on, no ESC byte needed —
801+
* and confusable characters, neither of which a line-oriented ASCII protocol
802+
* has a reason to carry. Note the range cannot be split byte-wise: 0x80..0x9F
803+
* is also the UTF-8 continuation range, so banning C1 alone would corrupt any
804+
* multi-byte sequence, and permitting it would leave the controls intact. */
805+
static int wire_text_ok(const char *s, int allow_space) {
806+
for (; *s; s++) {
807+
unsigned char c = (unsigned char)*s;
808+
if (c >= 0x21 && c <= 0x7e)
809+
continue;
810+
if (allow_space && c == 0x20)
811+
continue;
812+
return 0;
813+
}
814+
return 1;
815+
}
816+
783817
/* Split a "region[:name]" wire token into a resolved region + name_buf (sized
784818
* NAME_LEN). The split is on the FIRST `:` only — names may contain subsequent
785819
* colons (e.g. PCI BDF "0000:00:14.0"); region wire names are short
@@ -805,6 +839,10 @@ static enum kasld_region parse_region_field(const char *region_field,
805839
return REGION_UNKNOWN;
806840
memcpy(name_buf, name_src, nlen);
807841
name_buf[nlen] = '\0';
842+
if (!wire_text_ok(name_buf, 0)) { /* control bytes reach the terminal */
843+
name_buf[0] = '\0';
844+
return REGION_UNKNOWN;
845+
}
808846
} else {
809847
size_t rlen = strlen(region_field);
810848
if (rlen >= sizeof(region_str))
@@ -1618,6 +1656,8 @@ static int parse_disposition(const char *s, struct component_disposition *d) {
16181656
tmp.gate[n] = '\0';
16191657
if (n == 0)
16201658
return 0; /* gate= naming nothing */
1659+
if (!wire_text_ok(tmp.gate, 0))
1660+
return 0; /* control bytes reach the hardening report */
16211661
} else if (klen == 3 && memcmp(key, "msg", 3) == 0) {
16221662
size_t n = 0;
16231663
if (seen_msg)
@@ -1635,6 +1675,8 @@ static int parse_disposition(const char *s, struct component_disposition *d) {
16351675
return 0; /* unterminated */
16361676
s++;
16371677
tmp.message[n] = '\0';
1678+
if (!wire_text_ok(tmp.message, 1))
1679+
return 0; /* quoted prose, but still no control bytes */
16381680
} else {
16391681
return 0; /* unknown key (spec: no forward-compat silence) */
16401682
}
@@ -1661,8 +1703,27 @@ static int handle_component_line(struct component_log *clog,
16611703
memcpy(line, content, len);
16621704
line[len] = '\0';
16631705

1664-
if (verbose && plain_output())
1665-
printf("%s\n", line);
1706+
/* Echoed verbatim except for control bytes: DEL and everything below 0x20,
1707+
* including a stray CR that would redraw the line just printed. What a filter
1708+
* removes here is an escape sequence on a line that never reaches a parser
1709+
* and so is never rejected by one — the same terminal the readout protects,
1710+
* reached through the debugging channel instead.
1711+
*
1712+
* The range is narrower than the one wire_text_ok() applies to the wire's
1713+
* free-text fields, which also refuse everything at or above 0x80. That is
1714+
* deliberate rather than an omission: those fields are identifiers whose
1715+
* sources are ASCII by their own specifications, while a diagnostic line
1716+
* quotes arbitrary kernel text — a dmesg line, a /proc string — where a high
1717+
* byte can be legitimate UTF-8 that the operator asked to see. The residue is
1718+
* that a raw C1 byte survives here, which a terminal in an 8-bit locale would
1719+
* act on; mangling quoted kernel text on every run is the worse trade. */
1720+
if (verbose && plain_output()) {
1721+
for (const char *p = line; *p; p++) {
1722+
unsigned char c = (unsigned char)*p;
1723+
putchar((c < 0x20 || c == 0x7f) ? '?' : c);
1724+
}
1725+
putchar('\n');
1726+
}
16661727

16671728
/* Capture line for verbose / JSON-with-output. Allocated on first use and
16681729
* grown geometrically — no fixed cap, so noisy components do not silently

tests/check-wire-text

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
#!/bin/sh
2+
# This file is part of KASLD - https://github.com/bcoles/kasld
3+
#
4+
# check-wire-text — a component cannot put an escape sequence on the operator's
5+
# terminal.
6+
#
7+
# Three wire fields carry text a component chooses: a result's `name`, and a
8+
# disposition's `gate` and `msg`. All three are rendered, `gate` and `msg` into
9+
# the hardening report, which is the output an operator forwards. A control byte
10+
# among them is not a cosmetic flaw: cursor-movement and erase-line sequences
11+
# redraw what was already printed, so a finding can be made to read as its
12+
# opposite by the report meant to expose it.
13+
#
14+
# Two paths carry such a byte and both are covered here. The parser rejects a
15+
# record whose free-text field leaves printable ASCII, so nothing reaches the
16+
# rendered readout. `--verbose` additionally echoes each component line before
17+
# any parser sees it, so that echo strips control bytes rather than relying on
18+
# the rejection.
19+
#
20+
# check-render-color proves KASLD's OWN escapes strip back to the plain
21+
# rendering. It says nothing about escapes arriving in data, which is the gap
22+
# this guard covers.
23+
#
24+
# The stubs are shell scripts: the orchestrator execs whatever is executable in
25+
# the component directory, so no compiler is needed and the guard runs anywhere
26+
# the host binary does.
27+
# ---
28+
# <bcoles@gmail.com>
29+
30+
set -u
31+
ROOT=$(CDPATH='' cd -- "$(dirname -- "$0")/.." && pwd)
32+
33+
if [ -t 1 ] || [ -n "${KASLD_COLOR:-}" ]; then
34+
RED=$(printf '\033[31m'); GREEN=$(printf '\033[32m'); YELLOW=$(printf '\033[33m')
35+
RESET=$(printf '\033[0m')
36+
else
37+
RED=; GREEN=; YELLOW=; RESET=
38+
fi
39+
40+
# Literal control bytes, built once. The textual form "\033" is not a portable
41+
# pattern -- some greps read it as an escape and some as four characters, so a
42+
# guard written that way matches nothing and reports success on every run.
43+
ESC=$(printf '\033')
44+
CR=$(printf '\r')
45+
46+
HOST_TRIPLE=$(cc -dumpmachine 2>/dev/null || echo unknown)
47+
BIN="$ROOT/build/$HOST_TRIPLE/kasld"
48+
if [ ! -x "$BIN" ]; then
49+
printf '%swire-text guard: SKIP%s (no host binary at %s; run make first)\n' \
50+
"$YELLOW" "$RESET" "$BIN"
51+
exit 0
52+
fi
53+
54+
WORK=$(mktemp -d "${TMPDIR:-/tmp}/kasld-wiretext.XXXXXX") || exit 1
55+
trap 'rm -rf "$WORK"' EXIT INT TERM
56+
EMPTY="$WORK/sysroot"
57+
STUBS="$WORK/components"
58+
mkdir -p "$EMPTY" "$STUBS" || exit 1
59+
60+
# An empty sysroot keeps the run hermetic: no host fact is read, so the output
61+
# is the stubs' contribution and the architectural defaults.
62+
63+
# A clean record, so a run that rejects everything cannot pass by accident.
64+
cat >"$STUBS/stub_clean" <<'STUB'
65+
#!/bin/sh
66+
echo "V kernel_text:cleanname pos=base conf=parsed lo=0xffffffff8b600000"
67+
STUB
68+
69+
# One stub per hostile field. Each carries an erase-line and a colour sequence,
70+
# the pair that rewrites a rendered line in place.
71+
cat >"$STUBS/stub_name" <<'STUB'
72+
#!/bin/sh
73+
printf 'V module:ev\033[2K\rHOSTILENAME pos=base conf=parsed lo=0xffffffffc0abc000\n'
74+
STUB
75+
76+
cat >"$STUBS/stub_gate" <<'STUB'
77+
#!/bin/sh
78+
printf 'R cat=mitigation gate=kpti\033[2K\rHOSTILEGATE msg="blocked"\n'
79+
exit 69
80+
STUB
81+
82+
cat >"$STUBS/stub_msg" <<'STUB'
83+
#!/bin/sh
84+
printf 'R cat=mitigation gate=kpti msg="blocked \033[7mHOSTILEMSG\033[0m here"\n'
85+
exit 69
86+
STUB
87+
88+
# A high byte, covering the other half of the rule. The admissible set stops at
89+
# 0x7E, not merely below 0x20: 0x80..0x9F is the C1 control range, which a
90+
# terminal in an 8-bit locale acts on with no ESC byte involved, and it cannot
91+
# be excluded byte-wise without also excluding UTF-8 continuation bytes. A guard
92+
# that only emitted ESC would keep passing if that half were dropped.
93+
cat >"$STUBS/stub_highbyte" <<'STUB'
94+
#!/bin/sh
95+
printf 'V module:hi\233HOSTILEHIGH pos=base conf=parsed lo=0xffffffffc0def000\n'
96+
STUB
97+
98+
chmod +x "$STUBS"/stub_* || exit 1
99+
100+
fail=0
101+
checked=0
102+
103+
# Every mode that renders component-supplied text: the default readout, the
104+
# verbose readout (which also echoes raw component lines), and the hardening
105+
# report (where a disposition's gate and msg are shown).
106+
for mode in "-q" "-q -v" "-q -H"; do
107+
# shellcheck disable=SC2086 # $mode is a deliberate word-split option list
108+
out=$(KASLD_SYSROOT="$EMPTY" KASLD_COMPONENT_DIR="$STUBS" \
109+
"$BIN" $mode 2>&1) || true
110+
checked=$((checked + 1))
111+
112+
case $out in
113+
*"$ESC"*)
114+
printf '%s escape byte reached the terminal%s: kasld %s\n' \
115+
"$RED" "$RESET" "$mode"
116+
fail=1 ;;
117+
esac
118+
case $out in
119+
*"$CR"*)
120+
printf '%s carriage return reached the terminal%s: kasld %s\n' \
121+
"$RED" "$RESET" "$mode"
122+
fail=1 ;;
123+
esac
124+
125+
# A record whose free-text field left printable ASCII must not RENDER,
126+
# whichever byte took it out of the set. Under --verbose the raw component
127+
# line is echoed on purpose, so the marker legitimately appears there with its
128+
# control bytes already neutralised; the rendered readout is every other line.
129+
# Dropping the echoed lines -- those that still carry a wire tag -- separates
130+
# the two without needing to know the readout's layout.
131+
# -a: the output can carry a high byte, and grep would otherwise report
132+
# "binary file matches" instead of the lines, leaving the check below with
133+
# nothing to search and passing vacuously.
134+
rendered=$(printf '%s\n' "$out" | LC_ALL=C grep -av '^[VPSR] ' || true)
135+
for marker in HOSTILENAME HOSTILEGATE HOSTILEMSG HOSTILEHIGH; do
136+
case $rendered in
137+
*"$marker"*)
138+
printf '%s a rejected record rendered anyway%s: %s in kasld %s\n' \
139+
"$RED" "$RESET" "$marker" "$mode"
140+
fail=1 ;;
141+
esac
142+
done
143+
144+
# The clean record must still render: a guard that passes because nothing at
145+
# all was accepted would report success on a parser that rejects everything.
146+
# Checked under --verbose, the mode that lists a record's name at all; the
147+
# default readout reports resolved quantities rather than per-record labels.
148+
case $mode in
149+
"-q -v")
150+
case $out in
151+
*cleanname*) ;;
152+
*)
153+
printf '%s the clean record did not render%s: kasld %s\n' \
154+
"$RED" "$RESET" "$mode"
155+
fail=1 ;;
156+
esac ;;
157+
esac
158+
done
159+
160+
if [ "$fail" -eq 0 ]; then
161+
printf '%swire-text guard: OK%s (%d modes, 4 hostile records, none rendered)\n' \
162+
"$GREEN" "$RESET" "$checked"
163+
else
164+
printf '%swire-text guard: FAIL%s\n' "$RED" "$RESET"
165+
fi
166+
exit "$fail"

0 commit comments

Comments
 (0)