mattrglobal / pairing_crypto

A library for pairing based cryptography
Apache License 2.0
10 stars 3 forks source link

Key generation only gives serialized key, not x/y point. #185

Open dwaite opened 4 months ago

dwaite commented 4 months ago

The JWK representation described at https://www.ietf.org/archive/id/draft-ietf-cose-bls-key-representations-04.html , no longer uses OKP based on the serialization in pairing-friendly-curves, but x/y coordinates. This is probably best solved by having a Key type, possibly with a PrivateKey type, to represent public and private keys. That way this/these type(s) can handle serialization and initialization from various representations, and can be supplied inside things like the BbsSignRequest object.

BasileiosKal commented 4 months ago

Hey David! Thank you for raising this issue.

I agree that we should be more agnostic to the representation of public keys. That said, we designed the API to deal with byte arrays as much as possible. Couple of options come to mind,

Update the pub key input description to something like,

/// Public key
pub public_key: [u8; <COMPRESSED_PK_LEN>] || [u8; <UNCOMPRESSED_PK_LEN>],

Go the node crypto route and allow both byte arrays and key objects (plus possibly strings, points etc). So something like,

/// Public key
pub public_key: KeyObject || [u8; <COMPRESSED_PK_LEN>] || [u8; <UNCOMPRESSED_PK_LEN>],

Do let me know if you have a preference between the two. Personally, I'm leaning towards the first one at the moment, since I'm not clear on the advantage of the second.

cc @tplooker and @selfissued

dwaite commented 4 months ago

I would generally recommend a more OO approach - have a type in the rust library which privately holds the most appropriate format - and have generators, constructors and accessors which will internally operate based on that type.

That also allows you to say have a constructor reject invalid x/y coordinates outright, which then makes this a precondition you can rely on.

It seems if you accept a union of types, you'll need this object internally anyway.

However, I can understand not wanting to bridge another type based on your wide array of language bindings.

BasileiosKal commented 4 months ago

That also allows you to say have a constructor reject invalid x/y coordinates outright, which then makes this a precondition you can rely on.

I would note that we perform this checks internally either way

However, I can understand not wanting to bridge another type based on your wide array of language bindings.

This is one of my concerns yes. Although I agree that the proposal will be useful, It's hard to justify the required work given that it is not necessary at the moment (since we only support a single curve).

I think the best course at the moment could be a utility object/functions to handle the necessary transformations. Would that be acceptable??

selfissued commented 3 months ago

Given that the key representation at https://datatracker.ietf.org/doc/draft-ietf-cose-bls-key-representations/ uses (x, y) pairs, I believe we need to address this.

cc: @tplooker @BasileiosKal

selfissued commented 2 months ago

@BasileiosKal and I spoke about this today. He's going to implement utility functions that convert between the key representations.