Name Keel in apt and dpkg, and stop leaking the appliance identity #8

Merged

marcos-mendez merged 5 commits from fix/keel-apt-identity into 19.x 2026-09-29 00:43:12 -03:00

Conversation 2 Commits 5 Files changed 14 +711 -48

marcos-mendez commented 2026-09-28 01:38:45 -03:00 (Migrated from github.com)

Copy link

Three of the four ways a Keel appliance still reported to TurnKey. The
fourth, the apt suites themselves, is measured here and deliberately not
touched: see "What this does not do".

Closes #6.

The dpkg vendor

/etc/dpkg/origins/default resolved to a TurnKey file, so dpkg-vendor
answered TurnKey and every bug reporting tool on the appliance addressed
turnkeylinux/tracker. The overlay now ships a Keel file and no TurnKey
one, and conf/turnkey.d/dpkg-vendor points default at it.

Parent: Debian is kept. That matters: there is no Dpkg::Vendor::Keel
perl module, so dpkg-parsechangelog and dpkg-buildpackage fall back to
the parent's vendor object, which is the same Dpkg::Vendor::Debian they
got through TurnKey before. Package building is unaffected.

How it is asserted. Not by reading back the symlink the script wrote.
tests/dpkg-vendor.bats arranges the origins tree the overlay ships, runs
the conf script, and then asks the real dpkg-vendor what it makes of
it, through dpkg's own DPKG_ORIGINS_DIR (Dpkg::Vendor honours it):
--query Vendor answers Keel, --query Bugs answers our tracker,
--is TurnKey fails, --is Keel succeeds, --derives-from Debian
succeeds and --derives-from Ubuntu fails. The upgrade path is covered
too: a default inherited as a symlink to TurnKey, as a regular file, and
as a directory are each replaced, and the vendor query is what says so.

The apt User-Agent

mk/turnkey.mk and mk/turnkey-desktop.mk each wrote a per-appliance
/etc/apt/apt.conf.d/01turnkey:

Acquire::http::User-Agent "TurnKey APT-HTTP/1.3 (turnkey-wordpress-19.0-trixie-amd64)";

so deb.debian.org, security.debian.org, every mirror in between and
anyone watching the connection were told which appliance this machine is
and which version it runs, on every apt run it ever made.

It is now a fixed file the overlay ships, 01keel, naming the distribution
and nothing else. Measured: apt's https method reads the same
Acquire::http::User-Agent, so one line covers both schemes.

conf/turnkey.d/apt-identity exists for a reason worth stating. apt reads
apt.conf.d in lexical order and the last assignment of a scalar wins, so
a 01turnkey inherited from a parent layer built before this change
silently beats the 01keel beside it:

$ ls apt.conf.d/            # 01keel and a stale 01turnkey
$ apt-config dump Acquire::http::User-Agent
Acquire::http::User-Agent "TurnKey APT-HTTP/1.3 (turnkey-wordpress-19.0-trixie-amd64)";

The script removes it, and refuses if 01keel is not there.

How it is asserted. Off the wire. tests/ua-recorder.py is a local
server that records the User-Agent header of every request; a real
apt-get update is pointed at it and the test reads what apt actually
sent, over http and over TLS. So the verdict is apt's behaviour, not the
file's content. One test drives the stale-01turnkey hazard both ways: it
measures TurnKey's header going out, runs the conf script, and measures
Keel's.

The strings the old header leaked each have their own refutation: the
sent header contains no (, no TurnKey, no turnkey, no wordpress,
no trixie, no amd64 and no 19.0.

https on the TurnKey archive

It is available. Measured 2026-09-28:

$ curl -sS -o /dev/null -D - https://archive.turnkeylinux.org/debian/dists/trixie/Release
HTTP/2 200
$ curl -sS -o /dev/null -w 'code=%{http_code} ssl_verify=%{ssl_verify_result}\n' \
    https://archive.turnkeylinux.org/debian/dists/trixie/Release
code=200 ssl_verify=0

ssl_verify=0 is a certificate that verifies against the system trust
store. trixie-security and trixie-testing answer 200 over https as
well. So all six places conf/bootstrap_apt wrote
http://archive.turnkeylinux.org (the three deb822 stanzas and the three
legacy sources.list lines still written for pre-Trixie releases) move to
https.

The Debian sources beside them stay on http in this change, for one reason: that is Debian's own default and the payload is signed either way. (An earlier version of this description also said the build host's caching proxy only feeds http; it does not, squid there bumps https too, so that is not a reason.) The request stream over http still tells the path which packages at which versions an appliance fetches, the same confidentiality question the User-Agent change answers, so flipping the Debian URIs is worth its own change and is recorded in #7.

How it is asserted. The test extracts the heredoc bodies from
conf/bootstrap_apt itself, renders them with a build's variables, so the
bytes under test are the generator's own output, not a copy in the test;
writes them into a scratch apt tree and asks apt with
apt-get indextargets which URIs it would fetch. No network. The verdict
is apt's resolution of our sources: nothing under
http://archive.turnkeylinux.org, and all three suites present under
https://. The Debian URIs are asserted unchanged so the scope of the
change is visible in the suite.

What this does not do

It does not remove the TurnKey apt suites. The measurement #6 asked
for is done and it says the suites are load bearing:

• The suites offer 141 package names (from 35 source packages; webmin alone is 103 of them). Measured on the wordpress-demo container (which never went through the container patch): 39 installed packages have their apt candidate in the TurnKey archive (41 of the 141 names are installed; confconsole and inithooks are the other two, served by archive.keellinux.org at 1001). The count is per appliance: on forum it is 32. None is available from Debian at all.
• apt-cache policy over every installed package finds 0 where the
  TurnKey archive overrides Debian today.
• trixie-testing is Enabled: no on a built appliance, so two suites are
  enabled, not three.
• trixie-security from TurnKey carries exactly one package, tklbam,
  at the version already installed.
• There is also /etc/apt/preferences pinning o=turnkeylinux at 999,
  above Debian's 500, from overlays/bootstrap_apt/etc/apt/preferences.
  It shadows nothing today; it would the day a name collides. Removing it is a change to how every appliance resolves packages, so by BRIEF section 12 (preserve upstream behaviour when in doubt) it goes with the migration in #7 rather than riding here.

Of the 39: 7 are appliance infrastructure that must move to our pool,
7 are TurnKey Hub clients that should be dropped, 1
(turnkey-keys) goes with the sources it exists to verify, and 24 are
webmin and its modules, which is a product decision before it is a
packaging one. Full table and plan per package: #7.

Removing a source the build still needs is worse than leaving it, so that
is a separate change.

Overlap with #5, and how to resolve it

This touches lines #5 also touches. #5 was read before these edits.
Trial-merged locally; three files conflict and every resolution is
mechanical. I have run the merged tree.

• mk/turnkey.mk: the real one. #5 replaces the version-string lines
  with a keel-version-files call; this removes the two turnkey_aptconf
  lines beside them. Take #5's call and drop the User-Agent lines:

  release_version=$$(... turnkey-version.py ...); \
  $(COMMON_BIN_PATH)/keel-version-files $$release_version $O/root.patched || exit 1; \
  

  #5's turnkey_version=$$(cat .../etc/turnkey_version) goes too; it only
  existed to build the header this deletes.

• tests/coverage.sh: both branches replace the single-target script
  with the same per-target loop. This branch deliberately uses #5's exact
  loop so the resolution is a union of the targets arrays, five
  entries, nothing else.

• COVERAGE.md: both add a table and prose; keep both.

Merged tree measured, all five files at 100 percent: postfix-local 17/17,
dpkg-vendor 6/6, apt-identity 4/4, version-files.sh 15/15,
keel-version-files 36/36.

Whichever merges first, the other rebases. Happy either way.

One thing #5 does not do and this does: mk/turnkey-desktop.mk is a
second copy of the same root.patched/post block. #5 changes only
mk/turnkey.mk, so desktop appliances would keep writing the old header
and would not get /etc/keel_version. This fixes the header in both; the
keel_version gap in the desktop makefile is #5's to close and is not
touched here.

Tests

33 bats, all three measured files at 100 percent under kcov 43 with
bats 1.11, the threshold the gate is set to:

postfix-local: 100.00 percent (17 of 17 lines) covered, threshold 100
dpkg-vendor:   100.00 percent (6 of 6 lines) covered, threshold 100
apt-identity:  100.00 percent (4 of 4 lines) covered, threshold 100

Both refusals of each new conf script are covered, which is every non-zero
exit either can produce.

Each assertion was mutation-checked rather than assumed: reverting one
deb822 URI to http fails tests 8 and 9; reverting the legacy line fails
test 11; setting Vendor: TurnKey fails 9 of the vendor tests; restoring
the leaking User-Agent fails 4 of the apt tests.

A defect this uncovered

A bare ! cmd in a bats test body asserts nothing. Bash does not apply
errexit to a negated command, so the test passes whatever happens and
execution continues. Measured:

@test "bare negation that should fail" {
    ! grep -q foo <<< "foo bar"     # this does not fail the test
    echo "REACHED THE LINE AFTER" >&3
}
ok 1 bare negation that should fail

Both new suites use run ! cmd with bats_require_minimum_version 1.5.0.
tests/postfix-local.bats has three bare ones (lines 66, 96, 97), of which only line 96 is inert (the other two are last in their tests, and bats takes the last status as the verdict); they are left alone here so this diff stays on its
own subject, and are recorded in COVERAGE.md. It is the same family as
the docs/traps.md entry "A bats suite cannot see a library that kills its
caller".

Test plan

• COVERAGE_THRESHOLD=100 tests/coverage.sh: three files, all 100 percent
• the same on the tree merged with #5: five files, all 100 percent
• shellcheck -S warning clean on every file this adds or changes
• https on archive.turnkeylinux.org measured before the scheme was changed
• the User-Agent read off the wire from a real apt-get update, http and TLS
• the vendor read from the real dpkg-vendor
• every assertion mutation-checked
• a core layer built on the build host, to see 01keel in the image,
  no 01turnkey, and dpkg-vendor --query Vendor answering Keel on a
  booted appliance. Not run: the build host's core.tar.zst is in a
  known-inconsistent state and a repair is pending, so the build lock
  was deliberately not taken.

appliance / build-and-boot boots the layer the mirror already serves, not
this branch, so it cannot prove a change to the core layer either way
(Keel-Linux/.github#11).

Related

• Keel-Linux/inithooks#14 removes the remote JavaScript from the first
  boot page, and with it the only reader of the 01turnkey this deletes
  (29tagid parsed the build tag out of that User-Agent).
  bin/secalerts.sh reads the same file but is guarded and falls back to
  turnkey-version; measured that it survives the file's removal.
• #7 is the migration this deliberately does not do.
• #5 overlaps; see above.

Org-wide context: Keel-Linux/tracker#15.

Three of the four ways a Keel appliance still reported to TurnKey. The fourth, the apt suites themselves, is measured here and deliberately not touched: see "What this does not do". Closes #6. ## The dpkg vendor `/etc/dpkg/origins/default` resolved to a `TurnKey` file, so `dpkg-vendor` answered TurnKey and every bug reporting tool on the appliance addressed `turnkeylinux/tracker`. The overlay now ships a `Keel` file and no TurnKey one, and `conf/turnkey.d/dpkg-vendor` points `default` at it. `Parent: Debian` is kept. That matters: there is no `Dpkg::Vendor::Keel` perl module, so `dpkg-parsechangelog` and `dpkg-buildpackage` fall back to the parent's vendor object, which is the same `Dpkg::Vendor::Debian` they got through TurnKey before. Package building is unaffected. **How it is asserted.** Not by reading back the symlink the script wrote. `tests/dpkg-vendor.bats` arranges the origins tree the overlay ships, runs the conf script, and then asks the **real `dpkg-vendor`** what it makes of it, through dpkg's own `DPKG_ORIGINS_DIR` (`Dpkg::Vendor` honours it): `--query Vendor` answers `Keel`, `--query Bugs` answers our tracker, `--is TurnKey` fails, `--is Keel` succeeds, `--derives-from Debian` succeeds and `--derives-from Ubuntu` fails. The upgrade path is covered too: a `default` inherited as a symlink to TurnKey, as a regular file, and as a directory are each replaced, and the vendor query is what says so. ## The apt User-Agent `mk/turnkey.mk` and `mk/turnkey-desktop.mk` each wrote a per-appliance `/etc/apt/apt.conf.d/01turnkey`: Acquire::http::User-Agent "TurnKey APT-HTTP/1.3 (turnkey-wordpress-19.0-trixie-amd64)"; so `deb.debian.org`, `security.debian.org`, every mirror in between and anyone watching the connection were told which appliance this machine is and which version it runs, on every apt run it ever made. It is now a fixed file the overlay ships, `01keel`, naming the distribution and nothing else. Measured: apt's https method reads the same `Acquire::http::User-Agent`, so one line covers both schemes. `conf/turnkey.d/apt-identity` exists for a reason worth stating. apt reads `apt.conf.d` in lexical order and the last assignment of a scalar wins, so a `01turnkey` inherited from a parent layer built before this change **silently beats** the `01keel` beside it: $ ls apt.conf.d/ # 01keel and a stale 01turnkey $ apt-config dump Acquire::http::User-Agent Acquire::http::User-Agent "TurnKey APT-HTTP/1.3 (turnkey-wordpress-19.0-trixie-amd64)"; The script removes it, and refuses if `01keel` is not there. **How it is asserted.** Off the wire. `tests/ua-recorder.py` is a local server that records the `User-Agent` header of every request; a real `apt-get update` is pointed at it and the test reads what apt actually sent, over http and over TLS. So the verdict is apt's behaviour, not the file's content. One test drives the stale-`01turnkey` hazard both ways: it measures TurnKey's header going out, runs the conf script, and measures Keel's. The strings the old header leaked each have their own refutation: the sent header contains no `(`, no `TurnKey`, no `turnkey`, no `wordpress`, no `trixie`, no `amd64` and no `19.0`. ## https on the TurnKey archive **It is available.** Measured 2026-09-28: $ curl -sS -o /dev/null -D - https://archive.turnkeylinux.org/debian/dists/trixie/Release HTTP/2 200 $ curl -sS -o /dev/null -w 'code=%{http_code} ssl_verify=%{ssl_verify_result}\n' \ https://archive.turnkeylinux.org/debian/dists/trixie/Release code=200 ssl_verify=0 `ssl_verify=0` is a certificate that verifies against the system trust store. `trixie-security` and `trixie-testing` answer 200 over https as well. So all six places `conf/bootstrap_apt` wrote `http://archive.turnkeylinux.org` (the three deb822 stanzas and the three legacy `sources.list` lines still written for pre-Trixie releases) move to https. The Debian sources beside them stay on http in this change, for one reason: that is Debian's own default and the payload is signed either way. (An earlier version of this description also said the build host's caching proxy only feeds http; it does not, squid there bumps https too, so that is not a reason.) The request stream over http still tells the path which packages at which versions an appliance fetches, the same confidentiality question the User-Agent change answers, so flipping the Debian URIs is worth its own change and is recorded in #7. **How it is asserted.** The test extracts the heredoc bodies from `conf/bootstrap_apt` itself, renders them with a build's variables, so the bytes under test are the generator's own output, not a copy in the test; writes them into a scratch apt tree and asks **apt** with `apt-get indextargets` which URIs it would fetch. No network. The verdict is apt's resolution of our sources: nothing under `http://archive.turnkeylinux.org`, and all three suites present under `https://`. The Debian URIs are asserted unchanged so the scope of the change is visible in the suite. ## What this does not do **It does not remove the TurnKey apt suites.** The measurement #6 asked for is done and it says the suites are load bearing: - The suites offer 141 package names (from 35 source packages; `webmin` alone is 103 of them). Measured on the `wordpress-demo` container (which never went through the container patch): **39 installed packages have their apt candidate in the TurnKey archive** (41 of the 141 names are installed; `confconsole` and `inithooks` are the other two, served by `archive.keellinux.org` at 1001). The count is per appliance: on `forum` it is 32. **None is available from Debian at all.** - `apt-cache policy` over every installed package finds **0** where the TurnKey archive overrides Debian today. - `trixie-testing` is `Enabled: no` on a built appliance, so two suites are enabled, not three. - `trixie-security` from TurnKey carries **exactly one package**, `tklbam`, at the version already installed. - There is also `/etc/apt/preferences` pinning `o=turnkeylinux` at **999**, above Debian's 500, from `overlays/bootstrap_apt/etc/apt/preferences`. It shadows nothing today; it would the day a name collides. Removing it is a change to how every appliance resolves packages, so by BRIEF section 12 (preserve upstream behaviour when in doubt) it goes with the migration in #7 rather than riding here. Of the 39: **7** are appliance infrastructure that must move to our pool, **7** are TurnKey Hub clients that should be dropped, **1** (`turnkey-keys`) goes with the sources it exists to verify, and **24** are `webmin` and its modules, which is a product decision before it is a packaging one. Full table and plan per package: **#7**. Removing a source the build still needs is worse than leaving it, so that is a separate change. ## Overlap with #5, and how to resolve it **This touches lines #5 also touches.** #5 was read before these edits. Trial-merged locally; three files conflict and every resolution is mechanical. I have run the merged tree. - **`mk/turnkey.mk`**: the real one. #5 replaces the version-string lines with a `keel-version-files` call; this removes the two `turnkey_aptconf` lines beside them. Take #5's call and drop the User-Agent lines: release_version=$$(... turnkey-version.py ...); \ $(COMMON_BIN_PATH)/keel-version-files $$release_version $O/root.patched || exit 1; \ #5's `turnkey_version=$$(cat .../etc/turnkey_version)` goes too; it only existed to build the header this deletes. - **`tests/coverage.sh`**: both branches replace the single-target script with the same per-target loop. This branch deliberately uses **#5's exact loop** so the resolution is a union of the `targets` arrays, five entries, nothing else. - **`COVERAGE.md`**: both add a table and prose; keep both. Merged tree measured, all five files at 100 percent: `postfix-local` 17/17, `dpkg-vendor` 6/6, `apt-identity` 4/4, `version-files.sh` 15/15, `keel-version-files` 36/36. Whichever merges first, the other rebases. Happy either way. One thing #5 does not do and this does: **`mk/turnkey-desktop.mk` is a second copy** of the same `root.patched/post` block. #5 changes only `mk/turnkey.mk`, so desktop appliances would keep writing the old header and would not get `/etc/keel_version`. This fixes the header in both; the `keel_version` gap in the desktop makefile is #5's to close and is not touched here. ## Tests 33 bats, all three measured files at **100 percent** under kcov 43 with bats 1.11, the threshold the gate is set to: postfix-local: 100.00 percent (17 of 17 lines) covered, threshold 100 dpkg-vendor: 100.00 percent (6 of 6 lines) covered, threshold 100 apt-identity: 100.00 percent (4 of 4 lines) covered, threshold 100 Both refusals of each new conf script are covered, which is every non-zero exit either can produce. Each assertion was mutation-checked rather than assumed: reverting one deb822 URI to http fails tests 8 and 9; reverting the legacy line fails test 11; setting `Vendor: TurnKey` fails 9 of the vendor tests; restoring the leaking User-Agent fails 4 of the apt tests. ### A defect this uncovered A bare `! cmd` in a bats test body **asserts nothing**. Bash does not apply errexit to a negated command, so the test passes whatever happens and execution continues. Measured: @test "bare negation that should fail" { ! grep -q foo <<< "foo bar" # this does not fail the test echo "REACHED THE LINE AFTER" >&3 } ok 1 bare negation that should fail Both new suites use `run ! cmd` with `bats_require_minimum_version 1.5.0`. `tests/postfix-local.bats` has three bare ones (lines 66, 96, 97), of which only line 96 is inert (the other two are last in their tests, and bats takes the last status as the verdict); they are left alone here so this diff stays on its own subject, and are recorded in `COVERAGE.md`. It is the same family as the `docs/traps.md` entry "A bats suite cannot see a library that kills its caller". ## Test plan - [x] `COVERAGE_THRESHOLD=100 tests/coverage.sh`: three files, all 100 percent - [x] the same on the tree merged with #5: five files, all 100 percent - [x] `shellcheck -S warning` clean on every file this adds or changes - [x] https on `archive.turnkeylinux.org` measured before the scheme was changed - [x] the User-Agent read off the wire from a real `apt-get update`, http and TLS - [x] the vendor read from the real `dpkg-vendor` - [x] every assertion mutation-checked - [ ] a core layer built on the build host, to see `01keel` in the image, no `01turnkey`, and `dpkg-vendor --query Vendor` answering Keel on a booted appliance. Not run: the build host's `core.tar.zst` is in a known-inconsistent state and a repair is pending, so the build lock was deliberately not taken. `appliance / build-and-boot` boots the layer the mirror already serves, not this branch, so it cannot prove a change to the core layer either way (Keel-Linux/.github#11). ## Related - **Keel-Linux/inithooks#14** removes the remote JavaScript from the first boot page, and with it the only reader of the `01turnkey` this deletes (`29tagid` parsed the build tag out of that User-Agent). `bin/secalerts.sh` reads the same file but is guarded and falls back to `turnkey-version`; measured that it survives the file's removal. - **#7** is the migration this deliberately does not do. - **#5** overlaps; see above. Org-wide context: Keel-Linux/tracker#15.

marcos-mendez commented 2026-09-28 03:29:42 -03:00 (Migrated from github.com)

Copy link

Review of fix/keel-apt-identity. Findings ordered by severity, each says whether I got it by running something or by reading. No CRITICAL and no HIGH.

The measurement, re-derived rather than accepted

Run read-only with lxc-attach on the build host, on two containers, because the numbers decide how urgent the rest of the migration is.

claim	re-derived
two TurnKey suites enabled, not three	holds. turnkey-testing.sources is Enabled: no on both containers
the suites offer 141 names	holds exactly. 141 unique Package: across trixie binary-amd64 + binary-all and trixie-security binary-amd64
none of the installed ones is available from Debian at all	holds, and it is stronger than stated: of all 141 names, exactly one (deck) exists in Debian trixie, and it is not installed
apt-cache policy over every installed package finds 0 where TurnKey overrides Debian	holds. 0 of 533 on wordpress-demo, 0 of 814 on forum
trixie-security carries exactly one package, tklbam, at the installed version	holds. Installed 1.5.3+2+g31134d3, candidate the same, from trixie-security

So the conclusion stands: the suites are load bearing and removing them today would remove packages nothing replaces. Three corrections to how it is stated are below.

MEDIUM — overlays/bootstrap_apt/etc/apt/preferences:1-3: the 999 pin should go with this change, not wait for #7

Verified in the overlay and on both live containers:

Package: *
Pin: release o=turnkeylinux
Pin-Priority: 999

999 is above Debian's 500 and below the 1000 that would permit a downgrade. Its entire effect is: for any package name both archives offer, apt takes TurnKey's version even when Debian's is newer, including when Debian's newer version is the security fix. There is no log line for that and nothing in keel verify that would notice.

The scenario: TurnKey publishes a binary whose name collides with a Debian one — a patched nginx, a curl, anything — and from the next apt-get update every Keel appliance silently prefers it, on a signature the project does not control, for as long as the source stays.

This PR's own measurement is what makes removing it free: 0 installed packages depend on the override, and only deck of the 141 collides with a Debian name at all. Dropping the file, or lowering it under 500, changes nothing measurable today and removes the mechanism entirely. /etc/apt/preferences.d/keel at 1001 is unaffected either way. Leaving it is the one place in this diff where the remaining migration is made to carry something that does not need to wait for it.

MEDIUM — the stated reason for leaving the Debian sources on http is not true of this build host

The body says the Debian mirrors stay on http because "the build host reaches them through a caching proxy (squid_proxyCA.crt ...) that http is what feeds". Measured, read-only, /etc/squid/squid.conf on the build host:

23: http_port 127.0.0.1:3128 ssl-bump cert=/etc/squid/cert/squid_proxyCA.pem generate-host-certificates=on ...
24: ssl_bump bump all

The proxy bumps and caches https as well as http — that is what squid_proxyCA.crt is for, and conf/bootstrap_apt:134-136 already runs update-ca-certificates so the bootstrap trusts it. Flipping the Debian URIs would cost no caching on this host. The remaining reason, "that is Debian's own default", is a real reason; the proxy one is not, and a justification that is wrong in a merged body is the reason nobody revisits the decision.

Worth weighing while it is open, because it is the same leak this PR is closing: the old header told every mirror turnkey-wordpress-19.0-trixie-amd64. Over plain http the request stream says the same thing — wordpress, apache2, php8.4, webmin, at exact versions — to anyone on the path, and firstboot.d/95secupdates plus cron-apt make that happen on every appliance with no operator present. Signature verification is unaffected either way; this is confidentiality, which is precisely what the User-Agent change was about. I would either flip Debian here or record the decision in docs/decisions/ with the reason that survives.

I checked the obvious way this could have broken the build and it does not: /turnkey/fab/bootstraps/trixie-amd64 has ca-certificates, openssl and a populated /etc/ssl/certs/ca-certificates.crt, so the first apt-get update of a build can verify https before anything is installed.

MEDIUM — "39 installed" needs its definition and its container written next to it

Both are load bearing and neither is in the body.

Definition. On wordpress-demo, 41 of the 141 names are installed. 39 is the count whose apt candidate comes from archive.turnkeylinux.org; confconsole and inithooks are the other two, and they are excluded only because archive.keellinux.org already serves them at priority 1001. Both numbers are defensible, they answer different questions, and #7 will plan from whichever is written down.

Container. The measurement was taken on wordpress-demo, which never went through the container patch — /sbin/modprobe is the real one (-> ../bin/kmod), /etc/fstab is present, and /etc/default/inithooks has REDIRECT_OUTPUT=false. That is docs/traps.md entry 1, and it makes the container atypical of a shipped appliance.

It does not move any conclusion here, which I checked rather than assumed: on forum, which did go through the patch (modprobe -> /bin/true, no /etc/fstab, REDIRECT_OUTPUT=true), the count is 32 — a different appliance, so a different set — and every other line of the table is identical, including the 0 overrides and the single trixie-security package. So: the count is per appliance, not a property of the distribution, and the sentence should say which appliance and which container it came from.

MEDIUM — COVERAGE.md:48-52 and the body: the recorded bats defect is wrong in two of its three cases

The rule as written — "bash does not apply errexit to a negated command, so a bare one passes whatever happens" — is missing its exception. bats takes the last command's status as the test's verdict, so a bare ! cmd in final position does assert. Measured with bats 1.11.1:

ok 1 bare negation NOT last: asserts nothing
not ok 2 bare negation AS LAST command: does assert

Applied to tests/postfix-local.bats, only line 96 is inert. Line 66 is the last command of "fails when port 25 is in use on IPv6 only" and line 97 is the last command of "stops at the first failing postconf"; both decide their tests. shellcheck agrees and grades them differently — SC2314 is an error on 96 and only a note on 66 and 97:

tests/postfix-local.bats:66:5: note:  ... will not fail the test if it is not the last command
tests/postfix-local.bats:96:5: error: ... does not cause a test failure
tests/postfix-local.bats:97:5: note:  ... will not fail the test if it is not the last command

So the sentence should read "one of them (line 96)". run ! everywhere is still the right convention — it asserts wherever it stands — but an over-broad rule in COVERAGE.md is the kind of thing that gets applied as written later.

The sweep the record implies is worth doing, and it is much larger than this repository: inithooks has 20 inert bare negations, 17 of them in tests/test-ipconfig.bats, where 11 of the 13 rejection cases of ipconfig_ip6_syntax assert nothing. Full list handed over separately; it wants its own issue rather than either of these branches.

LOW — conf/turnkey.d/dpkg-vendor:26-27 leaves an inherited TurnKey origin file on the image

The script replaces default and never removes the file it used to point at. fab-apply-overlay only adds, so on any layer built on a parent produced before this change, /etc/dpkg/origins/TurnKey stays on disk — I see it on both containers, one of them dated Nov 2023. It is inert for a vendor query, which is why this is LOW, but it is asymmetric with the sibling script: conf/turnkey.d/apt-identity:22 removes the stale 01turnkey for exactly this reason, and removelists-final/turnkey:2 already removes /etc/apt/apt.conf.d/01proxy the same way. One line in either place closes it, and tests/dpkg-vendor.bats:113 currently asserts the file is not shipped, which is not the same as not present.

LOW — the removal is a property of a build, not of an installed machine

01keel arrives by overlay and 01turnkey is deleted by a conf script, so both happen only while a layer is being built. An appliance already deployed and upgraded through apt keeps its 01turnkey and keeps announcing itself for the life of the machine. That is a reasonable scope for an image-based project, but "upgrade path" in the body means "a build on a stale parent layer", and it is worth saying so — the lexical-order hazard itself I confirmed independently:

$ APT_CONFIG=... apt-config dump Acquire::http::User-Agent    # 01keel + stale 01turnkey
Acquire::http::User-Agent "TurnKey APT-HTTP/1.3 (turnkey-wordpress-19.0-trixie-amd64)";

On forum the stale header also carries the build type — ... -amd64 proxmox) — so the leak is one field wider than the body shows.

LOW — the unit the remaining migration should be measured in

141 is a count of binaries. They come from 35 source packages, and webmin (103 binaries) plus webmin-tklbam (1) are 104 of the 141. Every other source but three produces one binary. The body already says the webmin block is one product decision; saying it in sources rather than binaries would stop #7 from reading as roughly four times the work it is.

LOW — https adds an availability dependency the payload did not have

archive.turnkeylinux.org answers HTTP/2 200 with a verifying certificate on the Release of all three suites — confirmed, ssl_verify=0, CN=turnkeylinux.org from Google Trust Services, valid to 2026-12-17, fronted by Cloudflare. The gain is real and worth having. The cost, which is not in the body: the archive is signed either way, so https buys confidentiality and nothing for integrity, while an expired certificate, a broken chain or a wrong clock now turns apt-get update into a hard failure where http would have carried on. 95secupdates runs that on every first boot. Not a reason to revert — a reason for the first-boot hook's failure mode to be worth a look.

Checked and found right

• Parent: Debian is not a convenience, it is required. Read Dpkg/Vendor.pm: get_vendor_object tries Dpkg::Vendor::Keel, the require fails, and lines 197-202 fall back to the parent's object — Dpkg::Vendor::Debian, the same one TurnKey resolved to. Drop Parent: and it falls to Dpkg::Vendor::Default instead, which loses Debian's update-buildflags hook, its hardening defaults and its changelog handling. The comment should probably say that outright, because the field looks droppable and is not.
• tests/dpkg-vendor.bats 15/15 and tests/apt-identity.bats 11/11 pass here, on dpkg 1.22.22, apt from trixie, bats 1.11.1, and the TLS test does exercise the https path — /usr/lib/apt/methods/https is a symlink to http, so the single Acquire::http::User-Agent genuinely covers both.
• The removal does happen on a build over a stale parent: product.mk's root.patched/body applies every overlays/turnkey.d/* first and then runs every conf/turnkey.d/*, on every product build, so 01keel is in place before apt-identity looks for it and the inherited 01turnkey is gone before root.patched/post.
• mk/turnkey-desktop.mk really is a second copy that #5 misses.
• shellcheck -S warning clean on conf/turnkey.d/apt-identity, conf/turnkey.d/dpkg-vendor, conf/bootstrap_apt and tests/coverage.sh.
• All six archive.turnkeylinux.org occurrences in conf/bootstrap_apt moved (208, 225, 251, 261, 268, 279); the NO_TURNKEY_APT_REPO comment-out at line 291 still matches, since its pattern carries no scheme.
• Both conf scripts fail loudly rather than silently when their overlay file is missing, which is the right choice for something whose failure would otherwise be invisible until an image shipped.

One security matter outside both diffs came out of reading the first-boot path. It is not introduced here and these repositories are public, so I have raised it separately rather than in this thread.

Approve. The change is correct, the assertions are taken from behaviour rather than from the files under test, and the measurement's conclusions survive being re-derived. Everything above is MEDIUM or below; the pin is the one I would fold in before merge, and the two inaccurate sentences (the proxy justification and the three bats line numbers) are worth fixing in place, because both will be read later as settled fact.

Review of `fix/keel-apt-identity`. Findings ordered by severity, each says whether I got it by running something or by reading. No CRITICAL and no HIGH. ## The measurement, re-derived rather than accepted Run read-only with `lxc-attach` on the build host, on two containers, because the numbers decide how urgent the rest of the migration is. | claim | re-derived | | --- | --- | | two TurnKey suites enabled, not three | holds. `turnkey-testing.sources` is `Enabled: no` on both containers | | the suites offer 141 names | holds exactly. 141 unique `Package:` across trixie `binary-amd64` + `binary-all` and `trixie-security` `binary-amd64` | | none of the installed ones is available from Debian at all | holds, and it is stronger than stated: of all **141** names, exactly **one** (`deck`) exists in Debian trixie, and it is not installed | | `apt-cache policy` over every installed package finds 0 where TurnKey overrides Debian | holds. 0 of 533 on `wordpress-demo`, 0 of 814 on `forum` | | `trixie-security` carries exactly one package, `tklbam`, at the installed version | holds. Installed `1.5.3+2+g31134d3`, candidate the same, from `trixie-security` | So the conclusion stands: the suites are load bearing and removing them today would remove packages nothing replaces. Three corrections to how it is *stated* are below. ## MEDIUM — `overlays/bootstrap_apt/etc/apt/preferences:1-3`: the 999 pin should go with this change, not wait for #7 Verified in the overlay and on both live containers: Package: * Pin: release o=turnkeylinux Pin-Priority: 999 999 is above Debian's 500 and below the 1000 that would permit a downgrade. Its entire effect is: for any package name both archives offer, apt takes TurnKey's version **even when Debian's is newer**, including when Debian's newer version is the security fix. There is no log line for that and nothing in `keel verify` that would notice. The scenario: TurnKey publishes a binary whose name collides with a Debian one — a patched `nginx`, a `curl`, anything — and from the next `apt-get update` every Keel appliance silently prefers it, on a signature the project does not control, for as long as the source stays. This PR's own measurement is what makes removing it free: 0 installed packages depend on the override, and only `deck` of the 141 collides with a Debian name at all. Dropping the file, or lowering it under 500, changes nothing measurable today and removes the mechanism entirely. `/etc/apt/preferences.d/keel` at 1001 is unaffected either way. Leaving it is the one place in this diff where the remaining migration is made to carry something that does not need to wait for it. ## MEDIUM — the stated reason for leaving the Debian sources on http is not true of this build host The body says the Debian mirrors stay on http because "the build host reaches them through a caching proxy (`squid_proxyCA.crt` ...) that http is what feeds". Measured, read-only, `/etc/squid/squid.conf` on the build host: 23: http_port 127.0.0.1:3128 ssl-bump cert=/etc/squid/cert/squid_proxyCA.pem generate-host-certificates=on ... 24: ssl_bump bump all The proxy bumps and caches https as well as http — that is what `squid_proxyCA.crt` is *for*, and `conf/bootstrap_apt:134-136` already runs `update-ca-certificates` so the bootstrap trusts it. Flipping the Debian URIs would cost no caching on this host. The remaining reason, "that is Debian's own default", is a real reason; the proxy one is not, and a justification that is wrong in a merged body is the reason nobody revisits the decision. Worth weighing while it is open, because it is the same leak this PR is closing: the old header told every mirror `turnkey-wordpress-19.0-trixie-amd64`. Over plain http the **request stream** says the same thing — `wordpress`, `apache2`, `php8.4`, `webmin`, at exact versions — to anyone on the path, and `firstboot.d/95secupdates` plus cron-apt make that happen on every appliance with no operator present. Signature verification is unaffected either way; this is confidentiality, which is precisely what the User-Agent change was about. I would either flip Debian here or record the decision in `docs/decisions/` with the reason that survives. I checked the obvious way this could have broken the build and it does not: `/turnkey/fab/bootstraps/trixie-amd64` has `ca-certificates`, `openssl` and a populated `/etc/ssl/certs/ca-certificates.crt`, so the first `apt-get update` of a build can verify https before anything is installed. ## MEDIUM — "39 installed" needs its definition and its container written next to it Both are load bearing and neither is in the body. *Definition.* On `wordpress-demo`, **41** of the 141 names are installed. **39** is the count whose apt *candidate* comes from `archive.turnkeylinux.org`; `confconsole` and `inithooks` are the other two, and they are excluded only because `archive.keellinux.org` already serves them at priority 1001. Both numbers are defensible, they answer different questions, and #7 will plan from whichever is written down. *Container.* The measurement was taken on `wordpress-demo`, which never went through the container patch — `/sbin/modprobe` is the real one (`-> ../bin/kmod`), `/etc/fstab` is present, and `/etc/default/inithooks` has `REDIRECT_OUTPUT=false`. That is `docs/traps.md` entry 1, and it makes the container atypical of a shipped appliance. It does not move any conclusion here, which I checked rather than assumed: on `forum`, which did go through the patch (`modprobe -> /bin/true`, no `/etc/fstab`, `REDIRECT_OUTPUT=true`), the count is **32** — a different appliance, so a different set — and every other line of the table is identical, including the 0 overrides and the single `trixie-security` package. So: the count is per appliance, not a property of the distribution, and the sentence should say which appliance and which container it came from. ## MEDIUM — `COVERAGE.md:48-52` and the body: the recorded bats defect is wrong in two of its three cases The rule as written — "bash does not apply errexit to a negated command, so a bare one passes whatever happens" — is missing its exception. bats takes the **last** command's status as the test's verdict, so a bare `! cmd` in final position does assert. Measured with bats 1.11.1: ok 1 bare negation NOT last: asserts nothing not ok 2 bare negation AS LAST command: does assert Applied to `tests/postfix-local.bats`, only **line 96** is inert. Line 66 is the last command of "fails when port 25 is in use on IPv6 only" and line 97 is the last command of "stops at the first failing postconf"; both decide their tests. shellcheck agrees and grades them differently — SC2314 is an **error** on 96 and only a **note** on 66 and 97: tests/postfix-local.bats:66:5: note: ... will not fail the test if it is not the last command tests/postfix-local.bats:96:5: error: ... does not cause a test failure tests/postfix-local.bats:97:5: note: ... will not fail the test if it is not the last command So the sentence should read "one of them (line 96)". `run !` everywhere is still the right convention — it asserts wherever it stands — but an over-broad rule in `COVERAGE.md` is the kind of thing that gets applied as written later. The sweep the record implies is worth doing, and it is much larger than this repository: `inithooks` has 20 inert bare negations, 17 of them in `tests/test-ipconfig.bats`, where 11 of the 13 rejection cases of `ipconfig_ip6_syntax` assert nothing. Full list handed over separately; it wants its own issue rather than either of these branches. ## LOW — `conf/turnkey.d/dpkg-vendor:26-27` leaves an inherited `TurnKey` origin file on the image The script replaces `default` and never removes the file it used to point at. `fab-apply-overlay` only adds, so on any layer built on a parent produced before this change, `/etc/dpkg/origins/TurnKey` stays on disk — I see it on both containers, one of them dated Nov 2023. It is inert for a vendor query, which is why this is LOW, but it is asymmetric with the sibling script: `conf/turnkey.d/apt-identity:22` removes the stale `01turnkey` for exactly this reason, and `removelists-final/turnkey:2` already removes `/etc/apt/apt.conf.d/01proxy` the same way. One line in either place closes it, and `tests/dpkg-vendor.bats:113` currently asserts the file is not *shipped*, which is not the same as not *present*. ## LOW — the removal is a property of a build, not of an installed machine `01keel` arrives by overlay and `01turnkey` is deleted by a conf script, so both happen only while a layer is being built. An appliance already deployed and upgraded through apt keeps its `01turnkey` and keeps announcing itself for the life of the machine. That is a reasonable scope for an image-based project, but "upgrade path" in the body means "a build on a stale parent layer", and it is worth saying so — the lexical-order hazard itself I confirmed independently: $ APT_CONFIG=... apt-config dump Acquire::http::User-Agent # 01keel + stale 01turnkey Acquire::http::User-Agent "TurnKey APT-HTTP/1.3 (turnkey-wordpress-19.0-trixie-amd64)"; On `forum` the stale header also carries the build type — `... -amd64 proxmox)` — so the leak is one field wider than the body shows. ## LOW — the unit the remaining migration should be measured in 141 is a count of binaries. They come from **35 source packages**, and `webmin` (103 binaries) plus `webmin-tklbam` (1) are **104 of the 141**. Every other source but three produces one binary. The body already says the webmin block is one product decision; saying it in sources rather than binaries would stop #7 from reading as roughly four times the work it is. ## LOW — https adds an availability dependency the payload did not have `archive.turnkeylinux.org` answers `HTTP/2 200` with a verifying certificate on the `Release` of all three suites — confirmed, `ssl_verify=0`, CN=turnkeylinux.org from Google Trust Services, valid to 2026-12-17, fronted by Cloudflare. The gain is real and worth having. The cost, which is not in the body: the archive is signed either way, so https buys confidentiality and nothing for integrity, while an expired certificate, a broken chain or a wrong clock now turns `apt-get update` into a hard failure where http would have carried on. `95secupdates` runs that on every first boot. Not a reason to revert — a reason for the first-boot hook's failure mode to be worth a look. ## Checked and found right - **`Parent: Debian` is not a convenience, it is required.** Read `Dpkg/Vendor.pm`: `get_vendor_object` tries `Dpkg::Vendor::Keel`, the `require` fails, and lines 197-202 fall back to the **parent's** object — `Dpkg::Vendor::Debian`, the same one TurnKey resolved to. Drop `Parent:` and it falls to `Dpkg::Vendor::Default` instead, which loses Debian's `update-buildflags` hook, its hardening defaults and its changelog handling. The comment should probably say that outright, because the field looks droppable and is not. - `tests/dpkg-vendor.bats` 15/15 and `tests/apt-identity.bats` 11/11 pass here, on dpkg 1.22.22, apt from trixie, bats 1.11.1, and the TLS test does exercise the https path — `/usr/lib/apt/methods/https` is a symlink to `http`, so the single `Acquire::http::User-Agent` genuinely covers both. - The removal does happen on a build over a stale parent: `product.mk`'s `root.patched/body` applies every `overlays/turnkey.d/*` first and then runs every `conf/turnkey.d/*`, on every product build, so `01keel` is in place before `apt-identity` looks for it and the inherited `01turnkey` is gone before `root.patched/post`. - `mk/turnkey-desktop.mk` really is a second copy that #5 misses. - `shellcheck -S warning` clean on `conf/turnkey.d/apt-identity`, `conf/turnkey.d/dpkg-vendor`, `conf/bootstrap_apt` and `tests/coverage.sh`. - All six `archive.turnkeylinux.org` occurrences in `conf/bootstrap_apt` moved (208, 225, 251, 261, 268, 279); the `NO_TURNKEY_APT_REPO` comment-out at line 291 still matches, since its pattern carries no scheme. - Both conf scripts fail loudly rather than silently when their overlay file is missing, which is the right choice for something whose failure would otherwise be invisible until an image shipped. One security matter outside both diffs came out of reading the first-boot path. It is not introduced here and these repositories are public, so I have raised it separately rather than in this thread. **Approve.** The change is correct, the assertions are taken from behaviour rather than from the files under test, and the measurement's conclusions survive being re-derived. Everything above is MEDIUM or below; the pin is the one I would fold in before merge, and the two inaccurate sentences (the proxy justification and the three bats line numbers) are worth fixing in place, because both will be read later as settled fact.

marcos-mendez commented 2026-09-29 00:40:22 -03:00 (Migrated from github.com)

Copy link

b10ae1e removes an inherited /etc/dpkg/origins/TurnKey (asserted through the real dpkg-vendor) and corrects the bats-negation record: only line 96 of postfix-local.bats is inert. The proxy sentence and the definition of "39" are corrected in the description. The 999 pin stays for #7: it changes package resolution on every appliance, and BRIEF section 12 says preserve and record.

b10ae1e removes an inherited `/etc/dpkg/origins/TurnKey` (asserted through the real `dpkg-vendor`) and corrects the bats-negation record: only line 96 of `postfix-local.bats` is inert. The proxy sentence and the definition of "39" are corrected in the description. The 999 pin stays for #7: it changes package resolution on every appliance, and BRIEF section 12 says preserve and record.

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

bug

Something isn't working

  

documentation

Improvements or additions to documentation

  

duplicate

This issue or pull request already exists

  

enhancement

New feature or request

  

good first issue

Good for newcomers

  

help wanted

Extra attention is needed

  

invalid

This doesn't seem right

  

question

Further information is requested

  

wontfix

This will not be worked on

No labels

bug

documentation

duplicate

enhancement

good first issue

help wanted

invalid

question

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Keel-Linux/common!8

No description provided.