packaging: restore the client payload, add manual pages, cover packaging with an install test ai:claude-opus-5 #50

Manually merged
heiko merged 17 commits from cleanup into master 2026-08-18 10:22:01 +02:00
Owner

Eleven commits, starting from the report in #45 and ending in the things found while verifying the fix. Every packaging claim below was checked against a package built with gogogo pack and installed in a container, not inferred from the config.

The reported bug (#45)

cert-proxy-client_1.20.0 stopped creating /etc/cert-proxy/, /etc/cert-proxy/hook and /var/lib/cert-proxy/certs. Confirmed: the move to gogogo packaging (eace6f5) dropped the old debian/ tree without carrying over its .install, .dirs and postinst. Eleven tags are affected, v1.19.1-0.20260524130539 through v1.20.1-0.20260628182040, including v1.20.0.

Fixed in four parts, because it turned out to be four problems:

  • /etc/cert-proxy/hook is shipped again, now as a conffile so local edits survive upgrades, and executable so pointing -hook at it works without a chmod. The old packaging did neither.
  • /var/lib/cert-proxy/certs (0750 root:ssl-cert) and the ssl-cert group come from tmpfiles.d/sysusers.d snippets, replacing the hand-written dpkg-statoverride dance.
  • Those snippets are invoked by a postinst that swallows failure (|| true), so on a host without the helpers the package installed cleanly and did nothing. The client package now depends on systemd | systemd-standalone-sysusers | systemd-sysusers and the tmpfiles equivalent, so dpkg refuses instead.
  • Nothing ever enabled cert-proxy-client.timer: the generated postinst only restarts on upgrade. A fresh install therefore never renewed anything. An after-install fragment presets it, first install only, and only once client-ssl.pem exists.

Also fixed, found on the way

  • Both packages shipped a junk /usr/lib/systemd/system/cert-proxy-*.default, because the systemd/cert-proxy-*.* glob also matched .default. Replaced with the dedicated systemd-units: key.
  • -connect only worked with an explicit scheme. url.Parse("host:4433") yields scheme host, so the client requested host:4433/v1/.... Shorthands now work.
  • The server logged req.URL verbatim under -verbose, and a PKCS12 bundle may carry ?pass=, so passwords reached the journal. Redacted.
  • cert-proxy-client man was intercepted before -help/-version, so -help man printed a manual page.
  • A missing -certbase surfaced as a mkdir failure mid-download; it is now checked at startup. The client deliberately still does not create the store: it creates per-domain directories only, and ownership of the store stays with whoever set it up.

Manual pages

There were none: the .pod sources were never wired into gogogo packaging, so every .deb tripped lintian no-manual-page. Replaced with Markdown + a pinned go-md2man, generated through go generate ./... into tracked gzip -9 pages, and embedded in both binaries behind <binary> man [<section>] [<page>].

Four pages after a section audit: cert-proxy-client(8), cert-proxy-server(8), cert-proxy-clients(5) (the clients/<cn> grammar) and cert-proxy(7) (endpoints, auth model, layout). The old POD had drifted — it documented a -certs flag that never existed and a -servername default that was wrong — so everything was rewritten from the source.

Test coverage

test/packaging/ (build tag packaging, so go test ./... still needs only Go) builds real packages and installs them in a podman container: 14 cases covering the #45 payload, the dependency and its refusal on a systemd-less host, conffile registration, the timer logic, manual pages, clean purge, and the CA payload. make test-packaging; make test-packaging-image builds the systemd-equipped image, which those two cases skip without.

It earned its keep immediately by catching a manpage path collision that made the two packages refuse to co-install.

Notes for review

  • The last three commits are grouped by when the finding appeared rather than by concern; de38366 in particular mixes the server password fix with client and generator changes. Happy to rebase into per-concern commits before merge.
  • Refs #45 — deliberately not written as a closing keyword; close it by hand if you agree it is covered.
  • #48 (the ssl-cert group grants traversal but cannot read keys) is not fixed here, only documented accurately.
  • Pre-existing and untouched: cert-proxy-server.service uses WantedBy=network.target, and the CA helper programs live under /etc as conffiles.

(authored by ai:claude-opus-5)

Eleven commits, starting from the report in #45 and ending in the things found while verifying the fix. Every packaging claim below was checked against a package built with `gogogo pack` and installed in a container, not inferred from the config. ## The reported bug (#45) `cert-proxy-client_1.20.0` stopped creating `/etc/cert-proxy/`, `/etc/cert-proxy/hook` and `/var/lib/cert-proxy/certs`. Confirmed: the move to gogogo packaging (`eace6f5`) dropped the old `debian/` tree without carrying over its `.install`, `.dirs` and `postinst`. Eleven tags are affected, `v1.19.1-0.20260524130539` through `v1.20.1-0.20260628182040`, including `v1.20.0`. Fixed in four parts, because it turned out to be four problems: * `/etc/cert-proxy/hook` is shipped again, now as a **conffile** so local edits survive upgrades, and **executable** so pointing `-hook` at it works without a chmod. The old packaging did neither. * `/var/lib/cert-proxy/certs` (`0750 root:ssl-cert`) and the `ssl-cert` group come from `tmpfiles.d`/`sysusers.d` snippets, replacing the hand-written `dpkg-statoverride` dance. * Those snippets are invoked by a postinst that swallows failure (`|| true`), so on a host without the helpers the package installed cleanly and did nothing. The client package now **depends** on `systemd | systemd-standalone-sysusers | systemd-sysusers` and the tmpfiles equivalent, so dpkg refuses instead. * Nothing ever enabled `cert-proxy-client.timer`: the generated postinst only restarts on *upgrade*. A fresh install therefore never renewed anything. An `after-install` fragment presets it, first install only, and only once `client-ssl.pem` exists. ## Also fixed, found on the way * Both packages shipped a junk `/usr/lib/systemd/system/cert-proxy-*.default`, because the `systemd/cert-proxy-*.*` glob also matched `.default`. Replaced with the dedicated `systemd-units:` key. * `-connect` only worked with an explicit scheme. `url.Parse("host:4433")` yields scheme `host`, so the client requested `host:4433/v1/...`. Shorthands now work. * The server logged `req.URL` verbatim under `-verbose`, and a PKCS12 bundle may carry `?pass=`, so passwords reached the journal. Redacted. * `cert-proxy-client man` was intercepted before `-help`/`-version`, so `-help man` printed a manual page. * A missing `-certbase` surfaced as a `mkdir` failure mid-download; it is now checked at startup. The client deliberately still does **not** create the store: it creates per-domain directories only, and ownership of the store stays with whoever set it up. ## Manual pages There were none: the `.pod` sources were never wired into gogogo packaging, so every `.deb` tripped lintian `no-manual-page`. Replaced with Markdown + a pinned `go-md2man`, generated through `go generate ./...` into tracked `gzip -9` pages, and embedded in both binaries behind `<binary> man [<section>] [<page>]`. Four pages after a section audit: `cert-proxy-client(8)`, `cert-proxy-server(8)`, `cert-proxy-clients(5)` (the `clients/<cn>` grammar) and `cert-proxy(7)` (endpoints, auth model, layout). The old POD had drifted — it documented a `-certs` flag that never existed and a `-servername` default that was wrong — so everything was rewritten from the source. ## Test coverage `test/packaging/` (build tag `packaging`, so `go test ./...` still needs only Go) builds real packages and installs them in a podman container: 14 cases covering the #45 payload, the dependency and its refusal on a systemd-less host, conffile registration, the timer logic, manual pages, clean purge, and the CA payload. `make test-packaging`; `make test-packaging-image` builds the systemd-equipped image, which those two cases skip without. It earned its keep immediately by catching a manpage path collision that made the two packages refuse to co-install. ## Notes for review * The last three commits are grouped by *when the finding appeared* rather than by concern; `de38366` in particular mixes the server password fix with client and generator changes. Happy to rebase into per-concern commits before merge. * Refs #45 — deliberately not written as a closing keyword; close it by hand if you agree it is covered. * #48 (the `ssl-cert` group grants traversal but cannot read keys) is **not** fixed here, only documented accurately. * Pre-existing and untouched: `cert-proxy-server.service` uses `WantedBy=network.target`, and the CA helper *programs* live under `/etc` as conffiles. *(authored by ai:claude-opus-5)*
Packaging moved to gogogo, so the Debian toolchain no longer drives the
Makefiles and nothing but humans does: the CI workflows only run nagonag,
demo/demo calls go build directly, and .gogogo.conf builds via its own
build.commands. What was left had rotted:

  * cmd/.makefile.common injected the version with
    -X go.schlittermann.de/heiko/cert-proxy.git/program.Version, which is
    the wrong module path (.git suffix) and the wrong package
    (internal/program), and Version is a func-initialised var reading
    debug.ReadBuildInfo() -- which -X cannot set at all. The flag was
    inert; both make- and gogogo-built binaries already report the same
    build-info version.

  * distclean ran "rm -rf ${GOPATH}", which wipes the user's whole GOPATH
    when it is set.

  * the CA install globbed lib/*, so a local (gitignored) vars.sh was
    installed along with the tracked files, and everything landed 0755.

Replaced by a single top-level Makefile: build into build/ via
go build ./cmd/..., install into a prefix, install-ca from an explicit
file list with the same modes the .deb uses (0755 scripts, 0644 conf),
plus test/update/man/clean. The documented GOOS=windows make install path
and every target name in README.md are preserved.

The pod2man rules survive as "make man" so the manpage tooling is not
lost with the per-command Makefiles.
Both binaries shipped no manual page at all -- the .pod sources were never
wired into the gogogo packaging path, so every .deb tripped lintian
no-manual-page. Replace them with the go-md2man workflow.

Section audit (1-8): sections 1, 2, 3, 4 and 6 do not apply. Both commands
are administrative, so they are section 8, not 1. Everything under
internal/ is unimportable, so there is no section 3 material. Added
beyond the two command pages:

  * cert-proxy-clients(5) -- the line grammar shared by the per-client
    authorization files and the client's -cnfile: one domain per line,
    "#" to end of line is a comment, surrounding blanks and CR stripped,
    empty lines skipped, literal comparison with no wildcards. Also the
    accepted file name, which is what keeps a common name from escaping
    the directory.

  * cert-proxy(7) -- endpoints, which of them are public and why, query
    parameters, the x-version header, the 401/400/304 behaviour and the
    on-disk layout of both sides.

Canonical sources are man/*.md; man/gen.go renders them through the
pinned go-md2man tool and installs deterministic gzip -9 pages, written
to a temporary file and renamed so a failed run cannot truncate a tracked
page. "go generate ./..." is the entry point.

The pages are embedded in man/man.go and exposed as
"<binary> man [<section>] [<page>]", resolving arguments the way man(1)
does: no argument gives the binary's own page, a digit gives a section's
default page, a bare name is searched across sections with the lowest
section winning, two arguments are explicit. Piped output is raw roff;
on a terminal the page goes to "man -l -", falling back to raw roff with
a warning when man(1) is missing. Viewer, PATH lookup and terminal
detection are injected, so the tests need no pty and start no pager.

Documentation facts were taken from the source, not from the old .pod,
which had drifted: it documented -certs (never existed, it is -certbase),
-servername defaulting to "cert-proxy" (it defaults to empty, meaning the
FQDN connected to), and omitted -shared-hook, -pkcs12-compat and -help.
AGENTS.md also described /v1/list as listing all available domains; it
returns only the requesting client's own authorized domains.

Verified: generation is byte-identical across runs and leaves no diff,
gzip -t passes, groff -man -ww is silent, every page was rendered and
read in full, go test ./... passes, and a real .deb puts each page at
/usr/share/man/man<N>/ with mode 0644 and renders from inside the
extracted package. gzip -9 is required: the default level made lintian
fail with poor-compression-in-manual-page.
The regression behind issue #45 -- packaging quietly stopping to ship
/etc/cert-proxy/hook and to create /var/lib/cert-proxy/certs -- passes
every unit test and every lintian check. Only an install shows it, so
add a test that performs one.

test/packaging/ is guarded by the "packaging" build tag, so "go test
./..." still needs nothing but Go. It builds real packages with gogogo
pack and installs them in a throwaway podman container. Cases:
co-installability of both packages, the issue #45 payload, conffile
registration, absence of the stray .default unit file, manual pages at
their FHS paths, the embedded "man" subcommand, both binaries running,
clean remove and purge, and -- on a prepared image -- the ssl-cert group
and the 0750 certificate store.

Two things this immediately found:

  * The manual pages were mapped into both packages, so dpkg refused to
    co-install them: "trying to overwrite
    /usr/share/man/man5/cert-proxy-clients.5.gz, which is also in package
    cert-proxy-client". Each page is now packaged exactly once. The two
    shared pages travel with the server, where they are configured; a
    client-only host still reads them with "cert-proxy-client man 5|7",
    because every binary embeds all four.

  * debian:trixie-slim ships a dpkg path-exclude for /usr/share/man, which
    silently discards the pages during install, so the test removes that
    configuration before installing.

Cases needing systemd-sysusers and systemd-tmpfiles use a prepared image
built from testdata/Containerfile. The slim image has neither, and the
generated postinst tolerates their absence silently -- which is worth
knowing in itself: on a host without them the package installs
successfully and still creates neither the group nor the store. Those
cases skip, rather than fail, where the Debian archive is unreachable.

Verified: 8 cases pass and the 2 systemd cases skip in this environment;
running the co-installation case against the packages built before the
manpage fix reproduces the dpkg failure, so the test does detect what it
claims to.
The two cases needing systemd-sysusers and systemd-tmpfiles skipped
because the image could not be built here. The cause was not a missing
network but a partly blocked one: port 80 is refused, https works, and
apt in the published debian images points at http://deb.debian.org.

The Containerfile now rewrites the sources to https, which is preferable
anyway, and takes two build arguments for networks that need more:
APT_PROXY for an apt proxy, and APT_VERIFY_PEER=false for an intercepting
one. Archive signatures are still verified in that case, so packages stay
authenticated; only the transport check is relaxed.

Building moved out of the test into "make test-packaging-image", because
the settings are a property of the network the operator is on, not of the
code under test. The test now skips while the image is absent and names
the target to run. On this host:

  make test-packaging-image APT_PROXY=http://localhost:3128/ \
       APT_VERIFY_PEER=false PODMAN_BUILD_FLAGS=--network=host

With the image present all 10 cases pass, including the 0750 root:ssl-cert
store, the ssl-cert group, and "man 8 cert-proxy-client" resolving through
man-db from the installed package.
Issue #45 was only half fixed. The certificate store and the ssl-cert
group are created by a tmpfiles.d and a sysusers.d snippet, and the
generated postinst calls systemd-tmpfiles and systemd-sysusers behind
"|| true". On a host without them -- a container, a chroot, a sysvinit
system -- the package installed with status 0 and created neither, and
the client then failed exactly as reported.

The client package now depends on

    systemd | systemd-standalone-sysusers | systemd-sysusers
    systemd | systemd-standalone-tmpfiles | systemd-tmpfiles

the real package first so a systemd host is not offered a standalone
variant, the standalone binaries next for non-systemd systems, the
virtual names last. This is the form vsftpd and wpasupplicant use. dpkg
now refuses to configure rather than quietly doing nothing.

Only the client gains the dependency; the server's postinst merely
reloads systemd, which is harmless when absent.

Providing -certbase stays outside the binary's scope: the client creates
the per-domain directory below it and never the store itself, so its
ownership and mode remain under the control of whoever set it up.
TestExecute_DoesNotCreateCertbase pins that, and cert-proxy-client(8) now
says so instead of leaving it implicit.

Two packaging cases cover the dependency: one on the Depends field, one
asserting that a host without the helpers leaves the package unconfigured
(state iU) rather than silently broken. The payload cases install with
--force-depends, since they test the payload rather than dependency
resolution. All 12 pass.
A missing -certbase used to surface as

    mkdir /var/lib/cert-proxy/certs/example.com: no such file or directory

from inside the download path: accurate, but it names the per-domain
directory rather than the store, and only after the run has begun and
some domains may already have failed.

Check it once in parseFlags instead, before any work starts:

    -certbase "/var/lib/cert-proxy/certs" does not exist; the certificate
    store is created by the package or by the administrator, not by
    cert-proxy-client

The message says whose job it is, because it is deliberately not ours:
the client creates the per-domain directories and nothing above them.
A path that exists but is not a directory is reported separately.

os.Stat, not Lstat, so a symlinked store keeps working. The check sits at
the end of parseFlags, so man, -help and -version still work on a host
that has no store at all.

cert-proxy-client(8) gains a DIAGNOSTICS entry for the message.
The hook was installed 0644, so pointing -hook at it silently did
nothing useful until the admin remembered the chmod the file itself had
to ask for in a comment. Install it 0755 instead; as shipped it is empty
and exits 0, so an unedited hook is harmless.

This reverses 713885b ("Hook script example should not be executable"),
which made sense while the file was an example installed next to the real
thing. It is now the real thing: a conffile at the path the service unit
refers to.

The mode is set inline on the conffiles entry rather than by making the
source file executable, so the installed mode is stated where the
installation is configured. Ownership stays root:root, so no
dpkg-statoverride is involved.

Dropped the script-not-executable lintian override, which the tag no
longer produces, reworded the comment in the file that demanded the
chmod, updated cert-proxy-client(8), and moved the packaging assertion
from 644 to 755. Verified in a container: the installed hook is
-rwxr-xr-x and runs.
gogogo v0.24.2 refuses payload sources that git does not track (its
issue #78), which removes the silent failure mode: a "CA/" entry no
longer ships the admin's gitignored vars.sh or stray key material, it
fails the build and names the file.

That covers untracked sources but not tracked ones that simply do not
belong under /etc: a directory entry would still install CA/.gitignore
there. Keep listing the four files individually, and note in the config
why, now that the glob merely fails instead of leaking.

Added a packaging case asserting the installed CA tree is exactly
mkssl-pem, mkca, openssl.cnf and vars.sh.example, with no vars.sh, no
.gitignore and no .pem. 13 cases pass.
* -connect accepted only forms carrying a scheme. url.Parse reads
    "host:4433" as scheme "host" with opaque "4433", so the client then
    requested "host:4433/v1/..." instead of talking to that host. A bare
    "host" happened to work, "host:4433" did not. withScheme() supplies
    the scheme before parsing, so both work, and "//host:4433" too.

  * The man subcommand was intercepted before -help and -version, so
    "cert-proxy-client -help man" printed a manual page instead of the
    usage. It now runs after both, and still before CN validation.
    manCommand, duplicated in each command, became man.Command.

  * checkCertbase reported a dangling symlink as "does not exist", which
    is confusing when the operator can see the symlink. Lstat now
    distinguishes the two.

  * The server logged req.URL verbatim under -verbose, and a PKCS12
    bundle may be requested with ?pass=..., so passwords reached the
    journal. redactedURL() replaces the value; the parameter list is a
    map so more can be added.

  * man/gen.go globbed "*.md" without anchoring, so running it from the
    repository root produced AGENTS.gz, GEMINI.gz and README.gz. It now
    only accepts <topic>.<section>.md and fails loudly otherwise.

  * TestRoffReportsDecompressionFailure never reached the decompression
    path: its file was not in the embed set, so it tested ReadFile. The
    gzip path is now covered by injecting damaged data through a new
    pagesFS seam, rather than shipping a corrupt page in the binaries.

  * The comment claiming lowest-section-wins is "the behaviour of man(1)"
    was wrong; man-db uses its configured SECTION order. The rule stays,
    the justification is corrected.
gogogo's generated postinst reloads the manager and, on upgrade only,
restarts the units. Nothing enabled cert-proxy-client.timer, so a fresh
install left it disabled and certificates were never fetched again: the
same silent omission as issue #45, with a slower failure mode.

An after-install fragment presets the timer, mirroring what the retired
Debian postinst did. Only on first install, so an admin who disabled it
keeps it disabled across upgrades, and only once client-ssl.pem exists,
because a timer without credentials just fails daily. systemctl is
probed, since the sysusers/tmpfiles dependency can be satisfied by the
standalone binaries, which do not bring the manager.

Also in this change:

  * TestBothPackagesCoInstall asserted Contains(out, "2"), which the
    version string 1.20.1~0.2026... satisfies on its own. It now counts
    the ii lines and compares installed=2.

  * A case asserting the preset logic reached the postinst and that the
    composed script is valid shell (sh -n).

  * The CA payload comment described gogogo refusing untracked files and
    concluded "hence one entry per file". Since v0.24.2 it omits them
    from directory sweeps instead, and the entry is a sweep again.
A review checked the pages introduced on this branch against the source
rather than against the retired POD they replaced, and found several
claims that were simply untrue.

cert-proxy-client(8):

  * The hook environment was described as "variables of the same names as
    the positional parameters". The variables are DOMAIN, KEYFILE,
    CERTFILE, CHAINFILE, FULLCHAINFILE and BUNDLEFILE -- note
    FULLCHAINFILE, not FULLCHAIN -- and TIMESTAMP is not among them: the
    timestamp is positional only. Someone reading "$TIMESTAMP" in a hook
    would have got an empty string.

  * -passout file: takes the whole file with trailing whitespace removed,
    not its first line. A two-line file yields a two-line password.

  * -passout now warns that the password travels as a query parameter.

  * The certs directory entry no longer implies ssl-cert membership grants
    access to the keys. It grants traversal; the per-domain directories
    are 0700 and the keys 0600 under the client's own umask.

cert-proxy(7):

  * 401 was described as one undistinguishable case with the body
    "unauthorized". There are three distinct bodies, and the one for an
    unlisted domain echoes the client's common name and the domain back.

  * x-version is set in the content handler, so responses rejected by
    authentication or authorization never carry it.

  * Only an over-long path yields 400; an unknown endpoint yields 404 from
    the mux.

  * PKCS12 is stored client-side as bundle.pfx, not under the PEM names.

The same false ssl-cert rationale is removed from the tmpfiles snippet,
which now records that the client refuses to create the store itself.
Build bundle query parameters with url.Values instead of interpolating them
into a URL template. Passwords containing &, %, #, semicolons, spaces or
newlines now round-trip correctly rather than changing the request or making
it unparsable.

Use one fail-closed URL redactor in both binaries. It replaces pass values in
normal queries and suppresses the entire raw query when ParseQuery rejects it,
so malformed escapes and semicolon-separated fields cannot bypass redaction.
The client now uses the safe URL in verbose output, HTTP status errors and
url.Error values without mutating the original request or error.

Tests cover reserved characters, malformed queries, source immutability and
preservation of the underlying network error.
Treat the embedded manual as a subcommand only in the documented
"binary man ..." form. The standard option terminator now escapes a literal
domain named man: "cert-proxy-client -- man". This restores access to a valid
positional domain without making -help or -version ambiguous.

Validate -connect as an HTTP(S) base URL before endpoint construction. Reject
missing hosts, unsupported schemes, user information, queries and fragments;
those last two previously swallowed the appended /v1 endpoint into the wrong
URL component. Keep the host:port shorthand fixed by adding the scheme before
url.Parse.

Make the symlink certbase test portable to Windows runners without symlink
privileges. Correct the command comments and all manual-page wording to match
Go's host/IP Subject Alternative Name verification rather than legacy common
name matching, then regenerate the tracked pages deterministically.
Preset and start cert-proxy-client.timer on every first install, including the
normal install-then-provision workflow. ConditionPathExists makes service runs
skip cleanly until client-ssl.pem arrives. Stop the timer before package
removal while its unit still exists, preserve enablement across ordinary
remove/reinstall, and delete preset state on purge.

Restore four explicit CA payload mappings. A recursive CA/ sweep depended on
tracked-file filtering in recent gogogo versions and could package ignored
configuration, PEM files or CA material with an older builder.

Make package tests independent of release signing credentials by placing a
no-op debsign in their private PATH. Ensure TestMain removes package output
before os.Exit. Execute the generated postinst/prerm/postrm with a fake
systemctl, verify the credential condition, compare the complete CA file and
symlink set, and assert purge removes the dangling timer enablement link.
heiko manually merged commit c3b66ce835 into master 2026-08-18 10:22:01 +02:00
Sign in to join this conversation.
No description provided.