Skip to content

tailscale, keys: randomize disco key, don't store#171

Open
npry wants to merge 1 commit intomainfrom
npry/keys.randomize-disco
Open

tailscale, keys: randomize disco key, don't store#171
npry wants to merge 1 commit intomainfrom
npry/keys.randomize-disco

Conversation

@npry
Copy link
Copy Markdown
Collaborator

@npry npry commented May 7, 2026

Stop storing the disco key in the persisted key state and regenerate it each time a tailscale::Device runs. The persisted state is now its own type, ts_keys::PersistState, which only contains non-disco private keys, rather than both public and private.

Provide rudimentary migration for the config load/generate function to automatically convert existing key files to the new format.

Update lang bindings to support the new types.

This appears to solve the ts_tunnel session timeout problem based on my local tests.

tailscale::Config::key_state

It's not entirely clear what this field should represent. I left it as NodeState so the user can fully control the keys used when running a tailscale::Device, even though we know that the disco key should always be random. Arguably, we might want to make that field PersistState and unilaterally take over the responsibility for randomizing it, but I lean towards giving the user the freedom to do what they want, with the relevant footguns documented and the easy default path (via the key state file) not invoking sad behavior. Open to pushback, lmk

Stop storing the disco key in the persisted key state and regenerate it
each time a `tailscale::Device` runs. The persisted state is now its own
type, `ts_keys::PersistState`, which only contains non-disco private
keys, rather than both public and private.

Provide rudimentary migration for the config load/generate function to
automatically convert existing key files to the new format.

Update lang bindings to support the new types.

This appears to solve the ts_tunnel session timeout problem based on
my local tests.

Signed-off-by: Nathan Perry <nathan@tailscale.com>
Change-Id: I9aadd9e4ca8e025d8b587467bd2edf7a6a6a6964
@npry npry marked this pull request as ready for review May 7, 2026 12:10
Copy link
Copy Markdown
Collaborator

@dylan-tailscale dylan-tailscale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, and appreciate the added context in doc comments around disco keys

@dylan-tailscale
Copy link
Copy Markdown
Collaborator

I lean towards giving the user the freedom to do what they want, with the relevant footguns documented and the easy default path (via the key state file) not invoking sad behavior

For me, the deciding factor is "why would a user want to persist/reload an identical disco key?" If there's a reasonably valid use-case there (as in, "oh yeah I could see someone wanting to do that") then no problem leaving it as-is, with footgun docs/etc as you mentioned. If we can't think of a valid use-case, and the community doesn't present one, then I'd prefer using PersistState instead, just to avoid an avenue for bugs/misuse that has no known application. We're never going to guess every valid use-case obviously, but I do use that as a vague metric for potential harm/bugs outweighing usability.

Off the top of my head, I can't think of a valid use-case for re-using disco keys in a way that keeping NodeState enables, but I haven't thought too deeply about it. I'm good leaving this as-is for now and discussing here/face-to-face as a team and making changes later if necessary!

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