Skip to content

Fix ECDSA key registration in Commander SSH agent - #2394

Open
rporter92 wants to merge 1 commit into
Keeper-Security:masterfrom
rporter92:ssh-agent-ecdsa-fix
Open

rporter92 wants to merge 1 commit into
Keeper-Security:masterfrom
rporter92:ssh-agent-ecdsa-fix

Conversation

@rporter92

Copy link
Copy Markdown

Problem

Commander's built-in SSH agent (ssh-agent start then ssh <record>) fails to add any ecdsa-sha2-* key:

SSH request process error: type object
'cryptography.hazmat.primitives.asymmetric.ec.EllipticCurvePublicNumbers'
has no attribute 'from_encoded_point'
Add ssh-key. Failed to add ssh key "<record>" to ssh-agent

Two issues in the ECDSA branch of the ADD_IDENTITY handler in keepercommander/commands/ssh_agent.py:

  1. EllipticCurvePublicNumbers.from_encoded_point() was removed in cryptography 39.0.0; requirements.txt requires >=46.0.6, so this raises on every supported install.
  2. The stored public key blob omits the curve-name string required by RFC 5656 §3.1. Previously masked by (1); once (1) is fixed alone, ssh-add -l reports elliptic curve does not match and authentication fails.

Fix

  • Use EllipticCurvePublicKey.from_encoded_point(curve, point).public_numbers().
  • Include curve_name in key.key_blob.

No dependency change; the replacement API exists in every cryptography version Commander has ever required.

Testing

  • Existing unit-tests/ suite passes.
  • Manually created ECDSA Key after change applied and confirmed ECDSA key is now accepted in Commander SSH Agent. Errors no longer occurring.

The ADD_IDENTITY handler called EllipticCurvePublicNumbers.from_encoded_point(), which cryptography removed in 39.0.0, and built the ECDSA public key blob without the RFC 5656 curve-name string. Use EllipticCurvePublicKey.from_encoded_point() and include the curve name so ssh-add and SSH clients accept the key. After implementing the changes, i tested the full process using an ECDSA key and confirmed the change resolved the issue.

This branch has not been deployed

No deployments
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.

1 participant