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

Add security check to App Service for removing plain FTP support #327

Open
tonybaloney opened this issue Feb 6, 2023 · 4 comments
Open
Assignees
Labels

Comments

@tonybaloney
Copy link
Contributor

By default, App Service will allow plain FTP uploads. There is an existing check that mentions the basic authentication, but not the protocol.

{
      "category": "Security",
      "subcategory": "Identity and Access Control",
      "text": "Disable basic authentication",
      "description": "Disable basic authentication for both FTP/FTPS and for WebDeploy/SCM.  This disables access to these services and enforces the use of Azure AD secured endpoints for deployment.  Note that the SCM site can also be opened using Azure AD credentials.",
      "guid": "5d04c2c3-919c-4a0b-8c12-159e114b933d",
      "severity": "High",
      "link": "https://docs.microsoft.com/azure/app-service/deploy-configure-credentials#disable-basic-authentication"
    },

There should be an additional rule to change the requirement to either 'Disabled' or 'FtpsOnly'. Plain FTP is insecure.

The setting is properties.siteConfig.ftpsState and it is one of 'AllAllowed', 'FtpsOnly' or 'Disabled'. AllAllowed is the default value (shown as null in resource graph)

@erjosito erjosito assigned erjosito and xstof and unassigned erjosito Feb 15, 2023
@erjosito
Copy link
Collaborator

@xstof your thoughts?

@xstof
Copy link
Collaborator

xstof commented Feb 16, 2023

I'm trying to understand the issue being raised here. By disabling basic auth as the checklist instructs, you effectively disable FTP and FTPS as those don't support AAD auth.

@tonybaloney
Copy link
Contributor Author

tonybaloney commented Feb 20, 2023

My assumption was that each recommendation was mutually exclusive since you can put "Does not apply" in the spreadsheet.

Disabling WebDeploy would be a big inconvenience for a lot of developers because it means you can't deploy using most of the automated tooling.

My suggestion was is to have a separate rule to have plain FTP disabled. I want to test that FTP/FTPS is correctly disabled when applying the security policy in the existing rule as well.

@xstof
Copy link
Collaborator

xstof commented Feb 20, 2023

As the recommendation says, you can still use WebDeploy with AAD credentials when disabling basic authentication. Of course we can add an entry to the checklist that explains to disable FTP/FTPS only, but I was just not sure if that would add much in terms of security; because that would still allow WebDeploy with basic auth; which we really want to avoid customers doing in the context of securing their setup. I sense I might not fully understand what you're driving for here however; so bear with me please. You have a concrete text/example on the change proposed on the checklist please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants