-
Notifications
You must be signed in to change notification settings - Fork 76
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
Premature end of ClamAV socket stream behind telepresence proxy #101
Comments
@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. |
@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 |
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 isInfected: async (contents: Buffer): Promise<void> => {
const inputStream = Readable.from(contents)
const result = await clamscan.scanStream(inputStream)
console.log(result)
} |
@kylefarris Thanks, that is much simpler indeed. I had to install
|
Well, that stinks. At least your code is prettier now, haha. |
As awful as it might sound, it looks like you're just going to have to drop some |
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? |
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? |
My instance of ClamAV is running inside a Kubernetes cluster. Locally, I deploy it to a single-node cluster created with minikube |
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. |
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 |
Do you still want to proceed with this issue @kylefarris ? |
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. |
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 |
@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? |
@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)
})
}) |
@martijnvanderwoud Thanks for providing your input. Using the passthrough method worked for my use case too. Really appreciate your prompt response :) |
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. |
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 ...:... the scan fails with an empty response when I am behind a telepresence proxy:
When running the samen within my minikube cluster, without the telepresence proxy, the scan works just fine:
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
methodI also tried using the passthrough method ...:
... and found that it consistently succeeds, both behind the telepresence proxy, and running within minikube without the proxy:
The text was updated successfully, but these errors were encountered: