Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TCP to TLS socket upgrade using the node compatibility layer is broken #27087

Closed
ufr-scf opened this issue Nov 26, 2024 · 3 comments · Fixed by #27125
Closed

TCP to TLS socket upgrade using the node compatibility layer is broken #27087

ufr-scf opened this issue Nov 26, 2024 · 3 comments · Fixed by #27125
Labels
bug Something isn't working correctly node compat tls Issues related to TLS implementation

Comments

@ufr-scf
Copy link

ufr-scf commented Nov 26, 2024

Version: Deno 2.0.3 and later
Reproduction: https://github.com/ufr-scf/deno-tls-upgrade-error

The following code used to work on version 2.0.2 and below:

import { Socket } from "node:net";
import tls from "node:tls";

const socket = new Socket();
socket.connect(443, "google.com");
socket.on("connect", () => {
  const secure = tls.connect({ socket });
  secure.on("secureConnect", () => console.log("tls connected"));
})

On version 2.0.3 and later it throws the following error:

error: Uncaught Error: Client network socket disconnected before secure TLS connection was established
    at connResetException (ext:deno_node/internal/errors.ts:1897:14)
    at TLSSocket.onConnectEnd (node:_tls_wrap:32:19)
    at TLSSocket.emit (ext:deno_node/_events.mjs:405:35)
    at endReadableNT (ext:deno_node/_stream.mjs:3210:16)
    at processTicksAndRejections (ext:deno_node/_next_tick.ts:36:15)
    at runNextTicks (ext:deno_node/_next_tick.ts:76:3)
    at eventLoopTick (ext:core/01_core.js:182:21)

The commit after which this started occuring is this one: c553666.

@ufr-scf
Copy link
Author

ufr-scf commented Nov 26, 2024

Debugging the code with breakpoints on caught exceptions reveals some BadResource exceptions:

Exception has occurred: BadResource: Bad resource ID
  at Object.op_read (ext:core/00_infra.js:260:33)
    at TcpConn.read (ext:deno_net/01_net.js:130:26)
    at TCP.#read (ext:deno_node/internal_binding/stream_wrap.ts:326:45)
    at TCP.readStart (ext:deno_node/internal_binding/stream_wrap.ts:143:17)
    at _tryReadStart (node:net:789:31)
    at Socket._read (node:net:1817:7)
    at Socket.Readable.read (ext:deno_node/_stream.mjs:2561:16)
    at Socket.read (node:net:1736:34)
    at _afterConnect (node:net:384:14)
    at _afterConnectMultiple (node:net:486:3)

Another one:

Exception has occurred: BadResource: Bad resource ID
  at TcpConn.close (ext:deno_net/01_net.js:145:10)
    at TCP._onClose (ext:deno_node/internal_binding/stream_wrap.ts:304:31)
    at TCP._onClose (ext:deno_node/internal_binding/tcp_wrap.ts:498:47)
    at TCP.close (ext:deno_node/internal_binding/handle_wrap.ts:41:10)
    at TLSSocket._destroy (node:net:1838:20)
    at _destroy (ext:deno_node/_stream.mjs:1539:15)
    at TLSSocket.destroy (ext:deno_node/_stream.mjs:1509:9)
    at TLSSocket.onConnectEnd (node:_tls_wrap:59:10)
    at TLSSocket.emit (ext:deno_node/_events.mjs:405:35)
    at endReadableNT (ext:deno_node/_stream.mjs:3210:16)

@ufr-scf
Copy link
Author

ufr-scf commented Nov 26, 2024

Running the code with NODE_DEBUG="net,tls,stream" produces the following output:

NET 96365: 'pipe' false undefined
NET 96365: 'connect: find host' 'google.com'
NET 96365: 'connect: dns options' { family: undefined, hints: 64, all: false }
NET 96365: 'connect: autodetecting'
NET 96365: 'connect/multiple: will try the following addresses' [
  { address: '142.250.185.110', family: 4 },
  { address: '2a00:1450:4001:828::200e', family: 6 }
]
NET 96365: 'connect/multiple: attempting to connect to %s:%d (addressType: %d)' '142.250.185.110' 443 4
NET 96365: 'connect/multiple: setting the attempt timeout to %d ms' 250
NET 96365: 'connect/multiple: connection attempt to %s:%s completed with status %s' '142.250.185.110' 443 0
NET 96365: 'afterConnect'
STREAM 96365: 'read' 0
STREAM 96365: 'need readable' false
STREAM 96365: 'length less than watermark' true
STREAM 96365: 'do read'
NET 96365: '_read'
NET 96365: 'Socket._handle.readStart'
STREAM 96365: 'readableAddChunk' null
STREAM 96365: 'onEofChunk'
STREAM 96365: 'emitReadable' false false
STREAM 96365: 'emitReadable' null
STREAM 96365: 'read' 0
STREAM 96365: 'endReadable' false
STREAM 96365: 'emitReadable_' false 0 true
STREAM 96365: 'flow' null
STREAM 96365: 'endReadableNT' false 0
NET 96365: 'destroy'
NET 96365: 'close'
NET 96365: 'close handle'
error: Uncaught Error: Client network socket disconnected before secure TLS connection was established
    at connResetException (ext:deno_node/internal/errors.ts:2620:14)
    at TLSSocket.onConnectEnd (node:_tls_wrap:50:24)
    at TLSSocket.emit (ext:deno_node/_events.mjs:405:35)
    at endReadableNT (ext:deno_node/_stream.mjs:3210:16)
    at processTicksAndRejections (ext:deno_node/_next_tick.ts:53:15)
    at runNextTicks (ext:deno_node/_next_tick.ts:96:3)
    at eventLoopTick (ext:core/01_core.js:182:21)

@lucacasonato lucacasonato added bug Something isn't working correctly tls Issues related to TLS implementation node compat labels Nov 27, 2024
littledivy added a commit that referenced this issue Nov 28, 2024
bartlomieju pushed a commit that referenced this issue Nov 28, 2024
@ufr-scf
Copy link
Author

ufr-scf commented Dec 2, 2024

Thanks a lot for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly node compat tls Issues related to TLS implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants