digitalbazaar / digitalbazaar/webkms-client

CapabilityAgent.fromSecret collapses distinct binary secrets to the same key

Open Beginner friendly
#68 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
JavaScript
Stars
6
Forks
4
PR merge metrics
No merged PRs in 30d

Description

Summary

CapabilityAgent.fromSecret accepts a Uint8Array secret, but _computeSaltedHash decodes it to a string via TextDecoder (UTF-8) before hashing. Invalid UTF-8 byte sequences are replaced with U+FFFD, so distinct binary secrets can produce an identical seed -- and therefore the same did:key. This silently reduces entropy in deterministic key derivation.

To Reproduce
  import {CapabilityAgent} from '@digitalbazaar/webkms-client';                                                   
                                                                                                                  
  const a = await CapabilityAgent.fromSecret({secret: new Uint8Array([0xFF]), handle: 'h'});                      
  const b = await CapabilityAgent.fromSecret({secret: new Uint8Array([0xFE]), handle: 'h'});                      
                                                                                                                  
  console.log(a.id === b.id); // true — different secrets, same key                                               
Cause

In lib/CapabilityAgent.js, _computeSaltedHash does:

  secret = _uint8ArrayToString(secret); // TextDecoder().decode(secret) — lossy for non-UTF-8 bytes               
  const toHash = _stringToUint8Array(`${encodeURIComponent(salt)}:${encodeURIComponent(secret)}`);                

The Uint8Array > string > Uint8Array round-trip is not injective for arbitrary binary input. (String secrets are
unaffected, since they round-trip losslessly.)

Suggested fix

Hash binary secrets as raw bytes instead of round-tripping through a string -- e.g. branch on the original type and, for a Uint8Array, concatenate the encoded salt prefix (encodeURIComponent(handle) + ':'), which contains no literal : with the raw secret bytes. The string path can be left byte-identical so existing string-derived keys don't change.

Impact

Anyone passing raw bytes (random/derived key material) as secret may unknowingly derive colliding identities with far less entropy than the input implies.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in lib/CapabilityAgent.js at _computeSaltedHash and reproduce the Uint8Array examples from the issue. Preserve the existing string-secret behavior while ensuring distinct binary secrets produce distinct derived IDs; verify the shown collision no longer occurs.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
cryptography, security
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.