From bd7fc44518f523a1b0491e8da2f9fa5453d23419 Mon Sep 17 00:00:00 2001 From: Bevan Arps Date: Thu, 31 Oct 2024 09:04:51 +1300 Subject: [PATCH] Improve secret detection heuristics (#4289) * Switch heuristic to use regexp * Add detection of token * Use rule based approach to secret detection * Update MachineLearningServices * Update Insights * Update CDN * Update Compute * Update ContainerService * Remove HockeyAppToken as it's not a secret * Document that ValidationToken is not a secret * Update generated files * Update deepcopy files * Tweak configuration file * Remove configuration we don't need * Fix wording of error --- v2/azure-arm.yaml | 66 +++++++++++++---- .../internal/codegen/pipeline/add_secrets.go | 70 +++++++++++++++---- 2 files changed, 111 insertions(+), 25 deletions(-) diff --git a/v2/azure-arm.yaml b/v2/azure-arm.yaml index 9baee4e8fea..403c33baa87 100644 --- a/v2/azure-arm.yaml +++ b/v2/azure-arm.yaml @@ -1361,6 +1361,9 @@ objectModelConfiguration: $isResource: false DeepCreatedOriginProperties_STATUS: $isResource: false + DomainValidationProperties: + ValidationToken: + $isSecret: false # Not a conventional secret; this is the token someone must provide to prove they own the domain Profile: $export: true $supportedFrom: v2.6.0 @@ -1454,6 +1457,12 @@ objectModelConfiguration: $isResource: false NetworkInterfaceReference_STATUS: $isResource: false + SshPublicKeySpec: + KeyData: + $isSecret: false + SshPublicKey_STATUS: + KeyData: + $isSecret: false VirtualMachine: $export: true $supportedFrom: v2.0.0-alpha.1 @@ -1517,6 +1526,12 @@ objectModelConfiguration: $isResource: false NetworkProfile_STATUS: $isResource: false + SshPublicKeySpec: + KeyData: + $isSecret: false + SshPublicKey_STATUS: + KeyData: + $isSecret: false VMGalleryApplication: PackageReferenceId: $armReference: true @@ -1631,6 +1646,12 @@ objectModelConfiguration: containerservice: $payloadType: explicitCollections 2021-05-01: + ContainerServiceSshPublicKey: + KeyData: + $isSecret: false + ContainerServiceSshPublicKey_STATUS: + KeyData: + $isSecret: false ManagedCluster: $export: true $supportedFrom: v2.0.0-alpha.1 @@ -1671,6 +1692,12 @@ objectModelConfiguration: Id: $armReference: true 2023-02-01: + ContainerServiceSshPublicKey: + KeyData: + $isSecret: false + ContainerServiceSshPublicKey_STATUS: + KeyData: + $isSecret: false ManagedCluster: $export: true $supportedFrom: v2.0.0 @@ -1724,6 +1751,9 @@ objectModelConfiguration: 2023-10-01: ContainerServiceNetworkProfile_NetworkPlugin: $renameTo: NetworkPlugin + ContainerServiceSshPublicKey: + KeyData: + $isSecret: false DelegatedResource: ResourceId: $armReference: true @@ -1762,13 +1792,19 @@ objectModelConfiguration: ManagedClusterWindowsProfile: AdminPassword: $isSecret: true - PrivateLinkResource: - Id: - $armReference: true ManagedClusters_TrustedAccessRoleBinding: $exportAs: TrustedAccessRoleBinding $supportedFrom: v2.8.0 + PrivateLinkResource: + Id: + $armReference: true 2023-11-02-preview: + ContainerServiceSshPublicKey: + KeyData: + $isSecret: false + ContainerServiceSshPublicKey_STATUS: + KeyData: + $isSecret: false DelegatedResource: ResourceId: $armReference: true @@ -1810,6 +1846,12 @@ objectModelConfiguration: Id: $armReference: true 2024-04-02-preview: + ContainerServiceSshPublicKey: + KeyData: + $isSecret: false + ContainerServiceSshPublicKey_STATUS: + KeyData: + $isSecret: false DelegatedResource: ResourceId: $armReference: true @@ -1847,12 +1889,12 @@ objectModelConfiguration: ManagedClusterWindowsProfile: AdminPassword: $isSecret: true - PrivateLinkResource: - Id: - $armReference: true ManagedClusters_TrustedAccessRoleBinding: $exportAs: TrustedAccessRoleBinding $supportedFrom: v2.8.0 + PrivateLinkResource: + Id: + $armReference: true datafactory: 2018-06-01: PurviewConfiguration: @@ -2371,6 +2413,8 @@ objectModelConfiguration: $armReference: false 2020-02-02: ApplicationInsightsComponentProperties: + HockeyAppToken: + $isSecret: false WorkspaceResourceId: $armReference: true Component: @@ -2583,6 +2627,9 @@ objectModelConfiguration: Databricks: ResourceId: $armReference: true + DatabricksProperties: + DatabricksAccessToken: + $isSecret: true AKS: ResourceId: $armReference: true @@ -2613,9 +2660,6 @@ objectModelConfiguration: $isSecret: true PublicKeyData: $isSecret: true - DatabricksProperties: - DatabricksAccessToken: - $isSecret: true Workspaces_Connection: $exportAs: WorkspacesConnection $supportedFrom: v2.0.0-beta.2 @@ -2722,15 +2766,11 @@ objectModelConfiguration: UserAccountCredentials: AdminUserPassword: $isSecret: true - AdminUserSshPublicKey: - $isSecret: true VirtualMachineSshCredentials: Password: $isSecret: true PrivateKeyData: $isSecret: true - PublicKeyData: - $isSecret: true Workspaces_Connection: $exportAs: WorkspacesConnection $supportedFrom: v2.10.0 diff --git a/v2/tools/generator/internal/codegen/pipeline/add_secrets.go b/v2/tools/generator/internal/codegen/pipeline/add_secrets.go index 2f7a60a73b8..134dbbe274e 100644 --- a/v2/tools/generator/internal/codegen/pipeline/add_secrets.go +++ b/v2/tools/generator/internal/codegen/pipeline/add_secrets.go @@ -7,9 +7,11 @@ package pipeline import ( "context" + "regexp" "strings" "github.com/pkg/errors" + kerrors "k8s.io/apimachinery/pkg/util/errors" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel" "github.com/Azure/azure-service-operator/v2/tools/generator/internal/config" @@ -68,11 +70,13 @@ func applyConfigSecretOverrides( isSecret, isSecretConfigured = config.ObjectModelConfiguration.IsSecret.Lookup(strippedTypeName, prop.PropertyName()) } - if maybeSecret && !prop.IsSecret() && !isSecretConfigured { + // If it's not a secret, but it looks like a secret, and we don't have any configuration to tell us for + // sure, request configuration so we know for sure. + if !prop.IsSecret() && maybeSecret && !isSecretConfigured { // Property might be a secret, but isn't already configured as one, // and we don't have config to tell us for sure return nil, errors.Errorf( - "property %q might be a secret and must to be configured with $isSecret", + "property %s might be a secret and must be configured with $isSecret", prop.PropertyName()) } @@ -89,6 +93,7 @@ func applyConfigSecretOverrides( VisitObjectType: applyConfigSecrets, }.Build() + var errs []error for _, def := range definitions { if def.Name().IsARMType() { // No need to process ARM types @@ -97,12 +102,19 @@ func applyConfigSecretOverrides( updatedDef, err := visitor.VisitDefinition(def, def.Name()) if err != nil { - return nil, errors.Wrapf(err, "visiting type %q", def.Name()) + errs = append(errs, errors.Wrapf(err, "visiting type %q", def.Name())) + continue } result.Add(updatedDef) } + if len(errs) > 0 { + return nil, errors.Wrap( + kerrors.NewAggregate(errs), + "encountered errors while applying config secrets") + } + // Verify that all 'isSecret' modifiers are consumed before returning the result err := config.ObjectModelConfiguration.IsSecret.VerifyConsumed() if err != nil { @@ -139,21 +151,55 @@ func mightBeSecretProperty( return false } - // If the property name contains any of the secret words, it might be a secret - n := strings.ToLower(string(prop.PropertyName())) - for _, secretWord := range secretWords { - if strings.Contains(n, secretWord) { - return true + // If the property name matches a detector, that tells us + // whether to expect it's a secret or not + propertyName := string(prop.PropertyName()) + for _, detector := range secretDetectors { + if detector.regex.MatchString(propertyName) { + return detector.isSecret } } return false } -// A list of secret words to scan for. -// Lowercase only, please. -var secretWords = []string{ - "password", +// Rules for detecting potentially secret properties. +// These are processed in order, with the first matching rule being used. +var secretDetectors = []struct { + regex regexp.Regexp // Regular expression to match + isSecret bool // Whether to treat the property as a secret +}{ + { + // Look for the word `password` in any position + regex: *regexp.MustCompile(`(?i)password`), + isSecret: true, + }, + { + // Look for the word `token` in any position + regex: *regexp.MustCompile(`(?i)token`), + isSecret: true, + }, + { + // a PublicKey is not a secret + regex: *regexp.MustCompile(`(?i)publickey`), + isSecret: false, + }, + { + // KeyData is always a secret + regex: *regexp.MustCompile(`(?i)keydata`), + isSecret: true, + }, + { + // URLs and URIs are not secrets + regex: *regexp.MustCompile(`(?i)url|uri`), + isSecret: false, + }, + { + // IDs, Identifiers, and Names are not secrets, they're used to look them up + // (must match at the end) + regex: *regexp.MustCompile(`(?i)(id|identifier|Identity|name)$`), + isSecret: false, + }, } func transformSpecSecrets(definitions astmodel.TypeDefinitionSet) (astmodel.TypeDefinitionSet, error) {