Base58Matcher and Base64Matcher Conflict

[bshambaugh]

Use the hex to base58 tool here:

https://appdevtools.com/base58-encoder-decoder

040f1dbf0a2ca86875447a7c010b0fc6d39d76859c458fbe8f2bf775a40ad74a hex
====>
Gqzym8nfnxR5ZYZ3wZo8rvTwKTqGn5cJsbHnEhUZDPo base58

Try the Existing base58Matcher:

/^([1-9A-HJ-NP-Za-km-z]{44}|[1-9A-HJ-NP-Za-km-z]{88})$/.test('Gqzym8nfnxR5ZYZ3wZo8rvTwKTqGn5cJsbHnEhUZDPo')

false

Refactor, and use a new base58Matcher:

/^([1-9A-HJ-NP-Za-km-z]{43,44}|[1-9A-HJ-NP-Za-km-z]{86,88})$/.test('Gqzym8nfnxR5ZYZ3wZo8rvTwKTqGn5cJsbHnEhUZDPo')

true

For reference see code: https://github.com/decentralized-identity/did-jwt/blob/master/src/util.ts#L87-L115

[bshambaugh]

Using the python math library, I have print (math.log((256**32),58)) = 43.70 which puts the length between 43 an 44.

[bshambaugh]

I added some tests for parseKey.change the matcher by switching between commenting out one of the base58matchers in util.ts 764f89f

[bshambaugh]

The base58 string I am matching is only 43 characters long. I will see if I can add some padding to get the matcher to work.

[bshambaugh]

I thought that maybe Gqzym8nfnxR5ZYZ3wZo8rvTwKTqGn5cJsbHnEhUZDPo could be rewritten as 1Gqzym8nfnxR5ZYZ3wZo8rvTwKTqGn5cJsbHnEhUZDPo . The first attempt to get this to work was not successful. I will have to try again.

[bshambaugh]

This test does not seem to care that I just tack on a leading zero (in base58):

Matcher: const base58Matcher = /^([1-9A-HJ-NP-Za-km-z]{44}|[1-9A-HJ-NP-Za-km-z]{88})$/

From:

describe('parseKey test #2', () => {
const privateKeyBase58 = 'Gqzym8nfnxR5ZYZ3wZo8rvTwKTqGn5cJsbHnEhUZDPo'
const privateKeyHex = '040f1dbf0a2ca86875447a7c010b0fc6d39d76859c458fbe8f2bf775a40ad74a'
const privateKeyHexPrefix = '0x040f1dbf0a2ca86875447a7c010b0fc6d39d76859c458fbe8f2bf775a40ad74a'
const privateKeyBase64 = 'BA8dvwosqGh1RHp8AQsPxtOddoWcRY++jyv3daQK10o'
const privateKeyBase64Url = 'BA8dvwosqGh1RHp8AQsPxtOddoWcRY--jyv3daQK10o'

To:
describe('parseKey test #2', () => {
const privateKeyBase58 = '1Gqzym8nfnxR5ZYZ3wZo8rvTwKTqGn5cJsbHnEhUZDPo'
const privateKeyHex = '040f1dbf0a2ca86875447a7c010b0fc6d39d76859c458fbe8f2bf775a40ad74a'
const privateKeyHexPrefix = '0x040f1dbf0a2ca86875447a7c010b0fc6d39d76859c458fbe8f2bf775a40ad74a'
const privateKeyBase64 = 'BA8dvwosqGh1RHp8AQsPxtOddoWcRY++jyv3daQK10o'
const privateKeyBase64Url = 'BA8dvwosqGh1RHp8AQsPxtOddoWcRY--jyv3daQK10o'

[bshambaugh]

/^([1-9A-HJ-NP-Za-km-z]{44}|[1-9A-HJ-NP-Za-km-z]{88})$/.test('1Gqzym8nfnxR5ZYZ3wZo8rvTwKTqGn5cJsbHnEhUZDPo') evaluates to true in the browser.

I might need to dig into the code some more and see how the extra zero in front (the 1 in base58) is causing a problem. Maybe it is causing an unexpected length elsewhere??

[mirceanis]

Actually you have stumbled upon a larger issue here.

040f1dbf0a2ca86875447a7c010b0fc6d39d76859c458fbe8f2bf775a40ad74a hex does indeed encode to Gqzym8nfnxR5ZYZ3wZo8rvTwKTqGn5cJsbHnEhUZDPo base58btc which is 43 characters long so your proposed matcher makes sense at first glance.

BUT, that matcher is used in an heuristic to differentiate between different key encodings and the fact that there are valid encodings 43 character long, means that the logic there is broken, since that string could also represent a base64 encoding.

So because of this ambiguity we should either remove one of the base58btc or base64 encodings from the heuristic, or have parseKey throw an error.

This is a classic example of some convenience code coming back to byte :)

[mirceanis]

Fixing this will affect the private key inputs accepted by the signers (ES256KSigner, EdDSASigner, etc).

We have a couple of options to fix this:

• 1. remove the base58btc matcher from the heuristic
• 2. remove the base64 matcher from the heuristic
• 3. remove all string encodings, and require only raw Uint8Arrays for private key inputs
• 4. replace heuristic with multibase string encoding, which uses prefixes instead of fumbling with heuristics.

All of these are breaking changes so fixing this will bump us to a new major version.

If you are following this repository, please weigh in with your opinion.

[bshambaugh]

I announced it here https://lists.w3.org/Archives/Public/public-credentials/2022Mar/0321.html and on the ceramic discord. This is for the matchers used by parseKey in util.ts[1] and called by "(ES256KSigner, EdDSASigner, etc).".

@oed , is it better that I just tag you on GitHub?

[1] https://github.com/decentralized-identity/did-jwt/blob/master/src/util.ts

[bshambaugh]

For reference, this is in Orie Steele's code:
https://github.com/transmute-industries/verifiable-data/blob/5f5d9289191b1844200fd03bbd0054608811719f/packages/web-crypto-key-pair/src/signatures/getSigner.ts

getSigner the JsonWebKey2020 or CryptoKey . CryptoKey is an interface from the Web Cryptograpy API.
https://w3c.github.io/webcrypto/#cryptokey-interface

The jws code is here: https://github.com/transmute-industries/verifiable-data/blob/5f5d9289191b1844200fd03bbd0054608811719f/packages/web-crypto-key-pair/src/signatures/jws.ts#L20-L36 .

In jws.ts in the webcrypto-key-pairs package in jws.ts there is a snippet of code (L140-148):

export const getJwsSigner = (cryptoKey: CryptoKey): JwsSigner => {
  return {
    sign: async ({ data }: JwsSignerOptions) => {
      const signer = getRawSigner(cryptoKey);
      const alg = await getAlg(cryptoKey);
      return createJws(signer, data, { alg });
    },
  };
};

Maybe there is something going with this subject area in the WG that I don't know about. Will need to find out.

Checking out Mattr:
https://github.com/mattrglobal/jwm/blob/master/samples/example-signed-jwm.js#L25

https://github.com/panva/jose/blob/c1e1e6b1cfa93c082ecd328ae3baa2e5f64c304d/docs/classes/jws_compact_sign.CompactSign.md#sign

Parameters:

Name | Type | Description
key | KeyLike | Uint8Array | Private Key or Secret to sign the JWS with.
options? | SignOptions | JWS Sign options.

"KeyLike are runtime-specific classes representing asymmetric keys or symmetric secrets. These are instances of CryptoKey and additionally KeyObject in Node.js runtime. Uint8Array instances are also accepted as symmetric secret representation only."

from https://www.npmjs.com/package/jose

[bshambaugh]

Option 3 sticks out at me, and after some review of code in the wild it still does.

Fixing this will affect the private key inputs accepted by the signers (ES256KSigner, EdDSASigner, etc).

We have a couple of options to fix this:

* [ ]  1. remove the base58btc matcher from the heuristic

* [ ]  2. remove the base64 matcher from the heuristic

* [ ]  3. remove all string encodings, and require only raw Uint8Arrays for private key inputs

* [ ]  4. replace heuristic with multibase string encoding, which uses prefixes instead of fumbling with heuristics.

All of these are breaking changes so fixing this will bump us to a new major version.

If you are following this repository, please weigh in with your opinion.

[mirceanis]

yeah, it seems like the cleanest approach to me too.

[ukstv]

Working on Ceramic, which uses did-jwt library extensively. IMO, using Uint8Arrays (option 3) trumps every other variant by removing unnecessary stringly-typed ambiguity.

[uport-automation-bot]

🎉 This issue has been resolved in version 6.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀