Repository: clouditor/clouditor (go)

Issue:
Assessment component is crashing
**Describe the bug**
If AssessEvidence()  is called (probably) with the following evidence the assessment component runs in a panic: 
`
"Evidence: id:\"08569c84-ef24-11ed-a1a3-eeda3d68524b\" timestamp:{seconds:1683717355} cloud_service_id:\"3104301c-e5a3-4890-ab02-a7bef87b7243\" tool_id:\"wazuh-vat-evidence-collector:v0.1.3\" raw:\"\\\"\\\"\" resource:{struct_value:{fields:{key:\"id\" value:{string_value:\"dummyVAT\"}} fields:{key:\"name\" value:{string_value:\"dummyVAT\"}} fields:{key:\"numberOfVulnerabilitiesFound\" value:{number_value:6}} fields:{key:\"type\" value:{list_value:{values:{string_value:\"VirtualMachine\"} values:{string_value:\"Compute\"} values:{string_value:\"Resource\"}}}}}}"
`

Log message: 
```
time="2023-05-10T11:15:55Z" level=debug msg="Evaluating evidence 08569c84-ef24-11ed-a1a3-eeda3d68524b (dummyVAT) collected by wazuh-vat-evidence-collector:v0.1.3 at 2023-05-10 11:15:55 +0000 UTC" component=assessment
time="2023-05-10T11:15:55Z" level=trace msg="Evidence: id:\"08569c84-ef24-11ed-a1a3-eeda3d68524b\" timestamp:{seconds:1683717355} cloud_service_id:\"3104301c-e5a3-4890-ab02-a7bef87b7243\" tool_id:\"wazuh-vat-evidence-collector:v0.1.3\" raw:\"\\\"\\\"\" resource:{struct_value:{fields:{key:\"id\" value:{string_value:\"dummyVAT\"}} fields:{key:\"name\" value:{string_value:\"dummyVAT\"}} fields:{key:\"numberOfVulnerabilitiesFound\" value:{number_value:6}} fields:{key:\"type\" value:{list_value:{values:{string_value:\"VirtualMachine\"} values:{string_value:\"Compute\"} values:{string_value:\"Resource\"}}}}}}" component=assessment
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0xca5b4d]

goroutine 412 [running]:
clouditor.io/clouditor/service/assessment.(*Service).handleEvidence(0xc0002694a0, 0xc000c7c070, {0xc0006a0128, 0x8})
	/go/pkg/mod/clouditor.io/clouditor@v1.7.9/service/assessment/assessment.go:332 +0x78d
clouditor.io/clouditor/service/assessment.(*Service).AssessEvidence(0xc0002694a0, {0x1012100, 0xc000e023f0}, 0xc000e020f0)
	/go/pkg/mod/clouditor.io/clouditor@v1.7.9/service/assessment/assessment.go:226 +0x1fc
clouditor.io/clouditor/api/assessment._Assessment_AssessEvidence_Handler.func1({0x1012100, 0xc000e023f0}, {0xe49820?, 0xc000e020f0})
	/go/pkg/mod/clouditor.io/clouditor@v1.7.9/api/assessment/assessment_grpc.pb.go:167 +0x78
github.com/grpc-ecosystem/go-grpc-middleware/auth.UnaryServerInterceptor.func1({0x1012100, 0xc000e022a0}, {0xe49820, 0xc000e020f0}, 0xc000d2c0a0, 0xc000c36090)
	/go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware@v1.4.0/auth/auth.go:47 +0xba
clouditor.io/clouditor/server.UnaryServerInterceptorWithFilter.func1({0x1012100, 0xc000e022a0}, {0xe49820, 0xc000e020f0}, 0x0?, 0xc000c36090)
	/go/pkg/mod/clouditor.io/clouditor@v1.7.9/server/grpc.go:177 +0xa3
google.golang.org/grpc.getChainUnaryHandler.func1({0x1012100, 0xc000e022a0}, {0xe49820, 0xc000e020f0})
	/go/pkg/mod/google.golang.org/grpc@v1.55.0/server.go:1156 +0xb9
github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus.UnaryServerInterceptor.func1({0x1012100, 0xc000e021b0}, {0xe49820, 0xc000e020f0}, 0xc000d2c0a0, 0xc000c7e2c0)
	/go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware@v1.4.0/logging/logrus/server_interceptors.go:31 +0x102
google.golang.org/grpc.getChainUnaryHandler.func1({0x1012100, 0xc000e021b0}, {0xe49820, 0xc000e020f0})
	/go/pkg/mod/google.golang.org/grpc@v1.55.0/server.go:1156 +0xb9
github.com/grpc-ecosystem/go-grpc-middleware/tags.UnaryServerInterceptor.func1({0x1012100?, 0xc000e020c0?}, {0xe49820, 0xc000e020f0}, 0xc000d2c0a0, 0xc000c7e280)
	/go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware@v1.4.0/tags/interceptors.go:23 +0xa6
google.golang.org/grpc.chainUnaryInterceptors.func1({0x1012100, 0xc000e020c0}, {0xe49820, 0xc000e020f0}, 0xc000aa1a58?, 0xd7a7a0?)
	/go/pkg/mod/google.golang.org/grpc@v1.55.0/server.go:1147 +0x8f
clouditor.io/clouditor/api/assessment._Assessment_AssessEvidence_Handler({0xe65740?, 0xc0002694a0}, {0x1012100, 0xc000e020c0}, 0xc000c7c000, 0xc0003456c0)
	/go/pkg/mod/clouditor.io/clouditor@v1.7.9/api/assessment/assessment_grpc.pb.go:169 +0x138
google.golang.org/grpc.(*Server).processUnaryRPC(0xc00024e1e0, {0x10166a0, 0xc0007d36c0}, 0xc000458000, 0xc00033fa70, 0x16b5558, 0x0)
	/go/pkg/mod/google.golang.org/grpc@v1.55.0/server.go:1337 +0xdf3
google.golang.org/grpc.(*Server).handleStream(0xc00024e1e0, {0x10166a0, 0xc0007d36c0}, 0xc000458000, 0x0)
	/go/pkg/mod/google.golang.org/grpc@v1.55.0/server.go:1714 +0xa36
google.golang.org/grpc.(*Server).serveStreams.func1.1()
	/go/pkg/mod/google.golang.org/grpc@v1.55.0/server.go:959 +0x98
created by google.golang.org/grpc.(*Server).serveStreams.func1
	/go/pkg/mod/google.golang.org/grpc@v1.55.0/server.go:957 +0x18c
```

**To Reproduce**
Nothing, see Medina deployment.

**Expected behavior**
Catch and log error.

**Additional context**
Locally, it cannot be reproduced.

Pull request description:
Fix assessment crash when assessing corrupt evidence due to metrics cache
Fixes #1106 

Patch:
diff --git a/api/evidence/evidence.go b/api/evidence/evidence.go
index f7a0a8cd..7a597b59 100644
--- a/api/evidence/evidence.go
+++ b/api/evidence/evidence.go
@@ -38,9 +38,9 @@ type EvidenceHookFunc func(result *Evidence, err error)
 var (
 	ErrResourceNotStruct             = errors.New("resource in evidence is not struct value")
 	ErrResourceNotMap                = errors.New("resource in evidence is not a map")
-	ErrResourceIdMissing             = errors.New("resource in evidence is missing the id field")
+	ErrResourceIdIsEmpty             = errors.New("resource id in evidence is empty")
 	ErrResourceIdNotString           = errors.New("resource id in evidence is not a string")
-	ErrResourceIdFieldMissing        = errors.New("field id is missing")
+	ErrResourceIdFieldMissing        = errors.New("resource in evidence is missing the id field")
 	ErrResourceTypeFieldMissing      = errors.New("field type in evidence is missing")
 	ErrResourceTypeNotArrayOfStrings = errors.New("resource type in evidence is not an array of strings")
 	ErrResourceTypeEmpty             = errors.New("resource type (array) in evidence is empty")
@@ -67,7 +67,7 @@ func (evidence *Evidence) ValidateWithResource() (resourceId string, err error)
 	if !ok {
 		return "", ErrResourceIdFieldMissing
 	} else if field == "" {
-		return "", ErrResourceIdMissing
+		return "", ErrResourceIdIsEmpty
 	}
 
 	resourceId, ok = field.(string)
diff --git a/policies/rego.go b/policies/rego.go
index 9d8632bc..ca5c1978 100644
--- a/policies/rego.go
+++ b/policies/rego.go
@@ -167,8 +167,15 @@ func (re *regoEval) Eval(evidence *evidence.Evidence, src MetricsSource) (data [
 			if err != nil {
 				return nil, err
 			}
-
-			data = append(data, runMap)
+			// Add runMap to data only if metric was applicable. runMap=nil and err=nil means the metric was not
+			// applicable.
+			// This shouldn't happen in theory since it was tested above when the metric cache got initialized. But when
+			// there is new evidence which has set the resource types and tool id correctly (their combination builds
+			// the key for the cache), all metrics are applied due to the cache - even when all corresponding resource
+			// fields are not set properly.
+			if runMap != nil {
+				data = append(data, runMap)
+			}
 		}
 	}
 
diff --git a/service/assessment/assessment.go b/service/assessment/assessment.go
index f768d337..c931bd69 100644
--- a/service/assessment/assessment.go
+++ b/service/assessment/assessment.go
@@ -329,6 +329,12 @@ func (svc *Service) handleEvidence(ev *evidence.Evidence, resourceId string) (re
 	}
 
 	for _, data := range evaluations {
+		// That there is an empty (nil) evaluation should be caught beforehand, but you never know.
+		if data == nil {
+			log.Errorf("One empty policy evaluation detected for evidence '%s'. That should not happen.",
+				ev.GetId())
+			continue
+		}
 		metricID := data.MetricID
 
 		log.Debugf("Evaluated evidence %v with metric '%v' as %v", ev.Id, metricID, data.Compliant)


Test patch:
diff --git a/api/evidence/evidence_test.go b/api/evidence/evidence_test.go
index bc53cbb4..45debb68 100644
--- a/api/evidence/evidence_test.go
+++ b/api/evidence/evidence_test.go
@@ -97,7 +97,7 @@ func Test_ValidateEvidence(t *testing.T) {
 				}},
 			wantResp: "",
 			wantErr: func(tt assert.TestingT, err error, i ...interface{}) bool {
-				return assert.ErrorContains(t, err, ErrResourceIdMissing.Error())
+				return assert.ErrorContains(t, err, ErrResourceIdIsEmpty.Error())
 			},
 		},
 		{
diff --git a/internal/testutil/servicetest/evidencetest/evidence.go b/internal/testutil/servicetest/evidencetest/evidence.go
index bc2bd899..5358287f 100644
--- a/internal/testutil/servicetest/evidencetest/evidence.go
+++ b/internal/testutil/servicetest/evidencetest/evidence.go
@@ -29,7 +29,7 @@ var (
 		Timestamp:      timestamppb.Now(),
 		CloudServiceId: testdata.MockCloudServiceID,
 		ToolId:         testdata.MockEvidenceToolID,
-		Raw:            util.Ref(""),
+		Raw:            util.Ref("This Raw field must be of length >1"),
 		Resource:       structpb.NewNullValue(),
 	}
 	MockEvidence2 = &evidence.Evidence{
diff --git a/service/assessment/assessment_test.go b/service/assessment/assessment_test.go
index 88638ef9..8c875e53 100644
--- a/service/assessment/assessment_test.go
+++ b/service/assessment/assessment_test.go
@@ -26,6 +26,7 @@
 package assessment
 
 import (
+	"clouditor.io/clouditor/internal/testutil/servicetest/evidencetest"
 	"context"
 	"errors"
 	"fmt"
@@ -47,6 +48,8 @@ import (
 	"clouditor.io/clouditor/policies"
 	"clouditor.io/clouditor/service"
 	"clouditor.io/clouditor/voc"
+
+	"github.com/google/uuid"
 	"github.com/stretchr/testify/assert"
 	"golang.org/x/oauth2/clientcredentials"
 	"google.golang.org/grpc"
@@ -324,7 +327,7 @@ func TestService_AssessEvidence(t *testing.T) {
 			},
 			wantResp: nil,
 			wantErr: func(tt assert.TestingT, err error, i ...interface{}) bool {
-				return assert.ErrorContains(t, err, "invalid evidence: resource in evidence is missing the id field")
+				return assert.ErrorContains(t, err, evidence.ErrResourceIdIsEmpty.Error())
 			},
 		},
 		{
@@ -372,6 +375,57 @@ func TestService_AssessEvidence(t *testing.T) {
 	}
 }
 
+// TestService_AssessEvidence_DetectMisconfiguredEvidenceEvenWhenAlreadyCached tests the following workflow: First an
+// evidence with a VM resource is assessed. The resource contains all required fields s.t. the metric cache is filled
+// with all applicable metrics. In a second step we assess another evidence. It is also of type "VirtualMachine" but all
+// other fields are not set (e.g. MalwareProtection). Thus, metric will be applied and therefore no error occurs in
+// AssessEvidence-handleEvidence (assessment.go) which loops over all evaluations
+// Todo: Add it to table test above (would probably need some function injection in test cases like we do with storage)
+func TestService_AssessEvidence_DetectMisconfiguredEvidenceEvenWhenAlreadyCached(t *testing.T) {
+	s := &Service{
+		evidenceStore: api.NewRPCConnection(
+			"bufnet", evidence.NewEvidenceStoreClient, grpc.WithContextDialer(bufConnDialer)),
+		orchestrator: api.NewRPCConnection(
+			"bufnet", orchestrator.NewOrchestratorClient, grpc.WithContextDialer(bufConnDialer)),
+		authz: servicetest.NewAuthorizationStrategy(true),
+		evidenceStoreStreams: api.NewStreamsOf(
+			api.WithLogger[evidence.EvidenceStore_StoreEvidencesClient, *evidence.StoreEvidenceRequest](log)),
+		orchestratorStreams: api.NewStreamsOf(
+			api.WithLogger[orchestrator.Orchestrator_StoreAssessmentResultsClient,
+				*orchestrator.StoreAssessmentResultRequest](log)),
+		cachedConfigurations: make(map[string]cachedConfiguration),
+		pe:                   policies.NewRegoEval(policies.WithPackageName(policies.DefaultRegoPackage)),
+	}
+	// First assess evidence with a valid VM resource s.t. the cache is created for the combination of resource type and
+	// tool id (="VirtualMachine-{testdata.MockEvidenceToolID}")
+	e := evidencetest.MockEvidence1
+	e.Resource = toStruct(voc.VirtualMachine{
+		Compute: &voc.Compute{
+			Resource: &voc.Resource{ID: testdata.MockResourceID, Type: []string{"VirtualMachine"}}}}, t)
+	_, err := s.AssessEvidence(context.Background(), &assessment.AssessEvidenceRequest{Evidence: e})
+	assert.NoError(t, err)
+
+	// Now assess a new evidence which has not a valid format other than the resource type and tool id is set correctly
+	// Prepare resource
+	r := map[string]any{
+		// Make sure both evidences have the same type (for caching key)
+		"type": e.Resource.GetStructValue().AsMap()["type"],
+		"id":   uuid.NewString(),
+	}
+	v, err := structpb.NewValue(r)
+	assert.NoError(t, err)
+	_, err = s.AssessEvidence(context.Background(), &assessment.AssessEvidenceRequest{Evidence: &evidence.Evidence{
+		Id:             uuid.NewString(),
+		Timestamp:      timestamppb.Now(),
+		CloudServiceId: testdata.MockCloudServiceID,
+		// Make sure both evidences have the same tool id (for caching key)
+		ToolId:   e.ToolId,
+		Raw:      nil,
+		Resource: v,
+	}})
+	assert.NoError(t, err)
+}
+
 // TestAssessEvidences tests AssessEvidences
 func TestService_AssessEvidences(t *testing.T) {
 	type fields struct {