Add key expression formats for ed25519. #102
Conversation
msporny
requested review from
dlongley,
kdenhartog,
OR13,
selfissued,
tplooker and
peacekeeper
index.html
Outdated
| base58-btc format using the <code>publicKeyBase58</code> property or | ||
| MAY be encoded in JSON Web Key (JWK) format using the <code>publicKeyJwk</code> | ||
| property. Both the raw 32-byte base58-btc format and JWK format MUST be | ||
| supported by DID Document Processors. The use of other key formats for RSA MUST |
There was a problem hiding this comment.
This line has RSA instead of ed25519?
Similarly in the other PRs.
There was a problem hiding this comment.
This was a mistake, will fix.
index.html
Outdated
| <td> | ||
| ed25519 key values MAY be expressed as the raw 32-byte value encoded in | ||
| base58-btc format using the <code>publicKeyBase58</code> property or | ||
| MAY be encoded in JSON Web Key (JWK) format using the <code>publicKeyJwk</code> |
There was a problem hiding this comment.
One question here is ... does anyone really use JWK for ed25519? If not, we could eliminate it and just have the common base58 representation.
There was a problem hiding this comment.
Yes, use of JWKs for Ed25519 is common. That's why members of the community developed RFC 8037 - to define how to use the 25519 and 448 curves with JSON.
There was a problem hiding this comment.
I'm not sure if anyone is currently using JWKs as a representation right now for ed25519 keys, (legacy implementations appear to use base58) but I read this as implementers of future DID registries are encouraged to use JWKs rather than base58.
There was a problem hiding this comment.
@selfissued wrote:
Yes, use of JWKs for Ed25519 is common.
Do you have data or a citation for this assertion? I expect that's true in certain communities, but which ones, and which implementations?
There was a problem hiding this comment.
I'm aware of all these implementations:
- python: https://github.com/latchset/jwcrypto/
- node.js: https://github.com/panva/jose
- elixir: https://github.com/joken-elixir/joken
- go: https://github.com/pascaldekloe/jwt
- php: https://github.com/web-token/jwt-framework
I'm sure there are others...
There was a problem hiding this comment.
Please revise the wording along the lines of that suggested for RSA keys in #100 (review) to indicate that allowing for PublicKeyBase58 is an exception, and explicitly limiting the algorithms and curves that it may be used with.
|
Note: We also need one of these PRs for Curve25519 (X25519KeyAgreementKeys) -- I'd expect it to look pretty much the same as this one. |
index.html
Outdated
| @@ -1348,6 +1348,19 @@ <h2> | |||
| MAY be encoded in JSON Web Key (JWK) format using the <code>publicKeyJwk</code> | |||
| property. Both the raw 32-byte base58-btc format and JWK format MUST be | |||
| supported by DID Document Processors. The use of other key formats for RSA MUST | |||
There was a problem hiding this comment.
RSA mentioned on the end of this line too?
There was a problem hiding this comment.
This was a mistake, will fix.
index.html
Outdated
| </td> | ||
| <td> | ||
| ed25519 key values MAY be expressed as the raw 32-byte value encoded in | ||
| base58-btc format using the <code>publicKeyBase58</code> property or |
There was a problem hiding this comment.
While base58 was introduced by btc, I don't think there's any difference between base58 and base58-btc. Would be more clear to simply say base58 here IMO.
There was a problem hiding this comment.
There are two base58 encodings... Bitcoin's and Flickr's... We need to be specific about which one we're using (most use Bitcoin's base58 encoding scheme).
There was a problem hiding this comment.
Tried to clarify in 41b0b47.
index.html
Outdated
| ed25519 key values MAY be expressed as the raw 32-byte value encoded in | ||
| base58-btc format using the <code>publicKeyBase58</code> property or | ||
| MAY be encoded in JSON Web Key (JWK) format using the <code>publicKeyJwk</code> | ||
| property. Both the raw 32-byte base58-btc format and JWK format MUST be |
There was a problem hiding this comment.
same comment here. s/base58-btc/base58/
There was a problem hiding this comment.
There are two base58 encodings... Bitcoin's and Flickr's... We need to be specific about which one we're using (most use Bitcoin's base58 encoding scheme).
There was a problem hiding this comment.
Tried to clarify in 41b0b47.
msporny
force-pushed
the
msporny-issue-67-secp256k1
branch
from
150bcee to
f21dcd1
Compare
msporny
force-pushed
the
msporny-issue-67-ed25519
branch
from
419c8ec to
41b0b47
Compare
Updated language to reflect the most recent straw poll results in #67 (comment). |
index.html
Outdated
| <td> | ||
| Ed25519 public key values MUST be encoded in either JSON Web Key (JWK) | ||
| format using the <code>publicKeyJwk</code> property or as the raw 32-byte | ||
| public key value encoded in Base58 Bitcoin format using the |
There was a problem hiding this comment.
Each occurrence of Base58 Bitcoin format should be linked to (a reference to) the relevant spec/RFC/whatever.
There was a problem hiding this comment.
There was a problem hiding this comment.
This appears to be a better choice, albeit not a standard in the usual sense, as it actually says something -- https://en.bitcoin.it/wiki/Base58Check_encoding
There was a problem hiding this comment.
This was done in the msporny-issue-67 PR.
There was a problem hiding this comment.
BTW, Base58Check is not Base58 BTC -- Base58Check is a bad name and requires a funky double sha256 + checksum algorithm above and beyond base encoding. We are suggesting base58btc. That said, the issue notes that we haven't made a decision here yet, and still need to do so.
msporny
force-pushed
the
msporny-issue-67-secp256k1
branch
from
f21dcd1 to
9d76711
Compare
msporny
force-pushed
the
msporny-issue-67-ed25519
branch
from
41b0b47 to
1fce006
Compare
index.html
Outdated
| ed25519 | ||
| </td> | ||
| <td> | ||
| Ed25519 public key values MUST be encoded as the raw 32-byte public key value |
There was a problem hiding this comment.
Ed25519 public key values MUST be encoded as the raw 32-byte public key value
encoded in Base58 Bitcoin format using the <code>publicKeyBase58</code> property
if they are not encoded in JWK format.
->
Ed25519 public key values MUST be encoded in JSON Web Key (JWK) format using
the <code>publicKeyJwk</code> property or with the raw 32-byte public key
value encoded in Base58 Bitcoin format using the <code>publicKeyBase58</code>
property.
There was a problem hiding this comment.
While I agree with your wording, the consensus position of the group on @selfissued's proposal was to not word things in the way that you worded them. What you are suggesting is more or less what we had before, and the group straw polled to change it so that we don't have normative statements about JWK's usage in the table.
There was a problem hiding this comment.
I do not recall such a straw poll (which you refer to nearby as a resolution, where you cite the special Monday call, which I was not able to join). I do not think I would have agreed with it, given the result, and I do not recall it being mentioned in the regular Tuesday call. Can you provide a specific pointer to the relevant section of minutes?
In any case -- I'm suggesting the text be changed because the current wording is clumsy at best, and confusing at worst. Either the table should only mention the alternative encodings (leaving all mention of JWK in the preceding text), or it should be fully explicit (and mention both/all encoding options for each public key in the row), as in the table text I've suggested.
There was a problem hiding this comment.
The table now does the following thing you requested:
it should be fully explicit (and mention both/all encoding options for each public key in the row)
There was a problem hiding this comment.
I would prefer that the wording be changed to "Secp256k1 public key values MUST either be encoded as a JWK or be encoded as the raw 32-byte public key value encoded in Base58 Bitcoin format using the publicKeyBase58 property." That would put the two representations on a more even editorial footing.
msporny
force-pushed
the
msporny-issue-67-secp256k1
branch
from
9d76711 to
943db24
Compare
Co-Authored-By: Ted Thibodeau Jr <[email protected]>
Co-Authored-By: Ted Thibodeau Jr <[email protected]>
|
Ok, this PR is good to be merged down into msporny-issue-67-secp256k1... please make all future comments there. |
Preview | Diff