Migrate to RustCrypto/SSH

[baloo]

This migrates the serialization to use https://crates.io/crates/ssh-encoding.
The key structures are provided by https://crates.io/crates/ssh-key.

This makes using the keys with rust-crypto abstractions a lot easier (no need to convert in and out). I think this should also clean up the serialization weirdness I've seen lately.

[baloo]

Opening early and as a draft to get feedback and judge whether this would be an avenue you'd be willing to pursue.

I think this should lighten up the code present here:

 src/agent.rs             |  12 +++--
 src/error.rs             |  14 +++--
 src/proto/de.rs          | 267 --------------------------------------------------------------------------------------------
 src/proto/error.rs       |  19 -------
 src/proto/extension.rs   |  18 ++-----
 src/proto/key_type.rs    |  63 ----------------------
 src/proto/message.rs     | 312 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------------
 src/proto/mod.rs         |  73 --------------------------
 src/proto/private_key.rs | 109 --------------------------------------
 src/proto/public_key.rs  | 191 ------------------------------------------------------------------
 src/proto/signature.rs   |  43 ---------------
 src/proto/tests/mod.rs   |   6 ++-
 12 files changed, 273 insertions(+), 854 deletions(-)

(obviously I'm missing a lot, take that with a grain of salt)

[wiktor-k]

🤔 To be honest I was surprised serde was used here when I initially saw this crate so this just may be the right avenue and your stats are really promising.

I think the most challenging part would be writing tests for at least the most used key types, since the crate supported quite a bit of them:

            PrivateKey::Dss(key) => PublicKey::Dss(DssPublicKey::from(key)),
            PrivateKey::Ed25519(key) => PublicKey::Ed25519(Ed25519PublicKey::from(key)),
            PrivateKey::SkEd25519(key) => PublicKey::SkEd25519(SkEd25519PublicKey::from(key)),
            PrivateKey::Rsa(key) => PublicKey::Rsa(RsaPublicKey::from(key)),
            PrivateKey::EcDsa(key) => PublicKey::EcDsa(EcDsaPublicKey::from(key)),
            PrivateKey::SkEcDsa(key) => PublicKey::SkEcDsa(SkEcDsaPublicKey::from(key)),

On the other hand it's not clear if it worked at all since it wasn't tested in the first place... ugh...

I'm 👍 on this though :)

[wiktor-k]

Hmm... the more I think about this the more advantages I see: we could later split the messages into requests and responses (so that it's impossible to return a request object etc.) and properly document the types. I was just about to add more docs but I'm glad that you filed the PR early so that I don't document stuff that's going to disappear :)

[baloo]

we'll also need to revisit some types. Like the Success response can be free-formed.

https://datatracker.ietf.org/doc/html/draft-miller-ssh-agent-11#section-3.8.1

[wiktor-k]

we'll also need to revisit some types. Like the Success response can be free-formed.

https://datatracker.ietf.org/doc/html/draft-miller-ssh-agent-11#section-3.8.1

What an interesting edge case. Usually success doesn't have any data but for this single command it has. Does that mean the SSH client needs to keep state (so if the question was SSH_AGENTC_EXTENSION expect SSH_AGENT_SUCCESS with answers)... If I were designing the protocol I'd just use a different reply codepoint... but 🤷‍♂️ (in our code we can still use some different variant... or just make the Success take a vector of Strings and if it's empty it's empty).

Tangentially but I'm also thinking we should be presenting some higher level interface on the Agent Session since handle is too low-level (let's leave it in but delegate to special purpose functions like sign_request or get_identities and we'd wrap and match everything for the Session user)... okay, but that's for later 😅

Okay, one more thing: it seems this PR will take some further time (just for adding all key types and so it's tedious but needs to be done :/ ) so what do you think about releasing 0.3.0 from main now? (maybe with #18 merged?). The tokio-1 migration already provides quite a lot of value to people (I've got reports) and it's not as if we'd run out of numbers anyway... 😅

[baloo]

What an interesting edge case. Usually success doesn't have any data but for this single command it has. Does that mean the SSH client needs to keep state (so if the question was SSH_AGENTC_EXTENSION expect SSH_AGENT_SUCCESS with answers)... If I were designing the protocol I'd just use a different reply codepoint... but 🤷‍♂️ (in our code we can still use some different variant... or just make the Success take a vector of Strings and if it's empty it's empty).

Yeah, I kind of rely on that actually. Currently my implementation was relying on replying an extension, but your comment made me re-read the spec :D.
I have a whole custom protocol over extensions to get across SSH sessions (With agent forward (forward is detected via session-bind and refuses to sign afterwards).

Okay, one more thing: it seems this PR will take some further time (just for adding all key types and so it's tedious but needs to be done :/ ) so what do you think about releasing 0.3.0 from main now? (maybe with #18 merged?). The tokio-1 migration already provides quite a lot of value to people (I've got reports) and it's not as if we'd run out of numbers anyway... 😅

Works for me.

[wiktor-k]

Works for me.

Just FYI it's done: https://github.com/wiktor-k/ssh-agent-lib/releases/tag/v0.3.0

[wiktor-k]

Yeah, I kind of rely on that actually. Currently my implementation was relying on replying an extension, but your comment made me re-read the spec :D.
I have a whole custom protocol over extensions to get across SSH sessions (With agent forward (forward is detected via session-bind and refuses to sign afterwards).

Is your stuff open-source? It looks really interesting... what exactly are you doing over extensions there 👀

[baloo]

closed in favor of #26