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.

Sign up for GitHub

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

Jump to bottom

Always set alg and kid on expansion #502

Closed
wants to merge 18 commits into from
Closed

Always set alg and kid on expansion #502

Changes from 7 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
c0eb781
Always set alg and kid on expansion
nitro-neal Apr 29, 2024
3e71313
update
nitro-neal Apr 29, 2024
ab67df8
update
nitro-neal Apr 29, 2024
7f8fd7d
update
nitro-neal Apr 29, 2024
24f308e
update
nitro-neal Apr 29, 2024
ec1c910
update
nitro-neal Apr 30, 2024
0c54f87
update
nitro-neal Apr 30, 2024
62cdc2f
Update packages/dids/src/methods/did-dht.ts
nitro-neal May 1, 2024
44b12b6
update
nitro-neal May 1, 2024
e409348
Update packages/dids/src/methods/did-dht.ts
nitro-neal May 2, 2024
d9608f1
Update packages/dids/src/methods/did-dht.ts
nitro-neal May 2, 2024
a62c204
Update packages/dids/src/methods/did-dht.ts
nitro-neal May 2, 2024
07dea7e
Update packages/dids/src/methods/did-dht.ts
nitro-neal May 2, 2024
847ecb1
Update packages/dids/src/methods/did-dht.ts
nitro-neal May 2, 2024
fe5d87f
updating for simpler logic
nitro-neal May 2, 2024
c9493ff
fix lint
nitro-neal May 6, 2024
964ca7b
Merge branch 'main' into set-alg-kid-expansion
nitro-neal May 6, 2024
143b293
wip
nitro-neal May 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 37 additions & 3 deletions packages/dids/src/methods/did-dht.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import { extractDidFragment } from '../utils.js';
import { DidError, DidErrorCode } from '../did-error.js';
import { DidVerificationRelationship } from '../types/did-core.js';
import { EMPTY_DID_RESOLUTION_RESULT } from '../types/did-resolution.js';
import { getJoseSignatureAlgorithmFromPublicKey } from '@web5/crypto/utils';

/**
* Represents a BEP44 message, which is used for storing and retrieving data in the Mainline DHT
Expand Down Expand Up @@ -347,6 +348,15 @@ export enum DidDhtRegisteredKeyType {
secp256r1 = 2
}

/**
* Private helper that maps did dht registered key types to their corresponding default algorithm identifiers.
*/
const KeyTypeToDefaultAlgorithmMap = {
[DidDhtRegisteredKeyType.Ed25519] : 'EdDSA',
thehenrytsai marked this conversation as resolved.
Show resolved Hide resolved
[DidDhtRegisteredKeyType.secp256k1] : 'ES256K',
[DidDhtRegisteredKeyType.secp256r1] : 'ES256',
} as const;
nitro-neal marked this conversation as resolved.
Show resolved Hide resolved

/**
* Maps {@link https://www.w3.org/TR/did-core/#verification-relationships | DID Core Verification Relationship}
* values to the corresponding record name in the DNS packet representation of a DHT DID document.
Expand Down Expand Up @@ -1014,17 +1024,31 @@ export class DidDhtDocument {
case dnsRecordId.startsWith('k'): {
// Get the method ID fragment (id), key type (t), Base64URL-encoded public key (k), and
// optionally, controller (c) from the decoded TXT record data.
const { id, t, k, c } = DidDhtUtils.parseTxtDataToObject(answer.data);
const { id, t, k, c, alg: parsedAlg } = DidDhtUtils.parseTxtDataToObject(answer.data);

// Convert the public key from Base64URL format to a byte array.
const publicKeyBytes = Convert.base64Url(k).toUint8Array();

// Use the key type integer to look up the cryptographic curve name.
const namedCurve = DidDhtRegisteredKeyType[Number(t)];

// TODO: Remove after testing
// Determine the algorithm from the key type or use the initial algorithm if provided.
// const alg = parsedAlg || DidDhtRegisteredKeyType[Number(t)];

// Convert the public key from a byte array to JWK format.
let publicKey = await DidDhtUtils.keyConverter(namedCurve).bytesToPublicKey({ publicKeyBytes });

// Always set the algorithm on did:dht expansion.
nitro-neal marked this conversation as resolved.
Show resolved Hide resolved
publicKey.alg = parsedAlg || getJoseSignatureAlgorithmFromPublicKey(publicKey);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sanity: getJoseSignatureAlgorithmFromPublicKey() does not seem to return Ed25519 that will be expected of vector 3.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

am I using this correctly?

export function getJoseSignatureAlgorithmFromPublicKey(publicKey: Jwk): string {
  const curveToJoseAlgorithm: Record<string, string> = {
    'Ed25519'   : 'EdDSA',
    'P-256'     : 'ES256',
    'P-384'     : 'ES384',
    'P-521'     : 'ES512',
    'secp256k1' : 'ES256K',
  };

  // If the key contains an `alg` property that matches a JOSE registered algorithm identifier,
  // return its value.
  if (publicKey.alg && Object.values(curveToJoseAlgorithm).includes(publicKey.alg)) {
    return publicKey.alg;
  }

  // If the key contains a `crv` property, return the corresponding algorithm.
  if (publicKey.crv && Object.keys(curveToJoseAlgorithm).includes(publicKey.crv)) {
    return curveToJoseAlgorithm[publicKey.crv];
  }

  throw new Error(
    `Unable to determine algorithm based on provided input: alg=${publicKey.alg}, crv=${publicKey.crv}. ` +
    `Supported 'alg' values: ${Object.values(curveToJoseAlgorithm).join(', ')}. ` +
    `Supported 'crv' values: ${Object.keys(curveToJoseAlgorithm).join(', ')}.`
  );
}

Copy link
Contributor Author

@nitro-neal nitro-neal May 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the alg is always there for our jwks, but is that not necessarily the case for outside jwks inside the did dht??

if the alg is always there we can just do

if(parsedAlg) {
   publicKey.alg = parsedAlg
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get if(parsedAlg) { publicKey.alg = parsedAlg }, if parsedAlg is always defined, then there is nothing do right? since:

const { id, t, k, c, a: parsedAlg } = DidDhtUtils.parseTxtDataToObject(answer.data);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parsed alg will not always be defined, but publicKey.alg is always defined


// Determine the Key ID (kid): '0' for the identity key or JWK thumbprint for others. Always set alg on expansion.
nitro-neal marked this conversation as resolved.
Show resolved Hide resolved
if (id !== '0' && publicKey.kid === undefined) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For my understanding: why would kid be defined when id is not 0? May have opportunity to simplify the condition here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

simplifying to:

          if (id === '0') {
            publicKey.kid = '0';
          } else if (publicKey.kid === undefined) {
            publicKey.kid = await computeJwkThumbprint({ jwk: publicKey });
          }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the publicKey.kid should be defined, but if it is not, we can recompute it deterministically with computeJwkThumbprint

publicKey.kid = await computeJwkThumbprint({ jwk: publicKey });
thehenrytsai marked this conversation as resolved.
Show resolved Hide resolved
} else if (id === '0') {
publicKey.kid = '0';
}

// Initialize the `verificationMethod` array if it does not already exist.
didDocument.verificationMethod ??= [];

Expand Down Expand Up @@ -1159,7 +1183,11 @@ export class DidDhtDocument {
let methodId = vm.id.split('#').pop()!; // Remove fragment prefix, if any.
idLookup.set(methodId, dnsRecordId);

const publicKey = vm.publicKeyJwk;
const publicKey = vm.publicKeyJwk!;

if(methodId === '0') {
publicKey.kid = '0';
}
nitro-neal marked this conversation as resolved.
Show resolved Hide resolved

if (!(publicKey?.crv && publicKey.crv in AlgorithmToKeyTypeMap)) {
throw new DidError(DidErrorCode.InvalidPublicKeyType, `Verification method '${vm.id}' contains an unsupported key type: ${publicKey?.crv ?? 'undefined'}`);
Expand All @@ -1168,14 +1196,20 @@ export class DidDhtDocument {
// Use the public key's `crv` property to get the DID DHT key type.
const keyType = DidDhtRegisteredKeyType[publicKey.crv as keyof typeof DidDhtRegisteredKeyType];

let alg;
if(KeyTypeToDefaultAlgorithmMap[keyType] !== getJoseSignatureAlgorithmFromPublicKey(publicKey))
{
alg = getJoseSignatureAlgorithmFromPublicKey(publicKey);
}

// Convert the public key from JWK format to a byte array.
const publicKeyBytes = await DidDhtUtils.keyConverter(publicKey.crv).publicKeyToBytes({ publicKey });

// Convert the public key from a byte array to Base64URL format.
const publicKeyBase64Url = Convert.uint8Array(publicKeyBytes).toBase64Url();

// Define the data for the DNS TXT record.
const txtData = [`id=${methodId}`, `t=${keyType}`, `k=${publicKeyBase64Url}`];
const txtData = [`id=${methodId}`, `t=${keyType}`, `k=${publicKeyBase64Url}`, ...(alg ? [`alg=${alg}`] : [])];
nitro-neal marked this conversation as resolved.
Show resolved Hide resolved

// Add the controller property, if set to a value other than the Identity Key (DID Subject).
if (vm.controller !== didDocument.id) txtData.push(`c=${vm.controller}`);
Expand Down
Loading