login: expire certificate with token exp and load it into ssh-agent - #606
login: expire certificate with token exp and load it into ssh-agent#606sini wants to merge 3 commits into
Conversation
c401cdd to
de3ac7b
Compare
Two related changes so an opkssh identity expires coherently: sshcert: set the certificate's ValidBefore to the ID Token's `exp` claim instead of ssh.CertTimeInfinity. A certificate that carries a token's authorization should not outlive that token, and a `Valid: forever` cert is rejected by stricter SSH certificate parsers (e.g. RustCrypto ssh-key < 0.7 errors with "invalid time" on valid_before = u64::MAX). Falls back to CertTimeInfinity when the OP omits exp. login: when SSH_AUTH_SOCK is set, additionally load the certificate and its private key into the ssh-agent with a lifetime bounded by the token's exp, so the agent drops the key when the certificate expires. This revives the intent of openpubkey#96 on current main. It is best-effort and non-fatal; the keys are still written to disk as before.
de3ac7b to
86ebb35
Compare
|
@sini Let me know when this is ready for review.
OPKSSH does not use the ID Token's expiration claim as the expiration claim for the PK Token. Thus the party that creates the SSH cert does not know what the expiration time will be and can't set it for the SSH cert. OPKSSH expiry and Token expiry are not the same. They originally were the same, but the number one complaint we got from nearly everyone that use OPKSSH was that SSH keys expired too quickly. This is because OPs set very very low exp times (between 30 seconds to 20 minutes). This means that someone would need to run OPKSSH login every time they SSHed. I ran into this issue myself and was running OPKSSH login like 20 times a day. As a result OPKSSH expiry is set from the iat (issued_at) claim. The default value is iat + 24 hours, but it is configurable on the OPKSSH server. See https://github.com/openpubkey/opkssh/blob/main/docs/config.md#allowed-openid-providers-etcopkproviders-linux-or-programdataopkproviders-windows See discussion in this issue here.
The SSH certificate here is simply a way to package the PK Token and are self-signed. Thus an attacker with a leaked private key + certificate could simply resign the SSH certificate with expiry they choose. OPKSSH is secure because we check the PK Token inside the certificate using AuthorizedKeysCommand. Without an AuthorizedKeysCommand, OpenSSHd would immediately reject the SSH certificate since it is self-signed. So this wouldn't get us any security.
Well that sucks. Rust libraries shouldn't place additional restrictions on certificates. Maybe just set the opkssh to
One way to get this to work is to continuously refresh the ID Token with What do you think the right approach here is, given the issues with |
|
Thanks for the detailed context. The RustCrypto tolerance fix is on a RC branch, so perhaps the correct move is to just wait for that to land (for my use-case) and do nothing. I'm not sure that (u64::MAX-1) is a proper solution, it feels 1000% like a hack -- which is exactly the kind of solution I was looking to avoid. :) I'll give the issue thread you linked a deeper read and update the PR after giving it some thought. |
|
@EthanHeilman Do you have a recommendation for how you would like to see this PR evolve? Let's say I take this PR and focus solely on the ssh-agent lifecycle aspect of it -- having the CLI auto-register to the local agent with a timeout so keys don't just accumulate indefinitely. In that scenario the expiration time is ideally equal to the certificates validity time. Does this become a discovery problem between client <=> server configuration in your mind? |
Yeah, that is the main issue. We need two expiry times, agent_expiry (key lifetime) and server_expiry, and what happens when they don't line up. | Default | Min | Max | | If we set If we set
We should stage this as two PRs. The first PR adds SSH-agent support via this 24 hour lifetime mechanism. The second PR adds full refresh-agent support and then sets What do you think about this plan? |
|
Yeah, I can split this and stack the delivery for ease of review.
Yes, that's entirely possible -- but if the cert is good for 1 week, why would we expire it at 24 hours? Likewise, if the cert is only good for 4-8 hours, why would we persist it for 24 hours? So I think this should be configurable with a matching default. So, for the split: PR 1: using PR 2: once background token refreshing is in place, we can dynamically manage key replacement and sync |
… agent_lifetime config
The OPKSSH server computes expiration using a configuration setting (8hour, 24hour, 1week) from on the ID Token's SSH-agent, AFAICT, doesn't have a delete-key/replace-key function. If it did, we could just use that. Since it doesn't we absolutely have to set a lifetime on keys we add to prevent it from filling up with expired keys. The split sounds good.
Only thing to add is that we currently have background token refreshing is in place, it just needs to be improved. Would you be up for building the improved token refreshing in PR2? |
Yep, just give me your expectations/guidelines for it and I'll happily volunteer my time. :) |
Apparently, I use ssh-agent-mux to proxy bitwarden backed keys (rbw), gpg-agent (my yubikey), and standalone ssh-agent (opkssh key + a separate LLM agent git signing key). If we're building a persistent daemon it might be worth giving it full ssh-agent capabilities (optionally). |
Current it is just a process that run refreshes the token and writes a new SSH key. This is bad for four reasons:
Instead the refresh-agent should run like ssh-agent or oidc-agent. A single instance spins up as a daemon listening on a socket, talking to ssh-agent. Since it is a single instance it can deconflict refreshes. opkssh can query its current status. Stretch goal: OpenID Key Binding refresh support: OpenID Key Binding refresh flow signs a random challenge to prove possession of the key. If this signature can be requested from ssh-agent, that is fantastic because then the refresh-agent doesn't need to protect the private key. If the refresh-agent does need to do the signing then we probably want similar isolation as exists in ssh-agent. This doesn't need to be done in PR2, but it is worth thinking about when planning PR2 so that PR2 can work with future a key-binding refresh-agent PR. |
|
@EthanHeilman How does this draft spec look to you? https://gist.github.com/sini/b645242d94b5174e306e4ab54674bf63 |
|
@sini That looks great! Using the signer in that way can help us plug into HSMs. |
|
Alright. I've got another project currently in flight but once I hit a stopping point on it I'll do a more concrete implementation plan and build it. Full disclosure ahead of time, I'm going to use Claude Opus 5 for the largely mechanical parts + the Windows integration components since that's not really my wheelhouse so please be extra critical of that. The actual important security related parts/architecture will be all me. |
|
@EthanHeilman Alright, I found a flaw with my original draft design where I incorrectly assumed the refresh token would be recoverable. I've restructured the responsibilities and flow to ensure the responsibility now lives within the daemon. Here's my new refined design spec: And here's a rough implementation ordering plan: As a predicate, I'd like to propose refactoring the storage layout for existing config + keys. For keys that means Update: Here's a spec for the proposed file changes: https://gist.github.com/sini/c32d1b73adbb56f1bdbbe620b10dc80f |
Summary
Two related changes so an opkssh identity expires coherently, and so the freshly minted certificate is usable without cluttering
~/.ssh.1. Bind the certificate's validity to the ID Token (
sshcert)sshcert.Newcurrently setsValidBefore: ssh.CertTimeInfinity, so every opkssh certificate isValid: forever. The certificate carries a PK token whose authorization is time-bounded (the OP'sexp), yet the certificate itself never expires. This change setsValidBeforeto the ID Token'sexpclaim, so the certificate cannot outlive the token it embeds (defense in depth: a leaked private key + certificate is useless once the token expires, independent of the verifier). Falls back toCertTimeInfinityif the OP omitsexp;ValidAfteris left unset to avoid clock-skew rejections.This also fixes real-world interop: stricter SSH certificate parsers reject
valid_before = u64::MAX. For example RustCryptossh-key< 0.7 errors withinvalid time(RustCrypto/SSH#504), which makesssh-agent-mux(and anything built on that library) unable to list an agent holding an opkssh cert (overhacked/ssh-agent-mux#56).2. Load the certificate into
ssh-agenton login (login)When
SSH_AUTH_SOCKis set,opkssh loginnow also adds the certificate and its private key to the running agent, withLifetimeSecsbounded by the tokenexp, so the agent drops the key exactly when the certificate expires. Best-effort and non-fatal — the keys are still written to disk as today. This revives the intent of #96 on currentmain, driven offexprather than a hardcoded lifetime.Fixes #6. Relates to #96, #56.
Notes
Draft: opening for discussion of the approach (in particular whether the agent-add should be gated behind an explicit flag rather than
SSH_AUTH_SOCKpresence, and whether to keep writing key files when the agent is used). Unit tests pass (go test ./sshcert/...); thecommandsintegration tests that shell out tosshdwere not run in my environment.