Repository navigation
Conversation
…} entry like Node's setKey() normalizePemKeyOption() checks the type of the key of an entry and then the type of its passphrase, with Node's property names, before the key goes to createPrivateKey() or to the native config. An entry with a passphrase in effect is decrypted there, so its decrypt error comes before the next entry, as in Node. The native config loads an entry with no passphrase after every entry is checked. A per-key passphrase that is null no longer becomes the passphrase of the context.
Collaborator
Author
Collaborator
Author
|
Status (checked on 2026-11-07)
|
This branch has not been deployed
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.
Draft until #46522 merges: this needs its
https.createServer()order, HTTP options before TLS options.Problem
{ pem, passphrase }entry ofkeythrows the error ofcrypto.createPrivateKey():The property 'options.passphrase' is invalid. Received 5, or one that names"key.key". Node throwsERR_INVALID_ARG_TYPEforoptions.keyoroptions.passphrase.normalizePemKeyOption(src/js/internal/tls.ts:339). It checks nothing itself.Fix
normalizePemKeyOptionchecks the key of each entry, then its passphrase, as Node'ssetKey()does.passphrase: nullno longer becomes the context passphrase.node-tls-create-secure-context-args.test.ts(24 rows at 8 entry points, in Bun and in Node, 137 tests fail without the fix),ws.test.ts,node-fetch.test.js.ws://URLs, a tracking issue (Notes).Background
throwOnInvalidTLSArraygate, ahead of thecacheck and thepfxkeys: 30 of 252 doubly-invalid rows move away from Node.Downsides
passphrase: nullon an encrypted key next to a context passphrase, a SharedArrayBufferpemwith a passphrase.https.createServer()now throws for apemthat is not a key. It emitted'error'atlisten().createServercalls report the key error first, until net, tls: consolidate the open node:net, node:tls, Bun.connect, Bun.listen and usockets pull requests #46522. Builtin JS: +691 bytes.Notes
There is no issue for this. A differential run against Node found it.
The report,
tls.createSecureContext({ key: [entry] }):{ pem: enc, passphrase: 5 }ERR_INVALID_ARG_TYPEThe "options.passphrase" property must be of type string. Received type number (5)ERR_INVALID_ARG_VALUEThe property 'options.passphrase' is invalid. Received 5{ pem: 5, passphrase: "x" }ERR_INVALID_ARG_TYPEThe "options.key" property must be of type string or an instance of Buffer, TypedArray, or DataView. Received type number (5)ERR_INVALID_ARG_TYPEThe "key.key" property must be of type string or an instance of ArrayBuffer, Buffer, TypedArray, or DataView. Received type number (5){ pem: 5, passphrase: 5 }options.keyerrorkey.keyerror{ pem: enc, passphrase: Buffer.from("pw") }ERR_INVALID_ARG_TYPE... Received an instance of Buffer{ pem: 5 }options.keyerrorERR_INVALID_ARG_TYPEelement of TLSOptions.key must be of type string, ArrayBuffer, or Blob{ pem: enc, passphrase: "pw" }Every entry point shares the function and shows the same rows:
tls.createSecureContext(),tls.connect(),new tls.TLSSocket(),tls.createServer(),server.setSecureContext(),server.addContext(),https.createServer(),http2.createSecureServer(), and the agent options ofwsandnode-fetch.What the function does now. For each entry, in order: the key is
entry.pem, or the entry itself whenpemis undefined. The passphrase isentry.passphrase, or the passphrase of the context when it is undefined. The key must be a string, a view, an ArrayBuffer, or a BunFile. Then the passphrase must be a string. An entry with a passphrase in effect is decrypted there, so its decrypt error comes before the next entry, as in Node. The native config loads an entry with no passphrase after every entry is checked, so its load error comes after the type error of a later entry (Node: before).Measured (debug build, merge base 7ca0042 against this branch, Node v26.3.0 and v26.12.1 give identical output):
createSecureContext: 16 -> 8 rows differ from Node.createSecureContext(1292 cells): equal to Node 152 -> 1001. 228 cells go from accepted to refused (the three kinds under Downsides). 0 go from refused to accepted.https.createServer/http.createServerwith a second invalid HTTP option (432 cells): equal to Node 164 -> 114 on main's layout, 64 from right to wrong. With the HTTP options checked first, as net, tls: consolidate the open node:net, node:tls, Bun.connect, Bun.listen and usockets pull requests #46522 does: 164 -> 366, 8 from right to wrong.createPrivateKey()calls per entry: unchanged (1 with a passphrase in effect, 0 without). One more for an entry withpassphrase: nullnext to a context passphrase.tls.createSecureContext()with the JIT off: 8.86 -> 7.86 with no key, 8.44 -> 7.44 with a string key. The loop has no closure. With the JIT on, main loses that allocation too after it tiers up.internal/tls.js).size build/release/bun: text 140,289,307 -> 140,289,998 (+691), data and bss unchanged, file size unchanged.Behaviour changes, all of them toward Node:
ERR_INVALID_ARG_TYPE).passphrase: nullon an encrypted key next to a context passphrase (ERR_OSSL_BAD_DECRYPT). A SharedArrayBufferpemwith a passphrase in effect (ERR_INVALID_ARG_TYPE).pemthat is not a key.https.createServer()andhttp.createServer()throw from the constructor for an entry with no passphrase whosepemis not a key. They emitted'error'atlisten(). The error of a key that does not load still arrives atlisten().http.createServer({ key, cert })serves TLS in Bun on purpose, so it refuses the same entries. Node ignores the key there.wsandnode-fetchcheck the options forws://andhttp://URLs too. Node never readskeyfor those. The inputs that are newly refused now throw there.pemand a BunFilepemstay accepted, as for a key that is not in an entry. Only the native config reads a file, so a BunFile with a passphrase of its own is refused, and a BunFile withpassphrase: nullstill gets the passphrase of the context.Tests. Without the
internal/tls.tschange these fail: 16 rows x 8 entry points, 2 rows x 4 entry points next to an emptypfxlist, thehttp.createServer()test (137 tests), thewstest and thenode-fetchtest. The rest are guards: the accepted rows, the handshakes, the rows where main already gave Node's error, the caller's array left as written, and no key handed to a setter ofArray.prototype. 32todotests hold Node's answer for 4 rows that Bun does not give yet. "node gives the same answers" runs every row in Node 26 where it is installed. 23 single-clause mutants of the function each fail at least one test.Self-review, the two items not done. A test that pins the new throw on a
ws://URL: Node never readskeythere, so the test would pin a difference from Node. A tracking issue: the list below is the record.Not in this change:
throwOnInvalidTLSArrayrefuses it. The same check refuses{ pfx, key: [entry] }ontls.createServer(),setSecureContext(),https.createServer()andhttp2.createSecureServer().{ pem }objects incert,caandcrl, and nestedkeyarrays, still get the native text.pfx: [{ buf, passphrase }]entries are not checked.passphrasethat no key uses is still checked. Node checks it only when a key uses it.wsandnode-fetchtake an entry that is not in an array. Node refuses it.wsthrows from the constructor for an option of its own. npmwsemits'error'.https-proxy-agentpath ofwsgoes through the same function. No test pins it.Order with #46522 and #46528. Only
src/js/internal/tls.tschanges, and the hunk applies to both heads (ddfd2d3, edb9ce8). A trial merge of this branch adds no conflicted file to either. On #46522's layoutnewNativeSecureContextcalls the function beforenativeCiphersAndVersions(), so the cut on top of it has to put the version checks first. Commit d2ee57f on branchrobobun/b79b9382/tls-key-entries-with-http-orderis an earlier cut for main alone: it also movesstoreHTTPOptionsahead of the TLS options inhttp.Server.#33535 (not merged) had the same two checks in another place. The variant with the checks in the gate moved 30 of 252 rows away from Node and 40 toward it, refused
[{ pem: DataView }], and took an entry outside an array next topfxat 4 entry points. Native per-key passphrases were also considered: the reported errors come from JS, so that change alone corrects none of them.Also run:
test-tls-passphrase.js,test-tls-basic-validations.js,test-tls-options-boolean-check.js,test-https-options-boolean-check.jsand 16 more vendored TLS files.node-tls-server.test.ts,node-tls-context.test.ts,node-https-server.test.ts,ws-proxy.test.ts.