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.

Sign up for GitHub

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

Jump to bottom

Antivirus relies on connection close to get ICAP result #6764

Closed
wkloucek opened this issue Jul 10, 2023 · 11 comments · Fixed by #8062
Closed

Antivirus relies on connection close to get ICAP result #6764

wkloucek opened this issue Jul 10, 2023 · 11 comments · Fixed by #8062
Assignees
@fschade
Labels
Priority:p2-high Escalation, on top of current planning, release blocker Topic:Performance Type:Bug

Comments

@wkloucek
Copy link
Contributor

wkloucek commented Jul 10, 2023 •
edited

Describe the bug

ICAP supports reusing a connection. When ICAP is not explicitly configured to close the connection (= send FIN) after a scan, the antivirus service hangs.

Steps to reproduce

Steps to reproduce the behavior:

  1. Have a oCIS with antivirus service
  2. Have a ClamAV which does not close the connection after a scan
  3. Upload a file

Expected behavior

The virusscan is finished as soon as the ICAP response is received.

Actual behavior

The virusscan takes as long as the connection is not interrupted, timed out or closed. (= eg. receives FIN). In the case of the wireshark dump below with connection reuse and keepalive it takes until 47 / 300 seconds for the antivirus service to actually use the result from 6 / 0.002026783 seconds.

Wireshark dumps

ICAP with disable connection reuse

1	0.000000000	172.18.0.1	172.18.0.2	TCP	74	57616 → 1344 [SYN] Seq=0 Win=64240 Len=0 MSS=1460 SACK_PERM TSval=442762913 TSecr=0 WS=128
2	0.000071674	172.18.0.2	172.18.0.1	TCP	74	1344 → 57616 [SYN, ACK] Seq=0 Ack=1 Win=65160 Len=0 MSS=1460 SACK_PERM TSval=3142759142 TSecr=442762913 WS=128
3	0.000104416	172.18.0.1	172.18.0.2	TCP	66	57616 → 1344 [ACK] Seq=1 Ack=1 Win=64256 Len=0 TSval=442762913 TSecr=3142759142
4	0.000725205	172.18.0.1	172.18.0.2	ICAP	329	REQMOD icap://localhost:1344/avscan ICAP/1.0
5	0.000793593	172.18.0.2	172.18.0.1	TCP	66	1344 → 57616 [ACK] Seq=1 Ack=264 Win=65024 Len=0 TSval=3142759143 TSecr=442762914
6	0.002201844	172.18.0.2	172.18.0.1	ICAP	179	ICAP/1.0 204 Unmodified
7	0.002269051	172.18.0.1	172.18.0.2	TCP	66	57616 → 1344 [ACK] Seq=264 Ack=114 Win=64256 Len=0 TSval=442762915 TSecr=3142759144
8	0.002631282	172.18.0.2	172.18.0.1	TCP	66	1344 → 57616 [FIN, ACK] Seq=114 Ack=264 Win=65024 Len=0 TSval=3142759145 TSecr=442762915
9	0.002995268	172.18.0.1	172.18.0.2	TCP	66	57616 → 1344 [FIN, ACK] Seq=264 Ack=115 Win=64256 Len=0 TSval=442762916 TSecr=3142759145
10	0.003055210	172.18.0.2	172.18.0.1	TCP	66	1344 → 57616 [ACK] Seq=115 Ack=265 Win=65024 Len=0 TSval=3142759145 TSecr=442762916

ICAP with connection reuse and keepalive

1	0.000000000	172.18.0.1	172.18.0.2	TCP	74	49534 → 1344 [SYN] Seq=0 Win=64240 Len=0 MSS=1460 SACK_PERM TSval=442349969 TSecr=0 WS=128
2	0.000071965	172.18.0.2	172.18.0.1	TCP	74	1344 → 49534 [SYN, ACK] Seq=0 Ack=1 Win=65160 Len=0 MSS=1460 SACK_PERM TSval=3142346199 TSecr=442349969 WS=128
3	0.000098485	172.18.0.1	172.18.0.2	TCP	66	49534 → 1344 [ACK] Seq=1 Ack=1 Win=64256 Len=0 TSval=442349970 TSecr=3142346199
4	0.000697672	172.18.0.1	172.18.0.2	ICAP	329	REQMOD icap://localhost:1344/avscan ICAP/1.0
5	0.000732788	172.18.0.2	172.18.0.1	TCP	66	1344 → 49534 [ACK] Seq=1 Ack=264 Win=65024 Len=0 TSval=3142346199 TSecr=442349970
6	0.002026783	172.18.0.2	172.18.0.1	ICAP	179	ICAP/1.0 204 Unmodified
7	0.002056869	172.18.0.1	172.18.0.2	TCP	66	49534 → 1344 [ACK] Seq=264 Ack=114 Win=64256 Len=0 TSval=442349972 TSecr=3142346200
8	15.700939197	172.18.0.1	172.18.0.2	TCP	66	[TCP Keep-Alive] 49534 → 1344 [ACK] Seq=263 Ack=114 Win=64256 Len=0 TSval=442365670 TSecr=3142346200
9	15.700991416	172.18.0.2	172.18.0.1	TCP	66	[TCP Keep-Alive ACK] 1344 → 49534 [ACK] Seq=114 Ack=264 Win=65024 Len=0 TSval=3142361899 TSecr=442349972
10	31.060941244	172.18.0.1	172.18.0.2	TCP	66	[TCP Keep-Alive] 49534 → 1344 [ACK] Seq=263 Ack=114 Win=64256 Len=0 TSval=442381030 TSecr=3142361899
11	31.061009903	172.18.0.2	172.18.0.1	TCP	66	[TCP Keep-Alive ACK] 1344 → 49534 [ACK] Seq=114 Ack=264 Win=65024 Len=0 TSval=3142377259 TSecr=442349972
12	46.421110326	172.18.0.1	172.18.0.2	TCP	66	[TCP Keep-Alive] 49534 → 1344 [ACK] Seq=263 Ack=114 Win=64256 Len=0 TSval=442396391 TSecr=3142377259
13	46.421374874	172.18.0.2	172.18.0.1	TCP	66	[TCP Keep-Alive ACK] 1344 → 49534 [ACK] Seq=114 Ack=264 Win=65024 Len=0 TSval=3142392620 TSecr=442349972
14	61.781359678	172.18.0.1	172.18.0.2	TCP	66	[TCP Keep-Alive] 49534 → 1344 [ACK] Seq=263 Ack=114 Win=64256 Len=0 TSval=442411751 TSecr=3142392620
15	61.781413920	172.18.0.2	172.18.0.1	TCP	66	[TCP Keep-Alive ACK] 1344 → 49534 [ACK] Seq=114 Ack=264 Win=65024 Len=0 TSval=3142407980 TSecr=442349972
16	77.141104139	172.18.0.1	172.18.0.2	TCP	66	[TCP Keep-Alive] 49534 → 1344 [ACK] Seq=263 Ack=114 Win=64256 Len=0 TSval=442427111 TSecr=3142407980
17	77.141187075	172.18.0.2	172.18.0.1	TCP	66	[TCP Keep-Alive ACK] 1344 → 49534 [ACK] Seq=114 Ack=264 Win=65024 Len=0 TSval=3142423340 TSecr=442349972
18	92.501252805	172.18.0.1	172.18.0.2	TCP	66	[TCP Keep-Alive] 49534 → 1344 [ACK] Seq=263 Ack=114 Win=64256 Len=0 TSval=442442471 TSecr=3142423340
19	92.501342394	172.18.0.2	172.18.0.1	TCP	66	[TCP Keep-Alive ACK] 1344 → 49534 [ACK] Seq=114 Ack=264 Win=65024 Len=0 TSval=3142438700 TSecr=442349972
20	107.860950391	172.18.0.1	172.18.0.2	TCP	66	[TCP Keep-Alive] 49534 → 1344 [ACK] Seq=263 Ack=114 Win=64256 Len=0 TSval=442457830 TSecr=3142438700
21	107.861028849	172.18.0.2	172.18.0.1	TCP	66	[TCP Keep-Alive ACK] 1344 → 49534 [ACK] Seq=114 Ack=264 Win=65024 Len=0 TSval=3142454059 TSecr=442349972
22	123.221161827	172.18.0.1	172.18.0.2	TCP	66	[TCP Keep-Alive] 49534 → 1344 [ACK] Seq=263 Ack=114 Win=64256 Len=0 TSval=442473191 TSecr=3142454059
23	123.221875961	172.18.0.2	172.18.0.1	TCP	66	[TCP Keep-Alive ACK] 1344 → 49534 [ACK] Seq=114 Ack=264 Win=65024 Len=0 TSval=3142469420 TSecr=442349972
24	138.581108258	172.18.0.1	172.18.0.2	TCP	66	[TCP Keep-Alive] 49534 → 1344 [ACK] Seq=263 Ack=114 Win=64256 Len=0 TSval=442488551 TSecr=3142469420
25	138.581163102	172.18.0.2	172.18.0.1	TCP	66	[TCP Keep-Alive ACK] 1344 → 49534 [ACK] Seq=114 Ack=264 Win=65024 Len=0 TSval=3142484780 TSecr=442349972
26	153.941346641	172.18.0.1	172.18.0.2	TCP	66	[TCP Keep-Alive] 49534 → 1344 [ACK] Seq=263 Ack=114 Win=64256 Len=0 TSval=442503911 TSecr=3142484780
27	153.941406704	172.18.0.2	172.18.0.1	TCP	66	[TCP Keep-Alive ACK] 1344 → 49534 [ACK] Seq=114 Ack=264 Win=65024 Len=0 TSval=3142500140 TSecr=442349972
28	169.300910362	172.18.0.1	172.18.0.2	TCP	66	[TCP Keep-Alive] 49534 → 1344 [ACK] Seq=263 Ack=114 Win=64256 Len=0 TSval=442519270 TSecr=3142500140
29	169.300959915	172.18.0.2	172.18.0.1	TCP	66	[TCP Keep-Alive ACK] 1344 → 49534 [ACK] Seq=114 Ack=264 Win=65024 Len=0 TSval=3142515499 TSecr=442349972
30	184.661480736	172.18.0.1	172.18.0.2	TCP	66	[TCP Keep-Alive] 49534 → 1344 [ACK] Seq=263 Ack=114 Win=64256 Len=0 TSval=442534631 TSecr=3142515499
31	184.662123766	172.18.0.2	172.18.0.1	TCP	66	[TCP Keep-Alive ACK] 1344 → 49534 [ACK] Seq=114 Ack=264 Win=65024 Len=0 TSval=3142530861 TSecr=442349972
32	200.021095131	172.18.0.1	172.18.0.2	TCP	66	[TCP Keep-Alive] 49534 → 1344 [ACK] Seq=263 Ack=114 Win=64256 Len=0 TSval=442549991 TSecr=3142530861
33	200.021670604	172.18.0.2	172.18.0.1	TCP	66	[TCP Keep-Alive ACK] 1344 → 49534 [ACK] Seq=114 Ack=264 Win=65024 Len=0 TSval=3142546220 TSecr=442349972
34	215.381379128	172.18.0.1	172.18.0.2	TCP	66	[TCP Keep-Alive] 49534 → 1344 [ACK] Seq=263 Ack=114 Win=64256 Len=0 TSval=442565351 TSecr=3142546220
35	215.381437087	172.18.0.2	172.18.0.1	TCP	66	[TCP Keep-Alive ACK] 1344 → 49534 [ACK] Seq=114 Ack=264 Win=65024 Len=0 TSval=3142561580 TSecr=442349972
36	230.741083806	172.18.0.1	172.18.0.2	TCP	66	[TCP Keep-Alive] 49534 → 1344 [ACK] Seq=263 Ack=114 Win=64256 Len=0 TSval=442580711 TSecr=3142561580
37	230.741130354	172.18.0.2	172.18.0.1	TCP	66	[TCP Keep-Alive ACK] 1344 → 49534 [ACK] Seq=114 Ack=264 Win=65024 Len=0 TSval=3142576940 TSecr=442349972
38	246.100904869	172.18.0.1	172.18.0.2	TCP	66	[TCP Keep-Alive] 49534 → 1344 [ACK] Seq=263 Ack=114 Win=64256 Len=0 TSval=442596070 TSecr=3142576940
39	246.100971244	172.18.0.2	172.18.0.1	TCP	66	[TCP Keep-Alive ACK] 1344 → 49534 [ACK] Seq=114 Ack=264 Win=65024 Len=0 TSval=3142592299 TSecr=442349972
40	261.464229295	172.18.0.1	172.18.0.2	TCP	66	[TCP Keep-Alive] 49534 → 1344 [ACK] Seq=263 Ack=114 Win=64256 Len=0 TSval=442611434 TSecr=3142592299
41	261.464282936	172.18.0.2	172.18.0.1	TCP	66	[TCP Keep-Alive ACK] 1344 → 49534 [ACK] Seq=114 Ack=264 Win=65024 Len=0 TSval=3142607663 TSecr=442349972
42	276.820913059	172.18.0.1	172.18.0.2	TCP	66	[TCP Keep-Alive] 49534 → 1344 [ACK] Seq=263 Ack=114 Win=64256 Len=0 TSval=442626790 TSecr=3142607663
43	276.821753621	172.18.0.2	172.18.0.1	TCP	66	[TCP Keep-Alive ACK] 1344 → 49534 [ACK] Seq=114 Ack=264 Win=65024 Len=0 TSval=3142623020 TSecr=442349972
44	292.180904566	172.18.0.1	172.18.0.2	TCP	66	[TCP Keep-Alive] 49534 → 1344 [ACK] Seq=263 Ack=114 Win=64256 Len=0 TSval=442642150 TSecr=3142623020
45	292.180949862	172.18.0.2	172.18.0.1	TCP	66	[TCP Keep-Alive ACK] 1344 → 49534 [ACK] Seq=114 Ack=264 Win=65024 Len=0 TSval=3142638379 TSecr=442349972
46	300.001031412	172.18.0.1	172.18.0.2	TCP	66	49534 → 1344 [FIN, ACK] Seq=264 Ack=114 Win=64256 Len=0 TSval=442649970 TSecr=3142638379
47	300.001282204	172.18.0.2	172.18.0.1	TCP	66	1344 → 49534 [FIN, ACK] Seq=114 Ack=265 Win=65024 Len=0 TSval=3142646200 TSecr=442649970
48	300.001301660	172.18.0.1	172.18.0.2	TCP	66	49534 → 1344 [ACK] Seq=265 Ack=115 Win=64256 Len=0 TSval=442649971 TSecr=3142646200
@micbar micbar added Topic:Performance Priority:p2-high Escalation, on top of current planning, release blocker labels Jul 10, 2023
@micbar
Copy link
Contributor

micbar commented Jul 11, 2023

@kobergj Can you provide more context?

@micbar
Copy link
Contributor

micbar commented Jul 11, 2023

@wkloucek How important is this in regards of Release 3.1.0?

@wkloucek
Copy link
Contributor Author

@wkloucek How important is this in regards of Release 3.1.0?

Not important at all

@micbar micbar added Priority:p3-medium Normal priority and removed Priority:p2-high Escalation, on top of current planning, release blocker labels Jul 11, 2023
@kobergj
Copy link
Collaborator

kobergj commented Jul 11, 2023

Can you provide more context?

This is an issue of the icap lib we are using (https://github.com/egirna/icap-client) It is rather unmaintained (last commit 2020) and we planned to replace it with an alternative. However back then there were no alternatives. I see two fixes for this issue, but both are more a story than a bugfix.

A) Find an alternative for the used icap-lib and use this.

B) Write an icap lib by ourselves (we only need to implement the features we really need) and use this.

@kobergj kobergj mentioned this issue Jul 14, 2023
Open
@micbar
Copy link
Contributor

micbar commented Sep 1, 2023

The used icap Library is very "basic" and unmaintained.

See the proposal from @kobergj

Does anybody know a better library?
I would like to avoid developing an ICAP golang library on our own.

@micbar micbar added Priority:p2-high Escalation, on top of current planning, release blocker and removed Priority:p3-medium Normal priority labels Sep 1, 2023
@d7oc
Copy link
Contributor

d7oc commented Sep 1, 2023

Don't know if it matters, but the ICAP client in here: https://github.com/egirna/icapeg/tree/master/icap-client seems to have some changes newer than the stuff here: https://github.com/egirna/icap-client.

@d7oc
Copy link
Contributor

d7oc commented Sep 1, 2023

Might wrapping a c library be an option? If yes this library is also used by many distributions: https://github.com/c-icap/c-icap-server. It also features a CLI: https://c-icap.sourceforge.net/install.html (which I used to test my ICAP server, for the record /usr/bin/c-icap-client -f /root/eicar.com -s "srv_clamav?allow204=on&force=on&sizelimit=off&mode=simple" -i 192.168.179.2)

@jvillafanez
Copy link
Member

Might wrapping a c library be an option?

From my little experience, this should be the last option, if any other option has failed.
There are multiple drawbacks:

  • Safety features from the Go language won't be available in the wrapper. We need to be extremely careful with the wrapper code
  • Debugging code will be very complex. I don't think we'll be able to debug anything inside the wrapped library.
  • Some features, such as callbacks, might be difficult to make in the wrapper.
  • Need to pay special attention on memory management (variables "malloc"ed from C code must be manually freed), and transferring data (C types vs Go types)
  • The library could have additional limitations (thread-safety) which might condition the development of the wrapper or anything on top of it.

Due to these drawbacks, wrappers are usually an extremely thin interface to call C methods in order to reduce the "problematic" surface. Any complexity is managed by creating a class in Go that will use the simple methods of the wrapper to achieve its goal.

Note that the user might still need to have installed the icap shared library in his system, which might be another problem. I haven't look into it too much, but there should be an option to compile the library code statically to include the library code into the binary.

@micbar
Copy link
Contributor

micbar commented Dec 20, 2023

Proposal: Put a timeboxed effort on understanding if the upstream bug is fixable in their code base.

@wkloucek
Copy link
Contributor Author

From what I remember:

KeepAliveTimeout 0

should force a connection close after the request.

So if you have KeepAliveTimeout > 0 or even -1 you'll get a late / no scan result

@fschade fschade self-assigned this Dec 22, 2023
@fschade fschade mentioned this issue Dec 22, 2023
Merged
9 tasks
@fschade
Copy link
Contributor

fschade commented Dec 22, 2023

@wkloucek can you confirm #8062 works as expected and fixes the tcp keepalive thing!?

we can pair on that if help is needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@fschade fschade

Labels
Priority:p2-high Escalation, on top of current planning, release blocker Topic:Performance Type:Bug
Projects
Infinite Scale Team Board
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

enhancement: enable icap preview mode and fix client according to the spec fschade/ocis
6 participants
@jvillafanez @micbar @kobergj @wkloucek @fschade @d7oc