Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should this library throw errors defined in the did:key spec? #18

Open
aljones15 opened this issue Aug 1, 2022 · 0 comments
Open

Should this library throw errors defined in the did:key spec? #18

aljones15 opened this issue Aug 1, 2022 · 0 comments
Assignees

Comments

@aljones15
Copy link
Contributor

aljones15 commented Aug 1, 2022

Did key spec now has standardized errors for did:key.
While this library is not specifically a did:key library it does checks that the did:key spec expects to happen.
These checks need to raise specific errors in order to conform to the spec.
The errors that need to be raised are:

  1. invalidDid (when the publicKeyMultibase doesn't start with z)
  2. invalidPublicKeyLength (when the publicKey keyBuffer is not 32 bytes in length)

Currently this library has the following check:

if(!publicKeyMultibase || !_isValidKeyHeader(
publicKeyMultibase, MULTICODEC_ED25519_PUB_HEADER)) {
throw new Error(
'"publicKeyMultibase" has invalid header bytes: ' +
`"${publicKeyMultibase}".`);
}

// check a multibase key for an expected header
function _isValidKeyHeader(multibaseKey, expectedHeader) {
if(!(typeof multibaseKey === 'string' &&
multibaseKey[0] === MULTIBASE_BASE58BTC_HEADER)) {
return false;
}
const keyBytes = base58btc.decode(multibaseKey.slice(1));
return expectedHeader.every((val, i) => keyBytes[i] === val);
}

For a public key this ensure the first letter in the multibaseKey is z and then it checks that the first bytes of the resulting Uint8Array matches the bytes of the const MULTICODEC_ED25519_PUB_HEADER = new Uint8Array([0xed, 0x01]);

So should we update this library to throw errors that conform to the did:key specs errors?

Currently not starting with a z should result in an invalidDid error (this might be updated to invalidDidKey) This would need to happen before the byte check.

Secondly we now need to throw this error: invalidPublicKeyLength

This just requires changing the error here so that it can be caught and reported back to an implementation:

/**
* Takes a Uint8Array of public key bytes and encodes it in DER-encoded
* SubjectPublicKeyInfo (SPKI) format.
* Allows Uint8Arrays to be interoperable with node's crypto functions.
*
* @param {object} options - Options to use.
* @param {Uint8Array} options.publicKeyBytes - The keyBytes.
*
* @throws {TypeError} Throws if the bytes are not Uint8Array or of length 32.
*
* @returns {Buffer} DER Public key Prefix + key bytes.
*/
function publicKeyDerEncode({publicKeyBytes}) {
if(!(publicKeyBytes instanceof Uint8Array && publicKeyBytes.length === 32)) {
throw new TypeError('`publicKeyBytes` must be a 32 byte Buffer.');
}
return Buffer.concat([DER_PUBLIC_KEY_PREFIX, publicKeyBytes]);
}

I think we should use Error.code to report these errors back like this:

export class InvalidDid extends Error {
  constructor(message) {
    super(message);
    this.name = 'InvalidDidError';
    this.code = 'invalidDid';
  }
}

export class InvalidPublicKeyLength extends Error {
  constructor(message) {
    super(message);
    this.name = 'InvalidPublicKeyLengthError';
    this.code = 'invalidPublicKeyLength';
  }
}

These can be caught later down the line and used for various aspects of conformance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant