Secret Token Validation #128
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…o token_validation
attiasas
added
new feature
github-actions
bot
removed
the
safe to test
…o token_validation
attiasas
requested changes
Aug 12, 2024
| Watches: components.NewStringFlag(Watches, "A comma-separated list of Xray watches, to determine Xray's violations creation."), | ||
| RepoPath: components.NewStringFlag(RepoPath, "Target repo path, to enable Xray to determine watches accordingly."), | ||
| Licenses: components.NewBoolFlag(Licenses, "Set to true if you'd like to receive licenses from Xray scanning."), | ||
| SecretValidation: components.NewBoolFlag(SecretValidation, "Set it if you want exposures scanner to validate api tokens"), |
There was a problem hiding this comment.
Suggested change
| SecretValidation: components.NewBoolFlag(SecretValidation, "Set it if you want exposures scanner to validate api tokens"), | |
| SecretValidation: components.NewBoolFlag(SecretValidation, "Set to true if you want exposures scanner to validate api tokens"), |
Comment on lines
129
to
+150
|
|
||
| func GetResultPropertyTokenValidation(result *sarif.Result) string { | ||
| if result != nil && result.Properties != nil && result.Properties["tokenValidation"] != nil { | ||
| status, ok := result.Properties["tokenValidation"].(string) | ||
| if !ok { | ||
| return "" | ||
| } | ||
| return status | ||
| } | ||
| return "" | ||
| } | ||
|
|
||
| func GetResultPropertyMetadata(result *sarif.Result) string { | ||
| if result != nil && result.Properties != nil && result.Properties["metadata"] != nil { | ||
| metadata, ok := result.Properties["metadata"].(string) | ||
| if !ok { | ||
| return "" | ||
| } | ||
| return metadata | ||
| } | ||
| return "" | ||
| } |
There was a problem hiding this comment.
Suggested change
| func GetResultPropertyTokenValidation(result *sarif.Result) string { | |
| if result != nil && result.Properties != nil && result.Properties["tokenValidation"] != nil { | |
| status, ok := result.Properties["tokenValidation"].(string) | |
| if !ok { | |
| return "" | |
| } | |
| return status | |
| } | |
| return "" | |
| } | |
| func GetResultPropertyMetadata(result *sarif.Result) string { | |
| if result != nil && result.Properties != nil && result.Properties["metadata"] != nil { | |
| metadata, ok := result.Properties["metadata"].(string) | |
| if !ok { | |
| return "" | |
| } | |
| return metadata | |
| } | |
| return "" | |
| } | |
| func GetResultProperty(key string, result *sarif.Result) string { | |
| if result != nil && result.Properties != nil && result.Properties[key] != nil { | |
| status, ok := result.Properties[key].(string) | |
| if !ok { | |
| return "" | |
| } | |
| return status | |
| } | |
| return "" | |
| } | |
| func GetResultPropertyTokenValidation(result *sarif.Result) string { | |
| return GetResultProperty("tokenValidation", result) | |
| } | |
| func GetResultPropertyMetadata(result *sarif.Result) string { | |
| return GetResultProperty("metadata", result) | |
| } |
reduce code duplications
Comment on lines
+36
to
+45
| func CreateResultWithTokenValidation(msg, ruleId, level string, tokenValidation string, metadata string, locations ...*sarif.Location) *sarif.Result { | ||
| result := &sarif.Result{ | ||
| Message: *sarif.NewTextMessage(msg), | ||
| Level: &level, | ||
| RuleID: &ruleId, | ||
| Locations: locations, | ||
| } | ||
| result.Properties = map[string]interface{}{"tokenValidation": tokenValidation, "metadata": metadata} | ||
| return result | ||
| } |
There was a problem hiding this comment.
Suggested change
| func CreateResultWithTokenValidation(msg, ruleId, level string, tokenValidation string, metadata string, locations ...*sarif.Location) *sarif.Result { | |
| result := &sarif.Result{ | |
| Message: *sarif.NewTextMessage(msg), | |
| Level: &level, | |
| RuleID: &ruleId, | |
| Locations: locations, | |
| } | |
| result.Properties = map[string]interface{}{"tokenValidation": tokenValidation, "metadata": metadata} | |
| return result | |
| } | |
| func CreateResultWithProperties(msg, ruleId, level string, properties map[string]string, locations ...*sarif.Location) *sarif.Result { | |
| result := &sarif.Result{ | |
| Message: *sarif.NewTextMessage(msg), | |
| Level: &level, | |
| RuleID: &ruleId, | |
| Locations: locations, | |
| Properties: make(sarif.Properties), | |
| } | |
| for key, val := range properties { | |
| result.Properties[key] = value | |
| } | |
| return result | |
| } |
Make it general and not specific
Comment on lines
+128
to
+134
| var tokenValidationOrder = map[string]int{ | ||
| "Active": 1, | ||
| "Inactive": 2, | ||
| "Unsupported": 3, | ||
| "Unavailable": 4, | ||
| "": 5, | ||
| } |
There was a problem hiding this comment.
type TokenStatus string
Move this to jasutils, create a new type to be used in all places (just like ApplicableStatus)
Comment on lines
+136
to
+151
| type SourceCodeRows []SourceCodeRow | ||
|
|
||
| func (a SourceCodeRows) Less(i, j int) bool { | ||
| if tokenValidationOrder[a[i].TokenValidation] != tokenValidationOrder[a[j].TokenValidation] { | ||
| return tokenValidationOrder[a[i].TokenValidation] < tokenValidationOrder[a[j].TokenValidation] | ||
| } | ||
| return a[i].SeverityNumValue > a[j].SeverityNumValue | ||
| } | ||
|
|
||
| func (a SourceCodeRows) Len() int { | ||
| return len(a) | ||
| } | ||
|
|
||
| func (a SourceCodeRows) Swap(i, j int) { | ||
| a[i], a[j] = a[j], a[i] | ||
| } |
There was a problem hiding this comment.
No need for new type, please remove. move logic to the actual usage at resultstable.go use anonymus func
Comment on lines
+443
to
+445
| amEnvVars := jas.GetAnalyzerManagerXscEnvVars("", techutils.Technology(graphScanResults.ScannedPackageType)) | ||
| jas.AppendTokenValidationToEnvVars(amEnvVars, validateSecrets) | ||
| scanner, err = jas.CreateJasScanner(scanner, jfrogAppsConfig, scanCmd.serverDetails, amEnvVars) |
There was a problem hiding this comment.
Suggested change
| amEnvVars := jas.GetAnalyzerManagerXscEnvVars("", techutils.Technology(graphScanResults.ScannedPackageType)) | |
| jas.AppendTokenValidationToEnvVars(amEnvVars, validateSecrets) | |
| scanner, err = jas.CreateJasScanner(scanner, jfrogAppsConfig, scanCmd.serverDetails, amEnvVars) | |
| scanner, err = jas.CreateJasScanner(scanner, jfrogAppsConfig, scanCmd.serverDetails, jas.GetAnalyzerManagerXscEnvVars("", techutils.Technology(graphScanResults.ScannedPackageType), validateSecrets)) |
no need for AppendTokenValidationToEnvVars this is a new var for running the AM, it should be in the right method
Comment on lines
+218
to
+224
| dynamicTokenVersionMismatchErr := clientutils.ValidateMinimumVersion(clientutils.Xray, xrayVersion, jasutils.DynamicTokenValidationMinXrayVersion) | ||
| if dynamicTokenVersionMismatchErr != nil && (scanCmd.validateSecrets) { | ||
| log.Warn("Token validation (--validate-secrets flag) is not supported in your xray version") | ||
| scanResults.ExtendedScanResults.SecretValidation = false | ||
| } else { | ||
| scanResults.ExtendedScanResults.SecretValidation = jas.CheckForSecretValidation(xrayManager, scanCmd.validateSecrets) | ||
| } |
There was a problem hiding this comment.
to reduce code duplication and to make sure this is always running, put clientutils.ValidateMinimumVersion inside of CheckForSecretValidation right before running the API call.
(also true for the one in audit)
Comment on lines
+203
to
+209
| dynamicTokenVersionMismatchErr := clientutils.ValidateMinimumVersion(clientutils.Xray, auditParams.xrayVersion, jasutils.DynamicTokenValidationMinXrayVersion) | ||
| if dynamicTokenVersionMismatchErr != nil && (auditParams.AuditBasicParams.ValidateSecrets) { | ||
| log.Warn("Token validation (--validate-secrets flag) is not supported in your xray version") | ||
| results.ExtendedScanResults.SecretValidation = false | ||
| } else { | ||
| results.ExtendedScanResults.SecretValidation = jas.CheckForSecretValidation(xrayManager, auditParams.AuditBasicParams.ValidateSecrets) | ||
| } |
There was a problem hiding this comment.
to reduce code duplication and to make sure this is always running, put clientutils.ValidateMinimumVersion inside of CheckForSecretValidation right before running the API call.
(also true for the one in scan)
Comment on lines
+276
to
+278
| amEnvVars := jas.GetAnalyzerManagerXscEnvVars(auditParams.commonGraphScanParams.MultiScanId, scanResults.GetScaScannedTechnologies()...) | ||
| jas.AppendTokenValidationToEnvVars(amEnvVars, scanResults.ExtendedScanResults.SecretValidation) | ||
| scanner, err = jas.CreateJasScanner(scanner, jfrogAppsConfig, serverDetails, amEnvVars, auditParams.Exclusions()...) |
There was a problem hiding this comment.
Suggested change
| amEnvVars := jas.GetAnalyzerManagerXscEnvVars(auditParams.commonGraphScanParams.MultiScanId, scanResults.GetScaScannedTechnologies()...) | |
| jas.AppendTokenValidationToEnvVars(amEnvVars, scanResults.ExtendedScanResults.SecretValidation) | |
| scanner, err = jas.CreateJasScanner(scanner, jfrogAppsConfig, serverDetails, amEnvVars, auditParams.Exclusions()...) | |
| scanner, err = jas.CreateJasScanner(scanner, jfrogAppsConfig, scanCmd.serverDetails, jas.GetAnalyzerManagerXscEnvVars("", techutils.Technology(graphScanResults.ScannedPackageType), validateSecrets)) |
no need for AppendTokenValidationToEnvVars this is a new var for running the AM, it should be in the right method
commands/audit/scarunner.go
Outdated
| @@ -212,7 +212,7 @@ type DependencyTreeResult struct { | |||
| } | |||
|
|
|||
| func GetTechDependencyTree(params xrayutils.AuditParams, artifactoryServerDetails *config.ServerDetails, tech techutils.Technology) (depTreeResult DependencyTreeResult, err error) { | |||
| logMessage := fmt.Sprintf("Calculating %s dependencies", tech.ToFormal()) | |||
| logMessage := fmt.Sprintf("Calculating %s dependencies DEBUG MODE ACTIVATED", tech.ToFormal()) | |||
There was a problem hiding this comment.
Suggested change
| logMessage := fmt.Sprintf("Calculating %s dependencies DEBUG MODE ACTIVATED", tech.ToFormal()) | |
| logMessage := fmt.Sprintf("Calculating %s dependencies", tech.ToFormal()) |
remove to old value
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
devbranch.go vet ./....go fmt ./....Depends on - jfrog/documentation#145
Depends on - jfrog/jfrog-client-go#991
Description - adding a flag --validate-secrets to jf audit and jf docker scan so secrets found will trigger token validation on XRAY. Token validation takes secrets that are api tokens for example amazon secret key and checks if this key is still valid on amazon side. The capability is identical to audit and docker scan.
What I do is I pass an env var to analyzermanager because analyzers contain an env variable which according to its value (true/false) turns on the Gadget which is responsible for token validation.
As you see, there are multiple options to pass this env var, first through flag --validate-secrets, second through env var defined in user setup, third XRAY API which exists only from 3.101.0. otherwise it returns False.