Handle AddIdConstrained (msg 25) for SK keys#67
Handle AddIdConstrained (msg 25) for SK keys#67Fletch153 wants to merge 2 commits intoobelisk:developfrom
Conversation
Fletch153
force-pushed
the
handle-add-id-constrained
branch
4 times, most recently
from
3e1719d to
b2a9c57
Compare
ssh-add sends message 25 instead of 17 for security key types (ed25519-sk, ecdsa-sk) because it needs to pass along additional constraint metadata (sk-provider extension). The message type was already mapped in the enum but returned Request::Unknown, causing "agent refused operation" when trying to ssh-add an SK key. Parse the key data the same way as AddIdentity (message 17). Trailing constraint bytes are safely ignored by from_bytes().
Fletch153
force-pushed
the
handle-add-id-constrained
branch
from
b2a9c57 to
fc040a2
Compare
obelisk
left a comment
obelisk
left a comment
There was a problem hiding this comment.
Could you please parse out the constraints (and also log them as debug) that way if we want/need to implement them later we're in a better position.
Fletch153
force-pushed
the
handle-add-id-constrained
branch
2 times, most recently
from
0381dad to
f41fb83
Compare
Parse SSH agent key constraints (Lifetime, Confirm, Extension) from AddIdConstrained messages and log them at debug level, positioning us to implement constraint enforcement later. Tests use real wire data captured from ssh-add -t 3600 -c.
Fletch153
force-pushed
the
handle-add-id-constrained
branch
from
f41fb83 to
b98ea95
Compare
| debug!("AddIdConstrained constraints: {:?}", constraints); | ||
| } | ||
| match sshcerts::PrivateKey::from_bytes(buf) { | ||
| Ok(private_key) => Ok(Request::AddIdentity { private_key }), |
There was a problem hiding this comment.
This should be a trait member AddIdentityConstrained. That way we can pass constraints in properly. Then the parsing and debug printing can happen in there.
obelisk
left a comment
obelisk
left a comment
There was a problem hiding this comment.
Adding in the trait member should be the final refactor this needs. It will also provide a good place to add doc strings about what this is and what makes it different (also linking to external documentation would be good).
|
|
||
| // Captured from: ssh-add -t 3600 -c /tmp/test_ed25519 (comment "test-key") | ||
| // Message type 25 (SSH_AGENTC_ADD_ID_CONSTRAINED), body only (no msg type byte). | ||
| const REAL_ADD_ID_CONSTRAINED: &[u8] = &[ |
There was a problem hiding this comment.
Interesting. Can you provide more details on the command that generated this? As well as do a test with an SK key so we can test the more common use case?
|
Closing in favour of #68 which relies on a change to sshcerts to better support this case. |
2 participants
Summary
ssh-addsendsSSH2_AGENTC_ADD_ID_CONSTRAINED(message 25) instead ofSSH2_AGENTC_ADD_IDENTITY(message 17) for security key types (ed25519-sk, ecdsa-sk). This is because it needs to pass along additional constraint metadata (sk-provider extension).The message type was already mapped in the
MessageRequestenum but the match arm returnedRequest::Unknown, causingResponse::Failure— surfacing as"agent refused operation"when trying tossh-addan SK key file.Fix
Parse the key data in the
AddIdConstrainedarm the same way asAddIdentity. The wire format is identical except for trailing constraint bytes after the key data + comment.sshcerts::PrivateKey::from_bytes()is a lenient parser that reads only the key fields and ignores trailing bytes, so this works without any additional parsing.Testing
Verified by running
rustica-agent-cli singlewith the fix and successfully:ssh-add <ed25519-sk-key>— identity added (previously refused)ssh-add -l— SK key appears in listed identities