enhancement: Support Skyhigh Security ICAP as an ICAP server

changelog/unreleased/enhancement-skyhigh-security-icap-support.md

@@ -0,0 +1,7 @@
+Enhancement: Support Skyhigh Security ICAP as an ICAP server
+
+We have upgraded the antivirus ICAP client library, bringing enhanced performance and reliability to our antivirus scanning service.
+With this update, the Skyhigh Security ICAP can now be used as an ICAP server, providing robust and scalable antivirus solutions.
+
+https://github.com/owncloud/ocis/issues/9720
+https://github.com/fschade/icap-client/pull/6

go.mod

@@ -358,7 +358,7 @@ require (
 
 replace github.com/studio-b12/gowebdav => github.com/aduffeck/gowebdav v0.0.0-20231215102054-212d4a4374f6
 
-replace github.com/egirna/icap-client => github.com/fschade/icap-client v0.0.0-20240123094924-5af178158eaf
+replace github.com/egirna/icap-client => github.com/fschade/icap-client v0.0.0-20240802074440-aade4a234387
 
 replace github.com/unrolled/secure => github.com/DeepDiver1975/secure v0.0.0-20240611112133-abc838fb797c
 

go.sum

@@ -1118,8 +1118,8 @@ github.com/foxcpp/go-mockdns v1.1.0 h1:jI0rD8M0wuYAxL7r/ynTrCQQq0BVqfB99Vgk7Dlme
 github.com/foxcpp/go-mockdns v1.1.0/go.mod h1:IhLeSFGed3mJIAXPH2aiRQB+kqz7oqu8ld2qVbOu7Wk=
 github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHkI4W8=
 github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0=
-github.com/fschade/icap-client v0.0.0-20240123094924-5af178158eaf h1:3IzYXRblwIxeis+EtLLWTK0QitcefZT7YfpF7jfTFYA=
+github.com/fschade/icap-client v0.0.0-20240802074440-aade4a234387 h1:Y3wZgTr29sLxWSMz4KF91o0x87EaJF6FIPNJFepRIiw=
-github.com/fschade/icap-client v0.0.0-20240123094924-5af178158eaf/go.mod h1:Curjbe9P7SKWAtoXuu/huL8VnqzuBzetEpEPt9TLToE=
+github.com/fschade/icap-client v0.0.0-20240802074440-aade4a234387/go.mod h1:HpntrRsQA6RKNXy2Nbr4kVj+NO3OYWpAQUVxeya+3sU=
 github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo=
 github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4IgpuI1SZQ=
 github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nosvA=
@@ -1819,6 +1819,8 @@ github.com/pbnjay/memory v0.0.0-20210728143218-7b4eea64cf58 h1:onHthvaw9LFnH4t2D
 github.com/pbnjay/memory v0.0.0-20210728143218-7b4eea64cf58/go.mod h1:DXv8WO4yhMYhSNPKjeNKa5WY9YCIEBRbNzFFPJbWO6Y=
 github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic=
 github.com/pelletier/go-toml v1.8.1/go.mod h1:T2/BmBdy8dvIRq1a/8aqjN41wvWlN4lrapLU/GW4pbc=
+github.com/phayes/freeport v0.0.0-20220201140144-74d24b5ae9f5 h1:Ii+DKncOVM8Cu1Hc+ETb5K+23HdAMvESYE3ZJ5b5cMI=
+github.com/phayes/freeport v0.0.0-20220201140144-74d24b5ae9f5/go.mod h1:iIss55rKnNBTvrwdmkUpLnDpZoAHvWaiq5+iMmen4AE=
 github.com/phpdave11/gofpdf v1.4.2/go.mod h1:zpO6xFn9yxo3YLyMvW8HcKWVdbNqgIfOOp2dXMnm1mY=
 github.com/phpdave11/gofpdi v1.0.12/go.mod h1:vBmVV0Do6hSBHC8uKUQ71JGW+ZGQq74llk/7bXwjDoI=
 github.com/phpdave11/gofpdi v1.0.13/go.mod h1:vBmVV0Do6hSBHC8uKUQ71JGW+ZGQq74llk/7bXwjDoI=

services/antivirus/.mockery.yaml

@@ -0,0 +1,9 @@
+with-expecter: true
+filename: "{{.InterfaceName | snakecase }}.go"
+dir: "pkg/{{.PackageName}}/mocks"
+mockname: "{{.InterfaceName}}"
+outpkg: "mocks"
+packages:
+    github.com/owncloud/ocis/v2/services/antivirus/pkg/scanners:
+        interfaces:
+            Scanner:

services/antivirus/pkg/scanners/icap.go

@@ -10,9 +10,15 @@ import (
 	"time"
 
 	"github.com/cs3org/reva/v2/pkg/mime"
+
 	ic "github.com/egirna/icap-client"
 )
 
+// Scanner is the interface that wraps the basic Do method
+type Scanner interface {
+	Do(req ic.Request) (ic.Response, error)
+}
+
 // NewICAP returns a Scanner talking to an ICAP server
 func NewICAP(icapURL string, icapService string, timeout time.Duration) (ICAP, error) {
 	endpoint, err := url.Parse(icapURL)
@@ -26,14 +32,17 @@ func NewICAP(icapURL string, icapService string, timeout time.Duration) (ICAP, e
 	client, err := ic.NewClient(
 		ic.WithICAPConnectionTimeout(timeout),
 	)
+	if err != nil {
+		return ICAP{}, err
+	}
 
-	return ICAP{client: client, url: *endpoint}, nil
+	return ICAP{Client: &client, URL: endpoint.String()}, nil
 }
 
 // ICAP is responsible for scanning files using an ICAP server
 type ICAP struct {
-	client ic.Client
+	Client Scanner
-	url    url.URL
+	URL    string
 }
 
 // Scan scans a file using the ICAP server
@@ -41,6 +50,16 @@ func (s ICAP) Scan(in Input) (Result, error) {
 	ctx := context.TODO()
 	result := Result{}
 
+	optReq, err := ic.NewRequest(ctx, ic.MethodOPTIONS, s.URL, nil, nil)
+	if err != nil {
+		return result, err
+	}
+
+	optRes, err := s.Client.Do(optReq)
+	if err != nil {
+		return result, err
+	}
+
 	httpReq, err := http.NewRequest(http.MethodPost, in.Url, in.Body)
 	if err != nil {
 		return result, err
@@ -51,17 +70,7 @@ func (s ICAP) Scan(in Input) (Result, error) {
 		httpReq.Header.Set("Content-Type", mt)
 	}
 
-	optReq, err := ic.NewRequest(ctx, ic.MethodOPTIONS, s.url.String(), nil, nil)
+	req, err := ic.NewRequest(ctx, ic.MethodREQMOD, s.URL, httpReq, nil)
-	if err != nil {
-		return result, err
-	}
-
-	optRes, err := s.client.Do(optReq)
-	if err != nil {
-		return result, err
-	}
-
-	req, err := ic.NewRequest(ctx, ic.MethodREQMOD, s.url.String(), httpReq, nil)
 	if err != nil {
 		return result, err
 	}
@@ -73,7 +82,7 @@ func (s ICAP) Scan(in Input) (Result, error) {
 		}
 	}
 
-	res, err := s.client.Do(req)
+	res, err := s.Client.Do(req)
 	if err != nil {
 		return result, err
 	}
@@ -89,5 +98,14 @@ func (s ICAP) Scan(in Input) (Result, error) {
 		}
 	}
 
+	if result.Infected || res.ContentResponse == nil {
+		return result, nil
+	}
+
+	// mcafee forwards the scan result as HTML in the content response;
+	// status 403 indicates that the file is infected
+	result.Infected = res.ContentResponse.StatusCode == http.StatusForbidden
+	result.Description = res.ContentResponse.Status
+
 	return result, nil
 }

services/antivirus/pkg/scanners/icap_test.go

@@ -0,0 +1,186 @@
+package scanners_test
+
+import (
+	"bytes"
+	"errors"
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+
+	ic "github.com/egirna/icap-client"
+	"github.com/owncloud/ocis/v2/services/antivirus/pkg/scanners"
+	"github.com/owncloud/ocis/v2/services/antivirus/pkg/scanners/mocks"
+)
+
+func TestICAP_Scan(t *testing.T) {
+	var (
+		earlyExitErr = errors.New("stop here")
+		testUrl      = "icap://test"
+		client       = mocks.NewScanner(t)
+		scanner      = &scanners.ICAP{Client: client, URL: testUrl}
+	)
+
+	t.Run("it sends a OPTIONS request to determine details", func(t *testing.T) {
+		client.EXPECT().Do(mock.Anything).RunAndReturn(func(request ic.Request) (ic.Response, error) {
+			assert.Equal(t, ic.MethodOPTIONS, request.Method)
+			assert.Equal(t, testUrl, request.URL.String())
+			return ic.Response{}, earlyExitErr
+		}).Once()
+
+		_, err := scanner.Scan(scanners.Input{})
+		assert.ErrorIs(t, earlyExitErr, err) // we can exit early, just in case check the error to be identical to the early exit error
+	})
+
+	t.Run("it sends a REQMOD request with all the details", func(t *testing.T) {
+
+		t.Run("request with ContentLength", func(t *testing.T) {
+			t.Run("with size", func(t *testing.T) {
+				client.EXPECT().Do(mock.Anything).Return(ic.Response{}, nil).Once()
+
+				client.EXPECT().Do(mock.Anything).RunAndReturn(func(request ic.Request) (ic.Response, error) {
+					assert.Equal(t, ic.MethodREQMOD, request.Method)
+					assert.Equal(t, testUrl, request.URL.String())
+					assert.EqualValues(t, 999, request.HTTPRequest.ContentLength)
+					return ic.Response{}, earlyExitErr
+				}).Once()
+
+				_, err := scanner.Scan(scanners.Input{Size: 999})
+				assert.ErrorIs(t, earlyExitErr, err)
+			})
+
+			t.Run("without size", func(t *testing.T) {
+				client.EXPECT().Do(mock.Anything).Return(ic.Response{}, nil).Once()
+
+				client.EXPECT().Do(mock.Anything).RunAndReturn(func(request ic.Request) (ic.Response, error) {
+					assert.Equal(t, ic.MethodREQMOD, request.Method)
+					assert.Equal(t, testUrl, request.URL.String())
+					assert.EqualValues(t, 0, request.HTTPRequest.ContentLength)
+					return ic.Response{}, earlyExitErr
+				}).Once()
+
+				_, err := scanner.Scan(scanners.Input{})
+				assert.ErrorIs(t, earlyExitErr, err)
+			})
+		})
+
+		t.Run("request with Content-Type header", func(t *testing.T) {
+			t.Run("name contains known extension", func(t *testing.T) {
+				client.EXPECT().Do(mock.Anything).Return(ic.Response{}, nil).Once()
+
+				client.EXPECT().Do(mock.Anything).RunAndReturn(func(request ic.Request) (ic.Response, error) {
+					assert.Equal(t, "application/pdf", request.HTTPRequest.Header.Get("Content-Type"))
+					return ic.Response{}, earlyExitErr
+				}).Once()
+
+				_, err := scanner.Scan(scanners.Input{Name: "report.pdf"})
+				assert.ErrorIs(t, earlyExitErr, err)
+			})
+
+			t.Run("name with unknown extension", func(t *testing.T) {
+				client.EXPECT().Do(mock.Anything).Return(ic.Response{}, nil).Once()
+
+				client.EXPECT().Do(mock.Anything).RunAndReturn(func(request ic.Request) (ic.Response, error) {
+					assert.Equal(t, "application/octet-stream", request.HTTPRequest.Header.Get("Content-Type"))
+					return ic.Response{}, earlyExitErr
+				}).Once()
+
+				_, err := scanner.Scan(scanners.Input{Name: "report.unknown"})
+				assert.ErrorIs(t, earlyExitErr, err)
+			})
+
+			t.Run("name without extension", func(t *testing.T) {
+				client.EXPECT().Do(mock.Anything).Return(ic.Response{}, nil).Once()
+
+				client.EXPECT().Do(mock.Anything).RunAndReturn(func(request ic.Request) (ic.Response, error) {
+					assert.Equal(t, "httpd/unix-directory", request.HTTPRequest.Header.Get("Content-Type"))
+					return ic.Response{}, earlyExitErr
+				}).Once()
+
+				_, err := scanner.Scan(scanners.Input{Name: "report"})
+				assert.ErrorIs(t, earlyExitErr, err)
+			})
+		})
+
+		t.Run("request with the OPTIONS response preview size ", func(t *testing.T) {
+			t.Run("with PreviewBytes set", func(t *testing.T) {
+				client.EXPECT().Do(mock.Anything).Return(ic.Response{PreviewBytes: 444}, nil).Once()
+
+				client.EXPECT().Do(mock.Anything).RunAndReturn(func(request ic.Request) (ic.Response, error) {
+					assert.Equal(t, 444, request.PreviewBytes)
+					return ic.Response{}, earlyExitErr
+				}).Once()
+
+				_, err := scanner.Scan(scanners.Input{Body: bytes.NewReader(make([]byte, 888))})
+				assert.ErrorIs(t, earlyExitErr, err)
+			})
+
+			t.Run("without PreviewBytes set", func(t *testing.T) {
+				client.EXPECT().Do(mock.Anything).Return(ic.Response{}, nil).Once()
+
+				client.EXPECT().Do(mock.Anything).RunAndReturn(func(request ic.Request) (ic.Response, error) {
+					assert.Equal(t, 0, request.PreviewBytes)
+					return ic.Response{}, earlyExitErr
+				}).Once()
+
+				_, err := scanner.Scan(scanners.Input{Body: bytes.NewReader(make([]byte, 888))})
+				assert.ErrorIs(t, earlyExitErr, err)
+			})
+		})
+	})
+
+	t.Run("request with the OPTIONS response preview size ", func(t *testing.T) {
+		t.Run("with PreviewBytes set", func(t *testing.T) {
+			client.EXPECT().Do(mock.Anything).Return(ic.Response{PreviewBytes: 444}, nil).Once()
+
+			client.EXPECT().Do(mock.Anything).RunAndReturn(func(request ic.Request) (ic.Response, error) {
+				assert.Equal(t, 444, request.PreviewBytes)
+				return ic.Response{}, earlyExitErr
+			}).Once()
+
+			_, err := scanner.Scan(scanners.Input{Body: bytes.NewReader(make([]byte, 888))})
+			assert.ErrorIs(t, earlyExitErr, err)
+		})
+
+		t.Run("it handles virus scan results", func(t *testing.T) {
+			t.Run("no virus", func(t *testing.T) {
+				client.EXPECT().Do(mock.Anything).Return(ic.Response{}, nil).Once()
+				client.EXPECT().Do(mock.Anything).Return(ic.Response{}, nil).Once()
+
+				result, err := scanner.Scan(scanners.Input{})
+				assert.Nil(t, err)
+				assert.False(t, result.Infected)
+			})
+
+			// clamav returns an X-Infection-Found header with the threat description
+			t.Run("X-Infection-Found header ", func(t *testing.T) {
+				client.EXPECT().Do(mock.Anything).Return(ic.Response{}, nil).Once()
+				client.EXPECT().Do(mock.Anything).Return(ic.Response{Header: http.Header{"X-Infection-Found": []string{"Threat=bad threat;"}}}, nil).Once()
+
+				result, err := scanner.Scan(scanners.Input{})
+				assert.Nil(t, err)
+				assert.True(t, result.Infected)
+				assert.Equal(t, "bad threat", result.Description)
+			})
+
+			// skyhigh returns the information via the content response
+			t.Run("X-Infection-Found header", func(t *testing.T) {
+				client.EXPECT().Do(mock.Anything).Return(ic.Response{}, nil).Once()
+				client.EXPECT().Do(mock.Anything).Return(ic.Response{ContentResponse: &http.Response{StatusCode: http.StatusForbidden, Status: "some status"}}, nil).Once()
+
+				result, err := scanner.Scan(scanners.Input{})
+				assert.Nil(t, err)
+				assert.True(t, result.Infected)
+				assert.Equal(t, "some status", result.Description)
+
+				client.EXPECT().Do(mock.Anything).Return(ic.Response{}, nil).Once()
+				client.EXPECT().Do(mock.Anything).Return(ic.Response{ContentResponse: &http.Response{StatusCode: http.StatusOK}}, nil).Once()
+
+				result, err = scanner.Scan(scanners.Input{})
+				assert.Nil(t, err)
+				assert.False(t, result.Infected)
+			})
+		})
+	})
+}

services/antivirus/pkg/scanners/mocks/scanner.go

@@ -0,0 +1,91 @@
+// Code generated by mockery v2.43.2. DO NOT EDIT.
+
+package mocks
+
+import (
+	icapclient "github.com/egirna/icap-client"
+	mock "github.com/stretchr/testify/mock"
+)
+
+// Scanner is an autogenerated mock type for the Scanner type
+type Scanner struct {
+	mock.Mock
+}
+
+type Scanner_Expecter struct {
+	mock *mock.Mock
+}
+
+func (_m *Scanner) EXPECT() *Scanner_Expecter {
+	return &Scanner_Expecter{mock: &_m.Mock}
+}
+
+// Do provides a mock function with given fields: req
+func (_m *Scanner) Do(req icapclient.Request) (icapclient.Response, error) {
+	ret := _m.Called(req)
+
+	if len(ret) == 0 {
+		panic("no return value specified for Do")
+	}
+
+	var r0 icapclient.Response
+	var r1 error
+	if rf, ok := ret.Get(0).(func(icapclient.Request) (icapclient.Response, error)); ok {
+		return rf(req)
+	}
+	if rf, ok := ret.Get(0).(func(icapclient.Request) icapclient.Response); ok {
+		r0 = rf(req)
+	} else {
+		r0 = ret.Get(0).(icapclient.Response)
+	}
+
+	if rf, ok := ret.Get(1).(func(icapclient.Request) error); ok {
+		r1 = rf(req)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
+// Scanner_Do_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Do'
+type Scanner_Do_Call struct {
+	*mock.Call
+}
+
+// Do is a helper method to define mock.On call
+//   - req icapclient.Request
+func (_e *Scanner_Expecter) Do(req interface{}) *Scanner_Do_Call {
+	return &Scanner_Do_Call{Call: _e.mock.On("Do", req)}
+}
+
+func (_c *Scanner_Do_Call) Run(run func(req icapclient.Request)) *Scanner_Do_Call {
+	_c.Call.Run(func(args mock.Arguments) {
+		run(args[0].(icapclient.Request))
+	})
+	return _c
+}
+
+func (_c *Scanner_Do_Call) Return(_a0 icapclient.Response, _a1 error) *Scanner_Do_Call {
+	_c.Call.Return(_a0, _a1)
+	return _c
+}
+
+func (_c *Scanner_Do_Call) RunAndReturn(run func(icapclient.Request) (icapclient.Response, error)) *Scanner_Do_Call {
+	_c.Call.Return(run)
+	return _c
+}
+
+// NewScanner creates a new instance of Scanner. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations.
+// The first argument is typically a *testing.T value.
+func NewScanner(t interface {
+	mock.TestingT
+	Cleanup(func())
+}) *Scanner {
+	mock := &Scanner{}
+	mock.Mock.Test(t)
+
+	t.Cleanup(func() { mock.AssertExpectations(t) })
+
+	return mock
+}

vendor/github.com/egirna/icap-client/conn.go

@@ -102,13 +102,12 @@ func (c *ICAPConn) Send(in []byte) ([]byte, error) {
 			data = append(data, tmp[:n]...)
 
 			// explicitly breaking because the Read blocks for 100 continue message
-			// fixMe: still unclear why this is happening, find out and fix it
 			if bytes.Equal(data, []byte(icap100ContinueMsg)) {
 				break
 			}
 
-			// EOF detected, 0 Double crlf indicates the end of the message
+			// EOF detected, double crlf indicates the end of the message
-			if bytes.HasSuffix(data, []byte("0\r\n\r\n")) {
+			if bytes.HasSuffix(data, []byte(doubleCRLF)) {
 				break
 			}
 

vendor/github.com/egirna/icap-client/icap_client.go

@@ -55,7 +55,7 @@ const (
 	schemeHTTPReq                   = "http_request"
 	schemeHTTPResp                  = "http_response"
 	crlf                            = "\r\n"
-	doubleCRLF                      = "\r\n\r\n"
+	doubleCRLF                      = crlf + crlf
 	lf                              = "\n"
 	bodyEndIndicator                = crlf + "0" + crlf
 	fullBodyEndIndicatorPreviewMode = "; ieof" + doubleCRLF

vendor/modules.txt

@@ -774,7 +774,7 @@ github.com/dutchcoders/go-clamd
 # github.com/egirna/icap v0.0.0-20181108071049-d5ee18bd70bc
 ## explicit
 github.com/egirna/icap
-# github.com/egirna/icap-client v0.1.1 => github.com/fschade/icap-client v0.0.0-20240123094924-5af178158eaf
+# github.com/egirna/icap-client v0.1.1 => github.com/fschade/icap-client v0.0.0-20240802074440-aade4a234387
 ## explicit; go 1.21
 github.com/egirna/icap-client
 # github.com/emirpasic/gods v1.18.1
@@ -2423,6 +2423,6 @@ sigs.k8s.io/yaml/goyaml.v2
 ## explicit; go 1.13
 stash.kopano.io/kgol/rndm
 # github.com/studio-b12/gowebdav => github.com/aduffeck/gowebdav v0.0.0-20231215102054-212d4a4374f6
-# github.com/egirna/icap-client => github.com/fschade/icap-client v0.0.0-20240123094924-5af178158eaf
+# github.com/egirna/icap-client => github.com/fschade/icap-client v0.0.0-20240802074440-aade4a234387
 # github.com/unrolled/secure => github.com/DeepDiver1975/secure v0.0.0-20240611112133-abc838fb797c
 # github.com/go-micro/plugins/v4/store/nats-js-kv => github.com/kobergj/plugins/v4/store/nats-js-kv v0.0.0-20240724102745-4bc93ffd7ab6