chore: audit implementation coverage and fix ECDH deriveBits#910
Merged
Conversation
Update implementation-coverage.md and docs/data/coverage.ts to reflect actual implementation status. Found 10+ items marked as missing that are actually implemented: - crypto.diffieHellman(options) — fully implemented in ed.ts - subtle.deriveBits ECDH — ecDeriveBits() exists - subtle.generateKey X25519, X448, AES-KW, ChaCha20-Poly1305 - subtle.exportKey Ed25519/Ed448 raw, ChaCha20-Poly1305 jwk/raw - subtle.importKey ChaCha20-Poly1305 jwk/raw - coverage.ts: createDiffieHellman, getDiffieHellman, createECDH Also adds quick-wins plan for follow-on work (crypto.hash oneshot, subtle.deriveKey ECDH, getCurves, Ed25519 JWK, etc).
ecDeriveBits was calling keyObject.export({ format: 'jwk' }) which throws
at the TypeScript layer since JWK export isn't wired through the export()
method. The C++ exportJwk() on the native handle works fine — use it
directly, matching the pattern already used in subtle.ts exportKeyJWK.
Also adds 'public' property to SubtleAlgorithm type to remove casting,
applies the same cleanup in ed.ts xDeriveBits, and adds ECDH deriveBits
and import/export test cases.
Contributor
🤖 End-to-End Test Results - AndroidStatus: ✅ Passed 📸 Final Test ScreenshotScreenshot automatically captured from End-to-End tests and will expire in 30 days This comment is automatically updated on each test run. |
Contributor
🤖 End-to-End Test Results - iOSStatus: ✅ Passed 📸 Final Test ScreenshotScreenshot automatically captured from End-to-End tests and will expire in 30 days This comment is automatically updated on each test run. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Summary
Audits
implementation-coverage.mdanddocs/data/coverage.tsagainst the actual codebase, correcting 10+ items that were marked as missing but are actually implemented. Also fixes ECDHsubtle.deriveBits()which was broken for P-256/P-384/P-521, and adds new test cases.Coverage Audit
Found and corrected status for:
crypto.diffieHellman(options)— fully implemented in ed.tssubtle.deriveBitsECDH —ecDeriveBits()existssubtle.generateKeyX25519, X448, AES-KW, ChaCha20-Poly1305subtle.exportKeyEd25519/Ed448 raw, ChaCha20-Poly1305 jwk/rawsubtle.importKeyChaCha20-Poly1305 jwk/rawcoverage.ts: createDiffieHellman, getDiffieHellman, createECDHAdds a quick-wins plan for follow-on work (crypto.hash oneshot, subtle.deriveKey ECDH, getCurves, Ed25519 JWK, etc).
ECDH deriveBits Fix
All 6 ECDH deriveBits tests were failing with "PrivateKey export for jwk is not implemented":
ecDeriveBitswas callingkeyObject.export({ format: 'jwk' })which throws at the TS layerhandle.exportJwk()directly (the C++ impl is complete, matching the pattern insubtle.ts)P-256) vs OpenSSL alias (prime256v1) to the ECDH constructorOther Changes
publicproperty toSubtleAlgorithmtype, removing unsafe casts inecDeriveBitsandxDeriveBitsTesting
Run the
subtle.deriveBitstest suite in the example app — all ECDH tests should now pass.Closes #907