login: load certificate into ssh-agent with configurable lifetime - #611
Open
sini wants to merge 6 commits into
Open
login: load certificate into ssh-agent with configurable lifetime#611sini wants to merge 6 commits into
sini wants to merge 6 commits into
Conversation
…time When SSH_AUTH_SOCK points at a running ssh-agent, opkssh login now also adds the freshly minted certificate and its private key to the agent, so the key is usable immediately without pointing ssh at the key files. Best-effort and non-fatal: the keys are still written to disk first, and any agent problem is reported as a warning rather than failing the login. The key is always added with a lifetime, since ssh-agent has no replace operation and keys would otherwise accumulate with every login. The lifetime is resolved as: --lifetime flag, then agent_lifetime in the client config (~/.opk/config.yml), then a default of 24h matching the server's default certificate expiration policy. Both duration strings (12h, 45m) and raw seconds (28800) are accepted, and an invalid value fails before the browser dance rather than after authentication. Relates to openpubkey#6 and openpubkey#96; first of the two PRs agreed in openpubkey#606 (the second adds a refresh daemon that rotates agent keys as tokens are refreshed).
Review findings on the agent-lifetime feature: - Reject lifetimes under one second: uint32(d / time.Second) truncated sub-second durations (500ms, 900ms) to LifetimeSecs 0, which the agent protocol treats as no lifetime at all — an immortal key, the exact state the lifetime exists to prevent. Bound raw seconds values before multiplying, since time.Duration(sec) * time.Second wraps int64 for large inputs and could land on a small bogus duration. - Set a 5s deadline on the ssh-agent exchange so a dead socket (e.g. a forwarded agent whose upstream connection is gone) degrades to a warning instead of hanging the login after authentication succeeded. - Unset SSH_AUTH_SOCK in the integration suite's TestMain so its seven login flows can never inject test keys into a developer's real agent. - Document that the agent is not currently reachable on Windows.
… in CLI docs Post-review cleanup, behavior-preserving: - The in-process test-agent scaffolding is extracted to startTestAgent (windows skip, socket lifecycle via t.Cleanup, SSH_AUTH_SOCK wiring in one place). - The SSH_AUTH_SOCK isolation guard moves into the shared Mocks fixture, so every mock login flow is isolated from the developer's real agent by construction instead of by three pasted guards; a test that needs an agent points SSH_AUTH_SOCK at its own after calling Mocks. - TestParseConfigWithAgentLifetime collapses its two verbatim copies into a loop over both accepted value shapes. - Two comment blocks that restated nearby doc comments are trimmed. - docs/cli/opkssh_login.md is regenerated to document the new --lifetime flag (only this file has content changes from the flag addition).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
First of the two PRs agreed in #606: when
SSH_AUTH_SOCKpoints at a running ssh-agent,opkssh loginnow also adds the freshly minted certificate and its private key to the agent, so the key is usable immediately without pointingsshat the key files. Best-effort and non-fatal — the keys are still written to disk first, and any agent problem is a warning rather than a login failure.The key is always added with a lifetime, since ssh-agent has no replace operation and keys would otherwise accumulate with every login. The lifetime is resolved as:
--lifetimeflag (opkssh login --lifetime 12h)agent_lifetimein the client config (~/.opk/config.yml)Both duration strings (
12h,45m) and raw seconds (28800) are accepted. An invalid value fails up front, before the browser dance.Per the discussion in #606, this PR makes no change to certificate
ValidBefore— the client cannot know the server's expiration policy (the server computes it from the ID token'siat), so the agent lifetime is a client-side bound to keep keys from accumulating, not a statement of the certificate's validity. Syncing expiries dynamically is the follow-up refresh-daemon PR's job.Relates to #6, #96. Split out of #606.
Notes
Run()(fail before the browser dance) and again inaddCertToAgent, becauseLogin/LoginWithRefreshare exported and reachable withoutRun()(the integration tests call them directly). Sub-second and overflowing values are rejected — a truncatedLifetimeSecsof 0 would mean an immortal key, the exact state the lifetime exists to prevent.--auto-refreshthe key is added to the agent once at initial login; refreshed certificates only go to disk as today. Live agent rotation on refresh lands with the refresh daemon (PR 2).--print-key(that mode's contract is stdout only).agent.ServeAgentserver over a unix socket (skipped on Windows), asserting the added key carries the certificate and the requestedLifetimeSecs. Both the unit login-flow tests and the integration suite now pinSSH_AUTH_SOCKempty sogo testnever touches the developer's real agent.SSH_AUTH_SOCKis typically unset, so the agent add is silently skipped (now noted indocs/config.md); named-pipe support is planned alongside the daemon work.