Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
vdods opened this issue Aug 16, 2023 · 2 comments

Comments

@vdods
Copy link
Contributor

vdods commented Aug 16, 2023

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
Copy link
Contributor Author

vdods commented Aug 16, 2023

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:

@timothee-haudebourg
Copy link
Contributor

This issue should be resolved with the new refactor (version 0.8).

  • The to_dataset_for_signing has been replaced
  • ssi_json_ld::syntax is based on the json-syntax crate which is compatible with how serde_json serializes arbitrary precision numbers.

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

No branches or pull requests

2 participants