eclipse-tractusx / SSI-agent-lib

Apache License 2.0
4 stars 14 forks source link

`JWSProofSigner` produces wrong signatures #43

Open matgnt opened 10 months ago

matgnt commented 10 months ago

This one here literally took me days to find out ;-)

To my understanding so far, this affects all signatures created by MIW.

At the end, it is very much related to #32 but as stated there, it not only is an issue for external libraries, but it leads to a situation, that the signatures produced in the JWSProofSigner are wrong as well.

To summarize the problem, I think this describes it very well: https://datatracker.ietf.org/doc/html/rfc7797#section-3

When we use b64: false, the payload part is NOT base64 urlencoded! And according to https://w3c.github.io/vc-jws-2020/#json-web-signature-2020 this is the case for JsonWebSignature2020.

But since the b64 header is not only NOT in the header, but also NOT set when creating the JOSE Object, the libary DOES base64 the payload. This leads to non-verifiable signatures.

I think a good starting point could be the Test Suite VCs: https://github.com/decentralized-identity/JWS-Test-Suite/tree/main The created VCs from there, helped my a lot to compare with the VCs I downloaded from MIW for my analysis.

So far, this issue did not pop up, because the only entity who verified those wrong signatures was the signer itself.

-- Matthias Binzer

koptan commented 10 months ago

@borisrizov-zf, please assign it to me.

borisrizov-zf commented 6 months ago

@matgnt I've reviewed the PR, and I'm not fully convinced that this issue actually applies. We're not using the detached JWS, but the normal JWS. This indeed requires us to base64url encode the content, which is what we're doing. Please elaborate as to why detached JWS would apply to our use-case?

matgnt commented 6 months ago

Hi @borisrizov-zf

after 4 months and not working on that topic anymore, I'll try ... :-)

I just fetched a VC from latest MIW INT a second ago and the proof looks liket this:

        "proof": {
            "proofPurpose": "assertionMethod",
            "verificationMethod": "did:web:managed-identity-wallets-new.int.demo.catena-x.net:BPNL00000003CRHK#049f920c-e702-4e36-9b01-540423788a90",
            "type": "JsonWebSignature2020",
            "created": "2024-01-23T06:00:15Z",
            "jws": "eyJhbGciOiJFZERTQSJ9..HqCu-2Wm8yIOEfv1sJcm1FYrRTJaILHiwnaO8gx1dEn-0PvcEiu4ukPATFoB6ZCVlj16P7EpXPySwEXfl8sZAQ"
        },

From the running instance of MIW, it seems this way of signing is still used. The jws field is detached mode. Examples here https://datatracker.ietf.org/doc/html/rfc7797#section-4.1 and there https://datatracker.ietf.org/doc/html/rfc7797#section-4.2

and in consequence, the table here applies https://datatracker.ietf.org/doc/html/rfc7797#section-3

As mentioned, I think a required test for the library would be to test with some other library created VCs. An example can be found here: https://github.com/decentralized-identity/JWS-Test-Suite/tree/main

Does this help?

borisrizov-zf commented 6 months ago

@matgnt Yes, now I see how you've come to this. I'll double-check on the current develop version, as we've updated quite a lot of components. If this applies, we'll go ahead and work on a fix. Thanks again for your input!

borisrizov-zf commented 6 months ago

@matgnt Ok, I've gone through all of the material and code. There is no doubt that the requirements from https://w3c.github.io/vc-jws-2020/#json-web-signature-2020 are not met. Koptan's fix will indeed remedy this. We'll add a test to ensure validity.