From edaec4a6f2a204acc5830a49bb92a367790b2431 Mon Sep 17 00:00:00 2001 From: Geno Roupsky <185580+groupsky@users.noreply.github.com> Date: Thu, 15 Oct 2026 06:47:13 +0300 Subject: [PATCH] feat(broker): nodkey broker command, start and stop order, systemd unit Wire the broker service, the approval core, the Telegram approver, the stores and the audit log into one daemon, with core dumps off and an example hardened unit. Closes #11 --- AGENTS.md | 5 + DESIGN.md | 219 +++++- THREAT-MODEL.md | 50 +- cmd/nodkey/broker.go | 704 +++++++++++++++++++ cmd/nodkey/broker_linux_test.go | 772 +++++++++++++++++++++ cmd/nodkey/broker_test.go | 362 ++++++++++ cmd/nodkey/brokerenv_linux_test.go | 938 ++++++++++++++++++++++++++ cmd/nodkey/brokerfake_linux_test.go | 583 ++++++++++++++++ cmd/nodkey/brokerkeys.go | 72 ++ cmd/nodkey/harden_linux.go | 52 ++ cmd/nodkey/harden_other.go | 11 + cmd/nodkey/main.go | 4 + contrib/systemd/nodkey.service | 140 ++++ internal/audit/audit.go | 21 + internal/audit/recovery_linux_test.go | 72 ++ internal/store/dir_test.go | 70 ++ internal/store/home.go | 33 +- internal/telegram/api.go | 30 +- internal/telegram/fake_test.go | 7 + internal/telegram/poll.go | 1 + internal/telegram/poll_test.go | 4 + internal/telegram/stop_linux_test.go | 111 +++ internal/telegram/telegram.go | 24 + 23 files changed, 4243 insertions(+), 42 deletions(-) create mode 100644 cmd/nodkey/broker.go create mode 100644 cmd/nodkey/broker_linux_test.go create mode 100644 cmd/nodkey/broker_test.go create mode 100644 cmd/nodkey/brokerenv_linux_test.go create mode 100644 cmd/nodkey/brokerfake_linux_test.go create mode 100644 cmd/nodkey/brokerkeys.go create mode 100644 cmd/nodkey/harden_linux.go create mode 100644 cmd/nodkey/harden_other.go create mode 100644 contrib/systemd/nodkey.service create mode 100644 internal/audit/recovery_linux_test.go create mode 100644 internal/store/dir_test.go create mode 100644 internal/telegram/stop_linux_test.go diff --git a/AGENTS.md b/AGENTS.md index 54506d6..2fb6042 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -11,6 +11,9 @@ Planned. Each v1 issue adds its part. - `cmd/nodkey/` - the one binary. Subcommands: `broker`, `run`, `ssh-agent`, `ssh-grant`, `keygen`, `audit verify`, `version`. + `nodkey broker` starts the broker's parts in a fixed order, and stops + them in a fixed order (DESIGN.md section 15). Its tests run it in a + child process, with a fake Telegram server behind a fake HTTPS proxy. - `internal/` - one package per part (config, transport, audit, approval, telegram, store, sshauth, client parts). Packages do not import `cmd/`. @@ -22,6 +25,8 @@ Planned. Each v1 issue adds its part. - `internal/client/` - the client side: the requests to the broker (config, TLS key, deadline, short code, strict answer), and `nodkey run` (DESIGN.md sections 5 and 15). +- `contrib/systemd/` - the example systemd unit of the broker, with + encrypted credentials and a sandbox (DESIGN.md section 16). - `test/e2e/` - end-to-end tests with `sshd` in a container. Build tag `e2e`. - `.claude/agents/security-reviewer.md` - the security review every change diff --git a/DESIGN.md b/DESIGN.md index aec3321..3b31786 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -733,10 +733,34 @@ Not masked, for example: taking answers, so the polling stops for good (below), and the broker writes an `error` entry with code `poll_conflict` to the audit log. A conflict says that someone else may read the answers, so the broker - does not go on. + does not go on: `nodkey broker` stops, and exits with code 76 (section + 15). The example systemd unit does not start it again after that code, + so a stolen token does not bring a new alert at each restart. +- When the broker stops, it stops the polling first. It cancels the + `getUpdates` call that waits, and closes its connection at once, with a + TCP reset (`SO_LINGER` 0): the kernel drops a request that it has not + sent yet. After that the broker sends no `getUpdates`. So after a + restart, the new broker is the only one that polls. One case remains: + a `getUpdates` request that was already on its way when the broker + stopped. If it reaches Telegram after the new broker started to poll, + Telegram ends the new poll with a conflict. Then the new broker sends + the alert and stops, although no one else uses the token. This needs a + stop at the moment when the old broker sends `getUpdates`, and a delay + on the network that is longer than the time until the new broker + polls. After a crash or an error, the example unit waits 5 seconds + before it starts the broker again (`RestartSec=`). But `systemctl + restart`, and a package upgrade that restarts the broker, start the + new broker as soon as the old one has ended: then only the end of the + stop and the start lie between them. A proxy (below) can also pass on + a request after the broker closed its connection to the proxy. If this + alert comes right after a restart, and no other program uses the + token, start the broker again. - If Telegram refuses `getUpdates` in another way, with a code from 400 to 499 other than 409 and 429, such as 401 for a token that was - revoked, the polling stops for good too. + revoked, the polling stops for good too. After 401 or 404, Telegram + does not accept the bot token, and only a new token helps: `nodkey + broker` exits with code 77, and the example unit does not start it + again. After another code, it exits with 1 (section 15). - The bot token is in the request URL, and Go's HTTP errors include the URL. So no error and no log line of the Telegram code holds Go's error text: an error names the Bot API method and a fixed reason, and at most @@ -753,7 +777,14 @@ Not masked, for example: authority in the system's store, sees the token, and it can fake the answers of Telegram, so also taps. Do not use such a proxy for the broker. At start, the broker logs one line when it uses a proxy: the - proxy's host, never a user name or a password from the setting. + proxy's host, never a user name or a password from the setting. Go + also takes the certificate authorities from other places when + `SSL_CERT_FILE` or `SSL_CERT_DIR` is set; a certificate authority there + sees the token too, as such a proxy does. So the broker logs the name + and the path of each one that is set. Only root can set these + variables for the broker. Every user can read the `Environment=` lines + of a unit (`systemctl show`), so a proxy setting with a password goes + into an `EnvironmentFile=` that only root can read. - The token file must hold one bot token: the bot's ID in digits, `:`, and 30 to 64 letters, digits, `_` or `-`. Space around it, such as a newline at the end, is fine. @@ -781,8 +812,8 @@ Not masked, for example: needs a notice is denied as well. The approval core would still let grants and "never ask" rules with `notify = false` allow requests, but the broker denies every request from then on (503), and `nodkey broker` - stops. On a polling conflict, the broker first writes the `error` entry - with code `poll_conflict`. + stops (section 15). On a polling conflict, the broker first writes the + `error` entry with code `poll_conflict`. - Messages about "never ask" uses can go to a second chat (`notice_chat_id`), so the approval chat stays for approvals only. They are silent (`disable_notification`). At most 20 notices go out in @@ -950,6 +981,13 @@ The request fields are `request_id`, `client`, `kind` (`secrets`, SHA256 fingerprint), `remote_user`, and the claimed `command` and `reason`. +The broker writes `broker_started` when it has passed every check of its +start, just before it starts to poll Telegram and to take requests +(section 15). So it comes before every entry of that run, except a +`log_recovered` entry. It writes `broker_stopped` as the last entry of a +stop in order. A crash, a kill or a power cut leaves no +`broker_stopped`. A start that fails writes neither. + For each request, the broker writes its entries in this order. First `request`. Then one of these: `decision` (and `denied` with cause `approver` after a Deny); `grant_used`; or `denied` (with an `error` entry @@ -1041,8 +1079,11 @@ What the anchors do not find: - On Linux, `chattr +a` on the file stops anyone except root from changing old lines. It does not stop the broker user from appending. For example, it can append a `log_recovered` entry that names the last real entry as - a torn line. The broker does not see this at the next start. The anchor - of that last entry then does not match, so check with every anchor. + a torn line. When that entry is the last one, the next start takes it + as a torn line that a start before recorded, and sends one alert (see + "A torn last line" below). But when more entries are appended after it, + nothing alerts at the start. The anchor of that last real entry then + does not match, so check with every anchor. - If a person moves the log away, the broker starts a new log at entry 1. The journal then has anchors of both logs, and they differ. Use only the anchors since the new log began, and keep the old file. @@ -1058,6 +1099,12 @@ of the last complete line. So the torn line is not part of the chain. But the entry records the length and SHA-256 of the torn bytes, so a change to the torn line, or its removal, is still found. The broker also sends an alert message. So the broker still starts on its own after a power cut. +The alert goes out only once the start has written `broker_started` +(section 15). If that start stops before, such as when its listen address +is not there yet, the log ends in the `log_recovered` entry. The next +start finds it as the last entry, and sends the alert then. When Telegram +cannot be reached yet, the broker tries the alert again until it is out, +or until the broker stops. The broker does not start, and a person must look, when the end of the file has more than 64 KiB without a newline, or when the last complete line @@ -1346,8 +1393,11 @@ private keys, the broker's TLS key, the bot token, the `bws` token and the (`LoadCredentialEncrypted=`). They are encrypted with `systemd-creds encrypt --with-key=host+tpm2`, so they can be opened only on that machine, with its TPM2 chip. A copy of the disk or a backup alone does -not give them. The broker reads them from `$CREDENTIALS_DIRECTORY` at start. -An SSH key stored this way needs no passphrase. +not give them. The broker reads them from `$CREDENTIALS_DIRECTORY` at start: +the config names each one as `${CREDENTIALS_DIRECTORY}/NAME` (section 14), +and the example unit (section 16) has a line for each. +An SSH key stored this way needs no passphrase; the broker refuses a key +with a passphrase. - Without a TPM2 chip, encrypt with `--with-key=host` instead. Then anyone with a full copy of the disk can open them. The docs say this plainly. - Plain files (mode `0600`, owned by the broker user) also work, for tests @@ -1355,6 +1405,11 @@ An SSH key stored this way needs no passphrase. secret file can be read by group or others. - Root on the running broker host can still get everything. That is the trust we place in the broker host. +- The keys and tokens are in the broker's memory, and secret values are + there for a moment. Swap and hibernation write memory to disk, where a + copy of the disk can give them. So use encrypted swap or none, and no + hibernation, on the broker host. The example unit lets the broker use + no swap (`MemorySwapMax=0`, `MemoryZSwapMax=0`). **The `bw` master password.** The Bitwarden Password Manager CLI must be unlocked with the account's master password. An API key alone is not @@ -1680,10 +1735,12 @@ any problem: links to, or next to `node` in one of those directories, also not after its links: bw would keep its state there, not in its home directory. - Each store's program gets its own home directory, `NAME` in the store - directory (section 16). The broker makes it with mode `0700` if it is - missing. If it is there, it must be a directory, not a symbolic link, - owned by the broker user, with mode `0700`. The directories on the way - must pass the same checks as those on the way to a key file. + directory (section 16). The store directory is `stores` in the + directory of `audit_log`, such as `/var/lib/nodkey/stores`. The broker + makes the store directory and each home directory with mode `0700` if + it is missing. If it is there, it must be a directory, not a symbolic + link, owned by the broker user, with mode `0700`. The directories on + the way must pass the same checks as those on the way to a key file. - The audit log may be missing; then the broker makes it. If it is there, it must be a regular file, not a symbolic link, owned by the broker user. Its mode may give only read and write to the owner, such as `0600`: an @@ -1725,6 +1782,92 @@ nodkey audit verify FILE [--anchor SEQ:HASH]... [--anchors FILE|-] nodkey version ``` +`nodkey broker` runs the broker. The default config is +`/etc/nodkey/broker.toml`. Run it as its own user, from a systemd unit; +[contrib/systemd/nodkey.service](contrib/systemd/nodkey.service) is an +example (section 16). It writes its log to stderr, which systemd keeps in +the journal, with the anchors of the audit log (section 9). All its +lines, also an error at start and a wrong argument, come from the logger +of section 10, so no line can look like an anchor. + +It starts in this order. If a step fails, it stops there, and ends what +it started before. + +1. It reads the config and checks the files that the config names + (section 14). +2. It sets its soft and hard `RLIMIT_CORE` to 1 and marks itself not + dumpable (section 16). If it cannot, it does not start. +3. It loads its TLS key, and each SSH key with the `hosts` of its + section. It signs a fixed text with each SSH key and checks the + signature. It refuses two `[key.*]` sections with the same public key. +4. It opens the audit log. Before this step it writes no entry. An alert + about a torn last line (section 9) waits until step 9. So does the + alert of an earlier start that recorded a torn line and stopped before + step 8: the log then ends in its `log_recovered` entry. +5. It makes the store directory (section 14) and each store, then starts + all stores at once, for at most 70 seconds. A store that does not + start, such as a `bw` that cannot unlock, does not stop the broker: + the broker denies the requests for its secrets until the store works + (each request tries again), and the approver gets one alert for all + such stores, at step 9. +6. It makes the broker's service, the Telegram approver (it reads the bot + token) and the approval core. It checks the client pins. +7. It opens the `listen` address. Nothing reads from it yet: a client + that connects waits. So an address that is not there yet, such as at + boot before the private network is up, stops the start here: before + any entry, any call to Telegram and any alert. systemd starts the + broker again later, and no alert goes out each time. +8. It writes the `broker_started` entry. No later step of the start can + fail. +9. It starts to poll Telegram, and sends the alerts that waited. After a + power cut, the host can be up before Telegram can be reached. So when + an alert does not go out, the broker tries again: after 5 seconds, + then twice as long each time, at most 5 minutes, until the alert is + out or the broker stops. Each failed try gets one log line. +10. Only then does it read the connections on the `listen` address and + take requests. + +It stops on SIGTERM or SIGINT, also during the start, when the polling +stops for good (section 7), and when its server fails. It stops in this +order: + +1. The polling: the `getUpdates` call that waits is cancelled and its + connection closed (section 7). From now on no answer counts: every + request that waits for the person is denied. +2. The service: new requests get `503`, and the requests in progress end + and get `503` too (section 10). The broker waits for them, and writes + the `foreign_tap` entries that still wait. +3. The server: it closes the `listen` address and every connection. +4. The alerts of the start: each one gets its first try, also when the + stop began right after the start, but no try again. An alert that did + not go out is lost; the journal says so. +5. The stores: they clear their tokens and session keys. +6. The `broker_stopped` entry. Then the audit log is closed. + +The stop takes at most 80 seconds. A request may need up to 30 seconds +to take its answer, and a try of an alert up to 30 seconds. After 80 seconds the +broker ends without the rest of the stop, and exits with 1. The example +unit gives it 90 seconds (`TimeoutStopSec=`); then systemd kills it. + +`nodkey broker` exits with: + +| code | meaning | +|---|---| +| 0 | stopped by SIGTERM or SIGINT, and the stop was clean | +| 1 | any other end: the start failed, the polling stopped for another reason (such as another refusal of `getUpdates` than 401, 404 or 409), the server failed, or the stop was not clean, such as when the `broker_stopped` entry could not be written | +| 64 | wrong usage | +| 76 | Telegram reported a polling conflict (section 7): the bot token may be stolen. Make a new bot token, then start the broker. | +| 77 | Telegram does not accept the bot token: `getUpdates` got 401 or 404, such as for a revoked token or a bot that is gone. Put a new token into the token file, then start the broker. | + +The example unit lists 64, 76 and 77 in `RestartPreventExitStatus=`: +only a person can fix these, and a new start with the same setup would +only fail again, or call Telegram with the same token, and write +`broker_started` and `broker_stopped`, every few seconds. After code 1, +or a crash, it starts the broker again after 5 seconds. If the cause +stays, such as a wrong config, this repeats until a person fixes it; the +journal says why. A start that fails before step 8 writes no entry and +calls no Telegram. + `nodkey run` exits with the command's own code, or: | code | meaning | @@ -1832,16 +1975,19 @@ text of a line. It exits with: old copy. - `bws` and `bw` write state files in their home directory. Each store gets its own home directory, `NAME` in the store directory, such as - `/var/lib/nodkey/stores/vault`. The store directory is under the - broker's state directory, and only the broker user can write to it; the - example systemd unit makes it. + `/var/lib/nodkey/stores/vault`. The store directory is `stores` in the + directory of the audit log, which is the broker's state directory. It + has mode `0700` and is owned by the broker user, so only that user can + write to it (section 14). The broker makes it if it is missing; the + example systemd unit makes it too (`StateDirectory=`). - Limits on every input: request size (2 MiB, see section 10), number of secrets per request (32), field lengths. JSON decoding rejects unknown and repeated keys, and missing and `null` fields. - The broker turns off core dumps and marks itself not dumpable - (`PR_SET_DUMPABLE`). For core dumps, it sets its own soft `RLIMIT_CORE` - to 1 at start, and the example systemd unit sets `LimitCORE=1`. The store - programs inherit this limit; Go's `os/exec` cannot set a limit for one + (`PR_SET_DUMPABLE`). For core dumps, it sets its own soft and hard + `RLIMIT_CORE` to 1 at start, and the example systemd unit sets + `LimitCORE=1`. The store programs inherit this limit, and cannot raise + it above the hard limit; Go's `os/exec` cannot set a limit for one child. If a store program crashes, a core handler such as systemd-coredump would write the program's environment, with the token or the session key, into the journal. With a limit of exactly 1 and a @@ -1849,7 +1995,9 @@ text of a line. It exits with: kernel calls no core handler and writes no core file. A limit of 0 does not stop a core handler, and a socket `core_pattern` (`@`) does not look at the limit at all. So keep a pipe or file `core_pattern` on the broker - host. + host. The broker reads both settings back, and does not start if it + cannot make them. A hard limit of 0 is such a case, because the soft + limit cannot be higher than the hard limit. - `nodkey run` marks itself not dumpable (`PR_SET_DUMPABLE` 0) before it sends its request, so it writes no core dump while it holds the values, also when the agent sets `GOTRACEBACK`, and other processes of the user @@ -1862,10 +2010,25 @@ text of a line. It exits with: minutes ends at 12:10 on the wall clock, also if the host slept from 12:00 to 20:00. A request whose end the wall clock has passed takes no answer. -- A systemd unit with sandbox settings ships as an example. Its sandbox - must let `bw` run: Node.js needs memory that is both writable and - executable, so the unit must not set `MemoryDenyWriteExecute=yes` when - it has a `bw` store. +- A systemd unit with sandbox settings ships as an example: + [contrib/systemd/nodkey.service](contrib/systemd/nodkey.service). Its + sandbox must let `bw` run: Node.js needs memory that is both writable + and executable, so the unit must not set `MemoryDenyWriteExecute=yes` + when it has a `bw` store. It must not set `PrivateUsers=yes` either: + then the credentials and the config, owned by root, would look owned + by `nobody`, and the broker would refuse them. The unit has one + `LoadCredentialEncrypted=` line for each key, token and password file + (section 11), and sets `StateDirectory=` (the audit log and the store + directory, mode `0700`), `UMask=0077`, `LimitCORE=1`, `MemoryMax=`, + `MemorySwapMax=0` and `MemoryZSwapMax=0` (no swap, section 11; + `MemoryZSwapMax=` needs systemd 253 or newer, an older one ignores it + with a warning, and `MemorySwapMax=0` alone is enough), + `TimeoutStopSec=90s`, `RestartPreventExitStatus=64 76 77` (section 15) + and `LogRateLimitIntervalSec=0`, so that journald never drops an anchor + of the audit log. +- Swap and hibernation write memory to disk, and the broker's memory + holds keys, tokens and, for a moment, secret values. So the broker host + uses encrypted swap or none, and no hibernation (section 11). ### Store programs @@ -1980,6 +2143,14 @@ bws 2.1.0 and bw 2026.9.1 on 2026-10-14 core, the fake approver and clock, fake stores in memory, and an audit log in memory that refuses what the real one refuses. SSH logins are real: an in-process SSH server checks the signatures. +- **Tests of `nodkey broker`**: the command runs in a child process, with + a config whose files pass every check of section 14, fake store + programs, and the real Telegram approver. The approver reaches a fake + Telegram server through a fake HTTPS proxy that answers TLS with a + certificate of a test CA, so nothing goes to the real Bot API. The + tests check the order of the start and the stop, the exit codes, that + core dumps are off, a store that does not start, and a canary from the + request to the fake Telegram server, end to end. - **Fake stores**: the test binary of `internal/store` acts like `bws` or `bw` when it runs under that name. It records its arguments and its environment, so the tests can check that no token, password or session diff --git a/THREAT-MODEL.md b/THREAT-MODEL.md index 0578573..de5799c 100644 --- a/THREAT-MODEL.md +++ b/THREAT-MODEL.md @@ -185,20 +185,31 @@ the end of grants after it still work (T8). The alert goes out then too, and the tap is written with cause `edited_message`, next to the Deny. When another program polls with the same token, Telegram reports a conflict. The broker logs an alert, sends an alert message, and stops -taking answers: every request that waits for the person is denied. With -the token, an attacker can stop the broker this way, but they can deny -service anyway, by reading the updates. +taking answers: every request that waits for the person is denied. Then +`nodkey broker` stops with exit code 76, and the example systemd unit +does not start it again, so the alert does not come again at each +restart. With the token, an attacker can stop the broker this way, but +they can deny service anyway, by reading the updates. When the broker +stops, it closes the connection of its waiting `getUpdates` call at once +and polls no more. Still, in rare cases a `getUpdates` request of a +broker that just stopped is on its way, reaches Telegram after the new +broker started to poll, and causes a conflict and an alert although +nobody else has the token (DESIGN.md section 7). A proxy that inspects TLS, with its own certificate authority in the system's store of the broker host, sees the token, and it can fake Telegram's answers, so also taps. The broker logs at start which proxy it uses (DESIGN.md section 7). Do not use such a proxy for the broker. -Risk that remains: denial of service, and misleading text in the chat. +Risk that remains: denial of service, misleading text in the chat, and, +rarely, a false conflict alert right after a restart. Only the button data says what a tap is, never the label. So a tap on a "Deny" label that the attacker gave the data of an Approve does not count: that request only expires, without the pause after a Deny. A tap on an "Approve" label with the data of a Deny denies, with the pause and the end of grants: that is denial of service too. Keep the token in an encrypted credential; make a new one if it leaks. +When Telegram no longer accepts the old token, the broker stops with +exit code 77, and the example unit does not start it again until a +person puts the new token in place. **T7. Someone else can use the approver's Telegram account.** Telegram runs on several devices at once. If the approver's account is @@ -321,9 +332,15 @@ claimed text stays a claim. The command itself may print a secret; version 1 does not filter the command's output. **T16. The broker host is broken into.** Everything is lost. Advice: a -dedicated user, the systemd sandbox settings in the example unit, secrets -as encrypted credentials sealed with the TPM, no agent running on that -host, updates. +dedicated user, the systemd sandbox settings in the example unit +(`contrib/systemd/nodkey.service`), secrets as encrypted credentials +sealed with the TPM, no agent running on that host, updates. The broker +marks itself not dumpable, so other processes of its user cannot attach +to it or read its memory. Swap and hibernation write memory to disk, with +the keys, the tokens and, for a moment, secret values in it: use +encrypted swap or none, and no hibernation, on the broker host. The +example unit lets the broker use no swap (`MemorySwapMax=0`, +`MemoryZSwapMax=0`). Risk that remains: real; out of scope. **T17. The audit log is changed.** @@ -369,8 +386,10 @@ Risk that remains: the later anchors cover it. The anchors do not find it; only the missing `audit anchor` line for that entry shows it. An appended `log_recovered` entry can also make the last real entry look like a - torn line. The anchor of that entry finds this, but nothing alerts at - the start. + torn line. The anchor of that entry finds this. When the forged entry + is the last entry, the next start also sends one alert about a torn + line; when more entries were appended after it, nothing alerts at the + start. - A log that was removed or moved away looks like a new log. Its anchors and those of the old log differ, so check with the anchors since the current log began, and keep the old file. @@ -387,12 +406,13 @@ output, and its errors hold only a fixed text and the exit code. Tests check all of this with made-up secrets and fake programs that record what they get. If a store program crashes, a core handler such as systemd-coredump would write its environment, with the token or the -session key, into the journal. So the broker sets its soft `RLIMIT_CORE` -to 1, which the programs inherit, and the example unit sets -`LimitCORE=1`: with exactly 1 and a pipe (`|`) or file `core_pattern` -(the systemd default), the kernel calls no core handler. A limit of 0 -does not stop one, and a socket `core_pattern` (`@`) does not look at the -limit (DESIGN.md section 16). +session key, into the journal. So the broker sets its soft and hard +`RLIMIT_CORE` to 1, which the programs inherit and cannot raise, and the +example unit sets `LimitCORE=1`: with exactly 1 and a pipe (`|`) or file +`core_pattern` (the systemd default), the kernel calls no core handler. A +limit of 0 does not stop one, and a socket `core_pattern` (`@`) does not +look at the limit (DESIGN.md section 16). The broker reads the limits +back, and does not start if it cannot set them. Risk that remains: low. The broker clears the token and the session key in its memory, but only as best effort: Go strings and the copies that the system call makes are not cleared. diff --git a/cmd/nodkey/broker.go b/cmd/nodkey/broker.go new file mode 100644 index 0000000..6898458 --- /dev/null +++ b/cmd/nodkey/broker.go @@ -0,0 +1,704 @@ +package main + +import ( + "context" + "encoding/hex" + "errors" + "flag" + "fmt" + "io" + "log/slog" + "maps" + "net" + "os/signal" + "path/filepath" + "slices" + "strconv" + "strings" + "sync" + "syscall" + "time" + + "golang.org/x/crypto/ssh" + + "github.com/code6x/nodkey/internal/approval" + "github.com/code6x/nodkey/internal/audit" + "github.com/code6x/nodkey/internal/broker" + "github.com/code6x/nodkey/internal/config" + "github.com/code6x/nodkey/internal/store" + "github.com/code6x/nodkey/internal/telegram" + "github.com/code6x/nodkey/internal/transport" +) + +// defaultBrokerConfig is the broker config of DESIGN.md section 14. +const defaultBrokerConfig = "/etc/nodkey/broker.toml" + +// exitPollConflict is the exit code of nodkey broker after Telegram +// reported a polling conflict: another program polls with the bot token, +// or a webhook is set, so the token may be stolen. The example systemd +// unit lists it in RestartPreventExitStatus=, so systemd does not start +// the broker again into the same conflict, with a new alert each time. It +// is EX_PROTOCOL from sysexits.h: Telegram answered what cannot happen +// while the broker is the only program that polls with the token. +const exitPollConflict = 76 + +// exitTokenRefused is the exit code of nodkey broker after Telegram +// answered getUpdates with 401 or 404: it does not accept the bot token, +// such as after the token was revoked or the bot deleted. Only a new +// token helps, so the example unit lists it in RestartPreventExitStatus= +// too: starting again would call Telegram with the same token, and write +// broker_started and broker_stopped, every few seconds. It is EX_NOPERM +// from sysexits.h. +const exitTokenRefused = 77 + +// storesDirName is the store directory: the directory next to the audit +// log that holds the home directory of each store (DESIGN.md section 16). +const storesDirName = "stores" + +// Time limits of the start and the stop. +const ( + // storeStartTimeout bounds the start of the stores. A bw store runs + // bw unlock, and after a failure bw status: two runs of at most + // store.DefaultTimeout each. + storeStartTimeout = 2*store.DefaultTimeout + 10*time.Second + // stopTimeout bounds the whole stop. The service waits for the + // requests in progress, which may write an answer for up to 30 + // seconds, and for its alert messages, up to 30 seconds more. The + // example unit sets TimeoutStopSec=90s, so the broker ends on its own + // first and logs why. + stopTimeout = 80 * time.Second +) + +// The tries of an alert message of the start (sendAlerts). After a power +// cut, the broker host can be up before the router or the link to the +// internet, so Telegram cannot be reached for a while. Each try takes at +// most alertTimeout, as telegram.Approver.Alert does. After a failed try, +// the next one comes after alertFirstWait, then after twice as long each +// time, at most alertMaxWait, until the alert is out or the stop begins. +// They are variables only so that a test can make them short. +var ( + alertTimeout = 30 * time.Second + alertFirstWait = 5 * time.Second + alertMaxWait = 5 * time.Minute +) + +// newStore makes the store of one [store.*] section. It is a variable +// only so that a test can run nodkey broker in a child process with a +// store whose start fails; nodkey never changes it. +var newStore = store.New + +// errPanicked is the error of a part of the broker that panicked. The +// panic value is never logged: it could hold text from a request. +var errPanicked = errors.New("it panicked; the panic value is not logged") + +const brokerUsage = `Usage: nodkey broker [--config FILE] + +Run the broker: the daemon that holds the SSH keys and the access to the +secret stores. It asks the approver on Telegram before it hands out a +secret or signs an SSH login. Run it as its own user, from a systemd +unit such as contrib/systemd/nodkey.service. + + --config FILE the broker config (default /etc/nodkey/broker.toml) + +It logs to stderr, which systemd keeps in the journal, with the anchors +of the audit log. SIGTERM or SIGINT stops it. + +Exit codes: 0 stopped by SIGTERM or SIGINT, 1 any other error, 64 wrong +usage, 76 Telegram reports a polling conflict: the bot token may be +stolen, 77 Telegram does not accept the bot token. After 64, 76 and 77, +systemd must not start the broker again: only a person can fix them. +` + +// runBroker runs nodkey broker with args (after "broker") and returns the +// exit code. +func runBroker(args []string, stderr io.Writer) int { + // One logger for every part: the command, the service, the server, + // the approval core and Telegram. Its lines start with "time=" and + // escape line breaks, so no line can look like an audit anchor + // (DESIGN.md section 9). + log := broker.NewLogger(stderr) + fs := flag.NewFlagSet("nodkey broker", flag.ContinueOnError) + // The flag package would print a wrong argument as it is, so its + // errors go through the logger. + fs.SetOutput(io.Discard) + path := fs.String("config", defaultBrokerConfig, "the broker config") + if err := fs.Parse(args); err != nil { + if errors.Is(err, flag.ErrHelp) { + printBrokerUsage(stderr) + return exitOK + } + log.Error("nodkey broker: wrong usage", slog.String("error", err.Error())) + printBrokerUsage(stderr) + return exitUsage + } + if fs.NArg() > 0 { + log.Error("nodkey broker: wrong usage: it takes no arguments, only --config FILE") + printBrokerUsage(stderr) + return exitUsage + } + ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGTERM, syscall.SIGINT) + defer stop() + return serveBroker(ctx, *path, stderr, log) +} + +// printBrokerUsage writes the usage of nodkey broker to w. A failed write +// to stderr cannot be reported anywhere, so the error is ignored. +func printBrokerUsage(w io.Writer) { + _, _ = io.WriteString(w, brokerUsage) +} + +// serveBroker starts the broker with the config at path, serves until ctx +// ends (a signal) or a part stops, then stops the broker and returns the +// exit code (DESIGN.md section 15). +func serveBroker(ctx context.Context, path string, stderr io.Writer, log *slog.Logger) (code int) { + defer func() { + // The runtime would print the panic value, and no line of the + // broker may hold text it did not choose. + if recover() != nil { + log.Error("nodkey broker: a panic stopped the broker; the panic value is not logged") + code = exitFailure + } + }() + b := &brokerParts{log: log, stderr: stderr} + bySignal := false + switch err := b.start(ctx, path); { + case err != nil && errors.Is(err, context.Canceled) && ctx.Err() != nil: + log.Info("nodkey broker: a signal asked to stop during the start") + bySignal = true + case err != nil: + log.Error("nodkey broker: cannot start", slog.String("error", err.Error())) + default: + bySignal = b.wait(ctx) + } + runErr, clean := b.stop(time.Now().Add(stopTimeout)) + code = brokerExitCode(bySignal, runErr, clean) + switch code { + case exitOK: + log.Info("nodkey broker: stopped", slog.Int("exit_code", code)) + case exitPollConflict: + log.Error("nodkey broker: stopped after a polling conflict; the bot token may be stolen, so systemd "+ + "does not start the broker again", slog.Int("exit_code", code)) + case exitTokenRefused: + log.Error("nodkey broker: stopped, because Telegram does not accept the bot token; put a new token "+ + "into the token file, then start the broker", slog.Int("exit_code", code)) + default: + log.Error("nodkey broker: stopped", slog.Int("exit_code", code)) + } + return code +} + +// brokerExitCode returns the exit code of nodkey broker: exitPollConflict +// after a polling conflict, exitTokenRefused when Telegram does not accept +// the bot token, 0 after a clean stop that a signal asked for, and 1 for +// every other end, such as a failed start, an approver that stopped for +// another reason, or a stop that was not clean. +func brokerExitCode(bySignal bool, runErr error, clean bool) int { + switch { + case errors.Is(runErr, telegram.ErrPollConflict): + return exitPollConflict + case errors.Is(runErr, telegram.ErrTokenRefused): + return exitTokenRefused + case bySignal && clean: + return exitOK + default: + return exitFailure + } +} + +// brokerParts are the parts of a running broker. start makes them in the +// order of DESIGN.md section 15; stop ends those that start made, in the +// order of that section. Only the goroutine of serveBroker uses the +// fields, except where a comment says otherwise. +type brokerParts struct { + log *slog.Logger + stderr io.Writer + + cfg *config.Broker + audit *audit.Logger + stores map[string]store.Store + svc *broker.Service + tg *telegram.Approver + + // alerts are the alert messages that wait until the approver can send + // them: a torn line in the audit log, and stores that did not start. + alerts []string + // alertsDone is closed when the alerts went out, or the sender gave + // up because the stop began. stop closes alertsStop when it begins. + alertsDone chan struct{} + alertsStop chan struct{} + + // started is true once the broker_started entry is on disk. + started bool + + // stopRun ends the service's Run; runDone is closed when Run returned, + // and runErr is its error, set before runDone is closed. + stopRun context.CancelFunc + runDone chan struct{} + runErr error + + // ln is the listen address. Until serve, nothing reads from it. + ln net.Listener + + // srv serves the clients; serveDone is closed when Serve returned, and + // serveErr is its error, set before serveDone is closed. + srv *transport.Server + serveDone chan struct{} + serveErr error +} + +// start makes the parts of the broker, in this order (DESIGN.md section +// 15): the config and the checks of the files it names, no core dumps, +// the broker's TLS key, the SSH keys, the audit log, the stores, the +// service, the Telegram approver and the approval core, the server and +// its listen address, the broker_started entry, the approver's Run, and +// only then Serve. On an error it returns at once; stop then ends what +// start made. It returns context.Canceled when ctx ended during the start. +func (b *brokerParts) start(ctx context.Context, path string) error { + b.log.Info("nodkey broker: starting", slog.String("version", version), slog.String("config", path)) + cfg, err := config.LoadBroker(path) // it runs CheckFiles + if err != nil { + return err + } + b.cfg = cfg + b.log.Info("nodkey broker: the config and the files it names passed their checks") + if err := hardenProcess(); err != nil { + return fmt.Errorf("cannot turn off core dumps: %w", err) + } + b.log.Info("nodkey broker: core dumps are off: the soft and the hard limit of core dumps are 1, " + + "and the process is not dumpable") + tlsKey, err := transport.LoadKeyFile(cfg.TLSKey) + if err != nil { + return fmt.Errorf("tls_key: %w", err) + } + b.log.Info("nodkey broker: the TLS key is loaded", slog.String("pin", transport.PinOf(tlsKey.Leaf).String())) + keys, err := loadSSHKeys(cfg, config.ReadPrivateFile) + if err != nil { + return err + } + for _, name := range slices.Sorted(maps.Keys(keys)) { + b.log.Info("nodkey broker: an SSH key is loaded and can sign", slog.String("key", name), + slog.String("fingerprint", ssh.FingerprintSHA256(keys[name].PublicKey()))) + } + b.log.Info("nodkey broker: the SSH keys are loaded", slog.Int("keys", len(keys))) + if err := ctx.Err(); err != nil { + return err + } + + // The audit log is opened once, before anything can ask. Its Alert + // only queues the message: the approver does not exist yet. + l, err := audit.Open(cfg.AuditLog, audit.Options{Journal: b.stderr, Alert: b.logRecovered}) + if err != nil { + return err + } + b.audit = l + b.log.Info("nodkey broker: the audit log is open", slog.String("path", cfg.AuditLog)) + if r, ok := l.LastRecovery(); ok { + // An earlier start recorded a torn line and stopped before its + // broker_started entry, such as when its listen address was not + // there yet. Its alert may never have gone out. + b.log.Warn("nodkey broker: ALERT: the last entry of the audit log records a torn line, and the start "+ + "that recorded it stopped before it could send the alert; this start sends it", + slog.Uint64("seq", r.Seq), slog.Int("torn_length", r.TornLength), + slog.String("torn_sha256", hex.EncodeToString(r.TornSHA256[:]))) + b.alerts = append(b.alerts, tornLineAlert(r)) + } + if err := b.startStores(ctx); err != nil { + return err + } + + svc, err := broker.New(broker.Parts{Config: cfg, Audit: l, Stores: b.stores, Keys: keys, Logger: b.log}) + if err != nil { + return err + } + b.svc = svc + // The approval core takes the approver, so Telegram comes first. + tg, err := telegram.New(cfg.Telegram, telegram.Options{Logger: b.log, ForeignTap: svc.ForeignTap}) + if err != nil { + return err + } + b.tg = tg + core, err := approval.New(cfg, tg, approval.Options{Logger: b.log}) + if err != nil { + return err + } + // The server checks the client pins here. + srv, err := transport.NewServer(tlsKey, cfg.Clients, svc, b.log) + if err != nil { + return err + } + // The listen address is opened now, so that a missing address, such + // as at boot before the private network is up, stops the start before + // any entry and any call to Telegram; systemd starts the broker again + // later. Nothing reads from it until serve below: a client that + // connects waits. + ln, err := (&net.ListenConfig{}).Listen(ctx, "tcp", cfg.Listen.String()) + if err != nil { + if ctx.Err() != nil { + return ctx.Err() + } + return fmt.Errorf("cannot listen on %s: %w", cfg.Listen, err) + } + b.ln = ln + b.log.Info("nodkey broker: the listen address is open; nothing reads from it yet", + slog.String("address", ln.Addr().String())) + if err := ctx.Err(); err != nil { + return err + } + + // The broker_started entry is the last step of the start that can + // fail. + if _, err := l.Append(audit.BrokerStarted{Version: version}); err != nil { + return fmt.Errorf("cannot write the broker_started entry: %w", err) + } + b.started = true + b.startRun(core) + b.sendAlerts() + b.serve(srv, ln) + b.log.Info("nodkey broker: listening; the broker is ready", slog.String("address", ln.Addr().String())) + return nil +} + +// logRecovered is the Alert of the audit log: Open found a torn last line +// and recorded it with a log_recovered entry. It logs that, and queues an +// alert message for the approver. +func (b *brokerParts) logRecovered(r audit.Recovery) { + b.log.Warn("nodkey broker: ALERT: the audit log ended in a torn line, as after a power cut; "+ + "a log_recovered entry records its length and hash", slog.Uint64("seq", r.Seq), + slog.Int("torn_length", r.TornLength), slog.String("torn_sha256", hex.EncodeToString(r.TornSHA256[:]))) + b.alerts = append(b.alerts, tornLineAlert(r)) +} + +// tornLineAlert returns the alert message about the torn line that r +// records. +func tornLineAlert(r audit.Recovery) string { + return fmt.Sprintf("The audit log ended in a torn line, as after a power cut. Entry %d (log_recovered) "+ + "records it. Check the log with nodkey audit verify and the anchors in the journal.", r.Seq) +} + +// startStores makes the store of each [store.*] section, in the store +// directory, and starts them all at once. A store that does not start +// does not stop the broker: the broker denies its requests until it works +// (each request tries again), and the approver gets one alert for all +// such stores. +func (b *brokerParts) startStores(ctx context.Context) error { + if len(b.cfg.Stores) == 0 { + return nil + } + dir := filepath.Join(filepath.Dir(b.cfg.AuditLog), storesDirName) + if err := store.PrepareDir(dir); err != nil { + return err + } + names := slices.Sorted(maps.Keys(b.cfg.Stores)) + b.stores = make(map[string]store.Store, len(names)) + for _, name := range names { + s, err := newStore(b.cfg.Stores[name], dir) + if err != nil { + return err // it names the store and a file, never a token + } + b.stores[name] = s + } + startCtx, cancel := context.WithTimeout(ctx, storeStartTimeout) + defer cancel() + errs := make([]error, len(names)) + var wg sync.WaitGroup + for i, name := range names { + s := b.stores[name] + wg.Go(func() { errs[i] = s.Start(startCtx) }) + } + wg.Wait() + if err := ctx.Err(); err != nil { + return err + } + var failed []string + for i, name := range names { + if errs[i] != nil { + failed = append(failed, strconv.Quote(name)) + // Store errors have fixed texts: never a token or the program's output. + b.log.Error("nodkey broker: a store could not start; the broker denies requests for its secrets "+ + "until it works, and each request tries again", slog.String("store", name), + slog.String("error", errs[i].Error())) + continue + } + b.log.Info("nodkey broker: a store started", slog.String("store", name)) + } + if len(failed) > 0 { + b.alerts = append(b.alerts, "These stores could not start: "+strings.Join(failed, ", ")+ + ". The broker denies requests for their secrets until they work; each request tries again. "+ + "The journal of the broker says why.") + } + return nil +} + +// startRun runs the service with the approval core and the approver's +// alert, until stop ends it or the approver stops. +func (b *brokerParts) startRun(core *approval.Core) { + ctx, cancel := context.WithCancel(context.Background()) + b.stopRun = cancel + b.runDone = make(chan struct{}) + go func() { + defer close(b.runDone) + b.runErr = errPanicked + defer func() { + if recover() != nil { + b.log.Error("nodkey broker: the approver panicked; the panic value is not logged; " + + "every request is denied, and the broker stops") + } + }() + b.runErr = b.svc.Run(ctx, core, b.tg.Alert) + }() + b.log.Info("nodkey broker: the approver runs: it polls Telegram for the answers") +} + +// sendAlerts sends the alert messages that waited for the approver, one +// after the other, in a goroutine of its own. Each one gets a first try, +// also when the stop has begun; the stop waits for it. After a failed +// try, sendAlert tries again (see alertFirstWait) until the alert is out +// or the stop begins. Each failed try gets one log line; the growing wait +// limits them to one line in 5 seconds at first and one in 5 minutes +// later. An alert that did not go out before the stop is lost, and the +// journal says so. +func (b *brokerParts) sendAlerts() { + msgs := b.alerts + b.alerts = nil + b.alertsDone = make(chan struct{}) + b.alertsStop = make(chan struct{}) + go func() { + defer close(b.alertsDone) + defer func() { + if recover() != nil { + b.log.Error("nodkey broker: sending an alert message panicked; the panic value is not logged") + } + }() + lost := 0 + for _, msg := range msgs { + if !b.sendAlert(msg) { + lost++ + } + } + if lost > 0 { + b.log.Error("nodkey broker: alert messages of the start did not go out before the stop; they are lost", + slog.Int("lost", lost)) + } + }() +} + +// sendAlert tries to send msg until it is out, and reports true. After the +// stop began, it makes no new try, and reports false. +func (b *brokerParts) sendAlert(msg string) bool { + wait := alertFirstWait + for try := 1; ; try++ { + ctx, cancel := context.WithTimeout(context.Background(), alertTimeout) + err := b.tg.Alert(ctx, msg) + cancel() + if err == nil { + return true + } + select { + case <-b.alertsStop: + // Telegram's errors never hold the token or Go's error text. + b.log.Warn("nodkey broker: an alert message did not go out", slog.String("error", err.Error()), + slog.Int("try", try)) + return false + default: + } + b.log.Warn("nodkey broker: an alert message did not go out; the broker tries again", + slog.String("error", err.Error()), slog.Int("try", try), slog.Duration("next_try_in", wait)) + t := time.NewTimer(wait) + select { + case <-t.C: + case <-b.alertsStop: + t.Stop() + return false + } + wait = min(2*wait, alertMaxWait) + } +} + +// serve serves the clients on ln. +func (b *brokerParts) serve(srv *transport.Server, ln net.Listener) { + b.srv = srv + b.serveDone = make(chan struct{}) + go func() { + defer close(b.serveDone) + b.serveErr = errPanicked + defer func() { + if recover() != nil { + b.log.Error("nodkey broker: the server panicked; the panic value is not logged") + } + }() + b.serveErr = srv.Serve(ln) + }() +} + +// wait waits until ctx ends, the approver stops, or the server stops. It +// reports whether a signal asked to stop. +func (b *brokerParts) wait(ctx context.Context) bool { + select { + case <-ctx.Done(): + b.log.Info("nodkey broker: stopping, as a signal asked") + return true + case <-b.runDone: + // The service has logged why. + b.log.Error("nodkey broker: stopping, because the approver stopped: no answer can come any more") + return false + case <-b.serveDone: + b.log.Error("nodkey broker: stopping, because the server stopped", slog.String("error", errText(b.serveErr))) + return false + } +} + +// errText returns the text of err, or "" for nil. +func errText(err error) string { + if err == nil { + return "" + } + return err.Error() +} + +// stop ends the parts that start made, in this order (DESIGN.md section +// 15): the approver's Run, so the polling stops and its connection is +// closed; the service, which ends the requests in progress and waits for +// them; the server; the alert messages of the start; the stores; and last +// the broker_stopped entry and the audit log. It waits at most until +// deadline: when a part takes longer, it gives up on the rest, and the +// process ends. It returns the error of Run, and whether the stop was +// clean: every part ended in time without an error or a panic, and the +// audit log took its last entry. +func (b *brokerParts) stop(deadline time.Time) (runErr error, clean bool) { + b.log.Info("nodkey broker: stopping: the approver, the service, the server, the stores, then the audit log") + clean = true + if b.alertsStop != nil { + close(b.alertsStop) // no more tries again of an alert of the start + } + if b.stopRun != nil { + b.stopRun() + if !waitUntil(b.runDone, deadline) { + return nil, b.late("the approver") + } + runErr = b.runErr + b.log.Info("nodkey broker: the approver stopped; no answer counts any more") + } + if b.svc != nil { + done, ok := b.step("the service", b.svc.Close, deadline) + if !done { + return runErr, false + } + clean = clean && ok + b.log.Info("nodkey broker: the service stopped; every request has ended") + } + switch { + case b.srv != nil: + // It closes the listen address too. + if err := b.srv.Close(); err != nil { + b.log.Error("nodkey broker: cannot close the server", slog.String("error", err.Error())) + clean = false + } + if !waitUntil(b.serveDone, deadline) { + return runErr, b.late("the server") + } + b.log.Info("nodkey broker: the server is closed") + case b.ln != nil: + if err := b.ln.Close(); err != nil { + b.log.Error("nodkey broker: cannot close the listen address", slog.String("error", err.Error())) + clean = false + } + } + if b.alertsDone != nil && !waitUntil(b.alertsDone, deadline) { + return runErr, b.late("an alert message") + } + if b.stores != nil { + done, ok := b.step("the stores", b.closeStores, deadline) + if !done { + return runErr, false + } + clean = clean && ok + b.log.Info("nodkey broker: the stores are closed; their tokens and session keys are cleared") + } + if b.audit != nil { + closed := false + done, ok := b.step("the audit log", func() { closed = b.closeAudit() }, deadline) + if !done { + return runErr, false + } + clean = clean && ok && closed + } + return runErr, clean +} + +// step runs f, a step of the stop, in a goroutine of its own, and waits +// for it at most until deadline. done is false when f did not return in +// time: then f goes on, and the process ends without it. ok is false when +// f panicked. step logs both. +func (b *brokerParts) step(part string, f func(), deadline time.Time) (done, ok bool) { + finished := make(chan struct{}) + panicked := false + go func() { + defer close(finished) + defer func() { + if recover() != nil { + panicked = true + } + }() + f() + }() + if !waitUntil(finished, deadline) { + return b.late(part), false + } + if panicked { + b.log.Error("nodkey broker: a step of the stop panicked; the panic value is not logged", + slog.String("step", part)) + return true, false + } + return true, true +} + +// late logs that the stop took too long while it waited for part, and +// returns false: the stop is not clean. +func (b *brokerParts) late(part string) bool { + b.log.Error("nodkey broker: the stop takes too long; the broker ends without waiting for the rest", + slog.String("waiting_for", part), slog.Duration("limit", stopTimeout)) + return false +} + +// closeStores closes every store: each clears its token or session key. +func (b *brokerParts) closeStores() { + for _, name := range slices.Sorted(maps.Keys(b.stores)) { + b.stores[name].Close() + } +} + +// closeAudit writes the broker_stopped entry, if broker_started is on +// disk, and closes the audit log. It reports whether both worked. +func (b *brokerParts) closeAudit() bool { + ok := true + if b.started { + if _, err := b.audit.Append(audit.BrokerStopped{}); err != nil { + b.log.Error("nodkey broker: cannot write the broker_stopped entry", slog.String("error", err.Error())) + ok = false + } + } + if err := b.audit.Close(); err != nil { + b.log.Error("nodkey broker: cannot close the audit log", slog.String("error", err.Error())) + return false + } + b.log.Info("nodkey broker: the audit log is closed") + return ok +} + +// waitUntil waits until done is closed, at most until deadline. It +// reports whether done was closed in time. +func waitUntil(done <-chan struct{}, deadline time.Time) bool { + select { + case <-done: + return true + default: + } + t := time.NewTimer(time.Until(deadline)) + defer t.Stop() + select { + case <-done: + return true + case <-t.C: + return false + } +} diff --git a/cmd/nodkey/broker_linux_test.go b/cmd/nodkey/broker_linux_test.go new file mode 100644 index 0000000..73aef9e --- /dev/null +++ b/cmd/nodkey/broker_linux_test.go @@ -0,0 +1,772 @@ +package main + +import ( + "bytes" + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "io/fs" + "net" + "net/http" + "os" + "slices" + "strings" + "sync" + "syscall" + "testing" + "time" + + "golang.org/x/crypto/ssh" + + "github.com/code6x/nodkey/internal/api" + "github.com/code6x/nodkey/internal/audit" +) + +// The log lines of nodkey broker that the tests look for. +const ( + lineStarting = "nodkey broker: starting" + lineConfigChecked = "nodkey broker: the config and the files it names passed their checks" + lineNoCoreDumps = "nodkey broker: core dumps are off" + lineTLSKey = "nodkey broker: the TLS key is loaded" + lineSSHKey = "nodkey broker: an SSH key is loaded and can sign" + lineSSHKeys = "nodkey broker: the SSH keys are loaded" + lineAuditOpen = "nodkey broker: the audit log is open" + lineStoreStarted = "nodkey broker: a store started" + lineStoreFailed = "nodkey broker: a store could not start" + lineProxy = "calls to Telegram go through a proxy from the environment" + lineListenOpen = "nodkey broker: the listen address is open" + lineApproverRuns = "nodkey broker: the approver runs" + lineListening = "nodkey broker: listening; the broker is ready" + lineStopSignal = "nodkey broker: stopping, as a signal asked" + lineStopApprover = "nodkey broker: stopping, because the approver stopped" + lineRunStopped = "nodkey broker: the approver stopped; no answer counts any more" + lineSvcStopped = "nodkey broker: the service stopped" + lineServerClosed = "nodkey broker: the server is closed" + lineStoresClosed = "nodkey broker: the stores are closed" + lineAuditClosed = "nodkey broker: the audit log is closed" + lineStopped = "nodkey broker: stopped" +) + +// secretsRequest returns a request of one variable for the secret name. +func secretsRequest(variable, name string) api.SecretsRequest { + return api.SecretsRequest{ + Secrets: []api.SecretRef{{Var: variable, Name: name}}, + Command: []string{"gh", "release", "create", "v1.2.0"}, + Dir: "/home/dev/src/app", + Reason: "publish the release the user asked for", + } +} + +// secretValues decodes the answer to a request for secrets. +func secretValues(t *testing.T, a brokerAnswer) map[string]string { + t.Helper() + mustStatus(t, a, http.StatusOK) + ans, err := api.ReadAnswer[api.SecretsAnswer](bytes.NewReader(a.body)) + if err != nil { + t.Fatalf("the answer: %v", err) + } + m := make(map[string]string, len(ans.Secrets)) + for _, s := range ans.Secrets { + m[s.Var] = string(s.Value) + } + return m +} + +// checkCoreDumpsOff checks from outside that the process pid has a soft +// and a hard limit of core dumps of 1, and is not dumpable: the files of a +// process that is not dumpable are owned by root in /proc, and other +// processes of its user cannot read its environment. +func checkCoreDumpsOff(t *testing.T, pid int) { + t.Helper() + limits, err := os.ReadFile(fmt.Sprintf("/proc/%d/limits", pid)) + if err != nil { + t.Fatal(err) + } + found := false + for line := range strings.SplitSeq(string(limits), "\n") { + rest, ok := strings.CutPrefix(line, "Max core file size") + if !ok { + continue + } + found = true + if f := strings.Fields(rest); len(f) < 2 || f[0] != "1" || f[1] != "1" { + t.Errorf("the soft and hard limits of core dumps of the broker are not 1: %q", line) + } + } + if !found { + t.Errorf("/proc/%d/limits has no limit of core dumps", pid) + } + if os.Geteuid() == 0 { + t.Log("the test runs as root, so it cannot see whether the broker is dumpable") + return + } + environ := fmt.Sprintf("/proc/%d/environ", pid) + fi, err := os.Stat(environ) + if err != nil { + t.Fatal(err) + } + if st, ok := fi.Sys().(*syscall.Stat_t); !ok || st.Uid != 0 { + t.Errorf("the broker is dumpable: %s is not owned by root (%v)", environ, fi.Sys()) + } + if _, err := os.ReadFile(environ); !errors.Is(err, fs.ErrPermission) { //nolint:gosec // G304: a file of /proc. + t.Errorf("another process of the user reads the environment of the broker: %v", err) + } +} + +// TestBrokerStartsAndStops runs nodkey broker with a store whose start +// fails, and stops it with SIGTERM. It checks the order of the start, that +// core dumps are off, that the broken store does not stop the broker (one +// alert; its secrets are denied; the other store works), that the stop +// closes the connection of the waiting getUpdates call while the broker +// still runs, the order of the stop, exit code 0, and the audit log: from +// broker_started to broker_stopped, every entry with its anchor in the +// journal. +func TestBrokerStartsAndStops(t *testing.T) { + t.Parallel() + e := startListening(t, brokerOptions{holdAlerts: true}, seamStoreStartFails) + e.stderr.wait(t, lineListening) + e.stderr.inOrder(t, lineStarting, lineConfigChecked, lineNoCoreDumps, lineTLSKey, lineSSHKey, lineSSHKeys, + lineAuditOpen, lineStoreStarted, lineStoreFailed, lineProxy, "variable=SSL_CERT_FILE", lineListenOpen, + "audit anchor 1:", lineApproverRuns, lineListening) + checkCoreDumpsOff(t, e.cmd.Process.Pid) + + // One alert for the broken store; it waits at the fake Telegram until + // the test releases it. + if alert := e.tg.waitAlert("could not start"); !strings.Contains(alert, "vault") { + t.Errorf("the alert does not name the store: %s", alert) + } + c := e.httpClient() + // A secret of the broken store is denied: its fetch fails too. + mustStatus(t, e.post(c, api.PathSecrets, secretsRequest("DB_PASSWORD", "db-password")), + http.StatusServiceUnavailable) + // A secret of the other store is handed out after the tap. + if got := secretValues(t, e.approve(c, api.PathSecrets, secretsRequest("GH_TOKEN", "github-token"))); got["GH_TOKEN"] != e.values["github-token"] { + t.Error("the variable did not get its value") + } + + // SIGTERM while a getUpdates call waits. The stop waits for the held + // alert after the server is closed, so the broker still runs when the + // test looks at the polling. + e.tg.waitPolling() + e.signal(syscall.SIGTERM) + e.stderr.wait(t, lineServerClosed) + e.tg.waitPollsGone() + if !e.running() { + t.Fatal("the broker ended before the test released the alert") + } + polls := e.tg.pollCount() + e.tg.release() + e.mustExit(exitOK) + if n := e.tg.pollCount(); n != polls { + t.Errorf("%d getUpdates calls after the approver stopped", n-polls) + } + if n := e.tg.alertCount("could not start"); n != 1 { + t.Errorf("%d alerts about the store, want 1", n) + } + e.stderr.inOrder(t, lineStopSignal, lineRunStopped, lineSvcStopped, lineServerClosed, lineStoresClosed, + lineAuditClosed, lineStopped+`" exit_code=0`) + + // The audit log, from broker_started to broker_stopped. + got := eventNames(e.auditEntries()) + want := []string{ + "broker_started", + "request", "grant_used:never_rule", "error:store_failed", "denied:error", // db-password + "request", "decision:approve_once", "secret_sent", // github-token + "broker_stopped", + } + if !slices.Equal(got, want) { + t.Errorf("audit entries:\n%q\nwant:\n%q", got, want) + } + if v, _ := e.auditEntries()[0]["version"].(string); v != version { + t.Errorf("broker_started has version %q, want %q", v, version) + } + f, err := os.Open(e.auditPath) + if err != nil { + t.Fatal(err) + } + defer func() { _ = f.Close() }() + anchors, err := audit.ReadAnchors(strings.NewReader(e.stderr.text())) + if err != nil { + t.Fatal(err) + } + res, err := audit.Verify(f, anchors...) + if err != nil || res.Entries != uint64(len(want)) || res.Anchors != res.Entries { + t.Errorf("audit verify: %+v, %v; want every entry with its anchor", res, err) + } +} + +// TestBrokerPollConflict: when Telegram reports a polling conflict, the +// broker sends an alert, writes an error entry with poll_conflict, stops +// in order, and exits with exitPollConflict, which the example unit lists +// in RestartPreventExitStatus=. +func TestBrokerPollConflict(t *testing.T) { + t.Parallel() + e := startListening(t, brokerOptions{refusePolls: http.StatusConflict}) + e.mustExit(exitPollConflict) + e.tg.waitAlert("conflict") + e.stderr.inOrder(t, lineAuditOpen, "audit anchor 1:", lineApproverRuns, + "another program polls with the bot token", lineStopApprover, lineRunStopped, lineAuditClosed, + fmt.Sprintf("exit_code=%d", exitPollConflict)) + got := eventNames(e.auditEntries()) + if want := []string{"broker_started", "error:poll_conflict", "broker_stopped"}; !slices.Equal(got, want) { + t.Errorf("audit entries %q, want %q", got, want) + } +} + +// TestBrokerRefusesTheSameKeyTwice: two [key.*] sections whose files hold +// one key stop the start, before the audit log is opened and before +// anything listens or calls Telegram. +func TestBrokerRefusesTheSameKeyTwice(t *testing.T) { + t.Parallel() + e := startBroker(t, brokerOptions{sameKeyTwice: true}) + e.mustExit(exitFailure) + if !e.stderr.has("[key.copy] and [key.deploy] hold the same public key") { + t.Errorf("the log does not say why; its log:\n%s", e.stderr.text()) + } + for _, line := range []string{lineAuditOpen, lineListening} { + if e.stderr.has(line) { + t.Errorf("the log has %q", line) + } + } + if _, err := os.Lstat(e.auditPath); !errors.Is(err, fs.ErrNotExist) { + t.Errorf("the broker made the audit log: %v", err) + } + if p := e.tg.proxied(); len(p) != 0 { + t.Errorf("the broker called Telegram: %q", p) + } +} + +// TestBrokerNoListenBeforeTheAuditLog: when the audit log cannot be +// opened, because another process holds its lock as a second broker +// would, the broker stops, and never listens or polls. +func TestBrokerNoListenBeforeTheAuditLog(t *testing.T) { + t.Parallel() + e := newBrokerEnv(t, brokerOptions{}) + f, err := os.OpenFile(e.auditPath, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0o600) + if err != nil { + t.Fatal(err) + } + defer func() { _ = f.Close() }() + rc, err := f.SyscallConn() + if err != nil { + t.Fatal(err) + } + var lockErr error + if err := rc.Control(func(fd uintptr) { lockErr = syscall.Flock(int(fd), syscall.LOCK_EX|syscall.LOCK_NB) }); err != nil { + t.Fatal(err) + } + if lockErr != nil { + t.Fatal(lockErr) + } + + // Try to connect to the listen address all the time the broker runs. + ctx, cancel := context.WithCancel(t.Context()) + var wg sync.WaitGroup + var connected bool + wg.Go(func() { + d := net.Dialer{Timeout: 100 * time.Millisecond} + for ctx.Err() == nil { + if c, err := d.DialContext(ctx, "tcp", e.addr); err == nil { + _ = c.Close() + connected = true + return + } + time.Sleep(2 * time.Millisecond) + } + }) + e.start() + e.mustExit(exitFailure) + cancel() + wg.Wait() + if connected { + t.Error("the broker listened, though it could not open its audit log") + } + if !e.stderr.has("only one broker may write to one audit log") { + t.Errorf("the log does not say why; its log:\n%s", e.stderr.text()) + } + for _, line := range []string{lineAuditOpen, lineListenOpen, lineApproverRuns, lineListening} { + if e.stderr.has(line) { + t.Errorf("the log has %q", line) + } + } + if p := e.tg.proxied(); len(p) != 0 { + t.Errorf("the broker called Telegram: %q", p) + } +} + +// TestBrokerListenAddressInUse: when the broker cannot open its listen +// address, the start stops before the broker_started entry, before any +// call to Telegram and before any alert. So when systemd starts it again +// and again, such as at boot before the private network is up, no alert +// goes out each time, and the audit log does not grow. +func TestBrokerListenAddressInUse(t *testing.T) { + t.Parallel() + ln, err := (&net.ListenConfig{}).Listen(t.Context(), "tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer func() { _ = ln.Close() }() + e := newBrokerEnv(t, brokerOptions{listen: ln.Addr().String()}) + e.start(seamStoreStartFails) // a store alert waits + e.mustExit(exitFailure) + e.stderr.inOrder(t, lineAuditOpen, lineStoreFailed, "cannot listen on "+e.addr, lineStoresClosed, + lineAuditClosed, lineStopped+`" exit_code=1`) + for _, line := range []string{lineListenOpen, lineApproverRuns, lineListening} { + if e.stderr.has(line) { + t.Errorf("the log has %q", line) + } + } + if data, err := os.ReadFile(e.auditPath); err != nil || len(data) != 0 { + t.Errorf("the audit log: %d bytes, %v; want it empty", len(data), err) + } + if p := e.tg.proxied(); len(p) != 0 { + t.Errorf("the broker called Telegram: %q", p) + } +} + +// TestBrokerNeedsCoreDumpsOff: when the soft limit of core dumps cannot +// be set to 1, because the hard limit is 0, the broker does not start. It +// stops before it loads a key or opens the audit log. +func TestBrokerNeedsCoreDumpsOff(t *testing.T) { + t.Parallel() + e := startBroker(t, brokerOptions{}, seamNoCoreDumps) + e.mustExit(exitFailure) + if !e.stderr.has("the hard limit of core dumps is 0") { + t.Errorf("the log does not say why; its log:\n%s", e.stderr.text()) + } + for _, line := range []string{lineNoCoreDumps, lineTLSKey, lineAuditOpen, lineListening} { + if e.stderr.has(line) { + t.Errorf("the log has %q", line) + } + } + if _, err := os.Lstat(e.auditPath); !errors.Is(err, fs.ErrNotExist) { + t.Errorf("the broker made the audit log: %v", err) + } +} + +// TestBrokerCanaryThroughTelegram is the canary test of issue #11, end to +// end: the real broker, the real Telegram approver and the fake Telegram +// server, with the person's taps. A made-up secret X in the reason, the +// directory, the variable name, the command and the parent commands, as +// -pX, --token=X, password=X, "Authorization: X" and a long random word, +// never reaches the messages to Telegram, the audit log or the journal. +// Neither does a secret value or the bot token. +func TestBrokerCanaryThroughTelegram(t *testing.T) { + t.Parallel() + e := startListening(t, brokerOptions{}) + e.stderr.wait(t, lineListening) + c := e.httpClient() + x := canarySecret() + forms := []string{"-p" + x, "--token=" + x, "password=" + x, "Authorization: " + x, x} + reason := strings.Join(forms, " ") + + // Secrets, approved on the phone. + req := api.SecretsRequest{ + Secrets: []api.SecretRef{{Var: x, Name: "github-token"}}, + Command: append([]string{"mysql"}, forms...), + Dir: "/home/dev/" + x, + Reason: reason, + } + if got := secretValues(t, e.approve(c, api.PathSecrets, req)); got[x] != e.values["github-token"] { + t.Error("the variable did not get its value") + } + // An SSH login, approved on the phone. + sign, data := e.signRequest("web-1", "deploy") + sign.Commands = [][]string{ + {"ssh", "-p" + x, "deploy@web-1"}, {"bash", "-c", "deploy --token " + x}, append([]string{"make"}, forms...), + } + sign.Reason = reason + a := e.approve(c, api.PathSign, sign) + mustStatus(t, a, http.StatusOK) + ans, err := api.ReadAnswer[api.SignAnswer](bytes.NewReader(a.body)) + if err != nil { + t.Fatal(err) + } + if err := e.userKey.PublicKey().Verify(data, &ssh.Signature{Format: ans.Format, Blob: ans.Blob}); err != nil { + t.Errorf("the signature: %v", err) + } + // Denied by the broker's checks, before any message: an unknown + // secret named after the canary, and a login to a host that the key + // may not use. + req.Secrets = []api.SecretRef{{Var: "A", Name: x}} + mustStatus(t, e.post(c, api.PathSecrets, req), http.StatusForbidden) + other, _ := e.signRequest("db-1", "deploy") + other.Commands, other.Reason = sign.Commands, reason + mustStatus(t, e.post(c, api.PathSign, other), http.StatusForbidden) + + e.signal(syscall.SIGINT) + e.mustExit(exitOK) + auditLog, err := os.ReadFile(e.auditPath) + if err != nil { + t.Fatal(err) + } + texts := map[string]string{ + "the calls to Telegram": e.tg.bodies(), + "the audit log": string(auditLog), + "the journal": e.stderr.text(), + } + if !strings.Contains(texts["the calls to Telegram"], "***") { + t.Error("nothing was masked; the test does not test") + } + _, tokenSecret, _ := strings.Cut(e.tg.token, ":") + for where, text := range texts { + if strings.Contains(text, x) || strings.Contains(text, strings.ToLower(x)) { + t.Errorf("%s holds the canary", where) + } + if strings.Contains(text, e.values["github-token"]) { + t.Errorf("%s holds the secret value", where) + } + if strings.Contains(text, tokenSecret) { + t.Errorf("%s holds the bot token", where) + } + } + if e.tg.asked() != 2 { + t.Errorf("%d messages with buttons, want 2", e.tg.asked()) + } + for _, p := range e.tg.proxied() { + if p != "CONNECT "+telegramHost+":443" { + t.Errorf("the proxy was asked for %q", p) + } + } + got := eventNames(e.auditEntries()) + want := []string{ + "broker_started", + "request", "decision:approve_once", "secret_sent", + "request", "decision:approve_once", "signed", + "request", "denied:not_allowed", + "request", "denied:not_allowed", + "broker_stopped", + } + if !slices.Equal(got, want) { + t.Errorf("audit entries:\n%q\nwant:\n%q", got, want) + } +} + +// TestBrokerSignalDuringStart: a SIGTERM while the stores start ends the +// start. The broker closes what it opened, writes no broker_started +// entry, never listens or polls, and exits with 0. +func TestBrokerSignalDuringStart(t *testing.T) { + t.Parallel() + e := startBroker(t, brokerOptions{}, seamStoreStartWaits) + e.stderr.wait(t, lineAuditOpen) + e.signal(syscall.SIGTERM) + e.mustExit(exitOK) + e.stderr.inOrder(t, lineAuditOpen, "a signal asked to stop during the start", lineStoresClosed, lineAuditClosed, + lineStopped+`" exit_code=0`) + for _, line := range []string{lineApproverRuns, lineListening} { + if e.stderr.has(line) { + t.Errorf("the log has %q", line) + } + } + if data, err := os.ReadFile(e.auditPath); err != nil || len(data) != 0 { + t.Errorf("the audit log: %d bytes, %v; want it empty", len(data), err) + } + if p := e.tg.proxied(); len(p) != 0 { + t.Errorf("the broker called Telegram: %q", p) + } +} + +// tearAuditLog stops the broker of e with SIGTERM, and adds a torn last +// line to its audit log, as a power cut during a write would. +func tearAuditLog(t *testing.T, e *brokerEnv) { + t.Helper() + e.signal(syscall.SIGTERM) + e.mustExit(exitOK) + f, err := os.OpenFile(e.auditPath, os.O_WRONLY|os.O_APPEND, 0) + if err != nil { + t.Fatal(err) + } + if _, err := f.WriteString(`{"seq":3,"time":"2026-10-15T00:00:00Z","event":"req`); err != nil { + t.Fatal(err) + } + if err := f.Close(); err != nil { + t.Fatal(err) + } +} + +// tornLineEntries are the audit entries of two broker runs with a torn +// line between them. +var tornLineEntries = []string{ + "broker_started", "broker_stopped", "(torn line)", "log_recovered", "broker_started", "broker_stopped", +} + +// TestBrokerTornLineAlert: after a torn last line, the next start records +// it with a log_recovered entry, and the approver gets one alert. +func TestBrokerTornLineAlert(t *testing.T) { + t.Parallel() + e := startListening(t, brokerOptions{}) + e.stderr.wait(t, lineListening) + tearAuditLog(t, e) + e.start() + e.stderr.wait(t, lineListening) + e.signal(syscall.SIGTERM) // the stop waits for the alerts of the start + e.mustExit(exitOK) + if n := e.tg.alertCount("torn line"); n != 1 { + t.Errorf("%d alerts about the torn line, want 1", n) + } + if got := eventNames(e.auditEntries()); !slices.Equal(got, tornLineEntries) { + t.Errorf("audit entries %q, want %q", got, tornLineEntries) + } +} + +// TestBrokerTornLineAlertAfterAFailedStart: a power cut leaves a torn last +// line. At the next boot the listen address is not there yet, so the start +// fails at the listen step, after audit.Open recorded the torn line and +// queued the alert. systemd starts the broker again; this time the log +// ends in the log_recovered entry, with no broker_started after it, so the +// broker sends the alert then. The approver gets it once. +func TestBrokerTornLineAlertAfterAFailedStart(t *testing.T) { + t.Parallel() + e := startListening(t, brokerOptions{}) + e.stderr.wait(t, lineListening) + tearAuditLog(t, e) + + // The boot: the listen address is not there yet (here: taken). + ln, err := (&net.ListenConfig{}).Listen(t.Context(), "tcp", e.addr) + if err != nil { + t.Fatal(err) + } + e.start() + e.mustExit(exitFailure) + if !e.stderr.has("ALERT: the audit log ended in a torn line") || !e.stderr.has("cannot listen on") { + t.Fatalf("the first start did not record the torn line and fail at listen; its log:\n%s", e.stderr.text()) + } + if err := ln.Close(); err != nil { + t.Fatal(err) + } + if n := e.tg.alertCount("torn line"); n != 0 { + t.Errorf("the failed start sent %d alerts", n) + } + + // systemd starts it again, and the address is there now. + e.start() + e.stderr.wait(t, lineListening) + if !e.stderr.has("the start that recorded it stopped before it could send the alert") { + t.Errorf("the log does not say why the alert goes out again; its log:\n%s", e.stderr.text()) + } + e.signal(syscall.SIGTERM) // the stop waits for the alerts of the start + e.mustExit(exitOK) + if n := e.tg.alertCount("torn line"); n != 1 { + t.Errorf("%d alerts about the torn line reached the approver, want 1", n) + } + if got := eventNames(e.auditEntries()); !slices.Equal(got, tornLineEntries) { + t.Errorf("audit entries %q, want %q", got, tornLineEntries) + } + + // A third start sends no alert: broker_started follows the + // log_recovered entry now. + e.start() + e.stderr.wait(t, lineListening) + e.signal(syscall.SIGTERM) + e.mustExit(exitOK) + if n := e.tg.alertCount("torn line"); n != 1 { + t.Errorf("%d alerts about the torn line after a third start, want 1", n) + } +} + +// TestBrokerRefusedToken: when Telegram does not accept the bot token, the +// broker stops with exit code 77: 401 for a revoked token, 404 for a bot +// that is gone. The example unit lists 77 in RestartPreventExitStatus=, +// because a start with the same token would fail again: it would only call +// Telegram, and write broker_started and broker_stopped, every few +// seconds. Another refusal of getUpdates, such as 403, ends with 1. +func TestBrokerRefusedToken(t *testing.T) { + t.Parallel() + for _, tt := range []struct { + name string + opts brokerOptions + code int + }{ + {"a revoked token (401)", brokerOptions{unknownToken: true}, exitTokenRefused}, + {"a bot that is gone (404)", brokerOptions{refusePolls: http.StatusNotFound}, exitTokenRefused}, + {"another refusal (403)", brokerOptions{refusePolls: http.StatusForbidden}, exitFailure}, + } { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + e := startListening(t, tt.opts) + e.mustExit(tt.code) + if want := fmt.Sprintf(`" exit_code=%d`, tt.code); !e.stderr.has(want) { + t.Errorf("the log has no %q; its log:\n%s", want, e.stderr.text()) + } + got := eventNames(e.auditEntries()) + if want := []string{"broker_started", "broker_stopped"}; !slices.Equal(got, want) { + t.Errorf("audit entries %q, want %q", got, want) + } + }) + } +} + +// TestPortsOutside: the ports for the brokers of the tests are outside the +// kernel's own range of ports, below it or above it, or none. +func TestPortsOutside(t *testing.T) { + for _, tt := range []struct{ lo, hi, low, high int64 }{ + {32768, 60999, 10000, 32768}, // the default of Linux + {1024, 60999, 61000, 65536}, // a range from low ports + {10000, 65535, 0, 0}, // no room + {1024, 65535, 0, 0}, + {40000, 40999, 10000, 40000}, // more room below + {11000, 50000, 50001, 65536}, // more room above + } { + if low, high := portsOutside(tt.lo, tt.hi); low != tt.low || high != tt.high { + t.Errorf("portsOutside(%d, %d) = %d, %d; want %d, %d", tt.lo, tt.hi, low, high, tt.low, tt.high) + } + } +} + +// lineAlertAgain is the log line of a failed try of an alert message of +// the start. +const lineAlertAgain = "an alert message did not go out; the broker tries again" + +// TestBrokerAlertTriesAgain: after a power cut, the broker host can be up +// before Telegram can be reached. The broker writes broker_started, so the +// next start would not send the alert about the torn line again. So it +// tries again, with a growing wait, until Telegram can be reached, and the +// approver gets the alert once. +func TestBrokerAlertTriesAgain(t *testing.T) { + t.Parallel() + e := startListening(t, brokerOptions{}) + e.stderr.wait(t, lineListening) + tearAuditLog(t, e) + e.tg.setOffline(true) + e.start(seamFastAlertTries) + e.stderr.wait(t, lineListening) + e.stderr.waitCount(t, lineAlertAgain, 2) + if n := e.tg.alertCount("torn line"); n != 0 { + t.Fatalf("%d alerts while Telegram cannot be reached", n) + } + e.tg.setOffline(false) + e.tg.waitAlert("torn line") + e.signal(syscall.SIGTERM) + e.mustExit(exitOK) + if n := e.tg.alertCount("torn line"); n != 1 { + t.Errorf("%d alerts about the torn line, want 1", n) + } + if e.stderr.has("they are lost") { + t.Errorf("the log says an alert is lost; its log:\n%s", e.stderr.text()) + } +} + +// TestBrokerAlertTriesEndAtTheStop: while Telegram cannot be reached, the +// tries go on until the stop begins. Then the broker stops in time and +// cleanly, and its log says that the alert is lost. +func TestBrokerAlertTriesEndAtTheStop(t *testing.T) { + t.Parallel() + e := startListening(t, brokerOptions{}) + e.stderr.wait(t, lineListening) + tearAuditLog(t, e) + e.tg.setOffline(true) + e.start(seamFastAlertTries) + e.stderr.wait(t, lineListening) + e.stderr.waitCount(t, lineAlertAgain, 2) + e.signal(syscall.SIGTERM) + e.mustExit(exitOK) + if !e.stderr.has("they are lost") { + t.Errorf("the log does not say that the alert is lost; its log:\n%s", e.stderr.text()) + } + if n := e.tg.alertCount("torn line"); n != 0 { + t.Errorf("%d alerts, want none: Telegram could not be reached", n) + } +} + +// appendForgedRecovery appends a log_recovered entry that chains right +// onto the last entry of the audit log of e, as someone with write access +// to the log could: it names a torn line that never was. +func appendForgedRecovery(t *testing.T, e *brokerEnv, tornLength int, tornHash string) { + t.Helper() + data, err := os.ReadFile(e.auditPath) + if err != nil { + t.Fatal(err) + } + lines := bytes.Split(bytes.TrimSuffix(data, []byte("\n")), []byte("\n")) + last := lines[len(lines)-1] + var m struct { + Seq uint64 `json:"seq"` + } + if err := json.Unmarshal(last, &m); err != nil { + t.Fatal(err) + } + prev := sha256.Sum256(last) + line := fmt.Sprintf(`{"seq":%d,"time":"2026-10-15T00:00:00.000Z","event":"log_recovered","prev":"%s",`+ + `"torn_length":%d,"torn_sha256":"%s"}`+"\n", m.Seq+1, hex.EncodeToString(prev[:]), tornLength, tornHash) + f, err := os.OpenFile(e.auditPath, os.O_WRONLY|os.O_APPEND, 0) + if err != nil { + t.Fatal(err) + } + if _, err := f.WriteString(line); err != nil { + t.Fatal(err) + } + if err := f.Close(); err != nil { + t.Fatal(err) + } +} + +// TestBrokerForgedRecoveryAlertsOnce: a forged log_recovered entry as the +// last entry brings one alert at the next start, and no alert at any later +// start: never an endless alert (DESIGN.md section 9). +func TestBrokerForgedRecoveryAlertsOnce(t *testing.T) { + t.Parallel() + e := startListening(t, brokerOptions{}) + e.stderr.wait(t, lineListening) + e.signal(syscall.SIGTERM) + e.mustExit(exitOK) + appendForgedRecovery(t, e, 10, strings.Repeat("ab", 32)) + for round := 1; round <= 3; round++ { + e.start() + e.stderr.wait(t, lineListening) + e.signal(syscall.SIGTERM) // the stop waits for the first try of each alert + e.mustExit(exitOK) + if n := e.tg.alertCount("torn line"); n != 1 { + t.Errorf("after start %d: %d alerts about the torn line, want 1", round, n) + } + } +} + +// TestBrokerBadRecoveryStopsTheStart: a log_recovered last entry with a +// torn_length out of range, or a hash that is not 64 lower-case hex +// digits, is not a valid entry: the broker does not start, writes nothing +// and calls no Telegram; a person must look (DESIGN.md section 9). +func TestBrokerBadRecoveryStopsTheStart(t *testing.T) { + t.Parallel() + for _, tt := range []struct { + name string + length int + hash string + }{ + {"length 0", 0, strings.Repeat("ab", 32)}, + {"length over the line limit", 1 << 20, strings.Repeat("ab", 32)}, + {"a short hash", 10, "abcd"}, + {"upper-case hex", 10, strings.Repeat("AB", 32)}, + } { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + e := startListening(t, brokerOptions{}) + e.stderr.wait(t, lineListening) + e.signal(syscall.SIGTERM) + e.mustExit(exitOK) + appendForgedRecovery(t, e, tt.length, tt.hash) + before, err := os.ReadFile(e.auditPath) + if err != nil { + t.Fatal(err) + } + calls := len(e.tg.proxied()) + e.start() + e.mustExit(exitFailure) + after, err := os.ReadFile(e.auditPath) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(before, after) { + t.Error("the broker wrote to the audit log") + } + if n := len(e.tg.proxied()); n != calls { + t.Errorf("the broker called Telegram %d times", n-calls) + } + if !e.stderr.has("the last complete line is not a valid entry") { + t.Errorf("the log does not say why; its log:\n%s", e.stderr.text()) + } + }) + } +} diff --git a/cmd/nodkey/broker_test.go b/cmd/nodkey/broker_test.go new file mode 100644 index 0000000..e50ff3a --- /dev/null +++ b/cmd/nodkey/broker_test.go @@ -0,0 +1,362 @@ +package main + +import ( + "bytes" + "crypto/ed25519" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "encoding/pem" + "errors" + "fmt" + "os" + "path/filepath" + "slices" + "strings" + "testing" + "time" + + "golang.org/x/crypto/ssh" + + "github.com/code6x/nodkey/internal/broker" + "github.com/code6x/nodkey/internal/config" + "github.com/code6x/nodkey/internal/sshauth" + "github.com/code6x/nodkey/internal/telegram" +) + +// fakeAnchor is a line that looks like an audit anchor (DESIGN.md section +// 9). No line of the broker may start with "audit anchor ". +var fakeAnchor = "audit anchor 1:" + strings.Repeat("ab", 32) + +// noAnchorLine fails if a line of out starts like an audit anchor. +func noAnchorLine(t *testing.T, out string) { + t.Helper() + for line := range strings.SplitSeq(out, "\n") { + if strings.HasPrefix(line, "audit anchor ") { + t.Errorf("a line looks like an audit anchor: %q", line) + } + } +} + +func TestBrokerUsage(t *testing.T) { + for _, tt := range []struct { + name string + args []string + code int + log string // a part of the log line, if any + }{ + {"an unknown flag", []string{"broker", "--bogus"}, exitUsage, "nodkey broker: wrong usage"}, + {"an argument", []string{"broker", "extra"}, exitUsage, "it takes no arguments"}, + {"a flag with a line break", []string{"broker", "--x\n" + fakeAnchor}, exitUsage, "nodkey broker: wrong usage"}, + {"-h", []string{"broker", "-h"}, exitOK, ""}, + } { + var stdout, stderr bytes.Buffer + if code := run(tt.args, strings.NewReader(""), &stdout, &stderr); code != tt.code { + t.Errorf("%s: exit code %d, want %d", tt.name, code, tt.code) + } + if stdout.Len() != 0 { + t.Errorf("%s: stdout = %q, want it empty", tt.name, stdout.String()) + } + if !strings.Contains(stderr.String(), "Usage: nodkey broker [--config FILE]") { + t.Errorf("%s: stderr = %q, want the usage", tt.name, stderr.String()) + } + if tt.log != "" && (!strings.Contains(stderr.String(), "time=") || !strings.Contains(stderr.String(), tt.log)) { + t.Errorf("%s: stderr = %q, want a log line with %q", tt.name, stderr.String(), tt.log) + } + noAnchorLine(t, stderr.String()) + } + var stdout bytes.Buffer + if run([]string{"help"}, strings.NewReader(""), &stdout, &bytes.Buffer{}); !strings.Contains(stdout.String(), "broker") { + t.Errorf("help does not list broker: %q", stdout.String()) + } +} + +// TestBrokerConfigErrorIsOneLine: an error at the start goes through the +// broker's logger, which escapes a line break, also one in a path. The +// broker exits with 1. +func TestBrokerConfigErrorIsOneLine(t *testing.T) { + path := filepath.Join(t.TempDir(), "missing\n"+fakeAnchor) + var stderr bytes.Buffer + if code := run([]string{"broker", "--config", path}, strings.NewReader(""), &bytes.Buffer{}, &stderr); code != exitFailure { + t.Errorf("exit code %d, want %d", code, exitFailure) + } + if !strings.Contains(stderr.String(), "nodkey broker: cannot start") { + t.Errorf("stderr = %q", stderr.String()) + } + noAnchorLine(t, stderr.String()) +} + +func TestBrokerExitCode(t *testing.T) { + conflict := fmt.Errorf("telegram: %w", telegram.ErrPollConflict) + refused := fmt.Errorf("telegram: polling stopped: %w", telegram.ErrTokenRefused) + other := errors.New("made-up: the approver stopped") + for _, tt := range []struct { + bySignal, clean bool + runErr error + want int + }{ + {bySignal: true, clean: true, want: exitOK}, + {bySignal: true, clean: true, runErr: other, want: exitOK}, // Run ends with the context error + {bySignal: true, clean: false, want: exitFailure}, + {bySignal: false, clean: true, runErr: other, want: exitFailure}, + {bySignal: false, clean: true, want: exitFailure}, // a failed start, or the server stopped + {bySignal: false, clean: true, runErr: conflict, want: exitPollConflict}, + {bySignal: true, clean: false, runErr: conflict, want: exitPollConflict}, + {bySignal: false, clean: false, runErr: conflict, want: exitPollConflict}, + {bySignal: false, clean: true, runErr: refused, want: exitTokenRefused}, + {bySignal: true, clean: true, runErr: refused, want: exitTokenRefused}, + } { + if got := brokerExitCode(tt.bySignal, tt.runErr, tt.clean); got != tt.want { + t.Errorf("signal %v, clean %v, Run %v: exit code %d, want %d", tt.bySignal, tt.clean, tt.runErr, got, tt.want) + } + } +} + +// makeLogin returns what OpenSSH sends to its agent for a login as user +// with the key userKey to the host with the key hostKey: the content of +// the session-bind, with the host key's signature over a new session +// identifier, and the data to sign, a login request for that session (RFC +// 4252 section 7). +func makeLogin(t *testing.T, userKey ssh.PublicKey, hostKey ssh.Signer, user string) (bind, data []byte) { + t.Helper() + sessionID := make([]byte, 32) + if _, err := rand.Read(sessionID); err != nil { + t.Fatal(err) + } + sig, err := hostKey.Sign(rand.Reader, sessionID) + if err != nil { + t.Fatal(err) + } + bind = ssh.Marshal(struct { + HostKey []byte + SessionID []byte + Signature []byte + Forwarding bool + }{hostKey.PublicKey().Marshal(), sessionID, ssh.Marshal(sig), false}) + const msgUserAuthRequest = 50 + data = ssh.Marshal(struct { + SessionID []byte + Type byte + User string + Service string + Method string + HasSig bool + Algorithm string + PublicKey []byte + }{sessionID, msgUserAuthRequest, user, "ssh-connection", "publickey", true, userKey.Type(), userKey.Marshal()}) + return bind, data +} + +// newHostSigner makes an Ed25519 SSH key. +func newHostSigner(t *testing.T) ssh.Signer { + t.Helper() + _, priv, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + t.Fatal(err) + } + s, err := ssh.NewSignerFromSigner(priv) + if err != nil { + t.Fatal(err) + } + return s +} + +// TestLoadSSHKeys loads keys as nodkey broker does at start: each key +// file of a [key.*] section, with its hosts. It refuses two sections with +// the same public key in two files, a key with a passphrase, a file that +// holds no key, and a key that sshauth.NewKey refuses. No error shows the +// content of a file. +func TestLoadSSHKeys(t *testing.T) { + dir := t.TempDir() + write := func(name string, data []byte) string { + p := filepath.Join(dir, name) + if err := os.WriteFile(p, data, 0o600); err != nil { + t.Fatal(err) + } + return p + } + read := func(path string) ([]byte, error) { return os.ReadFile(path) } //nolint:gosec // G304: the test's own files. + _, priv, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + t.Fatal(err) + } + block, err := ssh.MarshalPrivateKey(priv, "") + if err != nil { + t.Fatal(err) + } + keyPEM := pem.EncodeToMemory(block) + deploy := write("deploy", keyPEM) + userKey, err := ssh.NewSignerFromSigner(priv) + if err != nil { + t.Fatal(err) + } + + // A key loads, with its hosts. + known := make(map[string]ssh.Signer) + var kh strings.Builder + for _, host := range []string{"web-1", "db-1"} { + known[host] = newHostSigner(t) + kh.WriteString(host + " " + string(ssh.MarshalAuthorizedKey(known[host].PublicKey()))) + } + knownHosts, err := config.ParseKnownHosts([]byte(kh.String())) + if err != nil { + t.Fatal(err) + } + key := func(file string, hosts ...string) config.Key { + return config.Key{File: file, Clients: []string{"laptop"}, Rule: config.Rule{Ask: config.AskAlways}, Hosts: hosts} + } + keys, err := loadSSHKeys(&config.Broker{Keys: map[string]config.Key{"deploy": key(deploy, "web-1")}}, read) + if err != nil { + t.Fatal(err) + } + k := keys["deploy"] + if k.Name() != "deploy" || !bytes.Equal(k.PublicKey().Marshal(), userKey.PublicKey().Marshal()) { + t.Fatalf("the key is %v", k) + } + for host, want := range map[string]error{"web-1": nil, "db-1": sshauth.ErrHostNotAllowed} { + bind, data := makeLogin(t, userKey.PublicKey(), known[host], "deploy") + _, err := k.Check(sshauth.Request{Key: userKey.PublicKey().Marshal(), Data: data, Binds: [][]byte{bind}}, knownHosts) + if !errors.Is(err, want) { + t.Errorf("a login to %s: %v, want %v", host, err, want) + } + } + + // Errors. + canaryText := "made-up " + canaryWord() + encrypted, err := ssh.MarshalPrivateKeyWithPassphrase(priv, "", []byte(canaryWord())) + if err != nil { + t.Fatal(err) + } + small, err := rsa.GenerateKey(rand.Reader, 1024) //nolint:gosec // G403: the test needs a key that is too small. + if err != nil { + t.Fatal(err) + } + smallPEM := pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(small)}) + for _, tt := range []struct { + name string + keys map[string]config.Key + want string + }{ + {"the same key in two files", map[string]config.Key{ + "deploy": key(deploy), "copy": key(write("copy", keyPEM), "db-1"), + }, "[key.copy] and [key.deploy] hold the same public key"}, + {"a passphrase", map[string]config.Key{"deploy": key(write("encrypted", pem.EncodeToMemory(encrypted)))}, + "the key has a passphrase"}, + {"not a key", map[string]config.Key{"deploy": key(write("text", []byte(canaryText)))}, + "does not hold an SSH private key"}, + {"a PEM block that is not a key", map[string]config.Key{"deploy": key(write("block", + pem.EncodeToMemory(&pem.Block{Type: canaryWord(), Bytes: []byte(canaryText)})))}, + "does not hold an SSH private key"}, + {"an RSA key that is too small", map[string]config.Key{"deploy": key(write("small", smallPEM))}, + "fewer than 2048 bits"}, + {"a file that cannot be read", map[string]config.Key{"deploy": key(filepath.Join(dir, "missing"))}, + "[key.deploy] file: open "}, + } { + keys, err := loadSSHKeys(&config.Broker{Keys: tt.keys}, read) + if err == nil || !strings.Contains(err.Error(), tt.want) { + t.Errorf("%s: %v, want an error with %q", tt.name, err, tt.want) + } + if keys != nil { + t.Errorf("%s: keys were returned with the error", tt.name) + } + if err != nil && strings.Contains(strings.ToLower(err.Error()), strings.ToLower(canaryWord())) { + t.Errorf("%s: the error shows the content of the file", tt.name) + } + } +} + +// canaryWord returns a made-up word that the tests look for, made at run +// time. +func canaryWord() string { + return "CANARY" + strings.Repeat("Q7", 8) +} + +// TestStopStep: a step of the stop that panics is not clean, and its panic +// value is not logged; a step that takes too long is left behind, and the +// stop ends; a closed channel counts also after the deadline. +func TestStopStep(t *testing.T) { + var logs bytes.Buffer + b := &brokerParts{log: broker.NewLogger(&logs)} + soon := time.Now().Add(time.Minute) + if done, ok := b.step("a step", func() {}, soon); !done || !ok { + t.Errorf("a step that returns: done %v, ok %v", done, ok) + } + if done, ok := b.step("a step that panics", func() { panic(canaryWord()) }, soon); !done || ok { + t.Errorf("a step that panics: done %v, ok %v", done, ok) + } + block := make(chan struct{}) + defer close(block) + if done, ok := b.step("a slow step", func() { <-block }, time.Now().Add(10*time.Millisecond)); done || ok { + t.Errorf("a step that takes too long: done %v, ok %v", done, ok) + } + for _, want := range []string{"a step of the stop panicked", `step="a step that panics"`, + "the stop takes too long", `waiting_for="a slow step"`} { + if !strings.Contains(logs.String(), want) { + t.Errorf("the log has no %q: %s", want, logs.String()) + } + } + if strings.Contains(logs.String(), canaryWord()) { + t.Error("the log holds the panic value") + } + closed := make(chan struct{}) + close(closed) + if !waitUntil(closed, time.Now().Add(-time.Second)) { + t.Error("a closed channel did not count after the deadline") + } + if waitUntil(make(chan struct{}), time.Now().Add(-time.Second)) { + t.Error("an open channel counted after the deadline") + } +} + +// TestUnitPreventsRestarts: the example systemd unit does not start the +// broker again after the exit codes that only a person can fix: wrong +// usage, a polling conflict and a bot token that Telegram refuses. +func TestUnitPreventsRestarts(t *testing.T) { + data, err := os.ReadFile(filepath.Join("..", "..", "contrib", "systemd", "nodkey.service")) + if err != nil { + t.Fatal(err) + } + want := fmt.Sprintf("RestartPreventExitStatus=%d %d %d", exitUsage, exitPollConflict, exitTokenRefused) + var found []string + for line := range strings.SplitSeq(string(data), "\n") { + if strings.HasPrefix(line, "RestartPreventExitStatus=") { + found = append(found, line) + } + } + if len(found) != 1 || found[0] != want { + t.Errorf("the unit has %q, want %q", found, want) + } +} + +// TestUnitHasEachCredential: the example unit loads a credential for each +// ${CREDENTIALS_DIRECTORY}/NAME of the example config in DESIGN.md +// section 14, and no other. +func TestUnitHasEachCredential(t *testing.T) { + design, err := os.ReadFile(filepath.Join("..", "..", "DESIGN.md")) + if err != nil { + t.Fatal(err) + } + unit, err := os.ReadFile(filepath.Join("..", "..", "contrib", "systemd", "nodkey.service")) + if err != nil { + t.Fatal(err) + } + _, example, ok := strings.Cut(string(design), "Broker, `/etc/nodkey/broker.toml`:\n\n```toml\n") + if !ok { + t.Fatal("DESIGN.md has no example of the broker config") + } + example, _, _ = strings.Cut(example, "```") + var inConfig, inUnit []string + for _, part := range strings.Split(example, "${CREDENTIALS_DIRECTORY}/")[1:] { + inConfig = append(inConfig, part[:strings.IndexByte(part, '"')]) + } + for line := range strings.SplitSeq(string(unit), "\n") { + if rest, ok := strings.CutPrefix(line, "LoadCredentialEncrypted="); ok { + inUnit = append(inUnit, rest[:strings.IndexByte(rest, ':')]) + } + } + slices.Sort(inConfig) + slices.Sort(inUnit) + if len(inConfig) == 0 || !slices.Equal(inConfig, inUnit) { + t.Errorf("the credentials of the example config %q and of the unit %q differ", inConfig, inUnit) + } +} diff --git a/cmd/nodkey/brokerenv_linux_test.go b/cmd/nodkey/brokerenv_linux_test.go new file mode 100644 index 0000000..51dd8b4 --- /dev/null +++ b/cmd/nodkey/brokerenv_linux_test.go @@ -0,0 +1,938 @@ +package main + +import ( + "bufio" + "bytes" + "context" + "crypto/ed25519" + "crypto/rand" + "crypto/tls" + "encoding/json" + "encoding/pem" + "errors" + "fmt" + "io" + "math/big" + "net" + "net/http" + "os" + "os/exec" + "path/filepath" + "slices" + "strconv" + "strings" + "sync" + "syscall" + "testing" + "time" + + "golang.org/x/crypto/ssh" + + "github.com/code6x/nodkey/internal/api" + "github.com/code6x/nodkey/internal/config" + "github.com/code6x/nodkey/internal/store" + "github.com/code6x/nodkey/internal/transport" +) + +// The tests of nodkey broker run the real command in a child process: the +// test binary again, with TestBrokerChild only, as the keygen test does. +// The child gets a config whose files pass every check of nodkey (in a +// private directory, see privateTestDir), fake store programs (small +// shell scripts), and a fake Telegram server. +// +// The child reaches the fake through a fake HTTPS proxy (HTTPS_PROXY). The +// proxy takes only a CONNECT to api.telegram.org, and answers the TLS +// handshake itself, with a certificate from a test CA that the child +// trusts through SSL_CERT_FILE (and an empty SSL_CERT_DIR). So the child +// runs the same code as in production, and nothing goes to the real Bot +// API. Go reads the proxy setting and the system's CAs once per process, +// which is one more reason for a child process. + +// Variables of the child process. +const ( + // brokerChildConfig is the config of the child's broker. Without it, + // TestBrokerChild skips. + brokerChildConfig = "NODKEY_TEST_BROKER_CONFIG" + // brokerChildSeams lists test changes in the child, with commas: see + // TestBrokerChild. + brokerChildSeams = "NODKEY_TEST_BROKER_SEAMS" + // brokerChildProxy is the address of the fake proxy. The child checks + // that every call to Telegram goes there. + brokerChildProxy = "NODKEY_TEST_BROKER_PROXY" +) + +// The test changes in the child (brokerChildSeams). +const ( + // seamStoreStartFails makes the start of the store brokenStore fail. + seamStoreStartFails = "store-start-fails" + // seamNoCoreDumps sets the hard limit of core dumps to 0 before the + // broker starts, so the broker cannot set its soft limit to 1. + seamNoCoreDumps = "core-hard-limit-0" + // seamStoreStartWaits makes the start of every store wait until the + // start of the broker ends, such as by a signal. + seamStoreStartWaits = "store-start-waits" + // seamFastAlertTries makes the tries of the alert messages of the + // start short: 1.5 seconds each, and 0.1 to 0.4 seconds between them. + seamFastAlertTries = "fast-alert-tries" +) + +// telegramHost is the host of the Bot API. +const telegramHost = "api.telegram.org" + +// approverID is the made-up Telegram user ID of the approver. +const approverID int64 = 7000001 + +// brokerWait bounds every wait of the tests of nodkey broker. +const brokerWait = 20 * time.Second + +// brokenStore is the store whose start fails with seamStoreStartFails. Its +// program fails too. +const brokenStore = "vault" + +// The item IDs of the test secrets. +const ( + idGitHub = "00000000-0000-0000-0000-000000000001" + idDB = "00000000-0000-0000-0000-000000000002" +) + +// TestBrokerChild runs only as the child of the tests of nodkey broker: +// it runs nodkey broker with the config in brokerChildConfig, and exits +// with its exit code. +func TestBrokerChild(t *testing.T) { + path := os.Getenv(brokerChildConfig) + if path == "" { + t.Skip("runs only as the child of the tests of nodkey broker") + } + for seam := range strings.SplitSeq(os.Getenv(brokerChildSeams), ",") { + switch seam { + case "": + case seamStoreStartFails: + real := newStore + newStore = func(cfg config.Store, dir string) (store.Store, error) { + s, err := real(cfg, dir) + if err != nil || cfg.Name != brokenStore { + return s, err + } + return startFails{s}, nil + } + case seamStoreStartWaits: + real := newStore + newStore = func(cfg config.Store, dir string) (store.Store, error) { + s, err := real(cfg, dir) + if err != nil { + return s, err + } + return startWaits{s}, nil + } + case seamFastAlertTries: + alertTimeout, alertFirstWait, alertMaxWait = 1500*time.Millisecond, 100*time.Millisecond, 400*time.Millisecond + case seamNoCoreDumps: + if err := syscall.Setrlimit(syscall.RLIMIT_CORE, &syscall.Rlimit{}); err != nil { + fmt.Fprintln(os.Stderr, "child: cannot set the hard limit of core dumps to 0") + os.Exit(3) + } + default: + fmt.Fprintln(os.Stderr, "child: unknown test change") + os.Exit(3) + } + } + // Every call to Telegram must go to the fake, through the proxy of + // the test: never to the real Bot API. The Telegram client uses + // http.ProxyFromEnvironment, and Go reads it once per process. + req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, "https://"+telegramHost+"/", nil) + if err != nil { + os.Exit(3) + } + proxy, err := http.ProxyFromEnvironment(req) + if err != nil || proxy == nil || proxy.Host != os.Getenv(brokerChildProxy) { + fmt.Fprintln(os.Stderr, "child: calls to Telegram would not go to the fake proxy; the broker does not start") + os.Exit(3) + } + os.Exit(run([]string{"broker", "--config", path}, os.Stdin, os.Stdout, os.Stderr)) +} + +// startFails is a store whose start fails. Fetch is the real store's. +type startFails struct{ store.Store } + +func (startFails) Start(context.Context) error { + return fmt.Errorf("store %q: made-up: the store cannot start", brokenStore) +} + +// startWaits is a store whose start waits until its context ends. Fetch is +// the real store's. +type startWaits struct{ store.Store } + +func (startWaits) Start(ctx context.Context) error { + <-ctx.Done() + return ctx.Err() +} + +// privateTestDir returns a new directory for the files of one test. Every +// directory on the way to it passes the checks of nodkey (DESIGN.md +// section 14): owned by root or by this user, and not writable by group or +// others. The system temporary directory fails them, so it is made in +// $XDG_RUNTIME_DIR or in the user's cache directory. It must also be on a +// local file system that nodkey knows, and allow running programs. The +// test is skipped if there is no such place, or fails when CI is set. +func privateTestDir(t *testing.T) string { + t.Helper() + var bases []string + if d := os.Getenv("XDG_RUNTIME_DIR"); d != "" { + bases = append(bases, d) + } + if d, err := os.UserCacheDir(); err == nil { + bases = append(bases, d) + } + for _, base := range bases { + dir, err := os.MkdirTemp(base, "nodkey-test-") + if err != nil { + continue + } + t.Cleanup(func() { _ = os.RemoveAll(dir) }) //nolint:gosec // G703: the test's own new directory. + if usableTestDir(t, dir) { + return dir + } + } + if os.Getenv("CI") != "" { + t.Fatalf("no directory passes the file checks of nodkey: tried %q", bases) + } + t.Skipf("no directory passes the file checks of nodkey: tried %q", bases) + return "" +} + +// usableTestDir reports whether dir passes the checks of a key file, and +// whether a program there can run. +func usableTestDir(t *testing.T, dir string) bool { + t.Helper() + if os.Chmod(dir, 0o700) != nil { //nolint:gosec // G302: a directory, private to its owner. + return false + } + probe := filepath.Join(dir, "probe") + //nolint:gosec // G703: a file in the test's own new directory. + if os.WriteFile(probe, []byte("#!/bin/sh\nexit 0\n"), 0o600) != nil || config.CheckPrivateFile(probe) != nil { + return false + } + if os.Chmod(probe, 0o700) != nil { //nolint:gosec // G302: a program of the test. + return false + } + ok := exec.CommandContext(t.Context(), probe).Run() == nil //nolint:gosec // G204: a program that the test wrote. + _ = os.Remove(probe) //nolint:gosec // G703: a file in the test's own new directory. + return ok +} + +// writeTestFile writes data to path with exactly mode. +func writeTestFile(t *testing.T, path string, data []byte, mode os.FileMode) { + t.Helper() + if err := os.WriteFile(path, data, 0o600); err != nil { //nolint:gosec // G703: a file in the test's own directory. + t.Fatal(err) + } + if err := os.Chmod(path, mode); err != nil { + t.Fatal(err) + } +} + +// madeUp returns a made-up value with what at its start, made at run time, +// so the source holds nothing that looks like a token. +func madeUp(what string) string { + return "made-up-" + what + "-" + rand.Text() +} + +// canarySecret returns a made-up secret, made at run time, as in the tests of +// package broker: 32 upper-case letters and digits that change often. It +// is a valid variable name, and package claim masks it in every form, as +// a random-looking word. +func canarySecret() string { + const letters = "QWERTYUPASDFGHJKLZXCVBNM" + const digits = "7391542860" + var b strings.Builder + for i := range 16 { + b.WriteByte(letters[(i*7+3)%len(letters)]) + b.WriteByte(digits[(i*3+1)%len(digits)]) + } + return b.String() +} + +// newBotToken returns a made-up bot token: digits, ":", and 35 random +// characters. +func newBotToken() string { + n, err := rand.Int(rand.Reader, big.NewInt(1_000_000_000)) + if err != nil { + panic(err) + } + secret := strings.Map(func(r rune) rune { + if r >= '2' && r <= '7' { + return r + } + return r | 0x20 // some lower case letters + }, rand.Text()+rand.Text()) + return strconv.FormatInt(1_000_000_000+n.Int64(), 10) + ":" + secret[:35] +} + +// brokerOptions change the broker of a test. +type brokerOptions struct { + // refusePolls, if set, is the status of Telegram's error answer to + // every getUpdates call, such as 409 for a polling conflict. + refusePolls int + // unknownToken puts a bot token into the token file that the fake + // Telegram does not know: it answers every call with 401, as Telegram + // does for a revoked token. + unknownToken bool + // holdAlerts makes alert messages (messages without buttons) wait at + // the fake Telegram until the test releases them. + holdAlerts bool + // sameKeyTwice adds [key.copy], whose file is a copy of the key file + // of [key.deploy]. + sameKeyTwice bool + // listen is the listen address of the broker; the default is a free + // port (freeAddress). + listen string +} + +// brokerEnv is nodkey broker in a child process, with its files and the +// fake Telegram server. +type brokerEnv struct { + t *testing.T + dir string // the private directory of the test + cfgPath string + auditPath string + addr string // the listen address + brokerPin config.Pin + client tls.Certificate // the TLS key of the client laptop + tg *fakeTelegram + values map[string]string // the secret values, by secret name + userKey ssh.Signer // the key of [key.deploy] + hostKeys map[string]ssh.Signer + + cmd *exec.Cmd + stderr *lineLog // the child's stderr: the broker's journal + exited chan struct{} + waitErr error // set before exited is closed +} + +// newBrokerEnv makes the files of a broker in a new private directory, +// and a fake Telegram server. It does not start the broker; see start. +func newBrokerEnv(t *testing.T, opts brokerOptions) *brokerEnv { + t.Helper() + dir := privateTestDir(t) + e := &brokerEnv{ + t: t, + dir: dir, + cfgPath: filepath.Join(dir, "etc", "broker.toml"), + auditPath: filepath.Join(dir, "state", "audit.jsonl"), + values: map[string]string{"github-token": madeUp("github"), "db-password": madeUp("db")}, + hostKeys: make(map[string]ssh.Signer), + stderr: newLineLog(), + } + for _, sub := range []string{"etc", "creds", "bin", "state"} { + if err := os.Mkdir(filepath.Join(dir, sub), 0o700); err != nil { + t.Fatal(err) + } + } + creds := filepath.Join(dir, "creds") + + // The TLS keys, as nodkey keygen makes them. + pin, err := transport.GenerateKeyFile(filepath.Join(creds, "broker-tls-key")) + if err != nil { + t.Fatal(err) + } + e.brokerPin = pin + clientPin, err := transport.GenerateKeyFile(filepath.Join(dir, "client.key")) + if err != nil { + t.Fatal(err) + } + if e.client, err = transport.LoadKeyFile(filepath.Join(dir, "client.key")); err != nil { + t.Fatal(err) + } + + // The tokens, and the fake store programs. The store "main" hands out + // github-token; the program of the store "vault" always fails. + e.tg = newFakeTelegram(t, opts) + token := e.tg.token + if opts.unknownToken { + token = newBotToken() + } + writeTestFile(t, filepath.Join(creds, "telegram-token"), []byte(token+"\n"), 0o600) + writeTestFile(t, filepath.Join(creds, "bws-token"), []byte(madeUp("bws-token")+"\n"), 0o600) + writeTestFile(t, filepath.Join(creds, "bws-token-vault"), []byte(madeUp("bws-token")+"\n"), 0o600) + bws := fmt.Sprintf("#!/bin/sh\n# A fake bws: it prints the secret with the ID $3.\n"+ + "case \"$3\" in\n%s) printf '%%s' '{\"id\":\"%s\",\"value\":\"%s\"}' ;;\n*) exit 1 ;;\nesac\n", + idGitHub, idGitHub, e.values["github-token"]) + writeTestFile(t, filepath.Join(dir, "bin", "bws"), []byte(bws), 0o700) + writeTestFile(t, filepath.Join(dir, "bin", "bws-vault"), []byte("#!/bin/sh\nexit 1\n"), 0o700) + + // The SSH key of [key.deploy], and the hosts. + _, priv, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + t.Fatal(err) + } + if e.userKey, err = ssh.NewSignerFromSigner(priv); err != nil { + t.Fatal(err) + } + block, err := ssh.MarshalPrivateKey(priv, "") + if err != nil { + t.Fatal(err) + } + writeTestFile(t, filepath.Join(creds, "ssh-deploy"), pem.EncodeToMemory(block), 0o600) + var knownHosts strings.Builder + for _, host := range []string{"web-1", "db-1"} { + _, hostPriv, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + t.Fatal(err) + } + if e.hostKeys[host], err = ssh.NewSignerFromSigner(hostPriv); err != nil { + t.Fatal(err) + } + knownHosts.WriteString(host + " " + string(ssh.MarshalAuthorizedKey(e.hostKeys[host].PublicKey()))) + } + writeTestFile(t, filepath.Join(dir, "etc", "known_hosts"), []byte(knownHosts.String()), 0o600) + + e.addr = opts.listen + if e.addr == "" { + e.addr = freeAddress(t) + } + cfg := fmt.Sprintf(`listen = %q +tls_key = "${CREDENTIALS_DIRECTORY}/broker-tls-key" +audit_log = %q +known_hosts = %q + +[telegram] +token_file = "${CREDENTIALS_DIRECTORY}/telegram-token" +user_id = %d + +[client.laptop] +pin = %q + +[store.main] +type = "bws" +program = %q +token_file = "${CREDENTIALS_DIRECTORY}/bws-token" + +[store.vault] +type = "bws" +program = %q +token_file = "${CREDENTIALS_DIRECTORY}/bws-token-vault" + +[secret.github-token] +store = "main" +id = %q +clients = ["laptop"] +ask = "always" + +[secret.db-password] +store = "vault" +id = %q +clients = ["laptop"] +ask = "never" +notify = false + +[key.deploy] +file = "${CREDENTIALS_DIRECTORY}/ssh-deploy" +clients = ["laptop"] +ask = "always" +hosts = ["web-1"] +`, e.addr, e.auditPath, filepath.Join(dir, "etc", "known_hosts"), approverID, clientPin.String(), + filepath.Join(dir, "bin", "bws"), filepath.Join(dir, "bin", "bws-vault"), idGitHub, idDB) + if opts.sameKeyTwice { + copied, err := os.ReadFile(filepath.Join(creds, "ssh-deploy")) //nolint:gosec // G304: the test's own key file. + if err != nil { + t.Fatal(err) + } + writeTestFile(t, filepath.Join(creds, "ssh-copy"), copied, 0o600) + cfg += ` +[key.copy] +file = "${CREDENTIALS_DIRECTORY}/ssh-copy" +clients = ["laptop"] +ask = "always" +hosts = ["db-1"] +` + } + writeTestFile(t, e.cfgPath, []byte(cfg), 0o600) + return e +} + +// The ports that freeAddress handed out in this process. +var ( + portsMu sync.Mutex + portsTaken = make(map[int64]bool) +) + +// freeAddress returns an address on the loopback interface with a port +// that is free now, for the broker of one test. The broker listens on it +// a moment later. So the port is outside the range of ports that the +// kernel gives out by itself, to sockets that ask for any port or connect +// (net.ipv4.ip_local_port_range): no other socket of the tests takes it in +// between. No two tests of this process get the same port. When the +// kernel's range leaves no room, the kernel chooses the port, and +// startListening tries again if another socket takes it first. +func freeAddress(t *testing.T) string { + t.Helper() + low, high := int64(0), int64(0) + if data, err := os.ReadFile("/proc/sys/net/ipv4/ip_local_port_range"); err == nil { + if f := strings.Fields(string(data)); len(f) == 2 { + lo, err1 := strconv.ParseInt(f[0], 10, 64) + hi, err2 := strconv.ParseInt(f[1], 10, 64) + if err1 == nil && err2 == nil { + low, high = portsOutside(lo, hi) + } + } + } + for range 200 { + if high <= low { + break + } + n, err := rand.Int(rand.Reader, big.NewInt(high-low)) + if err != nil { + t.Fatal(err) + } + port := low + n.Int64() + portsMu.Lock() + taken := portsTaken[port] + portsTaken[port] = true + portsMu.Unlock() + if taken { + continue + } + addr := "127.0.0.1:" + strconv.FormatInt(port, 10) + ln, err := (&net.ListenConfig{}).Listen(t.Context(), "tcp", addr) + if err != nil { + continue // in use + } + if err := ln.Close(); err != nil { + t.Fatal(err) + } + return addr + } + for range 200 { + ln, err := (&net.ListenConfig{}).Listen(t.Context(), "tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + addr := ln.Addr().(*net.TCPAddr) + if err := ln.Close(); err != nil { + t.Fatal(err) + } + portsMu.Lock() + taken := portsTaken[int64(addr.Port)] + portsTaken[int64(addr.Port)] = true + portsMu.Unlock() + if !taken { + return addr.String() + } + } + t.Fatal("found no free port for the broker") + return "" +} + +// portsOutside returns the ports [low, high) for the brokers of the tests, +// outside the kernel's own range of ports from lo to hi: the larger of the +// ports from 10000 to below lo, and the ports above hi. high is not more +// than low when neither has 1000 ports. +func portsOutside(lo, hi int64) (low, high int64) { + const first, last, enough = 10000, 65535, 1000 + below, above := lo-first, last-hi + switch { + case below >= enough && below >= above: + return first, lo + case above >= enough: + return hi + 1, last + 1 + } + return 0, 0 +} + +// startBroker makes the files of a broker and starts it. +func startBroker(t *testing.T, opts brokerOptions, seams ...string) *brokerEnv { + t.Helper() + e := newBrokerEnv(t, opts) + e.start(seams...) + return e +} + +// startListening makes the files of a broker, starts it, and waits until +// it has opened its listen address. The port was free a moment before the +// broker opens it, but another program on the machine can take it in +// between; then the broker stops, and the test starts it again with +// another port. +func startListening(t *testing.T, opts brokerOptions, seams ...string) *brokerEnv { + t.Helper() + e := newBrokerEnv(t, opts) + for try := 1; ; try++ { + e.start(seams...) + if e.stderr.waitOrEnd(t, lineListenOpen) { + return e + } + if try == 3 || !e.stderr.has("address already in use") { + t.Fatalf("the broker did not open its listen address; its log:\n%s", e.stderr.text()) + } + e.waitExit() + old := e.addr + e.addr = freeAddress(t) + cfg, err := os.ReadFile(e.cfgPath) + if err != nil { + t.Fatal(err) + } + writeTestFile(t, e.cfgPath, bytes.Replace(cfg, []byte(strconv.Quote(old)), []byte(strconv.Quote(e.addr)), 1), 0o600) + } +} + +// start starts the broker in a child process, with the test changes +// seams. The child's environment holds only what the broker needs, so +// nothing from the environment of the test, such as NO_PROXY, changes +// where the calls to Telegram go. +func (e *brokerEnv) start(seams ...string) { + t := e.t + t.Helper() + e.stderr = newLineLog() + cmd := exec.CommandContext(t.Context(), os.Args[0], "-test.run=^TestBrokerChild$") //nolint:gosec // G204: the test runs its own binary again. + cmd.Env = []string{ + brokerChildConfig + "=" + e.cfgPath, + brokerChildSeams + "=" + strings.Join(seams, ","), + brokerChildProxy + "=" + e.tg.proxyAddr, + "CREDENTIALS_DIRECTORY=" + filepath.Join(e.dir, "creds"), + "HTTPS_PROXY=http://" + e.tg.proxyAddr, + "SSL_CERT_FILE=" + e.tg.caFile, + "SSL_CERT_DIR=" + e.tg.caDir, + } + for _, name := range []string{"GORACE", "GOCOVERDIR", "TMPDIR"} { + if v, ok := os.LookupEnv(name); ok { + cmd.Env = append(cmd.Env, name+"="+v) + } + } + stderr, err := cmd.StderrPipe() + if err != nil { + t.Fatal(err) + } + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + e.cmd = cmd + exited, log := make(chan struct{}), e.stderr + e.exited = exited + go func() { + log.readFrom(stderr) + e.waitErr = cmd.Wait() + close(exited) + }() + t.Cleanup(func() { + select { + case <-exited: + default: + _ = cmd.Process.Kill() + <-exited + } + if strings.Contains(log.text(), "DATA RACE") { + t.Error("the race detector found a data race in the broker") + } + }) +} + +// running reports whether the broker still runs. +func (e *brokerEnv) running() bool { + select { + case <-e.exited: + return false + default: + return true + } +} + +// signal sends sig to the broker. +func (e *brokerEnv) signal(sig os.Signal) { + e.t.Helper() + if err := e.cmd.Process.Signal(sig); err != nil { + e.t.Fatal(err) + } +} + +// waitExit waits until the broker ends, and returns its exit code. +func (e *brokerEnv) waitExit() int { + e.t.Helper() + select { + case <-e.exited: + case <-time.After(brokerWait + stopTimeout): + e.t.Fatalf("the broker did not end; its log:\n%s", e.stderr.text()) + } + var exit *exec.ExitError + switch { + case e.waitErr == nil: + return 0 + case errors.As(e.waitErr, &exit) && exit.ExitCode() >= 0: + return exit.ExitCode() + } + e.t.Fatalf("the broker: %v; its log:\n%s", e.waitErr, e.stderr.text()) + return -1 +} + +// mustExit waits until the broker ends, and fails unless it exited with +// code. +func (e *brokerEnv) mustExit(code int) { + e.t.Helper() + if got := e.waitExit(); got != code { + e.t.Fatalf("the broker exited with %d, want %d; its log:\n%s", got, code, e.stderr.text()) + } +} + +// httpClient returns an HTTP client of the client laptop. +func (e *brokerEnv) httpClient() *http.Client { + e.t.Helper() + c, err := transport.NewClient(e.addr, e.client, e.brokerPin) + if err != nil { + e.t.Fatal(err) + } + e.t.Cleanup(c.CloseIdleConnections) + return c +} + +// brokerAnswer is what a client got. +type brokerAnswer struct { + status int + body []byte + err error +} + +// post sends body to path as JSON, and returns the answer. It does not +// fail the test, so it may run in its own goroutine. +func (e *brokerEnv) post(c *http.Client, path string, body any) brokerAnswer { + data, err := json.Marshal(body) + if err != nil { + return brokerAnswer{err: err} + } + ctx, cancel := context.WithTimeout(context.Background(), 2*brokerWait) + defer cancel() + req, err := http.NewRequestWithContext(ctx, http.MethodPost, "https://"+e.addr+path, bytes.NewReader(data)) + if err != nil { + return brokerAnswer{err: err} + } + req.Header.Set("Content-Type", api.ContentType) + resp, err := c.Do(req) + if err != nil { + return brokerAnswer{err: err} + } + defer func() { _ = resp.Body.Close() }() + b, err := io.ReadAll(io.LimitReader(resp.Body, api.MaxAnswer+1)) + return brokerAnswer{status: resp.StatusCode, body: b, err: err} +} + +// approve sends a request that needs a tap, taps "Approve once" on its +// message at the fake Telegram, and returns the answer. +func (e *brokerEnv) approve(c *http.Client, path string, body any) brokerAnswer { + e.t.Helper() + n := e.tg.asked() + res := make(chan brokerAnswer, 1) + go func() { res <- e.post(c, path, body) }() + msgID, requestID := e.tg.waitAsk(n + 1) + // A tap can come before the broker knows that its message was sent. + // Then the phone shows "Not ready yet. Tap again in a moment.", and + // the person taps again. + for try := 1; ; try++ { + answer := e.tg.waitTapAnswer(e.tg.tap(msgID, "o:"+requestID)) + if !strings.Contains(answer, "Not ready yet") { + break + } + if try == 100 { + e.t.Fatalf("every tap came too early; the log of the broker:\n%s", e.stderr.text()) + } + time.Sleep(10 * time.Millisecond) + } + select { + case a := <-res: + return a + case <-time.After(2 * brokerWait): + e.t.Fatalf("no answer; the log of the broker:\n%s", e.stderr.text()) + return brokerAnswer{} + } +} + +// mustStatus fails unless a has the status. +func mustStatus(t *testing.T, a brokerAnswer, status int) { + t.Helper() + if a.err != nil || a.status != status { + t.Fatalf("the answer: %d %s, %v; want status %d", a.status, a.body, a.err, status) + } +} + +// signRequest returns a request to sign an SSH login as user to host with +// the key of [key.deploy] (see makeLogin), and the data to sign. +func (e *brokerEnv) signRequest(host, user string) (api.SignRequest, []byte) { + e.t.Helper() + pub := e.userKey.PublicKey() + bind, data := makeLogin(e.t, pub, e.hostKeys[host], user) + return api.SignRequest{ + PublicKey: pub.Marshal(), + Data: data, + Binds: [][]byte{bind}, + Commands: [][]string{{"ssh", user + "@" + host}}, + }, data +} + +// auditEntries reads the audit log of the broker: one map for each line. +// A line that is not JSON, such as a torn line before its log_recovered +// entry, has the event "(torn line)". +func (e *brokerEnv) auditEntries() []map[string]any { + e.t.Helper() + data, err := os.ReadFile(e.auditPath) + if err != nil { + e.t.Fatal(err) + } + var entries []map[string]any + for line := range strings.SplitSeq(strings.TrimSuffix(string(data), "\n"), "\n") { + var m map[string]any + if err := json.Unmarshal([]byte(line), &m); err != nil { + m = map[string]any{"event": "(torn line)"} + } + entries = append(entries, m) + } + return entries +} + +// eventNames returns the events of entries, with the code or cause, such +// as "error:poll_conflict" or "denied:error". +func eventNames(entries []map[string]any) []string { + var names []string + for _, m := range entries { + name, _ := m["event"].(string) + for _, k := range []string{"code", "cause", "answer", "via"} { + if v, ok := m[k].(string); ok { + name += ":" + v + } + } + names = append(names, name) + } + return names +} + +// lineLog holds the lines of the broker's stderr as they come. +type lineLog struct { + mu sync.Mutex + lines []string + done bool // stderr is closed + changed chan struct{} // closed and made new on each change +} + +func newLineLog() *lineLog { return &lineLog{changed: make(chan struct{})} } + +// readFrom reads lines from r until it ends. +func (l *lineLog) readFrom(r io.Reader) { + s := bufio.NewScanner(r) + s.Buffer(make([]byte, 64<<10), 1<<20) + for s.Scan() { + l.mu.Lock() + l.lines = append(l.lines, s.Text()) + l.notifyLocked() + l.mu.Unlock() + } + _, _ = io.Copy(io.Discard, r) // after a line that is too long + l.mu.Lock() + l.done = true + l.notifyLocked() + l.mu.Unlock() +} + +func (l *lineLog) notifyLocked() { + close(l.changed) + l.changed = make(chan struct{}) +} + +// all returns the lines so far. +func (l *lineLog) all() []string { + l.mu.Lock() + defer l.mu.Unlock() + return slices.Clone(l.lines) +} + +// text returns the lines so far, each with a newline. +func (l *lineLog) text() string { + var b strings.Builder + for _, line := range l.all() { + b.WriteString(line + "\n") + } + return b.String() +} + +// has reports whether a line holds s. +func (l *lineLog) has(s string) bool { + return slices.ContainsFunc(l.all(), func(line string) bool { return strings.Contains(line, s) }) +} + +// count returns the number of lines that hold s. +func (l *lineLog) count(s string) int { + n := 0 + for _, line := range l.all() { + if strings.Contains(line, s) { + n++ + } + } + return n +} + +// waitCount waits until n lines hold s. +func (l *lineLog) waitCount(t *testing.T, s string, n int) { + t.Helper() + deadline := time.NewTimer(brokerWait) + defer deadline.Stop() + for { + l.mu.Lock() + done, changed := l.done, l.changed + l.mu.Unlock() + switch { + case l.count(s) >= n: + return + case done: + t.Fatalf("the broker ended with fewer than %d lines that hold %q; its log:\n%s", n, s, l.text()) + } + select { + case <-changed: + case <-deadline.C: + t.Fatalf("fewer than %d lines of the broker's log hold %q; its log:\n%s", n, s, l.text()) + } + } +} + +// wait waits until a line holds s. +func (l *lineLog) wait(t *testing.T, s string) { + t.Helper() + if !l.waitOrEnd(t, s) { + t.Fatalf("the broker ended, and no line of its log holds %q; its log:\n%s", s, l.text()) + } +} + +// waitOrEnd waits until a line holds s, and reports true, or until the +// log ends without such a line, and reports false. +func (l *lineLog) waitOrEnd(t *testing.T, s string) bool { + t.Helper() + deadline := time.NewTimer(brokerWait) + defer deadline.Stop() + for { + l.mu.Lock() + found := slices.ContainsFunc(l.lines, func(line string) bool { return strings.Contains(line, s) }) + done, changed := l.done, l.changed + l.mu.Unlock() + switch { + case found: + return true + case done: + return false + } + select { + case <-changed: + case <-deadline.C: + t.Fatalf("no line of the broker's log holds %q; its log:\n%s", s, l.text()) + } + } +} + +// inOrder fails unless lines hold each of parts, in this order. +func (l *lineLog) inOrder(t *testing.T, parts ...string) { + t.Helper() + lines := l.all() + i := 0 + for _, p := range parts { + for i < len(lines) && !strings.Contains(lines[i], p) { + i++ + } + if i == len(lines) { + t.Errorf("no line holds %q after the lines of %q; the log:\n%s", p, parts, strings.Join(lines, "\n")) + return + } + i++ + } +} diff --git a/cmd/nodkey/brokerfake_linux_test.go b/cmd/nodkey/brokerfake_linux_test.go new file mode 100644 index 0000000..7cbe195 --- /dev/null +++ b/cmd/nodkey/brokerfake_linux_test.go @@ -0,0 +1,583 @@ +package main + +import ( + "bufio" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/json" + "encoding/pem" + "fmt" + "io" + "math/big" + "net" + "net/http" + "os" + "path/filepath" + "slices" + "strconv" + "strings" + "sync" + "testing" + "time" +) + +// The fake Telegram server of the tests of nodkey broker, and the fake +// HTTPS proxy in front of it (see brokerenv_linux_test.go). + +// fakeTelegram is a fake Bot API server for the child broker, behind a +// fake HTTPS proxy. It answers like Telegram, and keeps every call. +type fakeTelegram struct { + t *testing.T + token string // the made-up bot token + proxyAddr string // the fake proxy + caFile string // SSL_CERT_FILE of the child: the test CA + caDir string // SSL_CERT_DIR of the child: an empty directory + + mu sync.Mutex + calls []tgCall + polls []*tgPoll + updates []tgUpdate + nextUpdate int64 + nextMsg int64 + markups map[int64]json.RawMessage // the buttons of each message, by message ID + refusePolls int // the status of the error answer to every getUpdates; 0 for none + hold chan struct{} // if set, alert messages wait until it is closed + offline bool // the proxy refuses every new connection, as if Telegram were unreachable + connects []string // what the proxy was asked to connect to + changed chan struct{} // closed and made new on each change +} + +// tgCall is one call that the fake got. +type tgCall struct { + method string + body []byte // the request body + msgID int64 // sendMessage: the message ID of its answer + buttons []string // sendMessage: the data of its buttons +} + +// tgPoll is one getUpdates call. +type tgPoll struct { + ended bool // the fake is done with it + gone bool // it ended because the client went away: it closed the connection +} + +// tgUpdate is a queued update. +type tgUpdate struct { + id int64 + raw []byte +} + +// newFakeTelegram starts a fake Telegram server and its proxy. +func newFakeTelegram(t *testing.T, opts brokerOptions) *fakeTelegram { + t.Helper() + f := &fakeTelegram{ + t: t, + token: newBotToken(), + nextUpdate: 1000, + nextMsg: 500, + markups: make(map[int64]json.RawMessage), + refusePolls: opts.refusePolls, + changed: make(chan struct{}), + } + if opts.holdAlerts { + f.hold = make(chan struct{}) + } + caPEM, leaf := newTestCA(t) + certs := t.TempDir() + f.caFile = filepath.Join(certs, "ca.pem") + writeTestFile(t, f.caFile, caPEM, 0o600) + f.caDir = filepath.Join(certs, "none") + if err := os.Mkdir(f.caDir, 0o700); err != nil { + t.Fatal(err) + } + + ln, err := (&net.ListenConfig{}).Listen(t.Context(), "tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + f.proxyAddr = ln.Addr().String() + tunnels := &chanListener{conns: make(chan net.Conn), done: make(chan struct{}), addr: ln.Addr()} + srv := &http.Server{Handler: f, ReadHeaderTimeout: brokerWait} + tlsConfig := &tls.Config{Certificates: []tls.Certificate{leaf}, NextProtos: []string{"http/1.1"}, MinVersion: tls.VersionTLS12} + var wg sync.WaitGroup + wg.Go(func() { _ = srv.Serve(tunnels) }) + wg.Go(func() { + for { + c, err := ln.Accept() + if err != nil { + return + } + wg.Go(func() { f.connect(c, tlsConfig, tunnels) }) + } + }) + t.Cleanup(func() { + _ = ln.Close() + _ = srv.Close() + f.release() + wg.Wait() + }) + return f +} + +// connect handles one connection to the fake proxy: a CONNECT to +// api.telegram.org, then TLS with the test CA's certificate, and the +// fake's HTTP server. Anything else is refused. +func (f *fakeTelegram) connect(c net.Conn, cfg *tls.Config, tunnels *chanListener) { + _ = c.SetReadDeadline(time.Now().Add(brokerWait)) + br := bufio.NewReader(c) + req, err := http.ReadRequest(br) + if err != nil { + _ = c.Close() + return + } + f.mu.Lock() + f.connects = append(f.connects, req.Method+" "+req.Host) + offline := f.offline + f.mu.Unlock() + if offline { + _, _ = io.WriteString(c, "HTTP/1.1 503 Service Unavailable\r\nContent-Length: 0\r\n\r\n") + _ = c.Close() + return + } + if req.Method != http.MethodConnect || req.Host != telegramHost+":443" { + _, _ = io.WriteString(c, "HTTP/1.1 403 Forbidden\r\nContent-Length: 0\r\n\r\n") + _ = c.Close() + return + } + _ = c.SetReadDeadline(time.Time{}) + if _, err := io.WriteString(c, "HTTP/1.1 200 Connection established\r\n\r\n"); err != nil { + _ = c.Close() + return + } + select { + case tunnels.conns <- tls.Server(&bufferedConn{Conn: c, r: br}, cfg): + case <-tunnels.done: + _ = c.Close() + } +} + +// bufferedConn is a connection whose first bytes may wait in r. +type bufferedConn struct { + net.Conn + r *bufio.Reader +} + +func (c *bufferedConn) Read(p []byte) (int, error) { return c.r.Read(p) } + +// chanListener is a net.Listener of the connections in conns. +type chanListener struct { + conns chan net.Conn + done chan struct{} + once sync.Once + addr net.Addr +} + +func (l *chanListener) Accept() (net.Conn, error) { + select { + case c := <-l.conns: + return c, nil + case <-l.done: + return nil, net.ErrClosed + } +} + +func (l *chanListener) Close() error { + l.once.Do(func() { close(l.done) }) + return nil +} + +func (l *chanListener) Addr() net.Addr { return l.addr } + +// newTestCA returns a new CA certificate in PEM, and a certificate for +// api.telegram.org that it signed, with its key. +func newTestCA(t *testing.T) ([]byte, tls.Certificate) { + t.Helper() + now := time.Now() + caKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + caTemplate := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "nodkey test CA"}, + NotBefore: now.Add(-time.Hour), + NotAfter: now.Add(time.Hour), + IsCA: true, + BasicConstraintsValid: true, + KeyUsage: x509.KeyUsageCertSign, + } + caDER, err := x509.CreateCertificate(rand.Reader, caTemplate, caTemplate, &caKey.PublicKey, caKey) + if err != nil { + t.Fatal(err) + } + ca, err := x509.ParseCertificate(caDER) + if err != nil { + t.Fatal(err) + } + leafKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + leafTemplate := &x509.Certificate{ + SerialNumber: big.NewInt(2), + Subject: pkix.Name{CommonName: telegramHost}, + DNSNames: []string{telegramHost}, + NotBefore: now.Add(-time.Hour), + NotAfter: now.Add(time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + } + leafDER, err := x509.CreateCertificate(rand.Reader, leafTemplate, ca, &leafKey.PublicKey, caKey) + if err != nil { + t.Fatal(err) + } + return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: caDER}), + tls.Certificate{Certificate: [][]byte{leafDER}, PrivateKey: leafKey} +} + +func (f *fakeTelegram) notifyLocked() { + close(f.changed) + f.changed = make(chan struct{}) +} + +// writeTelegram writes an answer of the Bot API. +func writeTelegram(w http.ResponseWriter, status int, body string) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _, _ = io.WriteString(w, body) +} + +func (f *fakeTelegram) ServeHTTP(w http.ResponseWriter, r *http.Request) { + rest, ok := strings.CutPrefix(r.URL.Path, "/bot") + token, method, ok2 := strings.Cut(rest, "/") + if !ok || !ok2 || token != f.token { + writeTelegram(w, http.StatusUnauthorized, `{"ok":false,"error_code":401,"description":"Unauthorized"}`) + return + } + body, err := io.ReadAll(io.LimitReader(r.Body, 1<<20)) + if err != nil { + return + } + var p struct { + ChatID int64 `json:"chat_id"` + MessageID int64 `json:"message_id"` + Offset int64 `json:"offset"` + Timeout int `json:"timeout"` + ReplyMarkup json.RawMessage `json:"reply_markup"` + } + _ = json.Unmarshal(body, &p) // the fields come from the broker; a test checks what it needs + call := tgCall{method: method, body: body} + f.mu.Lock() + if method == "sendMessage" { + f.nextMsg++ + call.msgID = f.nextMsg + call.buttons = buttonData(p.ReplyMarkup) + f.markups[call.msgID] = p.ReplyMarkup + } + f.calls = append(f.calls, call) + refusePolls, hold := f.refusePolls, f.hold + f.notifyLocked() + f.mu.Unlock() + + message := func(id int64) string { + return fmt.Sprintf(`{"ok":true,"result":{"message_id":%d,"date":1,"chat":{"id":%d,"type":"private"},`+ + `"text":"the text"}}`, id, p.ChatID) + } + switch method { + case "getUpdates": + if refusePolls != 0 { + description := map[int]string{ + http.StatusConflict: "Conflict: terminated by other getUpdates request", + http.StatusUnauthorized: "Unauthorized", + http.StatusForbidden: "Forbidden", + http.StatusNotFound: "Not Found", + }[refusePolls] + writeTelegram(w, refusePolls, fmt.Sprintf(`{"ok":false,"error_code":%d,"description":%q}`, + refusePolls, description)) + return + } + f.serveUpdates(w, r, p.Offset, time.Duration(p.Timeout)*time.Second) + case "sendMessage": + if len(call.buttons) == 0 && hold != nil { + select { + case <-hold: + case <-r.Context().Done(): + return + } + } + writeTelegram(w, http.StatusOK, message(call.msgID)) + case "editMessageText", "editMessageReplyMarkup": + writeTelegram(w, http.StatusOK, message(p.MessageID)) + case "answerCallbackQuery": + writeTelegram(w, http.StatusOK, `{"ok":true,"result":true}`) + default: + writeTelegram(w, http.StatusNotFound, `{"ok":false,"error_code":404,"description":"Not Found"}`) + } +} + +// serveUpdates answers getUpdates: the queued updates from offset on, or +// none after wait, as Telegram's long polling does. +func (f *fakeTelegram) serveUpdates(w http.ResponseWriter, r *http.Request, offset int64, wait time.Duration) { + poll := &tgPoll{} + f.mu.Lock() + f.polls = append(f.polls, poll) + f.notifyLocked() + f.mu.Unlock() + timer := time.NewTimer(min(max(wait, time.Second), 50*time.Second)) + defer timer.Stop() + end := func(gone bool) { + f.mu.Lock() + poll.ended, poll.gone = true, gone + f.notifyLocked() + f.mu.Unlock() + } + for { + f.mu.Lock() + f.updates = slices.DeleteFunc(f.updates, func(u tgUpdate) bool { return u.id < offset }) + var parts []string + for _, u := range f.updates { + parts = append(parts, string(u.raw)) + } + changed := f.changed + f.mu.Unlock() + if len(parts) > 0 { + end(false) + writeTelegram(w, http.StatusOK, `{"ok":true,"result":[`+strings.Join(parts, ",")+`]}`) + return + } + select { + case <-changed: + case <-timer.C: + end(false) + writeTelegram(w, http.StatusOK, `{"ok":true,"result":[]}`) + return + case <-r.Context().Done(): + end(true) + return + } + } +} + +// buttonData returns the callback data of each button in reply_markup. +func buttonData(markup json.RawMessage) []string { + var kb struct { + InlineKeyboard [][]struct { + CallbackData string `json:"callback_data"` + } `json:"inline_keyboard"` + } + if len(markup) == 0 || json.Unmarshal(markup, &kb) != nil { + return nil + } + var data []string + for _, row := range kb.InlineKeyboard { + for _, b := range row { + data = append(data, b.CallbackData) + } + } + return data +} + +// setOffline makes the proxy refuse every new connection, or take them +// again: Telegram cannot be reached, or it can again. A connection that +// the proxy took before stays. +func (f *fakeTelegram) setOffline(offline bool) { + f.mu.Lock() + defer f.mu.Unlock() + f.offline = offline +} + +// release lets the alert messages that wait go on. +func (f *fakeTelegram) release() { + f.mu.Lock() + defer f.mu.Unlock() + if f.hold != nil { + close(f.hold) + f.hold = nil + } +} + +// waitFor waits until cond, which runs with f.mu held, is true. +func (f *fakeTelegram) waitFor(what string, cond func() bool) { + f.t.Helper() + deadline := time.NewTimer(brokerWait) + defer deadline.Stop() + for { + f.mu.Lock() + ok, changed := cond(), f.changed + f.mu.Unlock() + if ok { + return + } + select { + case <-changed: + case <-deadline.C: + f.t.Fatalf("waited %s in vain for %s", brokerWait, what) + } + } +} + +// askedLocked returns the messages with buttons: those of requests. +func (f *fakeTelegram) askedLocked() []tgCall { + var out []tgCall + for _, c := range f.calls { + if c.method == "sendMessage" && len(c.buttons) > 0 { + out = append(out, c) + } + } + return out +} + +// asked returns how many messages with buttons the fake got. +func (f *fakeTelegram) asked() int { + f.mu.Lock() + defer f.mu.Unlock() + return len(f.askedLocked()) +} + +// waitAsk waits for the n-th message with buttons, and returns its +// message ID and the request ID in its buttons. +func (f *fakeTelegram) waitAsk(n int) (int64, string) { + f.t.Helper() + f.waitFor(fmt.Sprintf("message %d with buttons", n), func() bool { return len(f.askedLocked()) >= n }) + f.mu.Lock() + c := f.askedLocked()[n-1] + f.mu.Unlock() + _, id, _ := strings.Cut(c.buttons[len(c.buttons)-1], ":") + return c.msgID, id +} + +// waitAlert waits for a message without buttons whose request body holds +// text, and returns the body. +func (f *fakeTelegram) waitAlert(text string) string { + f.t.Helper() + var body string + f.waitFor("an alert with "+strconv.Quote(text), func() bool { + for _, c := range f.calls { + if c.method == "sendMessage" && len(c.buttons) == 0 && strings.Contains(string(c.body), text) { + body = string(c.body) + return true + } + } + return false + }) + return body +} + +// alertCount returns the number of messages without buttons whose request +// body holds text. +func (f *fakeTelegram) alertCount(text string) int { + f.mu.Lock() + defer f.mu.Unlock() + n := 0 + for _, c := range f.calls { + if c.method == "sendMessage" && len(c.buttons) == 0 && strings.Contains(string(c.body), text) { + n++ + } + } + return n +} + +// tap queues a press of the approver, in the private chat, on the message +// msgID with the buttons that the broker sent there, with the button data +// data. It returns the ID of the press (the callback query). +func (f *fakeTelegram) tap(msgID int64, data string) string { + f.t.Helper() + f.mu.Lock() + defer f.mu.Unlock() + f.nextUpdate++ + id := "cb" + strconv.FormatInt(f.nextUpdate, 10) + msg := map[string]any{ + "message_id": msgID, "date": 1, "text": "the text", + "chat": map[string]any{"id": approverID, "type": "private"}, + } + if m := f.markups[msgID]; len(m) > 0 { + msg["reply_markup"] = m + } + raw, err := json.Marshal(map[string]any{ + "update_id": f.nextUpdate, + "callback_query": map[string]any{ + "id": id, + "from": map[string]any{"id": approverID, "is_bot": false, "first_name": "Test"}, + "chat_instance": "1", + "data": data, + "message": msg, + }, + }) + if err != nil { + f.t.Fatal(err) + } + f.updates = append(f.updates, tgUpdate{id: f.nextUpdate, raw: raw}) + f.notifyLocked() + return id +} + +// waitTapAnswer waits for the answer to the press id +// (answerCallbackQuery), and returns the text that the phone shows. +func (f *fakeTelegram) waitTapAnswer(id string) string { + f.t.Helper() + var text string + f.waitFor("the answer to the press "+id, func() bool { + for _, c := range f.calls { + var p struct { + ID string `json:"callback_query_id"` + Text string `json:"text"` + } + if c.method == "answerCallbackQuery" && json.Unmarshal(c.body, &p) == nil && p.ID == id { + text = p.Text + return true + } + } + return false + }) + return text +} + +// waitPolling waits until a getUpdates call waits for an answer. +func (f *fakeTelegram) waitPolling() { + f.t.Helper() + f.waitFor("a getUpdates call that waits", func() bool { + return len(f.polls) > 0 && !f.polls[len(f.polls)-1].ended + }) +} + +// waitPollsGone waits until every getUpdates call has ended, the last one +// because the client went away: it closed the connection while the call +// waited. +func (f *fakeTelegram) waitPollsGone() { + f.t.Helper() + f.waitFor("the end of every getUpdates call", func() bool { + if len(f.polls) == 0 || !f.polls[len(f.polls)-1].gone { + return false + } + return !slices.ContainsFunc(f.polls, func(p *tgPoll) bool { return !p.ended }) + }) +} + +// pollCount returns the number of getUpdates calls so far. +func (f *fakeTelegram) pollCount() int { + f.mu.Lock() + defer f.mu.Unlock() + return len(f.polls) +} + +// bodies returns every request body that the fake got, one per line. +func (f *fakeTelegram) bodies() string { + f.mu.Lock() + defer f.mu.Unlock() + var b strings.Builder + for _, c := range f.calls { + b.WriteString(c.method + " " + string(c.body) + "\n") + } + return b.String() +} + +// proxied returns what the proxy was asked to connect to. +func (f *fakeTelegram) proxied() []string { + f.mu.Lock() + defer f.mu.Unlock() + return slices.Clone(f.connects) +} diff --git a/cmd/nodkey/brokerkeys.go b/cmd/nodkey/brokerkeys.go new file mode 100644 index 0000000..837538d --- /dev/null +++ b/cmd/nodkey/brokerkeys.go @@ -0,0 +1,72 @@ +package main + +import ( + "crypto" + "errors" + "fmt" + "maps" + "slices" + + "golang.org/x/crypto/ssh" + + "github.com/code6x/nodkey/internal/config" + "github.com/code6x/nodkey/internal/sshauth" +) + +// loadSSHKeys loads the SSH key of each [key.*] section. It reads each +// file with read, which is config.ReadPrivateFile: it checks who can read +// and change the file (DESIGN.md section 14). It makes each key with +// sshauth.NewKey and the hosts of its section; NewKey signs a fixed text +// with the key and checks the signature, so a broken key stops the start, +// not a login after the tap. Two sections must not hold the same public +// key: the config check finds one file named twice, not a copy of a key +// in a second file. Errors name the section and the file, never key +// material. +func loadSSHKeys(cfg *config.Broker, read func(string) ([]byte, error)) (map[string]sshauth.Key, error) { + keys := make(map[string]sshauth.Key, len(cfg.Keys)) + byPublicKey := make(map[string]string, len(cfg.Keys)) + for _, name := range slices.Sorted(maps.Keys(cfg.Keys)) { + key, err := loadSSHKey(name, cfg.Keys[name], read) + if err != nil { + return nil, err + } + blob := string(key.PublicKey().Marshal()) + if other, ok := byPublicKey[blob]; ok { + return nil, fmt.Errorf("[key.%s] and [key.%s] hold the same public key; each key may have only one "+ + "section, so that its rule and its hosts are clear", other, name) + } + byPublicKey[blob] = name + keys[name] = key + } + return keys, nil +} + +// loadSSHKey loads the key of the section [key.NAME] k. +func loadSSHKey(name string, k config.Key, read func(string) ([]byte, error)) (sshauth.Key, error) { + data, err := read(k.File) + if err != nil { + return sshauth.Key{}, fmt.Errorf("[key.%s] file: %w", name, err) // it names the path, never the content + } + defer clear(data) + raw, err := ssh.ParseRawPrivateKey(data) + if err != nil { + var passphrase *ssh.PassphraseMissingError + if errors.As(err, &passphrase) { + return sshauth.Key{}, fmt.Errorf("[key.%s] file %s: the key has a passphrase; store it without one, "+ + "as an encrypted systemd credential (DESIGN.md section 11)", name, k.File) + } + // The parser's error can quote the file, so it is left out. + return sshauth.Key{}, fmt.Errorf("[key.%s] file %s: does not hold an SSH private key in OpenSSH or PEM "+ + "form", name, k.File) + } + signer, ok := raw.(crypto.Signer) + if !ok { + return sshauth.Key{}, fmt.Errorf("[key.%s] file %s: the key type is not supported; use Ed25519, ECDSA "+ + "or RSA", name, k.File) + } + key, err := sshauth.NewKey(name, signer, k.Hosts) + if err != nil { + return sshauth.Key{}, fmt.Errorf("[key.%s] file %s: %w", name, k.File, err) // sshauth's errors hold no key material + } + return key, nil +} diff --git a/cmd/nodkey/harden_linux.go b/cmd/nodkey/harden_linux.go new file mode 100644 index 0000000..072eb3c --- /dev/null +++ b/cmd/nodkey/harden_linux.go @@ -0,0 +1,52 @@ +package main + +import ( + "errors" + "fmt" + "syscall" +) + +// hardenProcess turns off core dumps, and fails if it cannot (DESIGN.md +// section 16): +// +// - It sets the soft and the hard RLIMIT_CORE of the broker to 1. The +// store programs inherit both, and cannot raise the soft limit above +// the hard limit. With exactly 1 and a pipe or file core_pattern, such +// as the default of systemd, the kernel writes no core file and starts +// no core handler. A core handler such as systemd-coredump would copy +// the environment of a crashed bws or bw, with the token or the +// session key, into the journal. A limit of 0 does not stop a core +// handler. +// - It marks the broker not dumpable (PR_SET_DUMPABLE 0): no core dump +// of the broker, and other processes of its user cannot attach to it +// or read its memory. +// +// It reads both settings back. +func hardenProcess() error { + var lim syscall.Rlimit + if err := syscall.Getrlimit(syscall.RLIMIT_CORE, &lim); err != nil { + return fmt.Errorf("cannot read the limit of core dumps: %w", err) + } + if lim.Max == 0 { + return errors.New("the hard limit of core dumps is 0, so the soft limit cannot be 1; " + + "set LimitCORE=1 in the systemd unit, or run ulimit -c 1 first") + } + // The hard limit only goes down: a process without privileges cannot + // raise it again. + lim.Cur, lim.Max = 1, 1 + if err := syscall.Setrlimit(syscall.RLIMIT_CORE, &lim); err != nil { + return fmt.Errorf("cannot set the limit of core dumps to 1: %w", err) + } + var got syscall.Rlimit + if err := syscall.Getrlimit(syscall.RLIMIT_CORE, &got); err != nil || got.Cur != 1 || got.Max != 1 { + return errors.New("the soft and the hard limit of core dumps are not 1 after setting them") + } + if _, _, errno := syscall.RawSyscall6(syscall.SYS_PRCTL, syscall.PR_SET_DUMPABLE, 0, 0, 0, 0, 0); errno != 0 { + return fmt.Errorf("cannot mark the process not dumpable: %w", errno) + } + dumpable, _, errno := syscall.RawSyscall6(syscall.SYS_PRCTL, syscall.PR_GET_DUMPABLE, 0, 0, 0, 0, 0) + if errno != 0 || dumpable != 0 { + return errors.New("the process is still dumpable after PR_SET_DUMPABLE 0") + } + return nil +} diff --git a/cmd/nodkey/harden_other.go b/cmd/nodkey/harden_other.go new file mode 100644 index 0000000..f25e068 --- /dev/null +++ b/cmd/nodkey/harden_other.go @@ -0,0 +1,11 @@ +//go:build !linux + +package main + +import "errors" + +// hardenProcess fails: nodkey version 1 runs on Linux only, and it can +// turn off core dumps only there. +func hardenProcess() error { + return errors.New("nodkey broker runs on Linux only; it cannot turn off core dumps here") +} diff --git a/cmd/nodkey/main.go b/cmd/nodkey/main.go index 7c547f5..25ec59d 100644 --- a/cmd/nodkey/main.go +++ b/cmd/nodkey/main.go @@ -31,6 +31,8 @@ const ( const usageText = `Usage: nodkey [arguments] Commands: + broker [--config FILE] + run the broker: the daemon that asks the approver on Telegram keygen make a TLS key for the broker or a client, and print its pin run run a command with secrets, once a person approves on the phone audit verify FILE [--anchor SEQ:HASH]... [--anchors FILE|-] @@ -59,6 +61,8 @@ func run(args []string, stdin io.Reader, stdout, stderr io.Writer) int { name, rest := fs.Arg(0), fs.Args()[1:] switch name { + case "broker": + return runBroker(rest, stderr) case "keygen": ignoreBrokenPipe() return runKeygen(rest, stdout, stderr, writeKeyFile) diff --git a/contrib/systemd/nodkey.service b/contrib/systemd/nodkey.service new file mode 100644 index 0000000..904c7df --- /dev/null +++ b/contrib/systemd/nodkey.service @@ -0,0 +1,140 @@ +# An example systemd unit for the nodkey broker (DESIGN.md sections 11, +# 15 and 16). Copy it to /etc/systemd/system/nodkey.service, and change +# the paths, the user and the credentials to fit your config. +# +# Before the first start: +# +# 1. Make the broker's own user: +# useradd --system --home-dir /var/lib/nodkey --shell /usr/sbin/nologin nodkey +# 2. Put the config into /etc/nodkey/broker.toml and the known hosts into +# /etc/nodkey/known_hosts. Both owned by root, and only root may +# change them (mode 0644). +# 3. Make each key, token and password file of the config an encrypted +# credential, readable only on this machine. For example: +# systemd-creds encrypt --with-key=host+tpm2 --name=broker-tls-key \ +# broker.key /etc/nodkey/credentials/broker-tls-key.cred +# Without a TPM2 chip, use --with-key=host: then anyone with a full copy +# of the disk can open the credentials. Remove the plain files after. +# In the config, name each file as ${CREDENTIALS_DIRECTORY}/NAME, with +# the NAME before ":" in its LoadCredentialEncrypted= line below. +# 4. Check the unit: systemd-analyze verify /etc/systemd/system/nodkey.service +# +# Read the log and the anchors of the audit log with journalctl -u nodkey. + +[Unit] +Description=nodkey broker: secrets and SSH signatures after an approval on the phone +Documentation=https://github.com/code6x/nodkey/blob/main/DESIGN.md +# The broker listens on one address of the private network, and calls +# Telegram. If that address is not there yet, the start fails, and systemd +# tries again (Restart= below). +Wants=network-online.target +After=network-online.target + +[Service] +Type=exec +User=nodkey +Group=nodkey +ExecStart=/usr/local/bin/nodkey broker --config /etc/nodkey/broker.toml + +# Keys, tokens and passwords, as encrypted credentials. systemd opens them +# at each start, and gives them to the broker in $CREDENTIALS_DIRECTORY. +# One line for each file of the config; remove the lines you do not need. +LoadCredentialEncrypted=broker-tls-key:/etc/nodkey/credentials/broker-tls-key.cred +LoadCredentialEncrypted=telegram-token:/etc/nodkey/credentials/telegram-token.cred +LoadCredentialEncrypted=bws-token:/etc/nodkey/credentials/bws-token.cred +LoadCredentialEncrypted=bws-token-backup:/etc/nodkey/credentials/bws-token-backup.cred +LoadCredentialEncrypted=bw-master-password:/etc/nodkey/credentials/bw-master-password.cred +LoadCredentialEncrypted=ssh-deploy:/etc/nodkey/credentials/ssh-deploy.cred + +# /var/lib/nodkey holds the audit log (audit_log = "/var/lib/nodkey/audit.jsonl"), +# and /var/lib/nodkey/stores holds the home directory of each store. The +# broker checks that the store directory is a directory with mode 0700, +# owned by its user. +StateDirectory=nodkey nodkey/stores +StateDirectoryMode=0700 +UMask=0077 + +# A crash or an error starts the broker again, after 5 seconds. Not after +# these exit codes, which only a person can fix (DESIGN.md section 15): +# 64 wrong usage: check ExecStart=. +# 76 a polling conflict at Telegram: another program polls with the bot +# token, so the token may be stolen. Make a new token first; starting +# again would only send the same alert again. +# 77 Telegram does not accept the bot token, such as a revoked token. +# Put a new token into its credential first; starting again would only +# call Telegram with the old token every 5 seconds. +Restart=on-failure +RestartSec=5s +RestartPreventExitStatus=64 76 77 +# The stop waits for the requests in progress and for alert messages, at +# most 80 seconds; then the broker ends on its own. systemd kills it +# after 90 seconds, and stops every process of the unit, also a store +# program that left its process group. +TimeoutStopSec=90s +KillMode=control-group + +# No core dumps. With a limit of exactly 1, a crashed bws or bw starts no +# core handler that would copy its environment, with a token, into the +# journal. The broker sets its own soft and hard limit to 1 too, and +# refuses to start if it cannot. +LimitCORE=1 +# The broker reads at most 16 request bodies at a time, about 320 MiB at +# worst; bw runs on Node.js. Change this to fit your host. +MemoryMax=1G +# Keys, tokens and secret values are in the memory of the broker and of +# its store programs. Swap would write that memory to disk, so the unit +# may not use swap or zswap. Hibernation writes all memory to disk too: +# use encrypted swap or none, and no hibernation, on the broker host. +# MemoryZSwapMax= needs systemd 253 or newer; an older systemd ignores it +# with a warning, and MemorySwapMax=0 alone is enough. +MemorySwapMax=0 +MemoryZSwapMax=0 +# journald must never drop a line of the broker: the anchors of the audit +# log are in its log. The broker limits its own lines. +LogRateLimitIntervalSec=0 + +# A proxy for the calls to Telegram, if you need one. Never use a proxy +# that inspects TLS: it sees the bot token (DESIGN.md section 7). Every +# user can read Environment= with systemctl show, so a proxy with a user +# name and a password belongs in EnvironmentFile=, a file that only root +# can read (mode 0600). The broker logs the proxy's host at start, and +# SSL_CERT_FILE and SSL_CERT_DIR if they are set. +#Environment=HTTPS_PROXY=http://proxy.example.com:3128 +#EnvironmentFile=/etc/nodkey/proxy.env + +# Sandbox. These settings still let bws and bw run. +NoNewPrivileges=yes +CapabilityBoundingSet= +AmbientCapabilities= +ProtectSystem=strict +ProtectHome=yes +PrivateTmp=yes +PrivateDevices=yes +ProtectKernelTunables=yes +ProtectKernelModules=yes +ProtectKernelLogs=yes +ProtectControlGroups=yes +ProtectClock=yes +ProtectHostname=yes +ProtectProc=invisible +RestrictAddressFamilies=AF_INET AF_INET6 AF_UNIX AF_NETLINK +RestrictNamespaces=yes +RestrictRealtime=yes +RestrictSUIDSGID=yes +LockPersonality=yes +RemoveIPC=yes +KeyringMode=private +SystemCallArchitectures=native +SystemCallFilter=@system-service +SystemCallFilter=~@privileged +SystemCallErrorNumber=EPERM +# Not set on purpose: +# MemoryDenyWriteExecute=yes bw runs on Node.js, which needs memory that +# is writable and executable. +# PrivateUsers=yes files owned by root, such as the +# credentials and the config, would look +# owned by nobody, and the broker would +# refuse them. + +[Install] +WantedBy=multi-user.target diff --git a/internal/audit/audit.go b/internal/audit/audit.go index a528697..a1081a3 100644 --- a/internal/audit/audit.go +++ b/internal/audit/audit.go @@ -114,6 +114,9 @@ type Logger struct { now func() time.Time last Anchor // the last entry; seq 0 and a zero hash when there is none err error // set after a failed write, sync or journal write + // found is the log_recovered entry that Open found as the last entry + // of the log, if any (see LastRecovery). It never changes after Open. + found *Recovery } // Open opens the audit log at path for appending, and makes it if it is @@ -226,6 +229,9 @@ func (l *Logger) start(path string, self uint32, h hooks) (Recovery, bool, error "check the log with nodkey audit verify", path, bad) } l.last = Anchor{Seq: e.seq, Hash: sha256.Sum256(t.last)} + if e.event == eventLogRecovered { + l.found = &Recovery{Seq: e.seq, TornLength: e.tornLength, TornSHA256: e.tornHash} + } } if t.torn == nil { return Recovery{}, false, nil @@ -475,6 +481,21 @@ func appendLine(b []byte, seq uint64, t time.Time, name string, prev [sha256.Siz return append(b, '}') } +// LastRecovery reports whether the last entry that Open found in the log, +// before it wrote anything itself, is a log_recovered entry, and returns +// what that entry records. Then an earlier Open recorded a torn line, and +// nothing was written after it: the broker that opened the log then +// stopped before its broker_started entry, so the alert about the torn +// line may never have gone out (DESIGN.md section 9). The broker sends the +// alert again. A log_recovered entry that this Open wrote does not count; +// Options.Alert reports it. +func (l *Logger) LastRecovery() (Recovery, bool) { + if l.found == nil { + return Recovery{}, false + } + return *l.found, true +} + // Close closes the log and releases its lock. Append fails after Close. // The broker appends a broker_stopped entry first. func (l *Logger) Close() error { diff --git a/internal/audit/recovery_linux_test.go b/internal/audit/recovery_linux_test.go new file mode 100644 index 0000000..7c64848 --- /dev/null +++ b/internal/audit/recovery_linux_test.go @@ -0,0 +1,72 @@ +package audit + +import ( + "crypto/sha256" + "testing" +) + +// TestOpenFindsARecoveryWithoutAStart: a start records a torn line with a +// log_recovered entry, and stops before it writes anything else, such as +// broker_started; so its alert may never have gone out. The next Open +// finds the log_recovered entry as the last entry and reports it with +// LastRecovery, and does not call Alert again. That holds for each later +// start until an entry follows it. A log_recovered entry that Open writes +// itself is reported by Alert only. +func TestOpenFindsARecoveryWithoutAStart(t *testing.T) { + base := newChain(t) + base.add(BrokerStarted{Version: "dev"}) + base.add(BrokerStopped{}) + next := newChain(t) + next.lines, next.last = append([][]byte(nil), base.lines...), base.last + next.add(Request{Subject: secretsSubject()}) + torn := half(next.lines[2]) + path := logPath(t) + writeLog(t, path, append(base.bytes(), torn...), 0o600) + + first := mustOpen(t, path, testHooks()) + want := Recovery{Seq: 3, TornLength: len(torn), TornSHA256: sha256.Sum256(torn)} + if len(first.alerts) != 1 || first.alerts[0] != want { + t.Fatalf("alerts = %+v, want one: %+v", first.alerts, want) + } + if rec, ok := first.LastRecovery(); ok { + t.Errorf("LastRecovery after Open wrote the entry itself = %+v; want none", rec) + } + if err := first.Close(); err != nil { + t.Fatal(err) + } + + // Two starts that stop before broker_started. + for i := range 2 { + l := mustOpen(t, path, testHooks()) + if len(l.alerts) != 0 { + t.Errorf("start %d: alerts = %+v, want none: there is no new torn line", i+2, l.alerts) + } + if rec, ok := l.LastRecovery(); !ok || rec != want { + t.Errorf("start %d: LastRecovery = %+v, %v; want %+v", i+2, rec, ok, want) + } + if err := l.Close(); err != nil { + t.Fatal(err) + } + } + + // A start that writes broker_started ends it. + l := mustOpen(t, path, testHooks()) + mustAppend(t, l, BrokerStarted{Version: "dev"}) + if err := l.Close(); err != nil { + t.Fatal(err) + } + if rec, ok := mustOpen(t, path, testHooks()).LastRecovery(); ok { + t.Errorf("LastRecovery after broker_started = %+v; want none", rec) + } + mustVerify(t, path) + + // A new log, and a log whose last entry is another event. + if rec, ok := mustOpen(t, logPath(t), testHooks()).LastRecovery(); ok { + t.Errorf("LastRecovery of a new log = %+v; want none", rec) + } + other := logPath(t) + writeLog(t, other, base.bytes(), 0o600) + if rec, ok := mustOpen(t, other, testHooks()).LastRecovery(); ok { + t.Errorf("LastRecovery after broker_stopped = %+v; want none", rec) + } +} diff --git a/internal/store/dir_test.go b/internal/store/dir_test.go new file mode 100644 index 0000000..d05a820 --- /dev/null +++ b/internal/store/dir_test.go @@ -0,0 +1,70 @@ +package store + +import ( + "errors" + "os" + "path/filepath" + "slices" + "strings" + "testing" +) + +// TestPrepareDir: the store directory follows the rules of a store's home +// directory. A missing one is made with mode 0700; one with another mode, +// a file or a link is refused; the directories on the way are checked, and +// PrepareDir itself makes the real check of the config package. +func TestPrepareDir(t *testing.T) { + t.Parallel() + pass := func(string) error { return nil } + + dir := filepath.Join(t.TempDir(), "stores") + if err := prepareDir(dir, pass); err != nil { + t.Fatalf("a missing store directory: %v", err) + } + if fi, err := os.Lstat(dir); err != nil || !fi.IsDir() || fi.Mode().Perm() != homePerm { + t.Fatalf("the store directory: %v, %v; want a directory with mode 0700", fi, err) + } + if err := prepareDir(dir, pass); err != nil { + t.Errorf("a store directory that is there: %v", err) + } + + bad := errors.New("made-up: a directory that others can write to") + var checked []string + check := func(p string) error { + checked = append(checked, p) + return bad + } + if err := prepareDir(dir, check); !errors.Is(err, bad) { + t.Errorf("with a bad directory on the way: %v", err) + } + if !slices.Equal(checked, []string{dir}) { + t.Errorf("checked %q, want %q", checked, dir) + } + + chmod(t, dir, 0o750) + if err := prepareDir(dir, pass); err == nil || !strings.Contains(err.Error(), "has mode 0750") { + t.Errorf("a store directory with mode 0750: %v", err) + } + chmod(t, dir, 0o700) + + link := filepath.Join(filepath.Dir(dir), "link") + if err := os.Symlink(dir, link); err != nil { + t.Fatal(err) + } + if err := prepareDir(link, pass); err == nil || !strings.Contains(err.Error(), "is a symbolic link") { + t.Errorf("a link: %v", err) + } + + for _, p := range []string{"stores", dir + "/", dir + "/../stores", ""} { + if err := prepareDir(p, pass); err == nil || !strings.Contains(err.Error(), cleanAbsRule) { + t.Errorf("%q: %v, want an error about the path", p, err) + } + } + + // The real check refuses the system temporary directory, which + // everyone can write to. + if err := PrepareDir(filepath.Join(t.TempDir(), "stores")); err == nil || + !strings.Contains(err.Error(), "group or others can write") { + t.Errorf("PrepareDir below the system temporary directory: %v", err) + } +} diff --git a/internal/store/home.go b/internal/store/home.go index 737e423..1bf8d38 100644 --- a/internal/store/home.go +++ b/internal/store/home.go @@ -6,12 +6,41 @@ import ( "io/fs" "math" "os" + + "github.com/code6x/nodkey/internal/config" ) -// homePerm is the mode of a store's home directory: only its owner may -// read, write or enter it. +// homePerm is the mode of a store's home directory, and of the store +// directory that holds the homes: only its owner may read, write or enter +// it. const homePerm fs.FileMode = 0o700 +// PrepareDir makes sure that dir, the store directory, is private: New +// makes the home directory of each store in it. nodkey broker calls it +// before New. The rules are those of a store's home directory: +// +// - dir must be a clean absolute path. +// - Every directory on the way to dir must pass config.CheckParentDirs. +// - If dir is missing, it is made with mode 0700. Its parent must exist. +// - dir must be a directory, not a symbolic link, owned by the user that +// runs nodkey, with mode 0700. +// +// An error can name the path. +func PrepareDir(dir string) error { + return prepareDir(dir, config.CheckParentDirs) +} + +// prepareDir is PrepareDir with the check of the directories on the way. +func prepareDir(dir string, checkParentDirs func(string) error) error { + if !cleanAbs(dir) { + return fmt.Errorf("the store directory: %s", cleanAbsRule) + } + if err := prepareHome(dir, checkParentDirs); err != nil { + return fmt.Errorf("the store directory: %w", err) + } + return nil +} + // prepareHome makes sure that home is a private directory for the // program's state files. bws keeps its login state there, and bw keeps a // copy of the vault and its login. diff --git a/internal/telegram/api.go b/internal/telegram/api.go index 50e41d3..3f7cd5b 100644 --- a/internal/telegram/api.go +++ b/internal/telegram/api.go @@ -153,12 +153,14 @@ func newClient(token botToken, base string) *client { // proxy sees only the host name: the URL with the token is inside // TLS. A proxy that inspects TLS, with its own CA in the system's // store, sees the token (DESIGN.md section 7). +// - Closing a connection resets it at once (resetOnClose). func newHTTPClient() *http.Client { protocols := new(http.Protocols) protocols.SetHTTP1(true) + dialer := &net.Dialer{Timeout: 10 * time.Second, KeepAlive: 30 * time.Second} tr := &http.Transport{ Proxy: http.ProxyFromEnvironment, - DialContext: (&net.Dialer{Timeout: 10 * time.Second, KeepAlive: 30 * time.Second}).DialContext, + DialContext: resetOnClose(dialer.DialContext), TLSClientConfig: &tls.Config{MinVersion: tls.VersionTLS12}, TLSHandshakeTimeout: 10 * time.Second, ResponseHeaderTimeout: pollTimeout + pollMargin, @@ -175,6 +177,30 @@ func newHTTPClient() *http.Client { } } +// resetOnClose wraps dial so that closing one of its TCP connections +// resets the connection at once (SO_LINGER 0): the kernel drops the bytes +// that it has not sent yet, and sends no more. When the broker stops, Run +// cancels the getUpdates call that waits, and net/http closes its +// connection. Without the reset, the kernel would still try to send a +// getUpdates request that it had not sent yet, also after the broker +// ended. That late request could reach Telegram after a new broker started +// to poll, and Telegram would end the new poll with a conflict: a false +// alert that the token may be stolen (DESIGN.md section 7). Setting the +// option is best effort: if it fails, the connection still works. +func resetOnClose(dial func(ctx context.Context, network, addr string) (net.Conn, error), +) func(ctx context.Context, network, addr string) (net.Conn, error) { + return func(ctx context.Context, network, addr string) (net.Conn, error) { + c, err := dial(ctx, network, addr) + if err != nil { + return nil, err + } + if tc, ok := c.(*net.TCPConn); ok { + _ = tc.SetLinger(0) // best effort, see above + } + return c, nil + } +} + // proxyFor returns the proxy that the client uses for base, as Go reads // it from the environment (HTTPS_PROXY or https_proxy, and NO_PROXY or // no_proxy), or nil for none. ok is false when the setting is not valid; @@ -365,10 +391,12 @@ func refused(method string, code int, r apiResponse) *apiError { e.err = ErrPollConflict case code == http.StatusUnauthorized: e.reason = "Telegram does not accept the bot token" + e.err = ErrTokenRefused case code == http.StatusForbidden: e.reason = "the bot may not send to this chat: the person did not start the bot, blocked it, or removed it" case code == http.StatusNotFound: e.reason = "Telegram does not know the method or the bot token" + e.err = ErrTokenRefused case strings.Contains(desc, "message is not modified"): e.reason = "the message is not modified" e.err = errNotModified diff --git a/internal/telegram/fake_test.go b/internal/telegram/fake_test.go index 55d8ab3..5d168d3 100644 --- a/internal/telegram/fake_test.go +++ b/internal/telegram/fake_test.go @@ -162,6 +162,9 @@ type fakeTelegram struct { presses map[string]fakePress // the presses that tap queued, by callback query ID changed chan struct{} // closed and made new on each change pollWait time.Duration // how long getUpdates waits for an update + // pollsGone counts the getUpdates calls that ended because the client + // went away before the answer: it closed the connection. + pollsGone int } // fakePress is a button press that tap queued. @@ -344,6 +347,10 @@ func (f *fakeTelegram) serveUpdates(w http.ResponseWriter, r *http.Request, call writeBody(w, http.StatusOK, `{"ok":true,"result":[]}`) return case <-r.Context().Done(): + f.mu.Lock() + f.pollsGone++ + f.notifyLocked() + f.mu.Unlock() return } } diff --git a/internal/telegram/poll.go b/internal/telegram/poll.go index 412aae0..a70bc00 100644 --- a/internal/telegram/poll.go +++ b/internal/telegram/poll.go @@ -88,6 +88,7 @@ const ( // is set), it logs an alert, sends an alert message, and returns // ErrPollConflict. When Telegram refuses getUpdates in another way, // such as for a token that is no longer valid, it returns that error. +// After 401 or 404, that error wraps ErrTokenRefused. func (a *Approver) Run(ctx context.Context, answer func(approval.Answer) error) (err error) { if answer == nil { return errors.New("telegram: Run needs a function for the answers") diff --git a/internal/telegram/poll_test.go b/internal/telegram/poll_test.go index 707e0df..de588e8 100644 --- a/internal/telegram/poll_test.go +++ b/internal/telegram/poll_test.go @@ -375,6 +375,10 @@ func TestPollStopsWhenRefused(t *testing.T) { if !errors.As(err, &ae) || ae.code != status || errors.Is(err, ErrPollConflict) || errors.Is(err, context.Canceled) { t.Errorf("Run: %v, want the %s error", err, code) } + // Only 401 and 404 say that the token does not work. + if refused := status == 401 || status == 404; errors.Is(err, ErrTokenRefused) != refused { + t.Errorf("Run: %v; ErrTokenRefused: %v, want %v", err, !refused, refused) + } if n := len(e.f.callsOf("sendMessage")); n != 0 { t.Errorf("%d messages, want 0", n) } diff --git a/internal/telegram/stop_linux_test.go b/internal/telegram/stop_linux_test.go new file mode 100644 index 0000000..17cf202 --- /dev/null +++ b/internal/telegram/stop_linux_test.go @@ -0,0 +1,111 @@ +package telegram + +import ( + "context" + "errors" + "log/slog" + "net" + "net/http" + "os" + "strings" + "syscall" + "testing" + "time" +) + +// TestConnectionsResetOnClose: closing a connection of the HTTP client +// resets it (SO_LINGER 0), so the kernel drops what it has not sent yet. A +// getUpdates request that waits in the kernel when the broker stops never +// reaches Telegram later (DESIGN.md section 7). The other side sees the +// reset, not a normal end of the connection. +func TestConnectionsResetOnClose(t *testing.T) { + ln, err := (&net.ListenConfig{}).Listen(t.Context(), "tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer func() { _ = ln.Close() }() + tr, ok := newHTTPClient().Transport.(*http.Transport) + if !ok || tr.DialContext == nil { + t.Fatal("the transport has no dialer of its own") + } + c, err := tr.DialContext(t.Context(), "tcp", ln.Addr().String()) + if err != nil { + t.Fatal(err) + } + server, err := ln.Accept() + if err != nil { + t.Fatal(err) + } + defer func() { _ = server.Close() }() + if err := c.Close(); err != nil { + t.Fatal(err) + } + if err := server.SetReadDeadline(time.Now().Add(waitLimit)); err != nil { + t.Fatal(err) + } + _, err = server.Read(make([]byte, 1)) + if !errors.Is(err, syscall.ECONNRESET) { + t.Errorf("the other side read %v, want a reset (ECONNRESET)", err) + } +} + +// TestRunClosesThePollConnection: when Run returns because its context +// ended, the getUpdates call that waited has ended, and its connection is +// closed: the server sees the client go away. Run sends no getUpdates +// after it returned. +func TestRunClosesThePollConnection(t *testing.T) { + e := newEnv(t) + e.f.mu.Lock() + e.f.pollWait = 2 * waitLimit // the call waits until the client goes away + e.f.mu.Unlock() + stop := e.run(t, (&stubCore{}).answer) + e.f.waitCalls("getUpdates", 1) + if err := stop(); !errors.Is(err, context.Canceled) { + t.Errorf("Run: %v, want context.Canceled", err) + } + e.f.waitFor("the end of the waiting getUpdates call", func() bool { return e.f.pollsGone == 1 }) + time.Sleep(100 * time.Millisecond) // a late call would come now + if n := len(e.f.callsOf("getUpdates")); n != 1 { + t.Errorf("%d getUpdates calls, want 1: Run polled after it returned", n) + } +} + +// TestNewLogsTheCAPlaces: when SSL_CERT_FILE or SSL_CERT_DIR is set, Go +// takes the CA certificates that check Telegram's certificate from there, +// and a CA there sees the bot token. New logs each one that is set, with +// its name and its path only. +func TestNewLogsTheCAPlaces(t *testing.T) { + token := newCanaryToken(t) + read := func(string) ([]byte, error) { return []byte(token), nil } + newWithLog := func() string { + var logs syncBuffer + logger := slog.New(slog.NewTextHandler(&logs, nil)) + if _, err := newApprover(testTelegramConfig(), Options{Logger: logger}, read, "http://127.0.0.1:1"); err != nil { + t.Fatal(err) + } + return logs.String() + } + t.Setenv("SSL_CERT_FILE", "") + t.Setenv("SSL_CERT_DIR", "") + if err := os.Unsetenv("SSL_CERT_FILE"); err != nil { + t.Fatal(err) + } + if err := os.Unsetenv("SSL_CERT_DIR"); err != nil { + t.Fatal(err) + } + if logs := newWithLog(); strings.Contains(logs, "CA certificates") { + t.Errorf("a log line without SSL_CERT_FILE or SSL_CERT_DIR: %s", logs) + } + t.Setenv("SSL_CERT_FILE", "/etc/nodkey/test-ca.pem") + t.Setenv("SSL_CERT_DIR", "/etc/nodkey/ca:/etc/nodkey/more-ca") + logs := newWithLog() + for _, want := range []string{"variable=SSL_CERT_FILE path=/etc/nodkey/test-ca.pem", + `variable=SSL_CERT_DIR path=/etc/nodkey/ca:/etc/nodkey/more-ca`} { + if !strings.Contains(logs, want) { + t.Errorf("the log has no %q: %s", want, logs) + } + } + if strings.Contains(logs, token[strings.IndexByte(token, ':')+1:]) { + t.Error("the log holds the bot token") + } +} diff --git a/internal/telegram/telegram.go b/internal/telegram/telegram.go index 075d90e..df4f8a7 100644 --- a/internal/telegram/telegram.go +++ b/internal/telegram/telegram.go @@ -29,6 +29,7 @@ import ( "fmt" "io" "log/slog" + "os" "slices" "strings" "sync" @@ -87,6 +88,12 @@ var ( // nothing was sent: the core denies the use with cause limit, and the // use does not count for max_per_day. ErrNoticeLimit = fmt.Errorf("telegram: too many notices in the last minute: %w", approval.ErrLimit) + // ErrTokenRefused: Telegram answered a call with 401 or 404: it does + // not accept the bot token, or does not know the bot, such as after + // the token was revoked or the bot deleted. Only a new token helps. + // When Run returns it, nodkey broker exits with its own code, so that + // systemd does not start it again and again with the same token. + ErrTokenRefused = errors.New("telegram: Telegram does not accept the bot token") ) // Options are the optional parts of an Approver. @@ -186,6 +193,10 @@ func (w *reportWindow) end(now time.Time) int { return dropped } +// caVariables are the variables with which Go's TLS code on Linux takes +// its CA certificates from other places than the system's. +var caVariables = []string{"SSL_CERT_FILE", "SSL_CERT_DIR"} + var _ approval.Approver = (*Approver)(nil) // New returns the Telegram approver for the [telegram] section cfg. It @@ -228,6 +239,19 @@ func newApprover(cfg config.Telegram, opts Options, read func(string) ([]byte, e "telegram: calls to Telegram go through a proxy from the environment; a proxy that inspects TLS sees the bot token", slog.String("proxy_host", proxy.Host)) } + // Go takes the CA certificates that check Telegram's certificate from + // these variables, when they are set, instead of the system's places. + // A CA there could make a server that the broker takes for Telegram, + // as a proxy that inspects TLS does. Only the name and the path are + // logged. + for _, name := range caVariables { + if path, ok := os.LookupEnv(name); ok { + logger.LogAttrs(context.Background(), slog.LevelInfo, + "telegram: the CA certificates that check Telegram's certificate come from a place that the "+ + "environment sets; a CA there sees the bot token", + slog.String("variable", name), slog.String("path", path)) + } + } return &Approver{ api: newClient(token, base), userID: cfg.UserID,