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-20240123094924-5af178158eaf/go.mod h1:Curjbe9P7SKWAtoXuu/huL8VnqzuBzetEpEPt9TLToE=
+github.com/fschade/icap-client v0.0.0-20240802074440-aade4a234387 h1:Y3wZgTr29sLxWSMz4KF91o0x87EaJF6FIPNJFepRIiw=
+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,42 +32,45 @@ 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
-	url    url.URL
+	Client Scanner
+	URL    string
 }
 
 // Scan scans a file using the ICAP server
 func (s ICAP) Scan(in Input) (Result, error) {
 	ctx := context.TODO()
 	result := Result{}
 
-	httpReq, err := http.NewRequest(http.MethodPost, in.Url, in.Body)
+	optReq, err := ic.NewRequest(ctx, ic.MethodOPTIONS, s.URL, nil, nil)
 	if err != nil {
 		return result, err
 	}
 
-	httpReq.ContentLength = in.Size
-	if mt := mime.Detect(path.Ext(in.Name) == "", in.Name); mt != "" {
-		httpReq.Header.Set("Content-Type", mt)
-	}
-
-	optReq, err := ic.NewRequest(ctx, ic.MethodOPTIONS, s.url.String(), nil, nil)
+	optRes, err := s.Client.Do(optReq)
 	if err != nil {
 		return result, err
 	}
 
-	optRes, err := s.client.Do(optReq)
+	httpReq, err := http.NewRequest(http.MethodPost, in.Url, in.Body)
 	if err != nil {
 		return result, err
 	}
 
-	req, err := ic.NewRequest(ctx, ic.MethodREQMOD, s.url.String(), httpReq, nil)
+	httpReq.ContentLength = in.Size
+	if mt := mime.Detect(path.Ext(in.Name) == "", in.Name); mt != "" {
+		httpReq.Header.Set("Content-Type", mt)
+	}
+
+	req, err := ic.NewRequest(ctx, ic.MethodREQMOD, s.URL, 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)
+			})
+		})
+	})
+}