Skip to content

Commit

Permalink
Improve secret detection heuristics (#4289)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@theunrepentantgeek
theunrepentantgeek authored Oct 30, 2024
1 parent 0572b94 commit bd7fc44
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 25 deletions.
66 changes: 53 additions & 13 deletions v2/azure-arm.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1517,6 +1526,12 @@ objectModelConfiguration:
$isResource: false
NetworkProfile_STATUS:
$isResource: false
SshPublicKeySpec:
KeyData:
$isSecret: false
SshPublicKey_STATUS:
KeyData:
$isSecret: false
VMGalleryApplication:
PackageReferenceId:
$armReference: true
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1724,6 +1751,9 @@ objectModelConfiguration:
2023-10-01:
ContainerServiceNetworkProfile_NetworkPlugin:
$renameTo: NetworkPlugin
ContainerServiceSshPublicKey:
KeyData:
$isSecret: false
DelegatedResource:
ResourceId:
$armReference: true
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -2371,6 +2413,8 @@ objectModelConfiguration:
$armReference: false
2020-02-02:
ApplicationInsightsComponentProperties:
HockeyAppToken:
$isSecret: false
WorkspaceResourceId:
$armReference: true
Component:
Expand Down Expand Up @@ -2583,6 +2627,9 @@ objectModelConfiguration:
Databricks:
ResourceId:
$armReference: true
DatabricksProperties:
DatabricksAccessToken:
$isSecret: true
AKS:
ResourceId:
$armReference: true
Expand Down Expand Up @@ -2613,9 +2660,6 @@ objectModelConfiguration:
$isSecret: true
PublicKeyData:
$isSecret: true
DatabricksProperties:
DatabricksAccessToken:
$isSecret: true
Workspaces_Connection:
$exportAs: WorkspacesConnection
$supportedFrom: v2.0.0-beta.2
Expand Down Expand Up @@ -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
Expand Down
70 changes: 58 additions & 12 deletions v2/tools/generator/internal/codegen/pipeline/add_secrets.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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())
}

Expand All @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit bd7fc44

Please sign in to comment.