Skip to content

Security hardening - #1

Open
FiloSottile wants to merge 7 commits into
boldsoftware:mainfrom
filippo-agent:per-share-codes
Open

FiloSottile wants to merge 7 commits into
boldsoftware:mainfrom
filippo-agent:per-share-codes

Conversation

@FiloSottile

Copy link
Copy Markdown

This turns the tokens into high-entropy ones, enables tailcat's PSKs, and makes read-only-ness more robust.

The two main risks it mitigates are:

  1. an attacker with a read-only token had a 0.6% chance per month of guessing the right read-write one, which is small but not nothing, especially if used for e.g. a fleet;
  2. there was no protection against quantum harvest-now-decrypt-later attacks over tailcat.

The loss is the read-out-loud friendliness of the tokens. I am kinda skeptical folks are reading them over the phone.

It also makes a session started with -read-only have no way to start a read-write session, for more sensitive settings.

Codes used to belong to the session: one read-only and one read-write,
both generated at startup and both accepted on every share. Handing an
agent a read-only URL therefore also handed it a listener on which the
read-write code was valid, and the only thing between the two was the
code's entropy and the wrong-code rate limit.

A share now has its own read-only code from the moment it opens, and a
read-write code only once read-write access is asked for on it. A share
only ever shared read-only has no code that can type, and a code never
works on a share it wasn't issued for. Switching the launcher to
read-only revokes the share's read-write code. shoulder ls shows each
share's access.
Codes were a number and two words, about 22 bits, drawn from
math/rand/v2. That package's global source is unpredictable in current
Go, but its documentation makes no such promise, and the short format
left the wrong-code rate limit as the only thing making a guess hard.

crypto/rand.Text gives at least 128 bits as base32 text. A code is pasted
into an agent's command line rather than read aloud or typed, so the
length costs nothing.
Codes carry at least 128 bits, so guessing one is hopeless at any rate,
and the limit had costs of its own: ten wrong codes locked a share for
everyone, so anyone who could reach a lan or tailscale share could keep
its legitimate agent out, and its check-then-record structure was only
sound because the handler between the two steps was fast. What remains
is a constant-time comparison.
The tailcat share disabled WireGuard's pre-shared key for a shorter
address, leaving the tunnel's confidentiality to X25519 alone. tailcat
documents the key as what keeps a relay operator who sees the node keys
from joining the tunnel, and what keeps a recording of the traffic
confidential against a future quantum computer. The address grows by the
key; it is pasted, not typed.
The state column was 34 characters, which the share list already
overflowed with one network share, and the per-share access suffixes
made longer still. Both the name and state columns are now as wide as
their widest entry, and three spaces apart so they read as columns.
The session's own socket carries the control API, and nothing on it
checks a code: /ctl/attach types into the terminal, and /ctl/share
mints codes for any transport. Whoever can reach the socket, as the
same user or through a forwarded socket, controls the session, so file
permissions are its only boundary and a code on it enforces nothing.
A read-only unix code enforced even less, while looking like it did.

The unix share has no codes, and its paste reaches the API at the
socket's root. Asking for read-only access on it is an error, which the
-read-only flags and the launcher's [r] key report. The launcher shows
no access toggle on the unix socket and explains why under the paste
line, and a read-only choice made on a network transport survives a
hop through the unix socket instead of quietly becoming read-write.
Read-only was a property of a code on a share, negotiated on every
request: a share minted its read-write code when first asked for it and
dropped it when the launcher switched to read-only, and the exclusive
flag that closed other shares for the launcher's menu carried that
revocation too. That is more state than the use case needs, and the
bulk close under the flag had ordering and race problems.

Read-only is a session property, set by -read-only when the session
starts and never changed. A network share gets its read-only code, and
in a read-write session its read-write code, when it opens, and they
never change. A read-only session never has a code that can type, and
its network shares refuse to type whatever the code, so the property
holds even if a read-write code were ever made. The unix socket has no
codes and stays read-write: a read-only session starts on localhost
unless -t says otherwise, and its menu leaves [u] out.

[r] and `shoulder share -read-only` ask for a share's other code and
change nothing. The launcher switches transports by closing the share
it is on, over /ctl/unshare, and opening the new one, going back to the
old one if that fails, so a -port moves with it. /ctl/info lists open
shares by name, sorted, and shoulder ls marks read-only sessions.

The first screen says "on this machine" for the unix socket and
localhost, and the notes and help text are shorter.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants