From 87b4108db14a60b8cc8ad024d029967a50da99f6 Mon Sep 17 00:00:00 2001 From: Yevhen Zavhorodnii Date: Tue, 4 Jun 2024 15:34:43 +0100 Subject: [PATCH] Cover unguarded direct datastore access with unit tests --- .../unguarded_direct_datastore_access_rule.go | 7 +- ...arded_direct_datastore_access_rule_test.go | 411 ++++++++++++++++++ 2 files changed, 416 insertions(+), 2 deletions(-) create mode 100644 pkg/security/risks/builtin/unguarded_direct_datastore_access_rule_test.go diff --git a/pkg/security/risks/builtin/unguarded_direct_datastore_access_rule.go b/pkg/security/risks/builtin/unguarded_direct_datastore_access_rule.go index 112ce21a..b95a5fa0 100644 --- a/pkg/security/risks/builtin/unguarded_direct_datastore_access_rule.go +++ b/pkg/security/risks/builtin/unguarded_direct_datastore_access_rule.go @@ -53,9 +53,12 @@ func (r *UnguardedDirectDatastoreAccessRule) GenerateRisks(input *types.Model) ( continue } + acrossTrustBoundaryNetworkOnly := incomingAccess.IsAcrossTrustBoundaryNetworkOnly(input) + sharingSameParentTrustBoundary := isSharingSameParentTrustBoundary(input, technicalAsset, sourceAsset) + if technicalAsset.Confidentiality >= types.Confidential || technicalAsset.Integrity >= types.Critical { - if incomingAccess.IsAcrossTrustBoundaryNetworkOnly(input) && !fileServerAccessViaFTP(technicalAsset, incomingAccess) && - incomingAccess.Usage != types.DevOps && !isSharingSameParentTrustBoundary(input, technicalAsset, sourceAsset) { + if acrossTrustBoundaryNetworkOnly && !fileServerAccessViaFTP(technicalAsset, incomingAccess) && + incomingAccess.Usage != types.DevOps && !sharingSameParentTrustBoundary { highRisk := technicalAsset.Confidentiality == types.StrictlyConfidential || technicalAsset.Integrity == types.MissionCritical risks = append(risks, r.createRisk(technicalAsset, incomingAccess, diff --git a/pkg/security/risks/builtin/unguarded_direct_datastore_access_rule_test.go b/pkg/security/risks/builtin/unguarded_direct_datastore_access_rule_test.go new file mode 100644 index 00000000..f27939c9 --- /dev/null +++ b/pkg/security/risks/builtin/unguarded_direct_datastore_access_rule_test.go @@ -0,0 +1,411 @@ +package builtin + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/threagile/threagile/pkg/security/types" +) + +func TestUnguardedDirectDatastoreAccessRuleGenerateRisksEmptyModelNotRisksCreated(t *testing.T) { + rule := NewUnguardedDirectDatastoreAccessRule() + + risks, err := rule.GenerateRisks(&types.Model{}) + + assert.Nil(t, err) + assert.Empty(t, risks) +} + +type UnguardedDirectDatastoreAccessRuleTest struct { + outOfScope bool + isIdentityStore bool + isFileServer bool + assetType types.TechnicalAssetType + raa float64 + + isSourceIdentityProvider bool + + isAcrossTrustBoundary bool + isSharingSameParentTrustBoundary bool + + protocol types.Protocol + usage types.Usage + + confidentiality types.Confidentiality + integrity types.Criticality + + riskCreated bool + expectedImpact types.RiskExploitationImpact +} + +func TestUnguardedDirectDatastoreAccessRuleRuleGenerateRisks(t *testing.T) { + testCases := map[string]UnguardedDirectDatastoreAccessRuleTest{ + "out of scope": { + outOfScope: true, + assetType: types.Datastore, + + riskCreated: false, + }, + "no data store": { + outOfScope: false, + assetType: types.ExternalEntity, + + riskCreated: false, + }, + "identity store to identity provider is ok": { + outOfScope: false, + assetType: types.Datastore, + isSourceIdentityProvider: true, + isIdentityStore: true, + }, + "no risk when data store is not critical": { + outOfScope: false, + assetType: types.Datastore, + confidentiality: types.Restricted, + integrity: types.Operational, + }, + "not across trust boundary network": { + outOfScope: false, + assetType: types.Datastore, + + isAcrossTrustBoundary: false, + isSharingSameParentTrustBoundary: false, + usage: types.Business, + isFileServer: false, + protocol: types.HTTP, + + confidentiality: types.Confidential, + integrity: types.Critical, + + riskCreated: false, + }, + "sharing same parent trust boundary": { + outOfScope: false, + assetType: types.Datastore, + + isAcrossTrustBoundary: true, + isSharingSameParentTrustBoundary: true, + usage: types.Business, + isFileServer: false, + protocol: types.HTTP, + + confidentiality: types.Confidential, + integrity: types.Critical, + + riskCreated: false, + }, + "devops usage": { + outOfScope: false, + assetType: types.Datastore, + + isAcrossTrustBoundary: true, + isSharingSameParentTrustBoundary: false, + usage: types.DevOps, + isFileServer: false, + protocol: types.HTTP, + + confidentiality: types.Confidential, + integrity: types.Critical, + + riskCreated: false, + }, + "file server access via ftp": { + outOfScope: false, + assetType: types.Datastore, + + isAcrossTrustBoundary: true, + isSharingSameParentTrustBoundary: false, + usage: types.Business, + isFileServer: true, + protocol: types.FTP, + + confidentiality: types.Confidential, + integrity: types.Critical, + + riskCreated: false, + }, + "file server access via ftp (FTPS)": { + outOfScope: false, + assetType: types.Datastore, + + isAcrossTrustBoundary: true, + isSharingSameParentTrustBoundary: false, + usage: types.Business, + isFileServer: true, + protocol: types.FTPS, + + confidentiality: types.Confidential, + integrity: types.Critical, + + riskCreated: false, + }, + "file server access via ftp (SFTP)": { + outOfScope: false, + assetType: types.Datastore, + + isAcrossTrustBoundary: true, + isSharingSameParentTrustBoundary: false, + usage: types.Business, + isFileServer: true, + protocol: types.SFTP, + + confidentiality: types.Confidential, + integrity: types.Critical, + + riskCreated: false, + }, + "low impact": { + outOfScope: false, + assetType: types.Datastore, + + isAcrossTrustBoundary: true, + isSharingSameParentTrustBoundary: false, + usage: types.Business, + isFileServer: false, + protocol: types.HTTP, + + confidentiality: types.Confidential, + integrity: types.Critical, + + riskCreated: true, + expectedImpact: types.LowImpact, + }, + "high raa medium impact": { + outOfScope: false, + assetType: types.Datastore, + + isAcrossTrustBoundary: true, + isSharingSameParentTrustBoundary: false, + usage: types.Business, + isFileServer: false, + protocol: types.HTTP, + raa: 50, + + confidentiality: types.Confidential, + integrity: types.Critical, + + riskCreated: true, + expectedImpact: types.MediumImpact, + }, + "strict confidentiality medium impact": { + outOfScope: false, + assetType: types.Datastore, + + isAcrossTrustBoundary: true, + isSharingSameParentTrustBoundary: false, + usage: types.Business, + isFileServer: false, + protocol: types.HTTP, + + confidentiality: types.StrictlyConfidential, + integrity: types.Critical, + + riskCreated: true, + expectedImpact: types.MediumImpact, + }, + "mission critical integrity medium impact": { + outOfScope: false, + assetType: types.Datastore, + + isAcrossTrustBoundary: true, + isSharingSameParentTrustBoundary: false, + usage: types.Business, + isFileServer: false, + protocol: types.HTTP, + + confidentiality: types.Confidential, + integrity: types.MissionCritical, + + riskCreated: true, + expectedImpact: types.MediumImpact, + }, + } + + for name, testCase := range testCases { + t.Run(name, func(t *testing.T) { + rule := NewUnguardedDirectDatastoreAccessRule() + tb1 := &types.TrustBoundary{ + Id: "tb1", + Title: "First Trust Boundary", + TechnicalAssetsInside: []string{"source", "target"}, + Type: types.NetworkCloudProvider, + } + tb2 := &types.TrustBoundary{ + Id: "tb2", + Title: "Second Trust Boundary", + Type: types.NetworkCloudProvider, + } + if testCase.isAcrossTrustBoundary { + tb1.TechnicalAssetsInside = []string{"source"} + tb2.TechnicalAssetsInside = []string{"target"} + } + directContainingTrustBoundaryMappedByTechnicalAssetId := map[string]*types.TrustBoundary{ + "source": tb1, + "target": tb1, + } + if testCase.isAcrossTrustBoundary { + directContainingTrustBoundaryMappedByTechnicalAssetId["target"] = tb2 + } + + tb3 := &types.TrustBoundary{ + Id: "tb3", + Title: "Sharing Trust Boundary", + Type: types.NetworkCloudProvider, + } + if testCase.isSharingSameParentTrustBoundary { + tb3.TrustBoundariesNested = []string{"tb1", "tb2"} + } + risks, err := rule.GenerateRisks(&types.Model{ + TechnicalAssets: map[string]*types.TechnicalAsset{ + "source": { + Id: "source", + Title: "Source Technical Asset", + Technologies: types.TechnologyList{ + { + Attributes: map[string]bool{ + types.IdentityProvider: testCase.isSourceIdentityProvider, + }, + }, + }, + }, + "target": { + Id: "target", + Title: "Target Technical Asset", + OutOfScope: testCase.outOfScope, + Type: testCase.assetType, + RAA: testCase.raa, + CommunicationLinks: []*types.CommunicationLink{ + { + Title: "Test Communication Link", + SourceId: "source", + TargetId: "target", + Protocol: testCase.protocol, + Usage: testCase.usage, + }, + }, + Technologies: types.TechnologyList{ + { + Attributes: map[string]bool{ + types.FileServer: testCase.isFileServer, + types.IsIdentityStore: testCase.isIdentityStore, + }, + }, + }, + Confidentiality: testCase.confidentiality, + Integrity: testCase.integrity, + }, + }, + IncomingTechnicalCommunicationLinksMappedByTargetId: map[string][]*types.CommunicationLink{ + "target": { + { + Title: "Test Communication Link", + SourceId: "source", + TargetId: "target", + Protocol: testCase.protocol, + Usage: testCase.usage, + }, + }, + }, + TrustBoundaries: map[string]*types.TrustBoundary{ + "tb1": tb1, + "tb2": tb2, + "tb3": tb3, + }, + DirectContainingTrustBoundaryMappedByTechnicalAssetId: directContainingTrustBoundaryMappedByTechnicalAssetId, + }) + + assert.Nil(t, err) + if testCase.riskCreated { + assert.NotEmpty(t, risks) + assert.Equal(t, testCase.expectedImpact, risks[0].ExploitationImpact) + expectedMessage := "Unguarded Direct Datastore Access of Target Technical Asset by Source Technical Asset via Test Communication Link" + assert.Equal(t, risks[0].Title, expectedMessage) + } else { + assert.Empty(t, risks) + } + }) + } +} + +func TestIsSharingSameParentTrustBoundaryBothOutOfTrustBoundaryExpectTrue(t *testing.T) { + input := &types.Model{ + TrustBoundaries: map[string]*types.TrustBoundary{ + "tb1": { + Id: "tb1", + }, + "tb2": { + Id: "tb2", + }, + }, + } + left := &types.TechnicalAsset{ + Id: "ta1", + } + right := &types.TechnicalAsset{ + Id: "ta2", + } + assert.True(t, isSharingSameParentTrustBoundary(input, left, right)) +} + +func TestIsSharingSameParentTrustBoundaryLeftOutOfTrustBoundaryExpectFalse(t *testing.T) { + input := &types.Model{ + TrustBoundaries: map[string]*types.TrustBoundary{ + "tb1": { + Id: "tb1", + }, + "tb2": { + Id: "tb2", + TechnicalAssetsInside: []string{"ta2"}, + }, + }, + } + left := &types.TechnicalAsset{ + Id: "ta1", + } + right := &types.TechnicalAsset{ + Id: "ta2", + } + assert.False(t, isSharingSameParentTrustBoundary(input, left, right)) +} + +func TestIsSharingSameParentTrustBoundaryRightOutOfTrustBoundaryExpectFalse(t *testing.T) { + input := &types.Model{ + TrustBoundaries: map[string]*types.TrustBoundary{ + "tb1": { + Id: "tb1", + TechnicalAssetsInside: []string{"ta1"}, + }, + "tb2": { + Id: "tb2", + }, + }, + } + left := &types.TechnicalAsset{ + Id: "ta1", + } + right := &types.TechnicalAsset{ + Id: "ta2", + } + assert.False(t, isSharingSameParentTrustBoundary(input, left, right)) +} + +func TestIsSharingSameParentTrustBoundaryInDifferentTrustBoundariesExpectFalse(t *testing.T) { + input := &types.Model{ + TrustBoundaries: map[string]*types.TrustBoundary{ + "tb1": { + Id: "tb1", + TechnicalAssetsInside: []string{"ta1"}, + }, + "tb2": { + Id: "tb2", + }, + }, + } + left := &types.TechnicalAsset{ + Id: "ta1", + } + right := &types.TechnicalAsset{ + Id: "ta2", + } + assert.False(t, isSharingSameParentTrustBoundary(input, left, right)) +}