Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] SQL AVM Flagging false positives #3146

Closed
riosengineer opened this issue Oct 23, 2024 · 3 comments · Fixed by #3160
Closed

[BUG] SQL AVM Flagging false positives #3146

riosengineer opened this issue Oct 23, 2024 · 3 comments · Fixed by #3160
Assignees
Labels
bug Something isn't working rule: deployment Rule for Azure Resource Manager templates
Milestone

Comments

@riosengineer
Copy link

Existing rule

No response

Description of the issue

Hey @BernieWhite - hope you're well.

I am triggering these following rules:

  • Azure.Deployment.SecureParameter (AZR-000408)
  • Azure.Deployment.AdminUsername (AZR-000284)
    Running:
    PSRule v2.9.0 PSRule.Rules.Azure v1.38.0

When I am deploying the latest AVM for Azure SQL PaaS. I am actually passing an Entra security group for the login username and even with the ()secure param on this it still flags. I thought it would remove the alert by doing that to bypass it but no luck. I think this may be related or the same as: #2813 ?

  Recommend:
  Sensitive properties should be passed as parameters. Avoid using deterministic
  values for sensitive properties.

  Reason:
  - The property 'administratorLogin' uses a deterministic literal value.
  - The property 'administratorLoginPassword' uses a deterministic literal value.

  Help:
  - https://azure.github.io/PSRule.Rules.Azure/en/rules/Azure.Deployment.AdminUsername/

I am also getting a keys flag when I'm not using keys. I feel it is expanding the source module of the AVM SQL and maybe there is an issue from that.

Recommend:
 Consider using secure parameters for parameters that contain sensitive
 information.

 Reason:
 - Path keys: The parameter 'keys' with type 'array' is not secure.

 Help:
 - https://azure.github.io/PSRule.Rules.Azure/en/rules/Azure.Deployment.SecureParameter/

I know it's the AVM SQL because if I remove the module from my template, both these flags are not present. The source code is here: https://github.com/Azure/bicep-registry-modules/blob/main/avm/res/sql/server/main.bicep

Running the latest

Is this a potential bug? Thanks

Error messages

No response

Reproduction

Relevant code context to repro:

Param:

  sqlDatabase: {
      name: 'sqldb'
      tier: 'Basic'
      sku: 'Basic'
      maxSizeBytes: 2147483648
      collation: 'SQL_Latin1_General_CP1_CI_AS'
    }
  sqlAdministrators: {
      azureADOnlyAuthentication: true
      login: 'Some_Entra_Group'
      sid: 'asidhere'
      principalType: 'Group'
    }
  }

Template:

// Azure SQL DB
module sqlDb 'br/public:avm/res/sql/server:0.8.0' = {
  name: '${uniqueString(deployment().name, location)}-sql'
  scope: resourceGroup(rgName)
  params: {
    name: sqlServerName
    location: location
    minimalTlsVersion: '1.2'
    managedIdentities: {
      systemAssigned: true
    }
    privateEndpoints: [
      {
        name: 'pe'
        customNetworkInterfaceName: 'pe-sql-nic'
        subnetResourceId: vNet.outputs.subnetResourceIds[1]
        privateDnsZoneGroup: {
          privateDnsZoneGroupConfigs: [
            {
              privateDnsZoneResourceId: privateDNSZoneSql.id
            }
          ]
        }
      }
    ]
    publicNetworkAccess: 'Disabled'
    securityAlertPolicies: [
      {
        name: 'Default'
        state: 'Enabled'
        emailAccountAdmins: true
      }
    ]
    administrators: {
      azureADOnlyAuthentication: sqlAdministrators.azureADOnlyAuthentication
      login: sqlAdministrators.login
      sid: sqlAdministrators.sid
      principalType: sqlAdministrators.principalType
    }
    databases: [
      {
        name: sqlDatabase.name
        collation: sqlDatabase.collation
        maxSizeBytes: sqlDatabase.maxSizeBytes
        skuTier: sqlDatabase.tier
        skuName: sqlDatabase.sku
      }
    ]
    tags: union(tags, { Created: timeNow })
  }
}

Version of PSRule

2.9.0

Version of PSRule for Azure

No response

Additional context

No response

@riosengineer riosengineer added bug Something isn't working Needs: Triage 🔍 Needs attention from the team. labels Oct 23, 2024
@BernieWhite BernieWhite added rule: deployment Rule for Azure Resource Manager templates and removed Needs: Triage 🔍 Needs attention from the team. labels Oct 23, 2024
@BernieWhite BernieWhite added this to the v1.40.0 milestone Oct 23, 2024
@BernieWhite
Copy link
Collaborator

Thanks for reporting the issue @riosengineer.

@BernieWhite
Copy link
Collaborator

@riosengineer For Azure.Deployment.SecureParameter you can set the configuration option AZURE_DEPLOYMENT_NONSENSITIVE_PARAMETER_NAMES to override this behaviour. See: https://azure.github.io/PSRule.Rules.Azure/setup/configuring-rules/#azure_deployment_nonsensitive_parameter_names

For Azure.Deployment.AdminUsername I'm having trouble reproducing (on v1.39.3 and v1.38.0) locally are you able to provide a little more detail.

@riosengineer
Copy link
Author

@riosengineer For Azure.Deployment.SecureParameter you can set the configuration option AZURE_DEPLOYMENT_NONSENSITIVE_PARAMETER_NAMES to override this behaviour. See: https://azure.github.io/PSRule.Rules.Azure/setup/configuring-rules/#azure_deployment_nonsensitive_parameter_names

For Azure.Deployment.AdminUsername I'm having trouble reproducing (on v1.39.3 and v1.38.0) locally are you able to provide a little more detail.

Thanks, I should have known about that as I already use it in my ps-rule.yaml, apologises. That fixed the keys alert for Azure.Deployment.SecureParameter. I noticed actually, even AVM get the keys alert: https://github.com/Azure/bicep-registry-modules/actions/runs/11478762573/job/31943681905 too.

However, what is interesting is they aren't getting the alerts for the administratorLogin and administratorPassword as I notice they are running tests against a file that explicitly states those two params: https://github.com/Azure/bicep-registry-modules/blob/main/avm/res/sql/server/tests/e2e/defaults/main.test.bicep.

I did some more testing and found even explicitly stating the two params, PSRule returns the rule back twice? Almost like it's expecting this in the administrators: object as well?

main.bicep

@secure()
param sqlAdministrators object = {}
param sqlDatabase object = {}
param sqlAdministratorsUn string = 'test'
param sqlAdministratorsPw string = 'testing'
// Azure SQL DB
module sqlDb 'br/public:avm/res/sql/server:0.9.0' = {
  name: '${uniqueString(deployment().name, location)}-sql'
  scope: resourceGroup(rgName)
  params: {
    name: sqlServerName
    administratorLogin: sqlAdministratorsUn
    administratorLoginPassword: sqlAdministratorsPw
...
}

main.bicepparam

  sqlDatabase: {
      name: 'sqldb'
      tier: 'Basic'
      sku: 'Basic'
      maxSizeBytes: 2147483648
      collation: 'SQL_Latin1_General_CP1_CI_AS'
    }
  sqlAdministrators: {
      azureADOnlyAuthentication: true
      login: 'Some_Entra_Group'
      sid: 'asidhere'
      principalType: 'Group'
    }
  }

And noticed the rule returns it twice. The param is not required but I wonder if the object

FAIL  Azure.Deployment.AdminUsername (AZR-000284)

  Use secure parameters for sensitive resource properties.

  Template: bicep/.tests/main.test.bicep:0:0

  Recommend:
  Sensitive properties should be passed as parameters. Avoid using deterministic
  values for sensitive properties.

  Reason:
  - The property 'administratorLogin' uses a deterministic literal value.
  - The property 'administratorLogin' uses a deterministic literal value.
  - The property 'administratorLoginPassword' uses a deterministic literal value.
  - The property 'administratorLoginPassword' uses a deterministic literal value.

  Help:
  - https://azure.github.io/PSRule.Rules.Azure/en/rules/Azure.Deployment.AdminUsername/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working rule: deployment Rule for Azure Resource Manager templates
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants