search

dotnet / arcade

Tools that provide common build infrastructure for multiple .NET Foundation projects.
MIT License
664 stars 339 forks source link

Microsoft.DotNet.SignTool.Tests/Resources/MsiSetup.msi and MsiApplication.exe are flagged as malicious software by antiviral scanners #13843

Open aburmash opened 1 year ago

aburmash commented 1 year ago

arcade/src/Microsoft.DotNet.SignTool.Tests/Resources/MsiSetup.msi https://github.com/dotnet/arcade/blob/main/src/Microsoft.DotNet.SignTool.Tests/Resources/MsiSetup.msi is being flagged as malicious by multiple antiviruses

arcade/src/Microsoft.DotNet.SignTool.Tests/Resources/MsiApplication.exe https://github.com/dotnet/arcade/blob/main/src/Microsoft.DotNet.SignTool.Tests/Resources/MsiApplication.exe which is inside that msi is being flagged by much more scanners as malicious

To test that you can just submit that to virustotal and notice that it is being flagged as malicious.

While we do understand that it is most likely false positive, will be nice to fix it, otherwise scanners pick up source files provided for dotnet as containing malicious software. Since arcade sources, at least those two files, are being packed into source tarball to build SDK, scanning dotnet sources provided with the compiled SDK will generate a failed scan ( viruses reported ). While binaries themselves were committed back in 2020, looks like they were submitted for scanning around March 2023, so we expect more and more people will be getting those "your have a virus" reports over time.

Release Note Category

mmitche commented 1 year ago

@premun I took a quick look at this and was a bit unsure why these would end up in the VMR: https://github.com/dotnet/installer/blob/main/src/VirtualMonoRepo/allowed-binaries.txt#L23 seems to say that the * under the folder ends up there, but I don't think the globbing would also include the Resources folder. If it DID, then I wonder why the folder https://github.com/dotnet/dotnet/tree/main/src/arcade/src/Microsoft.DotNet.SignTool.Tests/Resources is so small compared to https://github.com/dotnet/arcade/tree/main/src/Microsoft.DotNet.SignTool.Tests/Resources

That said, I think the right thing to do here is to pull these binaries into an assets package and delete them from the repo. Only restore the assets package in non-SB configs.

mmitche commented 1 year ago

/cc @dotnet/source-build-internal

premun commented 1 year ago

@dkurepa is it possible we have set some allowed-binaries content but failed to clear the binaries properly?

GrabYourPitchforks commented 1 year ago

@mmitche Agree that using an assets package and removing from SB would be ideal.

AFAIK, these are only used for testing the signing pipeline and aren't ever launched. So another approach (feel free to do this in addition to the assets package approach!) is to use an innocuous already-signed file for testing. You could alternatively create a signable yet non-executable type file (like .cat) and test signing that. AV products shouldn't flag non-executable files.

dkurepa commented 1 year ago

@dkurepa is it possible we have set some allowed-binaries content but failed to clear the binaries properly?

The src/arcade/src/Microsoft.DotNet.SignTool.Tests/* line allows binaries to exist in the Resources folder, that's why the .msi is not getting flagged in the binary scanner. The reason why that folder is so much smaller in the VMR is because in the source-mappings we're filtering all of the dlls, nupkgs, etc.. Maybe we should just add .msi to cloaked files too?

premun commented 1 year ago

So if this is for testing, we should cloak the files rather and remove the allowed binaries?

dkurepa commented 1 year ago

I'm not sure, if we need the binaries inside of the folder, but don't want to see the msi, we can just cloak it. If we don't need it, we can just remove it I guess

premun commented 1 year ago

I opened https://github.com/dotnet/installer/pull/16210 and https://github.com/dotnet/dotnet/pull/27

missymessa commented 1 year ago

@premun @dkurepa is there an epic that this should belong in?

premun commented 1 year ago

I don't know.. I think this is one of those that are just tied to Arcade as a repository. Someone checked in a bunch of binaries couple of years ago which should be probably taken out and put in a NuGet or somewhere and restored during the test?

missymessa commented 1 year ago

Sounds like signing needs some love, so this could be done at the same time that happens.

tkapin commented 1 year ago

@missymessa / @mmitche - this is about removing test binaries used by the signing tool from the repo and consuming them another way (nuget, storage). It doesn't seem to be product construction related (unless we own the signing tool, which rather seems to belong to the core engineering services IMO). Also, the issue should live in arcade rather than in arcade-services. Any thoughts on this?

premun commented 1 year ago

Agreed, this does not belong to arcade-services

dkurepa commented 1 year ago

Should we transfer it back to arcade and leave it there?

tkapin commented 1 year ago

Transferred back to arcade as discussed as signing is the shared tooling.

aburmash commented 4 months ago

I once again noticed virus scan reports, and seems that issue was fixed for main, but affected files are still present in 6/7 arcade. Commit with fix reference: https://github.com/dotnet/dotnet/pull/27/files

Branches of arcade with files present: https://github.com/dotnet/arcade/tree/release/6.0/src/Microsoft.DotNet.SignTool.Tests/Resources https://github.com/dotnet/arcade/tree/release/7.0/src/Microsoft.DotNet.SignTool.Tests/Resources

mmitche commented 4 months ago

It's fixed in main, but only in the VMR projection. In the arcade repo those files appear to still be there in main. What are you doing when you get those errors?

/cc @MichaelSimons