decentralized-identity / decentralized-identity/ion-tools
Bug: Private key with only 42 bytes leads to error when creating an update operation
- 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.