kylefarris / clamscan

A robust ClamAV virus scanning library supporting scanning files, directories, and streams with local sockets, local/remote TCP, and local clamscan/clamdscan binaries (with failover).
MIT License
236 stars 69 forks source link

Premature end of ClamAV socket stream behind telepresence proxy #101

Open martijnvanderwoud opened 2 years ago

martijnvanderwoud commented 2 years ago

Hello and thanks for providing this library!

I have an issue that is NOT very problematic per se, but still it might be useful to report it, because maybe it could help uncover a subtle (timing?) issue with the ClamAV socket connection

When I use the scanStream method like this ...:

isInfected: async (contents: Buffer): Promise<void> => {
    let chunk: Buffer | null = contents
    const inputStream = new Readable()
    inputStream._read = () => {
        inputStream.push(chunk)
        chunk = null
    }

    const result = await clamscan.scanStream(inputStream)
    console.log(result)
}

... the scan fails with an empty response when I am behind a telepresence proxy:

node-clam: Provided stream is readable.
node-clam: Attempting to establish socket/TCP connection for "scanStream"
node-clam: using remote server: 10.96.253.36:3310
node-clam: Received final data from stream.
node-clam: The input stream has dried up.
node-clam: ClamAV is done scanning.
node-clam: Raw Response:   
node-clam: Error Response:  
node-clam: File may be INFECTED!
{
  isInfected: null,
  viruses: [],
  file: null,
  resultString: '',
  timeout: false
}
node-clam: Socket/Host connection closed.
node-clam: ClamAV socket has been closed! false

When running the samen within my minikube cluster, without the telepresence proxy, the scan works just fine:

node-clam: Provided stream is readable.
node-clam: Attempting to establish socket/TCP connection for "scanStream"
node-clam: using remote server: 10.96.253.36:3310
node-clam: Received final data from stream.
node-clam: The input stream has dried up.
node-clam: Received output from ClamAV Socket.
node-clam: ClamAV is done scanning.
node-clam: Raw Response:  stream: OK
node-clam: File is OK!
{
  isInfected: false,
  viruses: [],
  file: null,
  resultString: 'stream: OK\x00',
  timeout: false
}
node-clam: Socket/Host connection closed.
node-clam: ClamAV socket has been closed! false

What is also interesting is that the scan does complete successfully behind the telepresence proxy when I put a breakpoint on the chunk = null statement and let the debugger proceed step-by-step, which lets me think a timing issue might be the cause of a premature end of the ClamAV socket stream.

The behaviour is the same when I write the Buffer to a temporary file and then call the clamscan.isInfected method

I also tried using the passthrough method ...:

isInfected: (contents: Buffer) =>
    new Promise<boolean>((resolve, reject) => {
        let chunk: Buffer | null = contents
        const inputStream = new Readable()
        inputStream._read = () => {
            inputStream.push(chunk)
            chunk = null
        }
        const clamAVStream = clamscan.passthrough();
        inputStream.pipe(clamAVStream)
        clamAVStream
            .on("scan-complete", (result) => {
                const infected = result.isInfected;
                if (infected !== null) {
                    logger.debug(`Scan complete; contents infected: ${infected}`)
                    resolve(infected)
                }
            })
            .on('error', (error) => {
                reject(error)
            })
            .on('timeout', (error) => {
                const timeoutError = error || new Error("Scan timed out")
                reject(timeoutError)
            })
    })

... and found that it consistently succeeds, both behind the telepresence proxy, and running within minikube without the proxy:

node-clam: Attempting to establish socket/TCP connection for "passthrough"
node-clam: using remote server: 10.96.253.36:3310
node-clam: ClamAV Socket Initialized...
node-clam: Doing initial transform!
node-clam: Done with the full pipeline.
node-clam: Got result! stream: OK
node-clam: File is OK!
node-clam: Processed Result:  {
  isInfected: false,
  viruses: [],
  file: null,
  resultString: 'stream: OK\x00',
  timeout: false
} stream: OK
node-clam: ClamAV socket has received the last chunk!
node-clam: File is OK!
node-clam: Result of scan: {
  isInfected: false,
  viruses: [],
  file: null,
  resultString: 'stream: OK\x00',
  timeout: false
}
node-clam: It took 0 seconds to scan the file(s).
node-clam: Socket/Host connection closed.
node-clam: ClamAV socket has been closed! Because of Error: false
kylefarris commented 2 years ago

@martijnvanderwoud I don't really have the platform to test this specific issue. If you have the time and bandwidth to look into the solution for this issue, I'd be more than happy to work with you towards merging a fix through a PR.

martijnvanderwoud commented 2 years ago

@kylefarris Sure, if you can point me in the right direction on what could cause the issue and how it could be fixed (I have no clue at the moment), I would be happy to spend some time testing/debugging this

kylefarris commented 2 years ago

One thing that stands out as quite odd and almost-certainly unnecessary is this block of code in your examples:

inputStream._read = () => {
  inputStream.push(chunk)
  chunk = null
}

Try converting your Buffer to a Readable stream directly (Node 10.17.0 and up) and then pass that to scanStream:

isInfected: async (contents: Buffer): Promise<void> => {   
    const inputStream = Readable.from(contents)
    const result = await clamscan.scanStream(inputStream)
    console.log(result)
}
martijnvanderwoud commented 2 years ago

@kylefarris Thanks, that is much simpler indeed. I had to install @types/node as a devDependency before I could use the Readable.from method. Unfortunately, the result is still the same:

node-clam: Provided stream is readable.
node-clam: Attempting to establish socket/TCP connection for "scanStream"
node-clam: using remote server: 10.97.137.168:3310
node-clam: Received final data from stream.
node-clam: The input stream has dried up.
node-clam: ClamAV is done scanning.
node-clam: Raw Response:   
node-clam: Error Response:  
node-clam: File may be INFECTED!
{
  isInfected: null,
  viruses: [],
  file: null,
  resultString: '',
  timeout: false
}
node-clam: Socket/Host connection closed.
node-clam: ClamAV socket has been closed! false
kylefarris commented 2 years ago

Well, that stinks. At least your code is prettier now, haha.

kylefarris commented 2 years ago

As awful as it might sound, it looks like you're just going to have to drop some console.log lines in the node_modules/clamscan/index.js file of your project and just do some good ol'-fashioned debugging.

martijnvanderwoud commented 2 years ago

I already stepped through that code with my debugger, but was not able to detect the cause of the issue. Of course it does not help that I am not familiar with the code, which makes it hard to spot unintended behaviour. Maybe, if you have the time, and if you think it is worth the effort, I could share my screen during a debug session and we can take a look together?

kylefarris commented 2 years ago

Yeah, sorry about that. I see where you were talking about walking through the code and how you found out at what point it seemed to be failing. Of course, that part is gone now 😬.

Where is your instance of ClamAV? A remote server?

martijnvanderwoud commented 2 years ago

My instance of ClamAV is running inside a Kubernetes cluster. Locally, I deploy it to a single-node cluster created with minikube

kylefarris commented 2 years ago

So, you connect to it in your code with an IP & port, correct? And, if so, is the proxy situated between your code and the ClamAV instance? I'm just to get a better idea of topography of your setup.

martijnvanderwoud commented 2 years ago

Yes, that is correct, I connect to ClamAV with an IP (well, a host name, actually) and a port. The IP is a virtual IP assigned by the Kubernetes network proxy (see https://kubernetes.io/docs/concepts/services-networking/service/#virtual-ips-and-service-proxies). Normally, this IP is resolved within the internal network of the Kubernetes cluster, and it is not reachable from outside the cluster. In production, my NodeJS app with clamscan is running within the Kubernetes cluster, and clamscan connects to the hostname / virtual IP assigned to the ClamAV service. In this situation, the premature end of the socket stream does NOT happen and the scanStream method works just fine.

To be able to run my NodeJS application locally without deploying it to Kubernetes first, I use telepresence connect to route traffic from my local machine to the ClamAV service running inside Kubernetes. See https://www.telepresence.io/docs/latest/howtos/outbound/. So, in this situation, there is a Traffic Manager running inside Kubernetes. Telepresence adds a network interface to my machine so that traffic to the ClamAV virtual IP is routed to the Traffic Manager, which forwards it to the appropriate service inside the Kubernetes cluster. In this situation, the premature end of the socket DOES happen and the scanStream method does NOT work

martijnvanderwoud commented 2 years ago

Do you still want to proceed with this issue @kylefarris ?

kylefarris commented 2 years ago

I don't have the bandwidth or setup to test and fix this issue right now. If you do, please see if you can fix it and submit a PR.

martijnvanderwoud commented 2 years ago

As I said earlier, I need a shared debug session or at least some pointers from you before I can work on this. Un-assigning myself for now

AishwaryaKotla commented 1 year ago

@martijnvanderwoud I am facing the same issue - end of ClamAv socket connection, which is working fine using a debugger. Could you please provide the fix, if you were you able to resolve this issue?

martijnvanderwoud commented 1 year ago

@AishwaryaKotla unfortunately, no. I ended up using the passthrough method (see my first comment):

isInfected: (contents: Buffer) =>
    new Promise<boolean>((resolve, reject) => {
        let chunk: Buffer | null = contents
        const inputStream = new Readable()
        inputStream._read = () => {
            inputStream.push(chunk)
            chunk = null
        }
        const clamAVStream = clamscan.passthrough();
        inputStream.pipe(clamAVStream)
        clamAVStream
            .on("scan-complete", (result) => {
                const infected = result.isInfected;
                if (infected !== null) {
                    logger.debug(`Scan complete; contents infected: ${infected}`)
                    resolve(infected)
                }
            })
            .on('error', (error) => {
                reject(error)
            })
            .on('timeout', (error) => {
                const timeoutError = error || new Error("Scan timed out")
                reject(timeoutError)
            })
    })
AishwaryaKotla commented 1 year ago

@martijnvanderwoud Thanks for providing your input. Using the passthrough method worked for my use case too. Really appreciate your prompt response :)

origooo commented 6 months ago

I think this relates to an open issue over at Cisco-Talos/clamav. As has been discussed in another issue here by @benzino77, it seems that ClamAV has a timing error and sometimes closes the socket before the reply is sent back to the client.

It has been flagged as a bug by the maintainers.