feat: browser WASM host for TDF encrypt round-trip validation#840
feat: browser WASM host for TDF encrypt round-trip validation#840pflynn-virtru wants to merge 10 commits intomainfrom
Conversation
Implements a browser-based test harness that loads the TinyGo-compiled
WASM TDF encrypt module and validates round-trip encryption using
Web Crypto API (SubtleCrypto). Uses Worker + SharedArrayBuffer + Atomics
to bridge synchronous WASM host functions with async browser crypto.
Architecture:
- Worker thread loads WASM, blocks on Atomics.wait() for crypto calls
- Main thread performs async SubtleCrypto operations, signals via Atomics.notify()
- Cross-Origin Isolation headers (COOP/COEP) served for SharedArrayBuffer
Tests (3 browser round-trips):
- HS256 encrypt → ZIP parse → RSA-OAEP unwrap DEK → AES-GCM decrypt → assert match
- GMAC encrypt → verify GMAC segment hash → decrypt → assert match
- Invalid PEM → verify error propagation
Requires WASM binary built from opentdf/platform:
tinygo build -target=wasip1 -scheduler=none -gc=leaking \
-buildmode=c-shared -o wasm-host/tdfcore.wasm \
./sdk/experimental/tdf/wasm/
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello @pflynn-virtru, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request establishes a foundational browser-based environment for the TinyGo-compiled WASM TDF encryption module. It introduces a sophisticated inter-thread communication pattern using SharedArrayBuffer and Atomics to enable the WASM module to perform cryptographic operations via the browser's Web Crypto API. This setup is crucial for validating the TDF encryption process within a web context and includes automated testing capabilities to ensure reliability and correctness. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| const server = createServer(async (req, res) => { | ||
| const path = join(ROOT, req.url === '/' ? '/test.html' : req.url); | ||
| try { | ||
| const data = await readFile(path); |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 15 days ago
In general, to fix uncontrolled path usage, we must normalize the requested path relative to a fixed root directory and then ensure that the resolved path is still within that root. For Node, this can be done with path.resolve and optionally fs.realpath if we want to resolve symlinks as well. If the normalized path is outside the root, we reject the request (for example, with HTTP 403).
For this script, the best minimal fix is:
- Derive a safe, relative pathname from
req.url:- Strip the query string and fragment (everything after
?or#). - Default
/to/test.htmlas the current code does.
- Strip the query string and fragment (everything after
- Call
resolve(ROOT, '.' + safeUrlPath)to convert the URL path into a filesystem path underROOT. Prefixing with.ensures we get a path relative toROOT, not an absolute path that discards the root. - Verify that the resolved path starts with
ROOT(string prefix check). If not, respond with403and stop. - Use this validated
safePathfor bothreadFileandextname.
This preserves existing behavior for normal in-root requests (like /test.html, /foo.js) while preventing directory traversal or escaping from ROOT. To implement this, we need to:
- Add an import of
resolvefromnode:path(keeping existing imports). - Replace the direct
join(...)call with the normalization and check logic. - Change
extnamecall to use the validated filesystem path rather than the unsanitized joined path.
All edits must be within wasm-host/serve.mjs.
| @@ -3,7 +3,7 @@ | ||
|
|
||
| import { createServer } from 'node:http'; | ||
| import { readFile } from 'node:fs/promises'; | ||
| import { join, extname } from 'node:path'; | ||
| import { join, extname, resolve } from 'node:path'; | ||
| import { fileURLToPath } from 'node:url'; | ||
|
|
||
| const PORT = parseInt(process.argv[2] || '8080', 10); | ||
| @@ -19,10 +19,21 @@ | ||
| }; | ||
|
|
||
| const server = createServer(async (req, res) => { | ||
| const path = join(ROOT, req.url === '/' ? '/test.html' : req.url); | ||
| // Normalize request URL path and ensure it stays within ROOT | ||
| const urlPath = req.url.split(/[?#]/, 1)[0] || '/'; | ||
| const relativePath = urlPath === '/' ? '/test.html' : urlPath; | ||
| const fsPath = resolve(ROOT, '.' + relativePath); | ||
|
|
||
| // Prevent directory traversal: fsPath must remain under ROOT | ||
| if (!fsPath.startsWith(ROOT)) { | ||
| res.writeHead(403); | ||
| res.end('Forbidden'); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| const data = await readFile(path); | ||
| const ext = extname(path); | ||
| const data = await readFile(fsPath); | ||
| const ext = extname(fsPath); | ||
| res.writeHead(200, { | ||
| 'Content-Type': MIME[ext] || 'application/octet-stream', | ||
| 'Cross-Origin-Opener-Policy': 'same-origin', |
| const server = createServer(async (req, res) => { | ||
| const path = join(ROOT, req.url === '/' ? '/test.html' : req.url); | ||
| try { | ||
| const data = await readFile(path); |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 9 days ago
In general, to fix this issue we must ensure that any file path derived from req.url is constrained to a safe root directory. That means: (1) normalize the request path (and strip query/hash components), (2) resolve it relative to ROOT, and (3) reject the request if the resulting path is not within ROOT after resolution. We should also ensure that the default / route continues to serve /test.html as before.
The best way to fix it here without changing existing functionality is:
- Parse and normalize
req.urlso we only use the pathname (ignoring query strings and fragments) and prevent malformed paths. - Build a candidate path relative to
ROOTusingjoin(ROOT, relativePath)whererelativePathis a normalized path that does not allow going above root. - Use
path.resolveto normalize the final path andstartsWith(ROOT)to ensure it is still within the root directory. If not, respond with 404 (or 403) instead of reading the file. - Use this safe, resolved path for both
readFileand for determining the MIME type.
To implement this within the shown file:
- We need to import
resolvefromnode:pathalongside the existingjoinandextname. - Replace the simple
const path = join(ROOT, ...)logic with a slightly more robust block that:- Constructs a sanitized
urlPathfromreq.url, defaulting/to/test.html. - Uses
join(ROOT, urlPath)and thenresolveto get an absolute, normalizedfilePath. - Checks
filePath.startsWith(ROOT), and if the check fails, responds with 404 and returns.
- Constructs a sanitized
- Use
filePathinstead ofpathdownstream forreadFileand the MIME lookup.
All changes are confined to wasm-host/test-browser.mjs, specifically the import line for node:path and the body of the createServer callback around lines 23–32.
| @@ -5,7 +5,7 @@ | ||
|
|
||
| import { createServer } from 'node:http'; | ||
| import { readFile } from 'node:fs/promises'; | ||
| import { join, extname } from 'node:path'; | ||
| import { join, extname, resolve } from 'node:path'; | ||
| import { fileURLToPath } from 'node:url'; | ||
| import { chromium } from 'playwright'; | ||
|
|
||
| @@ -21,11 +21,23 @@ | ||
|
|
||
| // Start dev server with COOP/COEP headers | ||
| const server = createServer(async (req, res) => { | ||
| const path = join(ROOT, req.url === '/' ? '/test.html' : req.url); | ||
| // Normalize and constrain requested path to ROOT to avoid directory traversal | ||
| const rawUrl = req.url || '/'; | ||
| const urlPath = rawUrl === '/' ? '/test.html' : rawUrl.split('?')[0].split('#')[0]; | ||
| const joinedPath = join(ROOT, urlPath); | ||
| const filePath = resolve(joinedPath); | ||
|
|
||
| // Ensure the resolved path is within ROOT | ||
| if (!filePath.startsWith(ROOT)) { | ||
| res.writeHead(404); | ||
| res.end('Not found'); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| const data = await readFile(path); | ||
| const data = await readFile(filePath); | ||
| res.writeHead(200, { | ||
| 'Content-Type': MIME[extname(path)] || 'application/octet-stream', | ||
| 'Content-Type': MIME[extname(filePath)] || 'application/octet-stream', | ||
| 'Cross-Origin-Opener-Policy': 'same-origin', | ||
| 'Cross-Origin-Embedder-Policy': 'require-corp', | ||
| }); |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive test harness for validating a WASM-based TDF encryption module within a browser environment, utilizing a Web Worker with SharedArrayBuffer and Atomics for efficient communication. While the overall architecture is sound and adheres to modern browser security requirements (COOP/COEP), a high-severity path traversal vulnerability was identified in the development server (serve.mjs), and the outdated SHA-1 algorithm is used for RSA-OAEP operations in the crypto handler. Further improvements can be made to enhance maintainability by reducing code duplication and eliminating magic numbers, and to increase code consistency.
| const path = join(ROOT, req.url === '/' ? '/test.html' : req.url); | ||
| try { | ||
| const data = await readFile(path); |
There was a problem hiding this comment.
The development server is vulnerable to path traversal. The req.url is directly joined with the ROOT directory using path.join(), which does not prevent an attacker from using .. sequences to escape the root directory and read arbitrary files from the host system. While this is a development server, it poses a security risk if run in an environment where sensitive files are accessible.
| const path = join(ROOT, req.url === '/' ? '/test.html' : req.url); | |
| try { | |
| const data = await readFile(path); | |
| const url = new URL(req.url, 'http://localhost'); | |
| const targetPath = join(ROOT, url.pathname === '/' ? '/test.html' : url.pathname); | |
| if (!targetPath.startsWith(ROOT)) { | |
| res.writeHead(403); | |
| res.end('Forbidden'); | |
| return; | |
| } | |
| try { | |
| const data = await readFile(targetPath); |
| async _rsaOaepEncrypt(pubPemBytes, plaintext) { | ||
| const pem = new TextDecoder().decode(pubPemBytes); | ||
| const der = pemToDer(pem); | ||
| const key = await crypto.subtle.importKey('spki', der, { name: 'RSA-OAEP', hash: 'SHA-1' }, false, ['encrypt']); |
There was a problem hiding this comment.
The use of SHA-1 as the hash algorithm for RSA-OAEP is considered weak and outdated. Modern cryptographic standards recommend using SHA-256 or higher to ensure long-term security and resistance to potential vulnerabilities. It is highly recommended to upgrade to a more secure hashing algorithm, such as SHA-256, in both _rsaOaepEncrypt and _rsaOaepDecrypt if the corresponding WASM module supports it.
| async _rsaOaepDecrypt(privPemBytes, ciphertext) { | ||
| const pem = new TextDecoder().decode(privPemBytes); | ||
| const der = pemToDer(pem); | ||
| const key = await crypto.subtle.importKey('pkcs8', der, { name: 'RSA-OAEP', hash: 'SHA-1' }, false, ['decrypt']); |
There was a problem hiding this comment.
The use of SHA-1 as the hash algorithm for RSA-OAEP is considered weak and outdated. It is recommended to use SHA-256 instead.
|
|
||
| async _rsaGenerateKeypair(bits) { | ||
| const keyPair = await crypto.subtle.generateKey( | ||
| { name: 'RSA-OAEP', modulusLength: bits, publicExponent: new Uint8Array([1, 0, 1]), hash: 'SHA-1' }, |
There was a problem hiding this comment.
The use of SHA-1 as the hash algorithm for RSA-OAEP is considered weak and outdated. It is recommended to use SHA-256 instead.
| const buf = new Uint8Array(bin.length); | ||
| for (let i = 0; i < bin.length; i++) buf[i] = bin.charCodeAt(i); |
There was a problem hiding this comment.
This loop for converting a binary string to a Uint8Array can be simplified. A more modern and concise approach is to use Uint8Array.from(bin, c => c.charCodeAt(0)), which is also consistent with the pattern used in test.html.
| const buf = new Uint8Array(bin.length); | |
| for (let i = 0; i < bin.length; i++) buf[i] = bin.charCodeAt(i); | |
| const buf = Uint8Array.from(bin, c => c.charCodeAt(0)); |
| // Start dev server with COOP/COEP headers | ||
| const server = createServer(async (req, res) => { | ||
| const path = join(ROOT, req.url === '/' ? '/test.html' : req.url); | ||
| try { | ||
| const data = await readFile(path); | ||
| res.writeHead(200, { | ||
| 'Content-Type': MIME[extname(path)] || 'application/octet-stream', | ||
| 'Cross-Origin-Opener-Policy': 'same-origin', | ||
| 'Cross-Origin-Embedder-Policy': 'require-corp', | ||
| }); | ||
| res.end(data); | ||
| } catch { | ||
| res.writeHead(404); | ||
| res.end('Not found'); | ||
| } | ||
| }); |
There was a problem hiding this comment.
The server creation logic here is nearly identical to the code in serve.mjs. To avoid code duplication and improve maintainability, consider refactoring serve.mjs to export a function that creates and starts the server. This test script could then import and use that function, which would also centralize server configuration.
| } | ||
|
|
||
| function getWasmError() { | ||
| const bufSize = 1024; |
There was a problem hiding this comment.
The buffer size 1024 is a magic number. It would be better to define it as a constant at the top of the file, for example const WASM_ERROR_BUF_SIZE = 1024;. This improves readability and makes it easier to change if needed.
wasm-host/worker.mjs
Outdated
| const attr = writeStringToWasm(attrStr); | ||
| const pt = writeBytesToWasm(plaintext); | ||
|
|
||
| const outCapacity = 1024 * 1024; // 1MB |
There was a problem hiding this comment.
The output capacity 1024 * 1024 is a magic number. It's good practice to define this as a named constant at the top of the module, like const TDF_ENCRYPT_OUT_CAPACITY = 1 * 1024 * 1024;. This makes the code more self-documenting and easier to maintain.
Add synchronous Node.js WASM host with 8 crypto functions (node:crypto), inline ZIP parser, DEK unwrap, and tdf_encrypt/tdf_decrypt wrappers with OOM recovery. Uses local RSA keypair for DEK unwrap (no KAS needed). New --wasmBinary CLI flag. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
From dist/src/, need ../../../wasm-host/ (3 levels up) not ../../wasm-host/ (2 levels up) to reach the web-sdk root where wasm-host/ lives. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Rebuilt with TinyGo 0.37 in reactor mode (-buildmode=c-shared) from platform/sdk/experimental/tdf/wasm/ source. Now exports tdf_decrypt in addition to tdf_encrypt, enabling end-to-end WASM benchmarks. Also adds fd_fdstat_set_flags and poll_oneoff WASI stubs to the benchmark's WASM host for forward compatibility with standard Go wasip1 binaries. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds 'npm run benchmark' shortcut that builds and runs the cross-SDK benchmark. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
If these changes look good, signoff on them with: If they aren't any good, please remove them with: |
Replace the synthetic WASM-direct benchmarks (which skipped network I/O) with production-mode benchmarks that use the real KAS for both columns. WASM encrypt uses a cached KAS public key; WASM decrypt uses SDK client.read() which includes KAS rewrap — giving an honest apples-to- apples comparison. Also fix default clientId to opentdf-sdk (has rewrap entitlements). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
If these changes look good, signoff on them with: If they aren't any good, please remove them with: |
- Rebuild tdfcore.wasm with TinyGo 0.40.1 (149KB, streaming API) - worker.mjs: real read_input/write_output I/O callbacks, 10-param streaming tdf_encrypt with BigInt i64 plaintextSize - protocol.mjs: increase SAB to 4MB (2MB input + 2MB output) to support AES-GCM operations on segments up to ~2MB - wasm-tdf.mjs: add segmentSize option to encrypt() - test.html: add encrypt benchmarks (256B-10MB) with multi-segment round-trip verification via SubtleCrypto Browser benchmark (Chromium/V8, TinyGo, 3 iterations): 256B: 0.5ms, 16KB: 0.6ms, 64KB: 3.8ms, 256KB: 3.2ms, 1MB: 11.9ms, 10MB: 117.4ms Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add 100MB to browser encrypt benchmarks (1,144.8ms avg). Update cli/src/benchmark.ts to streaming 10-param tdf_encrypt API with real read_input/write_output callbacks. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
wasm-host/) that loads the TinyGo-compiled WASM TDF encrypt module and validates round-trip encryption using Web Crypto API (SubtleCrypto)Architecture
Files
protocol.mjscrypto-handler.mjsworker.mjswasm-tdf.mjsWasmTDFAPI classtest.htmltest-browser.mjsserve.mjsTest plan
How to run
Notes
.gitignore)test.htmlis test-only verification code; production integration would use the SDK's existingZipReaderopentdf/platformrename WASM exportsmalloc/free→tdf_malloc/tdf_freeto avoid TinyGo wasi-libc conflicts, and add TinyGo reactor mode (-buildmode=c-shared)🤖 Generated with Claude Code