Some serializers fail to serialize/deserialize `VerifyingKey`s & `Signature`s correctly (k256)

[BGluth]

It seems that some Serde serializers have issues when serializing VerifyingKeys and Signatures. They all serialize fine, but Serde returns an error for certain serializers when deserializing back to the original type.

I've tested it with three serializers, and it seems to be a bit of a hit or miss if it works. It's a bit verbose, but I've included tests that reproduce the issue. I would normally just include a Rust Playground link, but they don't support k256.

#[cfg(test)]
mod test {
    use k256::ecdsa::{Signature, SigningKey, signature::Signer};
    use rand::rngs::OsRng;
    use serde::{Deserialize, Serialize};
    use std::fmt::Debug;

    fn ser_deser_test<'a, D, S, T, U>(ser: S, deser: D, val: T)
    where
        D: Fn(&U) -> Result<T, String>,
        S: Fn(&T) -> Result<U, String>,
        T: Debug + Deserialize<'a> + PartialEq + Serialize,
    {
        let ser = ser(&val).unwrap();
        let deser = deser(&ser).expect("Deserialization failure");

        assert_eq!(val, deser);
    }

    fn create_fake_sig() -> Signature {
        let k = SigningKey::random(&mut OsRng);
        let msg: Vec<_> = (0..100).collect();

        k.sign(&msg)
    }

    #[test]
    fn json_signing_key() {
        ser_deser_test(
            |val| serde_json::to_string(val).map_err(|err| err.to_string()),
            |str| serde_json::from_str(str).map_err(|err| err.to_string()),
            SigningKey::random(&mut OsRng).verifying_key(),
        );
    }

    #[test]
    fn json_signature() {
        ser_deser_test(
            |val| serde_json::to_string(val).map_err(|err| err.to_string()),
            |str| serde_json::from_str(str).map_err(|err| err.to_string()),
            create_fake_sig(),
        );
    }

    #[test]
    fn toml_signing_key() {
        ser_deser_test(
            |val| toml::to_string(val).map_err(|err| err.to_string()),
            |str| toml::from_str(str).map_err(|err| err.to_string()),
            SigningKey::random(&mut OsRng).verifying_key(),
        );
    }

    #[test]
    fn toml_signature() {
        ser_deser_test(
            |val| toml::to_string(val).map_err(|err| err.to_string()),
            |str| toml::from_str(str).map_err(|err| err.to_string()),
            create_fake_sig(),
        );
    }

    #[test]
    fn bincode_signing_key() {
        ser_deser_test(
            |val| bincode::serialize(val).map_err(|err| err.to_string()),
            |bytes| bincode::deserialize(bytes).map_err(|err| err.to_string()),
            SigningKey::random(&mut OsRng).verifying_key(),
        );
    }

    #[test]
    fn bincode_signature() {
        ser_deser_test(
            |val| bincode::serialize(val).map_err(|err| err.to_string()),
            |bytes| bincode::deserialize(bytes).map_err(|err| err.to_string()),
            create_fake_sig(),
        );
    }
}

Dependencies:

[dependencies]
k256 = {version = "0.10.4", features = ["serde", "pem"] }
bincode = "1.3.3"
serde_json = "1.0.59"
rand = {version = "0.8.5", features = ["getrandom"] }
toml = "0.5.8"
serde = "1.0"
serde_yaml = "0.8"

With results:

test test::bincode_signing_key ... ok
test test::json_signature ... ok
test test::bincode_signature ... ok
test test::json_signing_key ... FAILED ("panicked at 'Deserialization failure: invalid type: sequence, expected a borrowed byte array at line 1 column 1")
test test::toml_signing_key ... FAILED ("panicked at 'Deserialization failure: "expected a right bracket, found a comma at line 1 column 4")
test test::toml_signature ... FAILED ("panicked at 'Deserialization failure: "expected an equals, found eof at line 1 column 131")

I'm not very experienced with cryptography, so let me know if I'm doing something that I shouldn't be. :slightly_smiling_face:

[tarcieri]

I'm guessing that the issue is JSON and TOML need toplevel objects/tables for a document to be valid. So you'll need to put the key/signature to be serialized in some kind of wrapper, e.g.

let signature = ...;
json! { signature }

[BGluth]

Hmm yeah good thought, but adding the json! macro doesn't seem to change the output in this case:

let val = serde_json::to_string(&SigningKey::random(&mut OsRng).verifying_key()).unwrap();

println!("No macro: {}", val);
println!("macro:    {}", json! { val });

No macro: [48,86,48,16,6,7,42,134,72,206,61,2,1,6,5,43,129,4,0,10,3,66,0,4,151,186,209,214,82,234,77,92,223,222,77,182,58,126,76,14,210,40,246,84,246,31,230,123,226,176,86,8,212,12,34,89,205,44,227,187,169,45,191,199,156,171,190,66,116,19,133,33,107,206,96,64,211,234,77,10,64,148,237,193,199,7,198,178]
macro:    "[48,86,48,16,6,7,42,134,72,206,61,2,1,6,5,43,129,4,0,10,3,66,0,4,151,186,209,214,82,234,77,92,223,222,77,182,58,126,76,14,210,40,246,84,246,31,230,123,226,176,86,8,212,12,34,89,205,44,227,187,169,45,191,199,156,171,190,66,116,19,133,33,107,206,96,64,211,234,77,10,64,148,237,193,199,7,198,178]"

As a sanity check, all three serializers seem ok if I give them some arbitrary type:

#[derive(Clone, Copy, Deserialize, Serialize)]
struct SomeStruct {
    x: f32,
    y: String
}    

#[test]
fn general_ser_deser_test() {
    let some_struct = SomeStruct {
        x: 4.2,
        y: "cool string",
    };

    ser_deser_test(
        |val| serde_json::to_string(val).map_err(|err| err.to_string()),
        |str| serde_json::from_str(str).map_err(|err| err.to_string()),
        some_struct,
    );

    ser_deser_test(
        |val| toml::to_string(val).map_err(|err| err.to_string()),
        |str| toml::from_str(str).map_err(|err| err.to_string()),
        some_struct,
    );

    ser_deser_test(
        |val| bincode::serialize(val).map_err(|err| err.to_string()),
        |bytes| bincode::deserialize(bytes).map_err(|err| err.to_string()),
        some_struct,
    );
}

[tarcieri]

You'll need to construct the outer JSON/TOML object before serializing as a string.

[BGluth]

I think it's possible that I may be misunderstanding, but if the failure was caused by the outer JSON/TOML missing, then shouldn't the serialization tests of SomeStruct be failing?

[tarcieri]

A struct maps directly to a JSON object or TOML table. So there isn't an issue there.

Can you try adding a json! wrapper which places the value inside a JSON object and confirm that fixes the issue?

[BGluth]

Yeah I'm still getting a failure on deserialization:

#[test]
fn json_signing_key() {
    let k = SigningKey::random(&mut OsRng).verifying_key();

    let ser = &json!{ k }.to_string();
    let deser = serde_json::from_str(ser).expect("Deserialization failure");

    assert_eq!(k, deser);
}

Deserialization failure: Error("invalid type: sequence, expected a borrowed byte array", line: 1, column: 1)

[tarcieri]

Now you have the problem that you are trying to deserialize a SigningKey, but the JSON document contains a toplevel object with a single key k.

Try defining a struct to put the SigningKey in and serializing/deserializing that.

[BGluth]

Thanks for the quick responses tarcieri.

It's unfortunately still failing, however it's failing when it tries to parse the value for the field of the struct, so I don't think it's related to something missing at the top of the JSON payload:

#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
struct SomeStruct {
    k: VerifyingKey,
}

#[test]
fn json_signing_key() {
    let some_struct = SomeStruct {
        k: SigningKey::random(&mut OsRng).verifying_key(),
    };

    let ser = serde_json::to_string(&some_struct).unwrap();
    let deser = serde_json::from_str(&ser).expect("Deserialization failure");

    assert_eq!(some_struct, deser);
}

'Deserialization failure: Error("invalid type: sequence, expected a borrowed byte array", line: 1, column: 6)'

With the actual serialized payload, column 6 is the start of the value for the field x:

{"k":[48,86,48, ...

[tarcieri]

Aah, that does appear to be a legitimate issue with the deserializer when used with a text-based format. It's currently decoding to a byte slice, which works with binary formats but not with text-based ones.

That's easy enough to fix by changing it to a Vec<u8>.

When encoding in JSON, there's also JWK as an interoperable format: https://docs.rs/elliptic-curve/latest/elliptic_curve/struct.JwkEcKey.html

[tarcieri]

So I elected to do what most of the other types in elliptic-curve and its downstream dependencies do: conditionally serialize the data as a hex string when the format is_human_readable:

https://github.com/RustCrypto/traits/pull/967

Unfortunately in the current form it's a bit hard to test.

I'd really like to extract this serialization logic into a separate crate which can have the sort of tests you're doing in this issue against a number of different encoding formats so we can solve these serialization problems once and for all in a single place.