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

RsaSignature2018 v1 spec definitions #1151

Closed
fabrii opened this issue Jun 13, 2023 · 12 comments
Closed

RsaSignature2018 v1 spec definitions #1151

fabrii opened this issue Jun 13, 2023 · 12 comments
Labels
pending close Close if no objection within 7 days

Comments

@fabrii
Copy link

fabrii commented Jun 13, 2023

Opening a new issue to keep track of this.

In #778 we can see that there is an error with the RsaSignature2018 spec. The issue was closed without specific actions taken.

Two libraries (vc.js, and verifiable-credentials-java), have implemented the RSA canonicalization following different approaches. For that reason, the canonicalization produced does not match, and neither does the signature.

I believe that a consensus/workaround should be made explicit in the spec, so we can have interoperability across all implementations.

@brentzundel
Copy link
Member

brentzundel commented Jun 13, 2023

The VCWG is not defining (and has never defined) an RsaSignature2018 spec. Such a thing does not fall under our purview.
We have done what we can, by removing mention of RsaSignature2018 from all examples and from the @context of the VC Data Model 2.0.

I recommend this issue be raised wherever RsaSignature2018 was defined (perhaps the CCG?), and closed here.

@brentzundel brentzundel added the pending close Close if no objection within 7 days label Jun 26, 2023
@brentzundel
Copy link
Member

No response. Marking as pending close. Will close in 7 days if there are no objections.

@fabrii
Copy link
Author

fabrii commented Jun 26, 2023

In #778 (comment) it's mentioned that the error is at the VC JSON-LD Context. This context is published by w3c.

If there is any other better place where I should open an issue about this, please share a link.

@brentzundel
Copy link
Member

The @context file for v2 of the VC Data Model is here: https://github.com/w3c/vc-data-model/blob/main/contexts/credentials/v2

It no longer has a reference to RsaSignature2018.

Are you proposing that we make changes to the published v1 @context?

FYI @msporny

@msporny
Copy link
Member

msporny commented Jun 26, 2023

@fabrii -- what would be the ideal outcome to your concern? I understand that you are using two implementations that produce different signatures for a particular input value. Can you provide an example of what is being signed?

The VCWG currently has no plans to support RSA, as most modern systems are suggesting a move away from RSA to at least Elliptic Curve cryptography. Can you elaborate on why you are unable to use Elliptic Curves for what you have implemented? It is possible to create an RSA cryptosuite for the new Data Integrity approach, but as I said above, guidance today is to not use RSA w/ new systems that are being built.

Some detail on the above would help us try to address your concern.

@fabrii
Copy link
Author

fabrii commented Jun 26, 2023

Changing v1 could break some working implementations. It is also static loaded in a lot of libraries.
As a less dangerous move, I am just hoping for the w3c to pronounce about how this case should be handled by the different implementations.

Do they have to canonicalize like option (1) or option (2):

(1) vc.js

_:c14n0 <http://purl.org/dc/terms/created> "2023-06-03T20:00:01Z"^^<xsd:dateTime> .
_:c14n0 <http://www.w3.org/1999/02/22-rdf-syntax-ns#type> <https://w3id.org/security#RsaSignature2018> .
_:c14n0 <sec:proofPurpose> <https://w3id.org/security#assertionMethod> .
_:c14n0 <sec:verificationMethod> <urn:oid:2.16.858.0.0.0.3.0#1> .

(2) verifiable-credentials-java

_:c14n0 <http://purl.org/dc/terms/created> "2023-06-03T20:00:01Z"^^<http://www.w3.org/2001/XMLSchema#dateTime> .
_:c14n0 <http://www.w3.org/1999/02/22-rdf-syntax-ns#type> <https://w3id.org/security#RsaSignature2018> .
_:c14n0 <https://w3id.org/security#proofPurpose> <https://w3id.org/security#assertionMethod> .
_:c14n0 <https://w3id.org/security#verificationMethod> <urn:oid:2.16.858.0.0.0.3.0#1> .

If an official statement is made, I can make the necessary changes in one of the two libraries, to canonicalize according to the spec. If this discussion is let open, I can follow the (1) path, and another organization might follow the (2) one. In that case we won't be able to correctly validate our credentials.

We are working with a PKI that relies on RSA. Work is being done to support Elliptic Curve cryptography but it will take at least half a year

@OR13
Copy link
Contributor

OR13 commented Jun 26, 2023

Top N-Quads are incorrect, sec: prefix should have been expanded, this issue would be better raised on the data integrity repo or a specific vendor implementation.

Too bad the working group withdrew JSON Web Signature 2020, it supported RSA...

@msporny
Copy link
Member

msporny commented Jun 27, 2023

If an official statement is made, I can make the necessary changes in one of the two libraries, to canonicalize according to the spec.

As @OR13 said, the top n-quads are clearly wrong.

I'd raise this as an issue on the vc.js library and we can take it from there: https://github.com/digitalbazaar/vc/issues

When you raise the issue, please include a complete example, the version of the library that you are using, and any other things that you might be doing (such as using a custom document loader).

In any case, this is the wrong venue for the discussion of this topic since it covers a topic that is not in scope for the VCWG in our current charter.

@msporny
Copy link
Member

msporny commented Jun 27, 2023

@OR13 wrote:

Too bad the working group withdrew JSON Web Signature 2020, it supported RSA...

To be clear, you, as the primary Editor of that specification, made a choice to withdraw JSON Web Signature 2020... and the WG agreed with you. What you wrote above makes it sound like it was a decision that was not initiated by you, which it was.

@fabrii -- if you feel like an RSA Data Integrity cryptography suite is a MUST HAVE for you, my suggestion is to create a work item in the W3C Credentials Community group that is modelled after this specification: https://w3c.github.io/vc-di-eddsa/

We won't be able to include that work item in this iteration of the Working Group, but if you get enough implemener support for it, it is possible to include it in a future WG. I will warn you, though, that there is increasing push back on implementing RSA for anything other than legacy systems since it's so easy to get a number of the parameters wrong, which leads to security vulnerabilities. That said, it's up to you to figure out what makes the most sense to you.

@OR13
Copy link
Contributor

OR13 commented Jun 27, 2023

@msporny yes, I withdrew the work item, based on the understanding that the new DataIntegrityProof and cryptosuite approach would be sufficient for implementers, and that others would be doing that work.

It seems like you are getting a feature request for an RSA cryptosuite... We are not planning to offer support for this moving forward.

@fabrii
Copy link
Author

fabrii commented Jun 27, 2023

Thank you for the responses. With this feedback, I will proceed to open an issue in the vc.js library. The correct approach would be to expand the N-Quads. I opened it first in digitalbazaar/jsonld.js#524, but this is not a problem with the jsonld library. This should be handled at vc.js level, as a "fix" for this specific case.

Regarding a new RSA Data Integrity suite, we hope we won't need it next year, so for now I'm okay with this solution.

Thanks a lot

EDIT:

Opened the issue on jsonld-signatures, as this is the place where the canonicalization is handled.

@brentzundel
Copy link
Member

Issue has been addressed, closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending close Close if no objection within 7 days
Projects
None yet
Development

No branches or pull requests

4 participants