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

Less common hashing algorithm for the message in Ed25519Signature2018 #21

Closed
kdenhartog opened this issue Apr 1, 2020 · 12 comments
Closed
Assignees
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@kdenhartog
Copy link
Contributor

kdenhartog commented Apr 1, 2020

In Ed25519Signature2018 it's defined that SHA256 should be used as the prehash function of the message. However, FIPS 186-5 and in OpenSSL it's defined that SHA512 should be used for this prehash function.

I propose that we define Ed25519Signature2020 to modify the algorithm and prepare to deprecate Ed25519Signature2018

RFC8032, did not specify this (it provided suggestions through example only) which is how the ambiguity occurred.

@dlongley @msporny @mikelodder7 thoughts?

@kdenhartog kdenhartog added bug Something isn't working help wanted Extra attention is needed labels Apr 1, 2020
@kdenhartog kdenhartog changed the title Incorrect hashing algorithm for Ed25519Signature2018 Less common hashing algorithm for the message in Ed25519Signature2018 Apr 1, 2020
@mikelodder7
Copy link

mikelodder7 commented Apr 1, 2020

I agree. However, as long as its just a prehash, we're not losing security by any means. Its just requiring an extra step. Libsodium, for example, will compute the SHA512 of the SHA256 hash. So its an unnecessary computation.

@msporny
Copy link
Collaborator

msporny commented Apr 1, 2020

I propose that we define Ed25519Signature2020 to modify the algorithm and prepare to deprecate Ed25519Signature2018

+1 to this. Yes, this came up the other day. There is a desire to do a few minor corrections to this cryptography suite in the next few months, namely:

  • Changes like the one you mentioned.
  • Removal of jws and replacement w/ proofValue. The JWS stuff was an experiment to see if we could unify the JOSE community and the Linked Data Security community. The attempt has failed for this cryptosuite, but the work is being carried on in JwsSignature2020... so we'd like to push all JOSE stuff to that cryptography suite and keep the Ed25519 stuff simpler and more auditable.

The problem is that Ed25519Signature2018 is deployed in production, so changing it at this point is probably not a good idea. That said, this is exactly what the cryptosuites were designed to do... so let's put some effort into Ed25519Signature2020 and fix the things we've learned from over the past several years. Nothing breaks in the ecosystem and we're left with something that all of us like a little bit more.

@dlongley
Copy link
Contributor

dlongley commented Apr 1, 2020

Note that Ed25519Signature2018 does use a SHA512 in the way @mikelodder7 noted. The linked data signature message works like this:

message = SHA256(canonicalized data) + SHA256(canonicalized proof meta data)
// note that this message is 64 bytes long

This is currently consistent across all the modern linked data signature types (RSA, ed25519, secp256k1, secp256r1, etc.) Then that "message" is signed using whatever signing algorithm, which typically includes hashing of the "message". So Ed25519Signature2018 requires that libraries internally SHA512(message) and then sign -- and this is what implementations do.

@kdenhartog
Copy link
Contributor Author

Note that Ed25519Signature2018 does use a SHA512 in the way @mikelodder7 noted. The linked data signature message works like this:

message = SHA256(canonicalized data) + SHA256(canonicalized proof meta data)
// note that this message is 64 bytes long

This is currently consistent across all the modern linked data signature types (RSA, ed25519, secp256k1, secp256r1, etc.) Then that "message" is signed using whatever signing algorithm, which typically includes hashing of the "message". So Ed25519Signature2018 requires that libraries internally SHA512(message) and then sign -- and this is what implementations do.

Ahh then I would think all we need to do then is update the language in Ed25519Signature2018 to make that a bit more clear and start 2020 to make some of the changes proposed by @msporny. I also recently proposed a change over email to Manu around the layering of signature suites and keys. I'd like to discuss that more broadly as well with the community and potentially start using that layering in the 2020 version.

@peacekeeper
Copy link
Member

I was also under the impression that Ed25519Signature2018 already uses SHA512. I wrote this crypto suite specification a while ago:

https://w3c-ccg.github.io/lds-ed25519-2018/#the-ed25519-2018-signature-suite

So it appears the language in https://w3c-ccg.github.io/security-vocab/#Ed25519Signature2018 is wrong.

@OR13
Copy link
Collaborator

OR13 commented Apr 3, 2020

I have removed the dupe, but looking around, it seems like the js implementation of Ed25519Signature2018 use sha-256... https://github.com/digitalbazaar/jsonld-signatures/blob/master/lib/suites/LinkedDataSignature.js#L223

This createVerifyData goes into the detached jws, here: https://github.com/transmute-industries/lds-jws2020/blob/master/src/__tests__/detached-jws.spec.js

as mike pointed out, jws is gonna hash it again anyway... but the algorithm used in createVerifyData needs to be specified correctly.... for example, we use sha-256 for https://lds.jsld.org/contexts/#suite-details

Our suite extends "LinkedDataSignature" so we inherit createVerifyData from DBs implementation.

Previous signature suites, we implemented createVerifyData manually:

https://github.com/transmute-industries/Ed25519Signature2018/blob/master/src/createVerifyData/index.js#L9

signatures will not interop if a different hash algorithm is used for createVerifyData.

This is why things like: https://tools.ietf.org/html/rfc7520

are useful...

how do we resolve this issue? we we update the crypto suite registry to say sha-256?

@peacekeeper
Copy link
Member

it seems like the js implementation of Ed25519Signature2018 use sha-256

how do we resolve this issue? we we update the crypto suite registry to say sha-256?

I think @dlongley 's earlier comment answers this. There is SHA-256 hashing during the RDF normalization step which creates the "input message", and then there's SHA-512 hashing during the signing step.

What I'm not sure about is which one of the two is considered the "message digest algorithm" according to the LD Proofs spec...

@OR13
Copy link
Collaborator

OR13 commented Apr 3, 2020

its the one that is used in createVerifyData... the one I linked above... the one used during the signature is the one defined by the JWS alg (https://tools.ietf.org/html/rfc8032 --- its sha512 afaik),

For example, here is sha256 in mastodon: https://github.com/tootsuite/mastodon/blob/cabdbb7f9c1df8007749d07a2e186bb3ad35f62b/app/lib/activitypub/linked_data_signature.rb#L30

Here is an ancient signature suite test i wrote for verifying ActivityPub stuff: https://github.com/transmute-industries/RsaSignature2017/blob/master/src/__tests__/Mastodon.Integration.spec.js

Here is the createVerifyData function: https://github.com/transmute-industries/RsaSignature2017/blob/master/src/common.js#L14

@OR13
Copy link
Collaborator

OR13 commented Apr 3, 2020

Can I close this issue now?

@OR13
Copy link
Collaborator

OR13 commented May 3, 2020

I'm closing this, as its not actionable. If you are interested in defining Ed25519Signature2020, please create a spec and get to work :) feel free to link back to this issue if you desire.

@OR13 OR13 closed this as completed May 3, 2020
@kdenhartog
Copy link
Contributor Author

Sorry I didn't get notified when you assigned this to me. I'm satisfied with the resulting discussion and agree the action would be to define a new Ed25519Signature2020, but want to continue discussion on two other topic first. Namely, whether we should be decoupling key representations (base58, jwk, etc) from the signing algorithm (EdDSA, EcDSA, etc). Additionally, how should we be categorizing signing algorithms? Should we classify it as EdDSASignature2020 and support the whole lot of the signing algorithms or just stick to the pattern we've followed.

I'm generally of the opinion that we should decouple key representation and keep the classification of signing algorithms as is. I'll open a separate issue to discuss this further though.

@OR13
Copy link
Collaborator

OR13 commented May 4, 2020

@kdenhartog you may also want to comment here: #32

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

6 participants