Merge pull request #4 from joreiche/pr-18-transfer

Stored, sent and received data assets are always processed

Author: joreiche

cmd/raa/main.go

@@ -167,12 +167,14 @@ func calculateAttackerAttractiveness(input *types.ParsedModel, techAsset types.T
 		score += dataAsset.Integrity.AttackerAttractivenessForProcessedOrStoredData() * dataAsset.Quantity.QuantityFactor()
 		score += dataAsset.Availability.AttackerAttractivenessForProcessedOrStoredData()
 	}
+	// NOTE: Assuming all stored data is also processed, this effectively scores stored data twice
 	for _, dataAssetStored := range techAsset.DataAssetsStored {
 		dataAsset := input.DataAssets[dataAssetStored]
 		score += dataAsset.Confidentiality.AttackerAttractivenessForProcessedOrStoredData() * dataAsset.Quantity.QuantityFactor()
 		score += dataAsset.Integrity.AttackerAttractivenessForProcessedOrStoredData() * dataAsset.Quantity.QuantityFactor()
 		score += dataAsset.Availability.AttackerAttractivenessForProcessedOrStoredData()
 	}
+	// NOTE: To send or receive data effectively is processing that data and it's questionable if the attractiveness increases further
 	for _, dataFlow := range techAsset.CommunicationLinks {
 		for _, dataAssetSent := range dataFlow.DataAssetsSent {
 			dataAsset := input.DataAssets[dataAssetSent]

pkg/model/parse.go

@@ -129,29 +129,35 @@ func ParseModel(modelInput *input.Model, builtinRiskRules map[string]risks.RiskR
 			return nil, errors.New("unknown 'usage' value of technical asset '" + title + "': " + asset.Usage)
 		}
 
-		var dataAssetsProcessed = make([]string, 0)
-		if asset.DataAssetsProcessed != nil {
-			dataAssetsProcessed = make([]string, len(asset.DataAssetsProcessed))
-			for i, parsedProcessedAsset := range asset.DataAssetsProcessed {
-				referencedAsset := fmt.Sprintf("%v", parsedProcessedAsset)
+		var dataAssetsStored = make([]string, 0)
+		if asset.DataAssetsStored != nil {
+			for _, parsedStoredAssets := range asset.DataAssetsStored {
+				referencedAsset := fmt.Sprintf("%v", parsedStoredAssets)
+				if contains(dataAssetsStored, referencedAsset) {
+					continue
+				}
+
 				err := parsedModel.CheckDataAssetTargetExists(referencedAsset, "technical asset '"+title+"'")
 				if err != nil {
 					return nil, err
 				}
-				dataAssetsProcessed[i] = referencedAsset
+				dataAssetsStored = append(dataAssetsStored, referencedAsset)
 			}
 		}
 
-		var dataAssetsStored = make([]string, 0)
-		if asset.DataAssetsStored != nil {
-			dataAssetsStored = make([]string, len(asset.DataAssetsStored))
-			for i, parsedStoredAssets := range asset.DataAssetsStored {
-				referencedAsset := fmt.Sprintf("%v", parsedStoredAssets)
+		var dataAssetsProcessed = dataAssetsStored
+		if asset.DataAssetsProcessed != nil {
+			for _, parsedProcessedAsset := range asset.DataAssetsProcessed {
+				referencedAsset := fmt.Sprintf("%v", parsedProcessedAsset)
+				if contains(dataAssetsProcessed, referencedAsset) {
+					continue
+				}
+
 				err := parsedModel.CheckDataAssetTargetExists(referencedAsset, "technical asset '"+title+"'")
 				if err != nil {
 					return nil, err
 				}
-				dataAssetsStored[i] = referencedAsset
+				dataAssetsProcessed = append(dataAssetsProcessed, referencedAsset)
 			}
 		}
 
@@ -227,22 +233,36 @@ func ParseModel(modelInput *input.Model, builtinRiskRules map[string]risks.RiskR
 				if commLink.DataAssetsSent != nil {
 					for _, dataAssetSent := range commLink.DataAssetsSent {
 						referencedAsset := fmt.Sprintf("%v", dataAssetSent)
-						err := parsedModel.CheckDataAssetTargetExists(referencedAsset, "communication link '"+commLinkTitle+"' of technical asset '"+title+"'")
-						if err != nil {
-							return nil, err
+						if !contains(dataAssetsSent, referencedAsset) {
+							err := parsedModel.CheckDataAssetTargetExists(referencedAsset, "communication link '"+commLinkTitle+"' of technical asset '"+title+"'")
+							if err != nil {
+								return nil, err
+							}
+
+							dataAssetsSent = append(dataAssetsSent, referencedAsset)
+							if !contains(dataAssetsProcessed, referencedAsset) {
+								dataAssetsProcessed = append(dataAssetsProcessed, referencedAsset)
+							}
 						}
-						dataAssetsSent = append(dataAssetsSent, referencedAsset)
 					}
 				}
 
 				if commLink.DataAssetsReceived != nil {
 					for _, dataAssetReceived := range commLink.DataAssetsReceived {
 						referencedAsset := fmt.Sprintf("%v", dataAssetReceived)
+						if contains(dataAssetsReceived, referencedAsset) {
+							continue
+						}
+
 						err := parsedModel.CheckDataAssetTargetExists(referencedAsset, "communication link '"+commLinkTitle+"' of technical asset '"+title+"'")
 						if err != nil {
 							return nil, err
 						}
 						dataAssetsReceived = append(dataAssetsReceived, referencedAsset)
+
+						if !contains(dataAssetsProcessed, referencedAsset) {
+							dataAssetsProcessed = append(dataAssetsProcessed, referencedAsset)
+						}
 					}
 				}
 
@@ -334,6 +354,29 @@ func ParseModel(modelInput *input.Model, builtinRiskRules map[string]risks.RiskR
 		}
 	}
 
+	// A target of a communication link implicitly processes all data assets that are sent to or received by that target
+	for id, techAsset := range parsedModel.TechnicalAssets {
+		for _, commLink := range techAsset.CommunicationLinks {
+			if commLink.TargetId == id {
+				continue
+			}
+			targetTechAsset := parsedModel.TechnicalAssets[commLink.TargetId]
+			dataAssetsProcessedByTarget := targetTechAsset.DataAssetsProcessed
+			for _, dataAssetSent := range commLink.DataAssetsSent {
+				if !contains(dataAssetsProcessedByTarget, dataAssetSent) {
+					dataAssetsProcessedByTarget = append(dataAssetsProcessedByTarget, dataAssetSent)
+				}
+			}
+			for _, dataAssetReceived := range commLink.DataAssetsReceived {
+				if !contains(dataAssetsProcessedByTarget, dataAssetReceived) {
+					dataAssetsProcessedByTarget = append(dataAssetsProcessedByTarget, dataAssetReceived)
+				}
+			}
+			targetTechAsset.DataAssetsProcessed = dataAssetsProcessedByTarget
+			parsedModel.TechnicalAssets[commLink.TargetId] = targetTechAsset
+		}
+	}
+
 	// Trust Boundaries ===============================================================================
 	checklistToAvoidAssetBeingModeledInMultipleTrustBoundaries := make(map[string]bool)
 	parsedModel.TrustBoundaries = make(map[string]types.TrustBoundary)
@@ -713,3 +756,12 @@ func lowerCaseAndTrim(tags []string) []string {
 	}
 	return tags
 }
+
+func contains(a []string, x string) bool {
+	for _, n := range a {
+		if x == n {
+			return true
+		}
+	}
+	return false
+}

pkg/report/colors.go

@@ -367,18 +367,12 @@ func determineTechnicalAssetLabelColor(ta types.TechnicalAsset, model *types.Par
 
 // red when mission-critical integrity, but still unauthenticated (non-readonly) channels access it
 // amber when critical integrity, but still unauthenticated (non-readonly) channels access it
-// pink when model forgery attempt (i.e. nothing being processed or stored)
-
+// pink when model forgery attempt (i.e. nothing being processed)
 func determineShapeBorderColor(ta types.TechnicalAsset, parsedModel *types.ParsedModel) string {
 	// Check for red
 	if ta.Confidentiality == types.StrictlyConfidential {
 		return Red
 	}
-	for _, storedDataAsset := range ta.DataAssetsStored {
-		if parsedModel.DataAssets[storedDataAsset].Confidentiality == types.StrictlyConfidential {
-			return Red
-		}
-	}
 	for _, processedDataAsset := range ta.DataAssetsProcessed {
 		if parsedModel.DataAssets[processedDataAsset].Confidentiality == types.StrictlyConfidential {
 			return Red
@@ -388,11 +382,6 @@ func determineShapeBorderColor(ta types.TechnicalAsset, parsedModel *types.Parse
 	if ta.Confidentiality == types.Confidential {
 		return Amber
 	}
-	for _, storedDataAsset := range ta.DataAssetsStored {
-		if parsedModel.DataAssets[storedDataAsset].Confidentiality == types.Confidential {
-			return Amber
-		}
-	}
 	for _, processedDataAsset := range ta.DataAssetsProcessed {
 		if parsedModel.DataAssets[processedDataAsset].Confidentiality == types.Confidential {
 			return Amber
@@ -427,7 +416,7 @@ func determineShapeBorderColor(ta types.TechnicalAsset, parsedModel *types.Parse
 // dotted when model forgery attempt (i.e. nothing being processed or stored)
 
 func determineShapeBorderLineStyle(ta types.TechnicalAsset) string {
-	if len(ta.DataAssetsProcessed) == 0 && len(ta.DataAssetsStored) == 0 || ta.OutOfScope {
+	if len(ta.DataAssetsProcessed) == 0 || ta.OutOfScope {
 		return "dotted" // dotted, because it's strange when too many technical communication links transfer no data... some ok, but many in a diagram ist a sign of model forgery...
 	}
 	return "solid"

pkg/security/risks/builtin/accidental-secret-leak-rule.go

@@ -29,7 +29,7 @@ func (*AccidentalSecretLeakRule) Category() types.RiskCategory {
 		Function:                   types.Operations,
 		STRIDE:                     types.InformationDisclosure,
 		DetectionLogic:             "In-scope sourcecode repositories and artifact registries.",
-		RiskAssessment:             "The risk rating depends on the sensitivity of the technical asset itself and of the data assets processed and stored.",
+		RiskAssessment:             "The risk rating depends on the sensitivity of the technical asset itself and of the data assets processed.",
 		FalsePositives:             "Usually no false positives.",
 		ModelFailurePossibleReason: false,
 		CWE:                        200,

pkg/security/risks/builtin/container-platform-escape-rule.go

@@ -34,7 +34,7 @@ func (*ContainerPlatformEscapeRule) Category() types.RiskCategory {
 		Function:       types.Operations,
 		STRIDE:         types.ElevationOfPrivilege,
 		DetectionLogic: "In-scope container platforms.",
-		RiskAssessment: "The risk rating depends on the sensitivity of the technical asset itself and of the data assets processed and stored.",
+		RiskAssessment: "The risk rating depends on the sensitivity of the technical asset itself and of the data assets processed.",
 		FalsePositives: "Container platforms not running parts of the target architecture can be considered " +
 			"as false positives after individual review.",
 		ModelFailurePossibleReason: false,

pkg/security/risks/builtin/cross-site-scripting-rule.go

@@ -27,7 +27,7 @@ func (*CrossSiteScriptingRule) Category() types.RiskCategory {
 		Function:       types.Development,
 		STRIDE:         types.Tampering,
 		DetectionLogic: "In-scope web applications.",
-		RiskAssessment: "The risk rating depends on the sensitivity of the data processed or stored in the web application.",
+		RiskAssessment: "The risk rating depends on the sensitivity of the data processed in the web application.",
 		FalsePositives: "When the technical asset " +
 			"is not accessed via a browser-like component (i.e not by a human user initiating the request that " +
 			"gets passed through all components until it reaches the web application) this can be considered a false positive.",

pkg/security/risks/builtin/ldap-injection-rule.go

@@ -15,7 +15,7 @@ func (*LdapInjectionRule) Category() types.RiskCategory {
 		Id:    "ldap-injection",
 		Title: "LDAP-Injection",
 		Description: "When an LDAP server is accessed LDAP-Injection risks might arise. " +
-			"The risk rating depends on the sensitivity of the LDAP server itself and of the data assets processed or stored.",
+			"The risk rating depends on the sensitivity of the LDAP server itself and of the data assets processed.",
 		Impact:     "If this risk remains unmitigated, attackers might be able to modify LDAP queries and access more data from the LDAP server than allowed.",
 		ASVS:       "V5 - Validation, Sanitization and Encoding Verification Requirements",
 		CheatSheet: "https://cheatsheetseries.owasp.org/cheatsheets/LDAP_Injection_Prevention_Cheat_Sheet.html",
@@ -27,7 +27,7 @@ func (*LdapInjectionRule) Category() types.RiskCategory {
 		Function:       types.Development,
 		STRIDE:         types.Tampering,
 		DetectionLogic: "In-scope clients accessing LDAP servers via typical LDAP access protocols.",
-		RiskAssessment: "The risk rating depends on the sensitivity of the LDAP server itself and of the data assets processed or stored.",
+		RiskAssessment: "The risk rating depends on the sensitivity of the LDAP server itself and of the data assets processed.",
 		FalsePositives: "LDAP server queries by search values not consisting of parts controllable by the caller can be considered " +
 			"as false positives after individual review.",
 		ModelFailurePossibleReason: false,

pkg/security/risks/builtin/missing-authentication-rule.go

@@ -14,7 +14,7 @@ func (*MissingAuthenticationRule) Category() types.RiskCategory {
 	return types.RiskCategory{
 		Id:          "missing-authentication",
 		Title:       "Missing Authentication",
-		Description: "Technical assets (especially multi-tenant systems) should authenticate incoming requests when the asset processes or stores sensitive data. ",
+		Description: "Technical assets (especially multi-tenant systems) should authenticate incoming requests when the asset processes sensitive data. ",
 		Impact:      "If this risk is unmitigated, attackers might be able to access or modify sensitive data in an unauthenticated way.",
 		ASVS:        "V2 - Authentication Verification Requirements",
 		CheatSheet:  "https://cheatsheetseries.owasp.org/cheatsheets/Authentication_Cheat_Sheet.html",
@@ -24,7 +24,7 @@ func (*MissingAuthenticationRule) Category() types.RiskCategory {
 		Check:    "Are recommendations from the linked cheat sheet and referenced ASVS chapter applied?",
 		Function: types.Architecture,
 		STRIDE:   types.ElevationOfPrivilege,
-		DetectionLogic: "In-scope technical assets (except " + types.LoadBalancer.String() + ", " + types.ReverseProxy.String() + ", " + types.ServiceRegistry.String() + ", " + types.WAF.String() + ", " + types.IDS.String() + ", and " + types.IPS.String() + " and in-process calls) should authenticate incoming requests when the asset processes or stores " +
+		DetectionLogic: "In-scope technical assets (except " + types.LoadBalancer.String() + ", " + types.ReverseProxy.String() + ", " + types.ServiceRegistry.String() + ", " + types.WAF.String() + ", " + types.IDS.String() + ", and " + types.IPS.String() + " and in-process calls) should authenticate incoming requests when the asset processes " +
 			"sensitive data. This is especially the case for all multi-tenant assets (there even non-sensitive ones).",
 		RiskAssessment: "The risk rating (medium or high) " +
 			"depends on the sensitivity of the data sent across the communication link. Monitoring callers are exempted from this risk.",

pkg/security/risks/builtin/missing-cloud-hardening-rule.go

@@ -35,7 +35,7 @@ func (*MissingCloudHardeningRule) Category() types.RiskCategory {
 		Function:       types.Operations,
 		STRIDE:         types.Tampering,
 		DetectionLogic: "In-scope cloud components (either residing in cloud trust boundaries or more specifically tagged with cloud provider types).",
-		RiskAssessment: "The risk rating depends on the sensitivity of the technical asset itself and of the data assets processed and stored.",
+		RiskAssessment: "The risk rating depends on the sensitivity of the technical asset itself and of the data assets processed.",
 		FalsePositives: "Cloud components not running parts of the target architecture can be considered " +
 			"as false positives after individual review.",
 		ModelFailurePossibleReason: false,

pkg/security/risks/builtin/missing-file-validation-rule.go

@@ -28,7 +28,7 @@ func (*MissingFileValidationRule) Category() types.RiskCategory {
 		Function:       types.Development,
 		STRIDE:         types.Spoofing,
 		DetectionLogic: "In-scope technical assets with custom-developed code accepting file data formats.",
-		RiskAssessment: "The risk rating depends on the sensitivity of the technical asset itself and of the data assets processed and stored.",
+		RiskAssessment: "The risk rating depends on the sensitivity of the technical asset itself and of the data assets processed.",
 		FalsePositives: "Fully trusted (i.e. cryptographically signed or similar) files can be considered " +
 			"as false positives after individual review.",
 		ModelFailurePossibleReason: false,

pkg/security/risks/builtin/missing-hardening-rule.go

@@ -32,7 +32,7 @@ func (r *MissingHardeningRule) Category() types.RiskCategory {
 		STRIDE:   types.Tampering,
 		DetectionLogic: "In-scope technical assets with RAA values of " + strconv.Itoa(r.raaLimit) + " % or higher. " +
 			"Generally for high-value targets like data stores, application servers, identity providers and ERP systems this limit is reduced to " + strconv.Itoa(r.raaLimitReduced) + " %",
-		RiskAssessment:             "The risk rating depends on the sensitivity of the data processed or stored in the technical asset.",
+		RiskAssessment:             "The risk rating depends on the sensitivity of the data processed in the technical asset.",
 		FalsePositives:             "Usually no false positives.",
 		ModelFailurePossibleReason: false,
 		CWE:                        16,

pkg/security/risks/builtin/missing-identity-store-rule.go

@@ -27,7 +27,7 @@ func (*MissingIdentityStoreRule) Category() types.RiskCategory {
 		STRIDE:         types.Spoofing,
 		DetectionLogic: "Models with authenticated data-flows authorized via end user identity missing an in-scope identity store.",
 		RiskAssessment: "The risk rating depends on the sensitivity of the end user-identity authorized technical assets and " +
-			"their data assets processed and stored.",
+			"their data assets processed.",
 		FalsePositives: "Models only offering data/services without any real authentication need " +
 			"can be considered as false positives after individual review.",
 		ModelFailurePossibleReason: true,

pkg/security/risks/builtin/missing-vault-rule.go

@@ -28,7 +28,7 @@ func (*MissingVaultRule) Category() types.RiskCategory {
 		Function:       types.Architecture,
 		STRIDE:         types.InformationDisclosure,
 		DetectionLogic: "Models without a Vault (Secret Storage).",
-		RiskAssessment: "The risk rating depends on the sensitivity of the technical asset itself and of the data assets processed and stored.",
+		RiskAssessment: "The risk rating depends on the sensitivity of the technical asset itself and of the data assets processed.",
 		FalsePositives: "Models where no technical assets have any kind of sensitive config data to protect " +
 			"can be considered as false positives after individual review.",
 		ModelFailurePossibleReason: true,

pkg/security/risks/builtin/missing-waf-rule.go

@@ -27,7 +27,7 @@ func (*MissingWafRule) Category() types.RiskCategory {
 		Function:       types.Operations,
 		STRIDE:         types.Tampering,
 		DetectionLogic: "In-scope web-services and/or web-applications accessed across a network trust boundary not having a Web Application Firewall (WAF) in front of them.",
-		RiskAssessment: "The risk rating depends on the sensitivity of the technical asset itself and of the data assets processed and stored.",
+		RiskAssessment: "The risk rating depends on the sensitivity of the technical asset itself and of the data assets processed.",
 		FalsePositives: "Targets only accessible via WAFs or reverse proxies containing a WAF component (like ModSecurity) can be considered " +
 			"as false positives after individual review.",
 		ModelFailurePossibleReason: false,

pkg/security/risks/builtin/path-traversal-rule.go

@@ -15,7 +15,7 @@ func (*PathTraversalRule) Category() types.RiskCategory {
 		Id:    "path-traversal",
 		Title: "Path-Traversal",
 		Description: "When a filesystem is accessed Path-Traversal or Local-File-Inclusion (LFI) risks might arise. " +
-			"The risk rating depends on the sensitivity of the technical asset itself and of the data assets processed or stored.",
+			"The risk rating depends on the sensitivity of the technical asset itself and of the data assets processed.",
 		Impact: "If this risk is unmitigated, attackers might be able to read sensitive files (configuration data, key/credential files, deployment files, " +
 			"business data files, etc.) from the filesystem of affected components.",
 		ASVS:       "V12 - File and Resources Verification Requirements",

pkg/security/risks/builtin/search-query-injection-rule.go

@@ -30,7 +30,7 @@ func (*SearchQueryInjectionRule) Category() types.RiskCategory {
 		Function:       types.Development,
 		STRIDE:         types.Tampering,
 		DetectionLogic: "In-scope clients accessing search engine servers via typical search access protocols.",
-		RiskAssessment: "The risk rating depends on the sensitivity of the search engine server itself and of the data assets processed or stored.",
+		RiskAssessment: "The risk rating depends on the sensitivity of the search engine server itself and of the data assets processed.",
 		FalsePositives: "Server engine queries by search values not consisting of parts controllable by the caller can be considered " +
 			"as false positives after individual review.",
 		ModelFailurePossibleReason: false,