Repository navigation
fix(nodejs)!: complete TCP JWK authentication support - #73
glasstiger wants to merge 4 commits into
Conversation
Accept full P-256 private JWKs through Sender and SenderOptions factories, retaining TCP/TCPS credential precedence and rejecting the new extra option on unsupported transports.\n\nUse SHA256 for EC challenge signing, verify factory handshakes with generated keys, pin the published declarations, and refresh the documentation.\n\nFixes #72
Review: approveReviewed at level 3, comparing No actionable defects found in JWK forwarding, credential precedence, TCP/TCPS signing, transport rejection, caller compatibility, or published declarations. ValidationAll checks passed locally on Node.js 24.3.0:
Regression-test efficacy was checked in isolation: removing JWK forwarding, removing the transport guard, or restoring the old digest identifier each caused the corresponding new tests to fail. Summary
Disposable worktrees were removed. The primary Git working tree remains clean. |
A JWK passed through extraOptions.jwk or the programmatic jwk option was first used when the server's challenge arrived, inside a socket listener. A key that Node could not import, such as one without kty and crv, escaped connect() as an unhandled rejection and terminated the process. Validate the key when the TCP transport is created: it must be an object with a non-empty kid, kty set to EC, crv set to P-256, and d, x and y that form a P-256 key pair. The errors never quote the key, which carries the private scalar. List kty and crv in the option's documentation, note the validation in the READMEs, drop the README's reference to an undocumented workaround, and regenerate the API reference.
validateJwk() relied on crypto.createPrivateKey() to prove that d, x and y form a P-256 key pair, but Node.js enforces that only from v26. Up to v24 it imports a JWK whose x and y belong to another key, and a d outside the curve's range, such as one written in hex, which crypto.sign() then rejects at the server's challenge. That error still escaped connect() as an unhandled rejection and ended the process. Derive the public point from d with ECDH, which rejects an out-of-range scalar on every version, and require it to equal x and y. The members are compared as integers, the way Node.js decodes them, so an encoding with a leading zero byte still names the same key. Catch errors from the challenge-response handshake in the socket listener: connect() now rejects with the error and destroys the socket instead of the process terminating. BREAKING CHANGE: Creating a TCP/TCPS sender now validates a full JWK passed through the jwk option or extraOptions.jwk, and throws when it is incomplete, not an EC P-256 key, or not a key pair. Such keys previously failed when the server's challenge arrived, ending the process, and a JWK whose x and y belong to a different key than d authenticated on Node.js 20 to 24. extraOptions.jwk is rejected for transports other than TCP/TCPS.
Review: approveCompleted a level 3 review, comparing No actionable defects found in JWK forwarding, credential precedence, key validation, TCP/TCPS signing and error handling, caller compatibility, or published declarations. ValidationPassed locally on Node.js 24.3.0:
The 96 focused options/transport tests also passed on Node.js 22.17.0. Regression-test efficacy was checked in isolation: removing forwarding, transport rejection, key validation, pair consistency, integer-coordinate handling, or handshake error containment—and restoring the old digest identifier—caused the corresponding tests to fail. Summary
Disposable worktrees were removed. The primary working tree remains clean. |
Review: approveCompleted a level 3 review, comparing Nothing blocking was found. The only item is an optional commit-message fix. CriticalNone. ModerateNone. Minor
CoverageThe test gate passes, with no coverage gaps.
ValidationRun locally, comparing base and head:
Summary
|
Summary
Completes the two remaining TCP authentication items from #72. The primary public/private-key consistency fix is already on main.
extraOptions.jwkinSender.fromConfig(),Sender.fromEnv(), and theSenderOptionsconstructor and factories.username/token, and reject the new extra option outside TCP/TCPS rather than silently ignoring authentication.kid,kty: "EC",crv: "P-256", andd,xandythat form a P-256 key pair. Previously the key was first used when the server's challenge arrived, inside a socket listener, where an error escapedconnect()as an unhandled rejection and terminated the process. That happened for a key Node could not import, such as one withoutktyandcrv, and on Node.js 20 to 24 also for adoutside the curve's range, such as one written in hex, which Node imports but cannot sign with. The factories andnew Sender({ jwk })now fail with an error that can be caught, and the error never quotes the key.crypto.createPrivateKey(), which enforces it only from Node.js 26. The client derives the public point fromdwith ECDH and compares it withxandyas integers, the way Node decodes them, so the same keys are accepted on every supported version.connect()rejects with the error and destroys the socket instead of the process terminating.SHA256instead of the legacyRSA-SHA256alias for EC challenge signing. The signatures are unchanged: both names produce DER-encoded ECDSA-SHA256 on Node.js 20, 22, 24 and 26.d), cover a failed handshake, and pin the published TypeScript declarations.Breaking changes
CONTRIBUTING.md treats a change to the errors a published API throws as breaking, even when it is a fix:
jwkoption orextraOptions.jwk, now throws when the key is incomplete, not an EC P-256 key, or not a key pair. Such keys previously failed when the server's challenge arrived, terminating the process, and a JWK whosexandybelong to a different key thandauthenticated on Node.js 20 to 24.extraOptions.jwkis rejected for transports other than TCP/TCPS.Validation
Closes #72.