paseto-standard / test-vectors

Test vectors for PASETO, PASERK, etc.
Other
6 stars 4 forks source link

Fix PASERK secret for Ed25519. #5

Closed dajiaji closed 2 years ago

dajiaji commented 2 years ago

Hi @paragonie-security

As I understand it, the private key of Ed25519 is 32 bytes and should not contain any seed information. However, in the test vectors, the private key (the "key" attribute) seems to be concatenated with the seed information and the PASERK data in the test vectors are based on the concatenated key and that seems wrong to me.

It would be great if you could check this PR. It is possible that I am making a big mistake, if so, please let me know.

Thank you in advance.

paragonie-security commented 2 years ago

As I understand it, the private key of Ed25519 is 32 bytes and should not contain any seed information.

The Ed25519 private key is 64 bytes, because its format is clamped_seed + public_key. See https://nacl.cr.yp.to/sign.html

However, in the test vectors, the private key (the "key" attribute) seems to be concatenated with the seed information and the PASERK data in the test vectors are based on the concatenated key and that seems wrong to me.

That's not seed information, it's the public key.

dajiaji commented 2 years ago

The Ed25519 private key is 64 bytes, because its format is clamped_seed + public_key. See https://nacl.cr.yp.to/sign.html

Hmm, RFC8032 section-5.1.5 says that "The private key is 32 octets". And for example, The pyca/cryptography that I use also says that the private key is 32 bytes. As you know, d-value (private key) of Ed25519 JWK is 32 byte long.

That's not seed information, it's the public key.

I don't understand why the private key should include the public key, since the public key can be derived from the 32-byte d-value (the private key in my definition), no matter how the private key is defined. It seems redundant.

paragonie-security commented 2 years ago

See #6

dajiaji commented 2 years ago

See #6

Thanks. It's very useful to me.

Anyway, 64-bit private key is dependent on libsodium implementation. It seems to me that it's not Platform Agnostic. I think k*.secret should not include public key, should be 32-byte long.

paragonie-security commented 2 years ago

They're more platform agnostic than doing only 32 byte keys.

Here's how you work around this in platforms that only support 32-byte keys;

const sk = decoded.slice(0, 32);

But if you go the other way, you have to do an Edwards scalar multiplication of the base point to recover the public key.

paragonie-security commented 2 years ago

I'm going to rename seed to secret-key-seed and include the public-key for these test vectors. That way everyone has the information they need but you can write tests against either key or secret-key-seed directly.

dajiaji commented 2 years ago

but I'll follow the current specifications for now.

I've updated my implementation to support libsodium Ed25519 key format and to pass the tests and released it.

You can close this PR but at least, PASERK {k2, k4}.secret spec should be revised. I can send you a PR, but if possible, could you change it to look like the following?

--- The [data] portion will be the Ed25519 secret key as raw bytes.
+++ The [data] portion will be the concatenation of Ed25519 private key (32 bytes) and public key (32 bytes) as raw bytes.
paragonie-security commented 2 years ago

https://github.com/paseto-standard/paserk/commit/423e82d1123f0385248590458455f60b9eea0771