spruceid / ssi

Core library for decentralized identity.
https://spruceid.dev
Apache License 2.0
180 stars 54 forks source link

Credential/Presentation verify fails if serde_json feature arbitrary_precision is enabled. #528

Open vdods opened 11 months ago

vdods commented 11 months ago

I'm currently digging into the precise cause, but it seems like the something in Credential/Presentation's impl of to_dataset_for_signing is not correctly forming the dataset when this arbitrary_precision feature is enabled in the serde_json crate. The arbitrary_precision feature causes numbers to be stored as strings, so perhaps to_dataset_for_signing is incorrectly canonicalizing numbers as strings.

The specific error returned by Credential::verify / Presentation::verify is "Verification equation was not satisfied" if ed25519-dalek is used, and "ring::error::Unspecified" if ring is used.

vdods commented 11 months ago

Ok, it looks like the call let json = ssi_json_ld::syntax::to_value_with(copy, Default::default).unwrap(); in Credential::to_dataset_for_signing is where it goes wrong.

Comparison of the json variable when serde_json/arbitrary_precision feature is not enabled and when it is enabled: When it is not enabled (irrelevant detail truncated):

...
Meta(
    "chainId",
    Span {
        start: 0,
        end: 0,
    },
): Meta(
    Number(
        "5",
    ),
    Span {
        start: 0,
        end: 0,
    },
),
...

When it is enabled:

...
Meta(
    "chainId",
    Span {
        start: 0,
        end: 0,
    },
): Meta(
    Object(
        {
            Meta(
                "$serde_json::private::Number",
                Span {
                    start: 0,
                    end: 0,
                },
            ): Meta(
                String(
                    "5",
                ),
                Span {
                    start: 0,
                    end: 0,
                },
            ),
        },
    ),
),
...

So it looks like because ssi_json_ld::syntax::to_value_with calls serde::Serialize, it's inheriting the problem described here. I'm not very familiar with how serde Serializers are implemented, but perhaps it's possible to somehow specify special behavior for "$serde_json::private::Number".

Unfortunately I'm not able to disable the arbitrary_precision feature, since it's buried my dependency on the ethers-rs crate. But ideally that crate would avoid using the arbitrary_precision flag.

Related: