dotnet / SqlClient

Microsoft.Data.SqlClient provides database connectivity to SQL Server for .NET applications.
MIT License
844 stars 281 forks source link

Package causes a new UWP app to fail Windows App Certification for Deployment to Microsoft Store #356

Closed bolyb closed 3 years ago

bolyb commented 4 years ago

Describe the bug

If you add the latest version of the NuGet package (1.1.0) to a blank UWP app, the app will fail the Windows App Certification with the following error and not be able to be deployed to the Microsoft Store. The failure relates to sni.dll failing the Binary analyzer Windows Security features test.

Windows security features test

FAILED
Binary analyzer
Error Found: The binary analyzer test detected the following errors:
File C:\Program Files\WindowsApps\(PackageName)\sni.dll has failed the AppContainerCheck check.
Impact if not fixed: If the app doesn’t use the available Windows protections, it can increase the vulnerability of the customer's computer to malware.
How to fix: Apply the required linker options - SAFESEH, DYNAMICBASE, NXCOMPAT, and APPCONTAINER - when you link the app. See links below for more information:
Fixing Binary Analyzer Errors

PASSED
Banned file analyzer

PASSED
Private code signing

Supported API test

FAILED
Supported APIs
Error Found: The supported APIs test detected the following errors:
API _mbschr in api-ms-win-crt-multibyte-l1-1-0.dll is not supported for this application type. sni.dll calls this API.
API _mbsnbicmp_l in api-ms-win-crt-multibyte-l1-1-0.dll is not supported for this application type. sni.dll calls this API.
API _mbsspn in api-ms-win-crt-multibyte-l1-1-0.dll is not supported for this application type. sni.dll calls this API.
API IsTokenRestricted in advapi32.dll is not supported for this application type. sni.dll calls this API.
Impact if not fixed: Using an API that is not part of the Windows SDK for Microsoft Store apps violates the Microsoft Store certification requirements.
How to fix: Review the error messages to identify the API that is not part of the Windows SDK for Microsoft Store apps. Please note, apps that are built in a debug configuration or without .NET Native enabled (where applicable) can fail this test as these environments may pull in unsupported APIs. Retest your app in a release configuration, and with .NET Native enabled if applicable. 

To reproduce

  1. Create a new, blank UWP app (File->New->Project->Blank App (Universal Windows))
  2. Right click the project -> Publish -> Create app packages -> Sideloading -> Yes, select a certificate: Create -> Next -> Next -> Choose an installer location -> Create -> Copy and Close (once it finishes running)
  3. Launch the Windows App Cert Kit (just search for app in Windows 10)
  4. Choose Validate Store App -> Browse for the app you want to validate -> Select the .msixbundle created in Step 2. -> Next -> leave all Selected, Next
  5. View the report once finished (should pass for everything except for default logos)
  6. Right click the project -> Manage NuGet Packages -> search for Microsoft.Data.SqlClient and install the latest version (v1.1.0 as of now)
  7. Repeat steps 2-5, report will show the failure documented above.

Expected behavior

Package should not prevent UWP apps from being deployed to store, as this will prevent production usage.

Further technical details

Microsoft.Data.SqlClient version: 1.1.0 .NET target: Universal Windows Target Version 1903 SQL Server version: SQL Server 2017 Operating system: Windows 10 Version 1909

Additional context This seems to have been an issue at one point with the System.Data.SqlClient package as well. Perhaps the same fix could be applied to this package? https://social.msdn.microsoft.com/Forums/en-US/5bcc17fa-5184-4fab-8ff2-cbee229a12a2/snidll-has-failed-the-appcontainercheck-check?forum=wpsubmit

Here is some documentation on fixing similar failures of this test with another dll (with the linker options /dynamicbase /nxcompat /appcontainer) https://github.com/grpc/grpc/issues/18188 Specifically, step 3 of this comment in the issue thread: https://github.com/grpc/grpc/issues/18188#issuecomment-544848727

cheenamalhotra commented 4 years ago

Hi @bolyb

I believe you're suggesting changes in SNI.dll source which loads from runtime.native.System.Data.SqlClient.sni when targeting UWP Apps.

Currently there are some hiccups with this library, we'll look into it if feasible and get back to you.

bolyb commented 4 years ago

Hi @cheenamalhotra

Are there any updates on the feasibility of this? We are approaching our deployment deadline, and would really like to use the Microsoft Store to deploy to our end users as we have in the past. We also do not want to start a new application using the System.Data.SqlClient package simply to pass the store submission process, since there will be no new work done on that package moving forward, as outlined here: https://devblogs.microsoft.com/dotnet/introducing-the-new-microsoftdatasqlclient/

What is the recommended course of action for us in this case? It seems logical that a Microsoft authored NuGet package should have no problems passing a Microsoft Windows Security features test.

David-Engel commented 4 years ago

@bolyb One of the things that was done in System.Data.SqlClient was specifically supporting a UWP build target. In doing that, S.D.SqlClient could provide different bits for UWP targets versus regular Windows targets. The UWP-specific bits in S.D.SqlClient do not include references to the native sni.dll (it uses a managed SNI implementation which is not quite as performant). In creating Microsoft.Data.SqlClient, that UWP-specific build target was not done and thus UWP applications which reference M.D.SqlClient will get the Windows bits with the native sni.dll references. It is on the roadmap to provide a UWP-specific build target in M.D.SqlClient with similar changes, but there is no date set for it at this time.

I recommend staying with System.Data.SqlClient for UWP applications until UWP-specific build support is added to Microsoft.Data.SqlClient.

cheenamalhotra commented 4 years ago

Hi @bolyb

Could you try with Microsoft.Data.SqlClient v2.0.0 and let us know if that works for you since the SNI DLLs are now different.

Alternatively, we also enabled switching native SNI to Managed implementation to avoid loading native DLL. You can set AppContext switch to Enable Managed SNI on Windows.

cheenamalhotra commented 4 years ago

As mentioned in https://github.com/dotnet/SqlClient/issues/703#issuecomment-677897900, the issue will be resolved by enabling Managed Networking on Windows for UWP apps. It's not enabled by default, but necessary for UWP applications.

bolyb commented 3 years ago

Hi @cheenamalhotra and @David-Engel

Thank you for your time investigating this issue, it is much appreciated.

I've just now had the opportunity to circle back on this. I've attempted to use Microsoft.Data.SqlClient 2.0.0, and 2.1.1, both with and without enabling Managed Networking on Windows for UWP apps. I have tried enabling this by adding the line AppContext.SetSwitch("Switch.Microsoft.Data.SqlClient.UseManagedNetworkingOnWindows", true); in App.xaml.cs as suggested in both the constructor and OnLaunched methods. Unfortunately in all cases, when following the reproduction steps outlined in the original bug report reproduction steps, the app still fails the Windows App Certification Kit with the same error.

FAILED Binary analyzer Error Found: The binary analyzer test detected the following errors: File C:\Program Files\WindowsApps\35d4df60-94b9-465f-a5bc-3d8bbd4e5f1e_1.0.2.0_x64__dt8z0k56dgxnc\Microsoft.Data.SqlClient.SNI.dll has failed the AppContainerCheck check.

In summary, there doesn't appear to have been any change on this original issue, what do you suggest as far as next steps to try?

wjvii commented 3 years ago

@bolyb @cheenamalhotra @David-Engel I too am circling back around this as I have run into the same thing again as well, I updated #703 mentioned above back in December. Right now, we cannot ship the newest versions of our Microsoft Windows application due to this issue.

cheenamalhotra commented 3 years ago

Hi @bolyb @wjvii

I think the error you're seeing is mainly due to SNI dependency that continues to exist even if you enable Managed SNI and do not use that DLL at all. I think the only solution that would work is when you we completely move away from Native SNI to managed for UAP targets. But because Microsoft.Data.SqlClient is now an external component and not built same way as System.Data.SqlClient was built in the .NET Core SDK, we have had challenges in the past when building for UAP targets.

I'll reopen this issue again, and we can revisit options that we can do here.

wjvii commented 3 years ago

@cheenamalhotra is there any chance that you can work with the Microsoft Store team and have them allow this Microsoft SNI dll?

ErikEJ commented 3 years ago

Remove the SNI files during build?

David-Engel commented 3 years ago

Is there any chance that you can work with the Microsoft Store team and have them allow this Microsoft SNI dll?

@wjvii The purpose of UWP is to provide a uniform set of APIs that apps can use, which are supported on all the devices that Windows 10 runs on. The Microsoft.Data.SqlClient.SNI.dll is not compiled specifically to use only the UWP core APIs, so there isn't any way the store can simply "allow" it because, without changing it, it likely would not work across all devices.

@bolyb As ErikEJ suggested, try deleting the SNI files from your build/publish folder (and maybe remove any references to it from your dep.json file, depending on your publish config), assuming you have set the switch to Enable Managed SNI on Windows. Those SNI dlls are not actually used when that switch is set.

bolyb commented 3 years ago

Hi @David-Engel

Thank you for your suggestions, I appreciate the insight. So far I have tried adding the following as both Pre and Post Build event commands: image

This successfully removes the Microsoft.Data.SqlClient.SNI.dll file from the bin directories, but unfortunately the application built this way is still failing the Windows App Certification Kit with the same error relating to this dll. This makes me think that the file is still making it's way into both the .msix and .msixbundle (I tested both with the same result).

I'm open to any suggestions as far as other methods to prevent the Microsoft.Data.SqlClient.SNI.dll from making it into the final .msix/.msixbundle in order to pass the Windows App Certification Kit. We publish our UWP app to our organization via the Microsoft Store for Business, and cannot submit the installer without passing these checks.

wjvii commented 3 years ago

Same issue here, when we package the app it still includes the dlls and fails to pass certification in the Microsoft Store.

David-Engel commented 3 years ago

@bolyb @wjvii Make sure you search and delete any occurrences of the SNI file recursively. In my test, there were 4 or them in runtime subfolders in my build folder for the different platforms that SNI supports. But this may vary depending on your project targets. Inspect the output folders manually to be sure. I also found I had to edit my *.deps.json file to remove references to the dependency in my .NET Core project.

bolyb commented 3 years ago

Hi @David-Engel

Thank you again for your suggestions. After further troubleshooting I was able to get it to successfully pass the Binary analyzer check on my local machine. I first had to add the following lines to my project's post build script to delete all files recursively as you suggested: cd $(TargetDir) DEL /F/Q/S Microsoft.Data.SqlClient.SNI.dll

image

Similar to your suggestions about the *.deps.json file for a .NET Core project, I then also had to remove all references (about 50 in my case) to Microsoft.Data.SqlClient.SNI.dll in the project.assets.json file located in the obj folder of my UWP App project. Doing this right before publishing the app packages for the project I was able to get a package created which successfully passes the Binary analyzer check. It's worth noting that a Rebuild All on the solution would trigger a NuGet Package Restore, which re inserts the ~50 references to the JSON file and then it will not pass or build successfully again until they are deleted.

3AM-Innovations commented 3 years ago

We are also having issues yet again with "Microsoft.Data.SqlClient.SNI.DLL".

image

We are referencing "Microsoft.Data.SqlClient" 2.1.2 directly.

image

We have switched the AppContext.

image

We are removing all SNI files from the files system. (recursively) We are editing the "project.assets.json" to remove all references to "Microsoft.Data.SqlClient.SNI".

However it still fails. I have now spent days on this... What else can I do?

We are building and bundling in one "msbuild" but I think we have to. I don't think I can build with one Azure Dev Ops Visual Studio Build that will build and one that bundles. This way we could build, then clean, then bundle... I'm going to try this next because I literally have no ideas on what to do to get our app in the store.

What frustrates me the most is if this was a WPF app it would have just worked. I think the best thing for us to do is walk away from UWP and just use WPF.

3AM-Innovations commented 3 years ago

Oh and another interesting piece. A previous build which use the exact same references is working and in the store.

3AM-Innovations commented 3 years ago

OK, I think I know how this was possible. This is only what I think happened. Our application was private for some time. Literally until the last release. I release the current version as private. Once we tested it and such I flipped our application to public. I'm guessing the app was just transition from private to public?

I'm not sure if they're different requirements but this is my guess...

3AM-Innovations commented 3 years ago

@bolyb How did you bundle your app? In VS or in ADO (Azure Dev Ops).

I have tried what you said (remove files and edit the json). The files get deleted and edited properly but AFTER the bundling process in ADO. I'm guessing you did a build locally, editing the files locally, then did a local "publish" to bundle it?

image

bolyb commented 3 years ago

Hi @3AM-Innovations

Yes, I built the project locally in VS and then did a local "Publish" -> "Create App Packages..." to bundle it exactly like in the screenshot you posted.

This ended up being such a problem for us that we stopped using the Microsoft Store for many of our apps. Even though we were publishing our application as a line of business application, we would still have to pass the store checks which became very limiting.

We now deploy our UWP application using App Installer with automatic updates, which offers a far better user experience than the older sideloading methods (no powershell visible by the user, and includes the ability to automatically update similar to the Microsoft Store). We publish the App Installer package to a company fileshare (or website) instead of to the Microsoft store, and users install the applications from there (more like traditional MSI with WPF). We had to jump through a few hoops to do this, acquiring a Code Signing Cert from Sectigo and publishing our app with this cert (which requires a hardware dongle that we share between developers on the company network using USB over IP).

Here's a link to some more information about App Installer if you are interested in going that route: https://docs.microsoft.com/en-us/archive/blogs/appconsult/handling-auto-updates-for-sideloaded-uwp-and-desktop-bridge-apps

dahovey commented 3 years ago

For me what worked is using PowerShell script to remove Microsoft.Data.SqlClient.SNI references in project.assets.json from the obj directory after restore, before build. I am using build pipeline to create the store upload package so I needed something automated. The PowerShell script:

function Remove-Property {
  param(
    [Parameter(Mandatory, ValueFromPipeline, Position = 0)]
    [object] $InputObject,
    [Parameter(Mandatory, Position = 1)]
    [string] $NamePattern
  )
  process {
    foreach ($el in $InputObject) {
      foreach ($propName in $el.psobject.Properties.Name) {
        if ($propName -like $NamePattern) {
          $el.psobject.Properties.Remove($propName)
        }
        else {
          $null = Remove-Property -InputObject $el.$propName -NamePattern $NamePattern
        }
      }
    }
    $InputObject
  }
}

$fileContent = Get-Content ".\obj\project.assets.json" -Raw | ConvertFrom-Json
$fileContent = Remove-Property -InputObject $fileContent -NamePattern "*Microsoft.Data.SqlClient.SNI*"
$fileContent | ConvertTo-Json -Depth 100 | Out-File ".\obj\project.assets.json" -Force

In my case the obj and bin folders do not exist. Restore target is run using msbuild, then the script above is run from the UWP project folder. Then I run msbuild with appropriate parameters to create store upload package, and it passes certification.

The above script creates a world where Microsoft.Data.SqlClient package does not depend on SNI package, and it removes any references to the SNI package from targets/libraries portions of json file.

(sigh). Sad this was needed, but hope it helps someone.

3AM-Innovations commented 3 years ago

@dahovey Thank you for your comment. I am going to try your script because mine just doesn't seem to work...

I'm trying to get the ADO build to work but cannot seem to find the correct "target" to make it happen at the correct time.

<Target Name="Cleanup 1" BeforeTargets="CoreCompile">
    <Message Importance="High" Text="Cleaning up SqlClient SNI files so Windows Store will accept the release..." />
    <Exec Command="powershell -NonInteractive -Command &quot;&amp; '$(ProjectDir)\Invoke-PostBuild.ps1' -Path \&quot;$(ProjectDir)\&quot;&quot;" WorkingDirectory="$(ProjectDir)" />
</Target>

I've have tried many many many different "BeforeTargets" and "AfterTargets"...

However this is still not working? Now my build / package step is a single pipeline step of "Visual Studio Build" using the following arguments

/p:AppxBundlePlatforms="x86|ARM64" /p:AppxBundle=Always  /p:UapAppxPackageBuildMode=StoreUpload 
/p:AppxPackageSigningEnabled=true /p:PackageCertificateThumbprint="aoeuaoeu"
/p:PackageCertificateKeyFile="$(signingCert.secureFilePath)" /p:PackageCertificatePassword="$(signingCert.password)"
3AM-Innovations commented 3 years ago

OK, we finally got a build from ADO to pass. Here is the final working for your CSPROJ file.

<Target Name="Cleanup SqlClient SNI" AfterTargets="PrepareForBuild">
    <Message Importance="High" Text="Cleaning up SqlClient SNI files so Windows Store will accept the release..." />
    <Exec Command="powershell -NonInteractive -Command &quot;&amp; '$(ProjectDir)\Invoke-PostBuild.ps1' -Path \&quot;$(ProjectDir)\&quot;&quot;" WorkingDirectory="$(ProjectDir)" />
</Target>
cheenamalhotra commented 3 years ago

Closing issue as above options are available to mitigate SNI.dll dependency problems during deployment.