Azure / bicep

Bicep is a declarative language for describing and deploying Azure resources
MIT License
3.23k stars 748 forks source link

Allow `getSecret()` to be used with `@secure()` objects #8733

Open Vox1984 opened 2 years ago

Vox1984 commented 2 years ago

Bicep version Bicep CLI version 0.11.1 (030248df55)

Describe the bug I am not able to use keyvault getSecret with secure object params passed directly to module.

To Reproduce

This code snippet works: main.bicep

module testmodule 'testmodule.bicep' = {
  name: 'testmodule'
  params: {
    linuxPassword: kv.getSecret('linuxPassword')
  }
}

testmodule.bicep

@secure()
param linuxPassword string

This snippet does not: main.bicep

module testmodule 'testmodule.bicep' = {
  name: 'testmodule'
  params: {
    config: {
      linuxPassword: kv.getSecret('linuxPassword')
      SomeOtherConfigParam1: kv.getSecret('foo')
      SomeOtherConfigParam2: kv.getSecret('boo')
    }
  }
}

testmodule.bicep

@secure()
param config object

It yields with an error Function "getSecret" is not valid at this location. It can only be used when directly assigning to a module parameter with a secure decorator.bicep(BCP180)

Additional context I need to pass dozens of app settings as secure params to the module and I was hoping this would work. Those settings have nested structure.

Is bicep really supporting only basic types as string, bool, array for keyvault getSecret() function? Any hints here Guys?

slavizh commented 2 years ago

You have to have a separate parameter for each secret not one parameter that is object and put multiple getSecret() operations in it.

Vox1984 commented 2 years ago

Thanks, I was afraid of this answer. That means for my specific scenario getSecret() is useless. If I do secrets one-by one it is going to be awful mess - as some of the params have same name but belong to different objects. I could prepend some unique string identifying object as a part of the name, but its ugly. also limit of 256 params will be hit soon in the future... so it is a no.

alex-frankel commented 1 year ago

@Vox1984 Is it possible to share more detail as to the type of bicep module that would have 256 secret values? It would be helpful to understand the scenario/complexity in more detail.

@miqm / @jeskew -- out of curiosity, is this limit related to untyped objects and could it be addressed after #4158?

jeskew commented 1 year ago

@alex-frankel I think we'll need to figure out how the @secure decorator should work on typed objects. Currently, the secure flag can only be declared and never inferred, so the type of config.linuxPassword in @Vox1984's example would be string, not secureString. In theory, if config used property-level declarations, this limitation would no longer apply, e.g.,

@secure()
type secret = string

@secure
param config { [string]: secret }

would assign a type of secureString to every property of config, but you need a @secure() decorator at every level of the object to prevent the value being output. It might make sense to define rules for how the secure flag should cascade (automatically apply to every property of config because it is marked as secure), taint (automatically apply to config because it has one or more secure properties), or both. The current state of #8673 is that @secure() cannot be used on types or type properties because the information flow control expectations are a bit vague.

slavizh commented 1 year ago

I can provide an example with array of secure strings that you need to provide. Container apps - https://learn.microsoft.com/en-us/azure/templates/microsoft.web/containerapps?pivots=deployment-language-bicep for example have secrets which is array. So if you want to give the option end user to choose to provide 1, 10 or 100 secrets no easy way to do this.

Vox1984 commented 1 year ago

Hi Guys,

My scenario might be not 100% typical. It is about keeping appServices configuration in key vault, which is probably not the usual case.

The following snippet shows settings I wanted to move to key vault:

appServices: [
(...)
siteConfig: {
        linuxFxVersion: 'ubuntu:22'
        acrUseManagedIdentityCreds: false
        alwaysOn: true
        appSettings: [
          {
            name: 'settingNr1'
            value: 'valueofNr1'
          }
          {
            name: 'settingNr2'
            value: 'valueofNr2'
          }
          {
            name: 'settingNrN'
            value: 'valueofNrN'
          }

each app have same section and the naming sometimes overlaps. So as you see it would be nice to be able to pass to the module with getSecret() secure object appSettings at least.

@alex-frankel I can hit 256 limit easily just multiply appSettings (if we pass them one by one) by the number of apps. also object solves the issue of overlaping parameters with the same name, but belonging to different apps.

afscrome commented 1 year ago

@Vox1984 For app service settings, you can use a key vault reference, rather than getSecret. When you use getSecret to populate an app service config setting, the values will end up in clear text in the azure resource manager, sort of defeating the point of using a key vault.

Here's an example of doing this with bicep. App servics will load the AzureWebJobsStorage value from the StorageConnectionString secret in keyvault (Note, you'll need to ensure your web app has a managed identity, with access to the keyvault)

resource functionApp 'Microsoft.Web/sites@2022-03-01' existing = {
  name: functionAppName
  resource appSettings 'config' = {
    name: 'appsettings'
    properties: {
      AzureWebJobsStorage: '@Microsoft.KeyVault(SecretUri=https://${keyVaultName}.vault.azure.net/secrets/StorageConnectionString)'
    }
  }
}
Vox1984 commented 1 year ago

Hi afscrome,

I know I could use the reference, and that was my first choice. But as you said I would need to have managed identity. For some stupid project reasons this is non an option.

Also as far I understand, If I use getSecret and a secure string with @secure() decorator, those values won't be available after the deployment and they won't be in the code repo as well

Use securestring for all passwords and secrets. If you pass sensitive data in a JSON object, use the secureObject type. Template parameters with secure string or secure object types can't be read after resource deployment. https://learn.microsoft.com/en-us/azure/azure-resource-manager/templates/best-practices

P.S I have moved the most sensitive secrets to parameters.json file and read them to secure strings... that would do for now. but you could improve bicep for the future, if you decide to do so.

alex-frankel commented 1 year ago

@SimonWahlin -- can you take a look at this one?

SimonWahlin commented 1 year ago

It is generally not recommended to use KeyVaults for configuration settings, mainly for performance and throttling reasons, see Best practices for secrets management.

I would recommend looking at Azure App Configuration to centralize configuration over multiple app services and/or function apps. If using managed identity is not an option, you can connect to App Configuration using a Connection String.

If you still want to use a secret to supply an object to your module, you can do this today by using a secret string in your module and use the function json() to convert it to a json object/array as a workaround.

As an example I defined a parameter in my module named appSettingString like this:

@secure()
param appSettingString string

In my sites resource I then convert this to json like this (with unrelated properties cut out):

resource sites 'Microsoft.Web/sites@2022-03-01' = {
  name: name
  location: location
  properties:{
    siteConfig: {
      appSettings: json(appSettingString)
    })
  }
}

This lets me consume the module in main.bicep like this:

param serverFarmId string

resource kv 'Microsoft.KeyVault/vaults@2022-07-01' existing = {
  name: 'Issue8733-kv'
}

module site 'sites-functionApp.bicep' ={
  name: 'site'
  params: {
    appSettingString: kv.getSecret('settings')
    location: 'westeurope'
    name: 'Issue8733-site'
    serverFarmId: serverFarmId
  }
}

The secret settings contains the following data:

[
    {
        "name": "settingNr1",
        "value": "valueofNr1"
    },
    {
        "name": "settingNr2",
        "value": "valueofNr2"
    },
    {
        "name": "settingNrN",
        "value": "valueofNrN"
    }
]

Once deployed it looks like this in the portal:

image

SimonWahlin commented 1 year ago

Closing due to no response. Please re-open if this is still an issue.

onionhammer commented 1 year ago

I'm not sure why this was closed if no code changes were made to fix it? Is this still an issue? The comments regarding key vault performance are irrelevant since this issue has very little to do with that.

matferrari-msft commented 1 year ago

@SimonWahlin @alex-frankel Could this be re-opened? I think there are legitimate uses cases for this.

Moreover, I think the @secure() decorator should be compatible with arrays to enable passing a list of secrets to a module. I guess the new ask would be "Allow getSecret to be used with both objects and array types".

I put an example below. In my use case, I wish to pull a variable number of secrets from a key vault and use them as protectedParameters in a VM run command. As a workaround, I am having to use a module and script invocation for each secret (put the module in a for loop and call a separate run command for each secret), but this workaround only works for this specific use case. If I wanted to pass a variable number of secrets to something like an app settings above, this workaround would not work

myModule 'module.bicep' = {
    name: ...
    scope: ...
    properties: {
        secrets: [for secretName in listOfSecretNames: kv.getSecret(secretName)]
    }
}
coolhome commented 1 year ago

Using LogicApps I have to create Microsoft.Web/connections but I am unable to pass the API Key from getSecrets to parameterValues.

Effectively the CARML Module for Microsoft.Web/connection is a secure object: https://github.com/Azure/ResourceModules/blob/d84d4f7287616a1b2883837fff5f2fcb14a45e67/modules/web/connections/main.bicep#L23-L25

Which prevents getSecret usage:

module MandrillConnection '../.carml/web/connections/main.bicep' = {
  name: 'TestWebConnection'
  params: {
    location: location
    name: 'mandrill'
    displayName: 'Emailer'
    api: {
      name: 'mandrill'
      displayName: 'Mandrill'
      type: 'Microsoft.Web/locations/managedApis'
      id: '/subscriptions/${subscription().subscriptionId}/providers/Microsoft.Web/locations/${location}/managedApis/mandrill'
    }
    customParameterValues: {}
    nonSecretParameterValues: {}
    parameterValues: {
      apiKey: KeyVault.getSecret('MySecretName') // Function "getSecret" is not valid at this location. It can only be used when directly assigning to a module parameter with a secure decorator.bicep(BCP180)
    }
  }
}

What would the recommended solution be for this?


EDIT: I created a wrapper module that consumes a param that I pass directly into the object instead. This appears to be working and aligns with the error message.

module MandrillConnection 'ConnectionMandrillApi.bicep' = {
  name: 'TestWebConnection'
  params: {
    name: 'connection-mandrill'
    location: location
    apikey: KeyVault.getSecret(keyVaultSecretMandrillApiKey)
  }
}