Default value of ip_restriction_default_action is Allow, which causes app services to unexpectedly be open to the internet

[rellis-of-rhindleton]

Is there an existing issue for this?

• I have searched the existing issues

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
• Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
• If you are interested in working on this issue or have submitted a pull request, please leave a comment and review the contribution guide to help.

Terraform Version

1.7.4

AzureRM Provider Version

3.95.0

Affected Resource(s)/Data Source(s)

azurerm_linux_web_app

Terraform Configuration Files

site_config { 
  # (no ip_restriction_default_action setting)
  # ...

Debug Output/Panic Output

n/a

Expected Behaviour

If the configuration does not have the ip_restriction_default_action setting, the default behavior should be as if the setting was set to Deny. This was the effective behavior in prior versions.

Actual Behaviour

Updating azurerm version to 3.95.0 causes the default action to be changed to Allow. If you don't notice this when reviewing the plan, it can expose the app service to public traffic.

Steps to Reproduce

The setting ip_restriction_default_action was added in 3.95.0, as pull request 25131. Updating a configuration to this release will result in the setting being set to Allow unless you add the setting at the same time.

Important Factoids

No response

References

#25131

[mattmshell]

I would like ip_restriction_default_action to be mandatory when there are firewall rules present

[xiaxyi]

@rellis-of-rhindleton Thanks for raising this issue, do you have any ip_restriction configured? The expected behavior is to allow all access if there isn't any deny list existed.

In the API request body, the property that we added in PR #25131 is to control the default IP list as below:
"ipSecurityRestrictions": [
{
"ipAddress": "Any",
"action": "Deny",
"priority": 2147483647,
"name": "Deny all",
"description": "Deny all access"
}
]
When there isn't any allowed ip list in TF config like below example, the default IP action is to allow all ip addresses.

resource "azurerm_linux_function_app" "test-standard" {
  name                      = "xiaixntest-LFA-standard"
  location                  = azurerm_resource_group.test.location
  resource_group_name       = azurerm_resource_group.test.name
  service_plan_id           = azurerm_service_plan.linux-standard.id
  virtual_network_subnet_id = azurerm_subnet.test1.id

  storage_account_name       = azurerm_storage_account.test.name
  storage_account_access_key = azurerm_storage_account.test.primary_access_key

  site_config {
    ip_restriction {
      ip_address = "13.107.6.152/31,13.107.128.0/22"
      name       = "test-restriction"
      priority   = 123
      action     = "Allow"
      headers {
        x_azure_fdid      = ["55ce4ed1-4b06-4bf1-b40e-4638452104da"]
        x_fd_health_probe = ["1"]
        x_forwarded_for   = ["9.9.9.9/32", "2002::1234:abcd:ffff:c0a8:101/64"]
        x_forwarded_host  = ["example.com"]
      }
    }
  }
}

Let me know if there is any misunderstanding of your situation. :)

[rellis-of-rhindleton]

The default action is being set to Allow on configurations with four Allow IP address restrictions. We haven’t used any Deny restrictions as the default action was effectively already Deny.

[xiaxyi]

Thanks @rellis-of-rhindleton , yes, if there are allowed ip list, the default action will be deny. Can you share the ip part of your state file and config file?

[rellis-of-rhindleton]

Here is an example from a module we sometimes use. I had to go through it to condense external variables into something you could actually see, and then anonymize it, so please forgive any syntax errors.

locals {

  app_ip_ranges = {
    "Allow-range-1" = {
      name       = "Allow-range-1"
      ip_address = "0.0.0.0/32"
    },
    "Allow-range-2" = {
      name       = "Allow-range-2"
      ip_address = "0.0.0.0/32"
    },
    "Allow-range-3" = {
      name       = "Allow-range-3"
      ip_address = "0.0.0.0/32"
    },
    "Allow-range-4" = {
      name       = "Allow-range-4"
      ip_address = "0.0.0.0/32"
    }
  }

  scm_ip_ranges = {
    "scm-ip-range" = {
      name        = "scm-ip-range"
      ip_address  = "0.0.0.0/16"
      range_start = "0.0.0.0"
      range_end   = "0.0.0.0"
    }
  }
}

resource "azurerm_linux_web_app" "this" {
  name                      = var.name
  location                  = var.location
  resource_group_name       = var.resource_group_name
  service_plan_id           = var.service_plan_id
  enabled                   = var.enabled
  virtual_network_subnet_id = var.virtual_network_subnet_id
  https_only                = true
  client_affinity_enabled   = false

  site_config {
    always_on                         = var.always_on
    app_command_line                  = var.app_command_line
    ftps_state                        = var.ftps_state
    http2_enabled                     = var.http2_enabled
    vnet_route_all_enabled            = true
    health_check_path                 = var.health_check_path
    health_check_eviction_time_in_min = var.health_check_eviction_time_in_min
    worker_count                      = var.worker_count

    dynamic "ip_restriction" {
      for_each = local.app_ip_ranges
      iterator = range
      content {
        name       = range.value.name
        ip_address = range.value.ip_address
        action     = "Allow"
        priority   = 20
      }
    }

    scm_use_main_ip_restriction = false

    dynamic "scm_ip_restriction" {
      for_each = local.scm_ip_ranges
      iterator = range
      content {
        name       = range.value.name
        ip_address = range.value.ip_address
        action     = "Allow"
        priority   = 20
      }
    }

  }

[rellis-of-rhindleton]

State excerpt. Also picked through and anonymized. I did not delete any entries from site_config, though I have removed some values from arrays etc.

This is the state the app was in before I tried to use the new version of the provider. Running the new version with the above configuration against this state attempts to set default action to Allow.

"site_config": [
  {
    "always_on": true,
    "api_definition_url": "",
    "api_management_api_id": "",
    "app_command_line": "",
    "application_stack": [
        // ...
    ],
    "auto_heal_enabled": false,
    "auto_heal_setting": [],
    "container_registry_managed_identity_client_id": "",
    "container_registry_use_managed_identity": false,
    "cors": [],
    "default_documents": [
        // ...
    ],
    "detailed_error_logging_enabled": false,
    "ftps_state": "Disabled",
    "health_check_eviction_time_in_min": 3,
    "health_check_path": "/health",
    "http2_enabled": false,
    "ip_restriction": [
      {
        "action": "Allow",
        "headers": [],
        "ip_address": "0.0.0.0/32",
        "name": "Allow-range-1",
        "priority": 20,
        "service_tag": "",
        "virtual_network_subnet_id": ""
      },
      {
        "action": "Allow",
        "headers": [],
        "ip_address": "0.0.0.0/32",
        "name": "Allow-range-2",
        "priority": 20,
        "service_tag": "",
        "virtual_network_subnet_id": ""
      },
      {
        "action": "Allow",
        "headers": [],
        "ip_address": "0.0.0.0/32",
        "name": "Allow-range-3",
        "priority": 20,
        "service_tag": "",
        "virtual_network_subnet_id": ""
      },
      {
        "action": "Allow",
        "headers": [],
        "ip_address": "0.0.0.0/32",
        "name": "Allow-range-4",
        "priority": 20,
        "service_tag": "",
        "virtual_network_subnet_id": ""
      }
    ],
    "linux_fx_version": "...",
    "load_balancing_mode": "LeastRequests",
    "local_mysql_enabled": false,
    "managed_pipeline_mode": "Integrated",
    "minimum_tls_version": "1.2",
    "remote_debugging_enabled": false,
    "remote_debugging_version": "...",
    "scm_ip_restriction": [
      {
        "action": "Allow",
        "headers": [],
        "ip_address": "0.0.0.0/16",
        "name": "scm-ip-range",
        "priority": 20,
        "service_tag": "",
        "virtual_network_subnet_id": ""
      }
    ],
    "scm_minimum_tls_version": "1.2",
    "scm_type": "None",
    "scm_use_main_ip_restriction": false,
    "use_32_bit_worker": true,
    "vnet_route_all_enabled": true,
    "websockets_enabled": false,
    "worker_count": 1
  }
]

[arkiaconsulting]

To me this attribute should be set to Deny by default in order to match the existing behavior, or it should not even exist.

[drdamour]

To me this attribute should be set to Deny by default in order to match the existing behavior, or it should not even exist.

The existing behaviour wasnt deny..it is null which then causes azure to do some sub decisions based on private endpoints being present.

[rellis-of-rhindleton]

Our services are not on private endpoints.

[brental]

As noted in #25394 having this default to "Allow" is a breaking change in a minor version and can have quite an impact on consumers if they don't notice the change when upgrading. If the default was previously null and Azure then decides then the default should have been null to maintain that.

Are there plans to actually fix the default similar to how #25367 was fixed? The responses in this issue don't make that clear.

EDIT: Not as much of an issue but naming the setting ip_restriction_default_action (singular not plural) when the setting in azure is ipSecurityRestrictionsDefaultAction (plural not singular) is also a strange decision. Seems like there needs to be a bit more thought put into naming of and default values for settings being added.

[jackofallops]

Hi All - apologies for the delay in responding here, we do recognise this is a hot topic.

This property was added as it should be a configurable setting and was a requested feature (in #22593). The default behaviour of the property changes in various conditions, leading to the confusion in the implementation. If no ip_restriction blocks exist, this value is returned as null in the API but is actually Allow behind the scenes, as the default rule is an implicit "allow all". This behaviour is where the default schema value in the addition of the property came from. When an Allow rule is added to the ip_restriction list the Service changes this to an implicit Deny all as the final rule item. Our existing testing did not detect this was overridden by the new property value and we're looking into ways we can catch this, and other potentially breaking changes, in the future.

In the meantime, we're looking into how we can correct this behaviour with the lowest forward impact to resources, however, we do anticipate a breaking change to be necessary. We will communicate this change as clearly and early as possible. Until the change is made and released, setting ip_restriction_default_action and scm_ip_restriction_default_action to their required values is recommended, or pinning to v3.94.0 is recommended where this is not possible. Note, resources that have been modified by v3.95.0 or later cannot have the value for the ip_restriction_default_action property reset, as the API does not allow this to be nulled once it has a concrete value.

[Noel-Jones]

Just come across this myself with similar concerns to many. Without getting into the debate on breaking change or not, my primary concern is that the provider does not detail the change that is being performed. With the defaults of Allow I would expect to see the following to indicate exactly what is being changed:

      ~ site_config {
          ~ scm_ip_restriction_default_action       = "Deny" -> "Allow"

Even after updating my code to match what it should be I see:

      ~ site_config {
          + ip_restriction_default_action           = "Allow"
          + scm_ip_restriction_default_action       = "Deny"

which of course does not fill me with confidence that I have correctly updated my code to zero out any change.

[Sefriol]

We noticed this in our Security Audit. I am surprised that this hasn't caused immediate stop on distribution of this version.

Not only will this go unnoticed, it can cause serious Security Implications.