Unable to make this crate work

[williamdes]

I tried my RSA key, created a new ecdsa key but nothing did work..
So I copied one of your test, and it still did not work. This Error:Crypto is cryptic. It get's me insane, I tried everything and could not figure out what's wrong: https://github.com/RustCrypto/SSH/search?q=Error%3A%3ACrypto

ssh-keygen -t ecdsa -C "williamdes+ecdsa-2023@wdes.fr" -f test_file

#[test]
fn sign_ecdsa() {
    use ssh_key::{PrivateKey, HashAlg};
    let signing_key = PrivateKey::from_openssh(include_str!("../test_file")).unwrap();
    let verifying_key = include_str!("../test_file.pub").parse::<PublicKey>().unwrap();

    let signature = signing_key
        .sign("example", HashAlg::Sha512, b"msg")
        .unwrap();

    assert_eq!(
        verifying_key.verify("example", b"msg", &signature),
        Ok(())
    );
}

test_file

-----BEGIN OPENSSH PRIVATE KEY-----
b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAaAAAABNlY2RzYS
1zaGEyLW5pc3RwMjU2AAAACG5pc3RwMjU2AAAAQQTZOpGE5QtUNvCErd6TxmvcmEqjg9g2
88wzq6wr273kmsQkc68BsNz85O6KV83juigQgiESW6JKEDl2qgZm0R0YAAAAuO1qa9vtam
vbAAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBNk6kYTlC1Q28ISt
3pPGa9yYSqOD2DbzzDOrrCvbveSaxCRzrwGw3Pzk7opXzeO6KBCCIRJbokoQOXaqBmbRHR
gAAAAhANoZUYbsXL333OcaSafXjQ1dtI29JwZY3ckCJCPM3/7WAAAAHXdpbGxpYW1kZXMr
ZWNkc2EtMjAyM0B3ZGVzLmZyAQI=
-----END OPENSSH PRIVATE KEY-----

test_file.pub

ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBNk6kYTlC1Q28ISt3pPGa9yYSqOD2DbzzDOrrCvbveSaxCRzrwGw3Pzk7opXzeO6KBCCIRJbokoQOXaqBmbRHRg= williamdes+ecdsa-2023@wdes.fr

I tried versions: 0.5.1 and 0.6.0-pre.0

Rust: 1.67.1
Debian: 12 (bookworm)

[williamdes]

And it's also in when trying to verify signatures

    let signature =
r#"-----BEGIN SSH SIGNATURE-----
....
-----END SSH SIGNATURE-----
"#.parse::<SshSig>().expect("Signature to be valid");

    let public_key = "ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBKlKFBOvaQUdMe3lqGz7GR7EA8fXwjTfdADvx2cmTioInNMsjC4c+Gf8wj5mMysDD524CvDlU8xFsPpbgXvPc0A= williamdes+ecdsa-2023@wdes.fr".parse::<PublicKey>().expect("Key to decode");

    match public_key.verify("example", include_bytes!("../README.md"), &signature) {
        Ok(data) => {
            println!("Valid: {:?}", data);
        },
        Err(err) => {
            eprintln!("Err: {}", err);
        }
    }

[tarcieri]

Your sign_rsa test should probably be named sign_ecdsa.

The individual algorithms are feature-gated per this table (which could perhaps be better described):

https://docs.rs/ssh-key/latest/ssh_key/#supported-algorithms

So you need to do the following in Cargo.toml:

[dependencies]
ssh-key = { version = "0.5", features = ["p256"] }

...then your example will work.

[williamdes]

Your sign_rsa test should probably be named sign_ecdsa.

Fixed, bad copy paste.

features = ["p256"]

Thank you !
One full after-noon for that 😄

Can you re-open this issue and add a bit more context for other users or this crate ?
Maybe changing a bit the struct ?

[tarcieri]

I can add some additional documentation and improve the errors.

If you have a concrete proposal to make, please open a new issue.

[williamdes]

I can add some additional documentation and improve the errors.

If you have a concrete proposal to make, please open a new issue.

Thank you, this will help.

In my implementation if I give a wrong message it also says Err: cryptographic error. Having a way to know it's not a missing crate feature or other stuff but just the message that is not valid would be awesome.

[tarcieri]

Opened #81

[williamdes]

Opened #81

Very cool, what get's me is that Error::Crypto should have been in the diff of this PR or something else is wrong since I never had Error::Algorithm printed to my screen 🤔

[tarcieri]

I'm guessing it was happening by way of signature::Error.

Propagating the error as AlgorithmUnsupported might take a bit of work.

[williamdes]

I'm guessing it was happening by way of signature::Error.

Propagating the error as AlgorithmUnsupported might take a bit of work.

Okay, I'm up for code reviews and testing
Currently building this for my amazon lambda to validate some input

[tarcieri]

#82 should hopefully preserve the error contexts

[tarcieri]

I confirmed #82 propagates the algorithm in your code example above:

thread 'sign_rsa' panicked at 'called `Result::unwrap()` on an `Err` value: AlgorithmUnsupported { algorithm: Ecdsa { curve: NistP256 } }', src/lib.rs:11:10

[williamdes]

I confirmed #82 propagates the algorithm in your code example above:

thread 'sign_rsa' panicked at 'called `Result::unwrap()` on an `Err` value: AlgorithmUnsupported { algorithm: Ecdsa { curve: NistP256 } }', src/lib.rs:11:10

Awesome !