Manual updates 20240502 security wave 1

[moljac]

Does this change any of the generated binding API's?

No.

Describe your contribution

C&AI Security Wave 1

This is 1st part of set of security improvements for AX repo

1. Added NuGetAudit properties
   https://learn.microsoft.com/en-us/nuget/concepts/auditing-packages
2. TODO: Added SAST tool .NET Security Guard
   Needs investigation.
   Adding NuGet SecurityCodeScan.VS2019causes build issues (see comments)
   https://owasp.org/www-community/Source_Code_Analysis_Tools

EDIT:

Builds with net9.0-rc1 revealed few security warnings

#996

.NET is designed with security in mind, but security issues can happen especially through transitive dependencies which is scanned by NuGetAudit, while SecurityCodeScan.VS2019 is a static analysis tool that can help identify security vulnerabilities in the code used in this repo. Admittedly this repo does not use much security critical .NET BCL APIs, but this NuGet will act preventive if such API calls might be added.

[jpobst]

If these NuGet Auditing warnings are important, we need to make them errors rather than warnings. Our build currently has over 15,000 warnings, we will never see any new warnings that show up.

https://learn.microsoft.com/en-us/nuget/concepts/auditing-packages#warning-codes

Note that this means our build can break at any time when a new advisory is published. We just need to be aware that sometimes it will break when we haven't pushed any changes to our repository.

[jpobst]

From Discord:

It is for Nuget Auditing only, though I tried to add SAST nuget but it causes some issues I need to check.

If this is true, then let's not add this package.

[jpobst]

Also note that our pipeline(s) already run the CodeQL static code analysis tool recommended and required by Microsoft security. We should likely rely on their expertise rather than trying to come up with our own solution here.

https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=10305045&view=logs&j=784e4eae-0a8d-50ee-7be1-df4337debdeb&t=fbdff2d1-992e-564e-2a8b-113c89c83f2b

[moljac]

If this is true, then let's not add this package.

I pushed to see whether the same problems can be reproduced on CI, but CI built OK. Now I know that updates borked my workloads locally. I had to nuke dotnet installations completely and after clean installation and workloads everything was OK.

[moljac]

Few additional thoughts:

1. "false negative" problem

   Functionality of security scanning tools (AV scanners, static code analyzer...) is usually expressed in percentage of detected issues and those are never 100%. Having more tools (eyes) should increase accuracy - decrease "false negative" percentage.

   .NET security issues are mostly ASP.net related. For this repo following could be interesting

   • SCS0001 - Command Injection

   • SCS0003 - XPath Injection

   • XML eXternal Entity Injection (XXE)

   • SCS0018 - Path Traversal

   • few crypto and security related (hardcoded passwords)

   • XSLT settings

   • SCS0028 - Insecure Deserialization

2. location (local or cloud)

   CodeQL works on CI - after commit and push. This nuget works during builds and issues can be fixed before commited. IMO prevention should occur as early as possible.

3. reporting, analysis, fixing

Reporting and analysis is also easier if done locally (on premise).

Before I started writing this comment I found out that CodeQL has some issues for our builds.

https://github.com/dotnet/android-libraries/security

https://github.com/dotnet/android-libraries/security/code-scanning/tools/CodeQL/status/configurations/automatic/9283c23397ab89b9cdebcdba43eab0be03d8f3e5cbdb50d380d9781e6e595f3f

[jpobst]

If these NuGet Auditing warnings are important, we need to make them errors rather than warnings. Our build currently has over 15,000 warnings, we will never see any new warnings that show up.

This does not seem to be addressed.

Also note that our pipeline(s) already run the CodeQL static code analysis tool recommended and required by Microsoft security. We should likely rely on their expertise rather than trying to come up with our own solution here.

I'm still sticking with this, and I believe we should do what Microsoft security requires we do rather than try to come up with our own security policies. I especially have concerns as this package hasn't been updated in over 2 years and it feels like a security product should be constantly maintained as security is a fast moving field.

If you want this, you'll need to get @jonpryor to approve it.

[moljac]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[moljac]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[moljac]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[moljac]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[moljac]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[moljac]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[moljac]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).