mitm doesn't intercept connection from Net.Socket#connect()

[VPagani]

After writing some tests using mitm, I stumbled upon this simple problem:

Net.Socket#connect() isn't stubbed by mitm, therefore the connection won't be intercepted.

const Net = require('net')
const socket = new Net.Socket()

socket.connect(22, 'example.org') // Not intercepted

[moll]

Hey,

Thanks for taking the time to create an issue. That's indeed the case right now. Mitm.js currently creates a new Socket itself and never calls its connect. I'll have to look into various Node implementations again to see if and how I can hook into an existing Socket rather than creating one. Until then, could you use Net.connect? It's got the function signature as Socket.prototype.connect.

[VPagani]

The only problem for me using Net.connect() is not beign able to disconnect and reconnect the socket without having to reattach the event handlers every time on the new socket created, because the socket client and some of these event handlers are created by an external package, so it is a lot easier to keep the same socket and just turn it on and off with socket.connect() and socket.end().

I've made a shitty workaround based on mitm code that works just for the connect test purpose:

const Socket = require('net').Socket

describe('Some Socket Class', () => {
	
    beforeEach(function() {
        this.mitm = Mitm()
        this.mitm.stubs.stub(
            Socket.prototype, 'connect',
            this.mitm.tcpConnect.bind(this.mitm, Socket.prototype.connect)
        )
    })
}

But the problem is that any other interaction between the socket and the fake server created by mitm is ignored as the new Socket client object returned from mitm.tcpConnect() is not used.

Maybe the solution is to replace the Net.Socket by a mocked one:

require('net').Socket = mitmSocket

I don't know for other Node implementations. What do you think?

[moll]

You say you're reusing the same Socket for multiple connections to the same host? I'm surprised that works. I remember seeing the Socket itself was a Writable Stream and I have a vague recollection from reading Node's code that once you call end on a Writable Stream, you can't restart it. Has reusing a Socket always worked for you?

Replacing Net's Socket like above won't have the same effect as all Node's internal references to that variable have already been set up (that is, bound) once Mitm.js's code runs. Calls to Socket.prototype.connect tend to be lazily bound, that's why your work-around above may work. I'll have to test converting an existing socket to a faked socket. If you read the code, it's actually the handle property that Mitm.js is passing to Socket's constructor that makes it a fake socket.

[VPagani]

I'm using Node v6.9.1. I don't know much about how sockets internally work, but yeah, multiple connections work in this version. I have a working script based on that for reconnecting the socket whenever an error or connection lost happens. Similar to the following code:

socket.on('close', (error) => {
    this.connected = false
    if (error) log.error(error)
    
    this.attemptToReconnect = setInterval(() => {
        if (!this.connected && !socket.connecting) {
            socket.connect(this.host, this.port, () => {
                this.connected = true
                clearInterval(this.attemptToReconnect)
            })
        }
    }, 2000)
})

Maybe it isn't the best way to do it, but it works for me since I started playing with these things (about 4 months ago).

I've created also a function that changes the host that it's connected to by calling socket.end() then socket.connect(newPort, newHost).

The node documentation says that socket.end() just half-closes the socket sending a FIN packet, but if you call socket.destroy() then it wouldn't be possible to connect again with the same socket.

[moll]

Sending traffic also works, right? It's a little iffy to be honest, because once Socket.prototype.close is called (to close the reading side of the socket), the close event is also emitted and the semantics for it state "The event indicates that no more events will be emitted, and no further computation will occur.". Reusing an existing socket breaks that contract, so it's likely not all consumers will work as expected if stream restarts like it.

However, I don't see a problem of supporting Socket.prototype.connect as a hijackable interface. If it ends up working for reconnects, all the better for you I guess. :)

[m-szyszka]

Hey @moll I'm using “mitm” lib to intercept requests and I can see it's not always working with "redis" connections. Connections are bypassed by default. I'm using this library to connect to redis: https://github.com/redis/node-redis . This library use internally node "net" module for sockets https://github.com/redis/node-redis/blob/c1fd86778a71072a805cbb0cf238bc38f387eea2/packages/client/lib/client/socket.ts#L2 I’m logging “connect” event and there are no logs for redis connections, but they are for other dependencies. Maybe it’s worth to add, that it’s working in Azure Function App. Could this issue be related to this thread?

[moll]

@m-szyszka, I'm not sure if they're related. It could also be related to Redis using ES6 imports. I don't know if import * as net lazily binds the exports from the Node.js' net module or not. As you might know, Mitm.js swaps out Net.connect with its hooked variant. If a library saves a reference to Net.connect prior to Mitm.js intercepting, it'll still use the original function.

You're trying to intercept the Redis library's connections, right? To debug this, I suppose you could try to add a few print statements to Redis' code and log out the Net.connect function --- see if it's the hooked version Mitm.js creates when you call Mitm.enable or not. You can probably create a few line file to try to reproduce this.

[m-szyszka]

@moll You are right, problem was that redis library saves reference to Net.connect prior to my Mitm.js intercepting. I solved it by refactoring and changing order. Thanks!