decentralized-identity / decentralized-identity/ion-tools

Bug: Private key with only 42 bytes leads to error when creating an update operation

Open
#23 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
143
Forks
31
PR merge metrics
No merged PRs in 30d

Description

It can happen, that one of the private keys (recovery or update) starts with a null byte and hence has a length of 42. This will lead to an error when running an update operation because the LocalSigner from ion-sdk requires it to be of length 43.

**Steps to reproduce:**
1. Clone this repository and install dependencies with `npm i`
(when using the packaged version, you may apply the fix from #14 locally in `node_modules\@decentralized-identity\ion-tools\ion.js\lib.js:172 (createUpdateRequest)`, otherwise you will get an error in `\@decentralized-identity\ion-sdk\dist\lib\InputValidator.js:25` when generating the update operation: `Cannot read properties of undefined (reading 'crv')`)

2. Copy the following code to a file in the top directory and run it, e.g. `node short-key.js`

```js
const ION = require("./ion.js/lib");

async function main() {
/*let i = 0;
while (true) {
i++;
const did = new ION.DID();
const operations = await did.getAllOperations();
const ukl = operations[0].update.privateJwk.d.length;
if (ukl != 43) {
console.log(JSON.stringify(operations, null, 2));
break;
}
if (i > 1000) {
console.log("no short key found after", i, "cycles");
break;
}
}*/

// create DID with "short" private key
const did = new ION.DID({
ops: [
{
operation: "create",
content: {},
recovery: {
publicJwk: {
kty: "EC",
crv: "secp256k1",
x: "qSYL-SqJLkOg1q_Ifox9m77W_PvJL7YZQQIjkT3c1Rw",
y: "dnm91aw22U7cYF-CMSmJa8LbAgGahlWnJmlwKJYTwEU",
},
privateJwk: {
kty: "EC",
crv: "secp256k1",
d: "mR-2ioKyTea6rBHmZR5YaGYEHRSljWpiJ8MVaBacN44",
x: "qSYL-SqJLkOg1q_Ifox9m77W_PvJL7YZQQIjkT3c1Rw",
y: "dnm91aw22U7cYF-CMSmJa8LbAgGahlWnJmlwKJYTwEU",
},
},
update: {
publicJwk: {
kty: "EC",
crv: "secp256k1",
x: "WS0zURYuE45OLuZTr6nNWfWO8xCLszHBqXQEDhSw2To",
y: "t3VlvV1zHxTLZkXJvUTJt-7b7BGitzOrRtm-Ogb0ODM",
},
privateJwk: {
kty: "EC",
crv: "secp256k1",
d: "MRKXkIv44DkLOQY1ZZj-mMfixI9aHh7i97xy9eewOw", // <-- private key starts with a null byte -> only has length 42
x: "WS0zURYuE45OLuZTr6nNWfWO8xCLszHBqXQEDhSw2To",
y: "t3VlvV1zHxTLZkXJvUTJt-7b7BGitzOrRtm-Ogb0ODM",
},
},
},
],
});

// the following workaround lets it work locally for me
const key43b = Buffer.from(Buffer.from(did._ops[0].update.privateJwk.d, "base64").toString("hex").padStart(64, '0'), "hex").toString("base64").slice(0,43);
//did._ops[0].update.privateJwk.d = key43b; // <-- uncomment this line to see the "fix"

// now, generate an update operation
const op = await did.generateOperation("update", {
addServices: [
{
id: "domain-1",
type: "LinkedDomains",
serviceEndpoint: "https://example.org",
},
],
});

const reqBody = await did.generateRequest(1);
console.log(op, reqBody);
}
main();

```

3. It will show an error:
```
[...]\ion-tools\node_modules\@decentralized-identity\ion-sdk\dist\lib\InputValidator.js:38
throw new IonError_1.default(ErrorCode_1.default.JwkEs256kHasIncorrectLengthOfD, `SECP256K1 JWK 'd' property must be 43 bytes.`);
^

IonError: JwkEs256kHasIncorrectLengthOfD: SECP256K1 JWK 'd' property must be 43 bytes.
at Function.validateEs256kOperationKey ([...]\ion-tools\node_modules\@decentralized-identity\ion-sdk\dist\lib\InputValidator.js:38:19)
at new LocalSigner ([...]\ion-tools\node_modules\@decentralized-identity\ion-sdk\dist\lib\LocalSigner.js:21:34)
at Function.create ([...]\ion-tools\node_modules\@decentralized-identity\ion-sdk\dist\lib\LocalSigner.js:27:16)
at ION.DID.generateRequest ([...]\ion-tools\ion.js\lib.js:171:59)

```

If you uncomment the highlighted lines,
```js
const key43b = Buffer.from(Buffer.from(did._ops[0].update.privateJwk.d, "base64").toString("hex").padStart(64, '0'), "hex").toString("base64").slice(0,43);
did._ops[0].update.privateJwk.d = key43b; // <-- uncomment this line to see the "fix"
```
the update is generated and runs without an error. Probably not the best fix.

---

**Questions:**
* Do you know what's the intention behind requiring the key to have a length of 43 [here in ion-sdk](https://github.com/decentralized-identity/ion-sdk/blob/d4cce6884ce3e873c49743cbcc01039b9f16ed0c/lib/InputValidator.ts#L43-L45)? (when testing it locally, the underlying library was able to correctly sign messages with the short key w/o any hacky pre-processing)
* If the length-check is needed security-wise, this should probably already happen when generating update and recovery keys so that a new one can be generated directly and users don't notice this error too late

The described bug does not only apply to `d`, but also to values `x` and `y`.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.