fsprojects / SQLProvider

A general F# SQL database erasing type provider, supporting LINQ queries, schema exploration, individuals, CRUD operations and much more besides.
https://fsprojects.github.io/SQLProvider
Other
572 stars 146 forks source link

make the provider FIPS compliant #586

Closed avanmeul closed 5 years ago

avanmeul commented 5 years ago

Description

SQLProvider isn't FIPS compliant, causing fatal errors on systems enforcing FIPS compliance.

Repro steps

1) Create registry key to enable FIPS compliance on a server. 2) Calls to SQLProvider will throw an exception "Exception has been thrown by the target of an invocation. ---> System.InvalidOperationException: This implementation is not part of the Windows Platform FIPS validated cryptographic algorithms.".

Expected behavior

SQLProvider should be FIPS compliant; otherwise domains like government (and other highly regulated industries like medical and financial organizations that care about security) may not be able to use it.

Actual behavior

Partial stack trace:

ToString=System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.InvalidOperationException: This implementation is not part of the Windows Platform FIPS validated cryptographic algorithms. at System.Security.Cryptography.SHA256Managed..ctor() --- End of inner exception stack trace --- at System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor) at System.Reflection.RuntimeConstructorInfo.Invoke(BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture) at System.Security.Cryptography.CryptoConfig.CreateFromName(String name, Object[] args) at System.Security.Cryptography.SHA256.Create(String hashName) at FSharp.Data.Sql.Common.Bytes.hash[a](FSharpFunc2 algo, Byte[] bs) at System.Collections.Concurrent.ConcurrentDictionary2.GetOrAdd(TKey key, Func2 valueFactory) at FSharp.Data.Sql.Common.QueryEvents.publishSqlQuery@94.Invoke(String connStr, String qry, IEnumerable1 parameters) at FSharp.Data.Sql.Runtime.QueryImplementation.executeQuery(ISqlDataContext dc, ISqlProvider provider, SqlExp sqlExp, List1 ti) at FSharp.Data.Sql.Runtime.QueryImplementation.SqlQueryable1.System-Collections-Generic-IEnumerable1-GetEnumerator() at System.Linq.Enumerable.WhereSelectEnumerableIterator2.MoveNext() at System.Collections.Generic.List1..ctor(IEnumerable1 collection) at Microsoft.FSharp.Collections.SeqModule.ToArray[T](IEnumerable`1 source)

Known workarounds

Turn off FIPS compliance; however some systems are locked down due to organizational policies that cannot be circumvented.

Related information

Presumably any system that requires FIPS compliance will be affected.

The above stack trace excerpt was created with the following configuration:

Thorium commented 5 years ago

Thanks. I think we can fix this by skipping adding the SHA256 connection hash to the event in FIPS mode, it's a very minor use case.

piaste commented 5 years ago

Thanks for the link.

Not working in the US, this is the first time I hear of FIPS. From googling, it seems it should be sufficient to replace the call to SHA256.Create() with a manual instantiation of SHA256CryptoProvider.

Since I requested that additional hash, I'll prepare a pull request, if that's OK with @Thorium.

Thorium commented 5 years ago

The funny part here is that the place throwing exception is ConcurrentDictionary2.GetOrAdd: We use hash for unique dictionary key in caching, not actually for crypting. Piaste go ahead. :-)

piaste commented 5 years ago

@Thorium

Actually it was to prevent leaking passwords and other sensitive info from the query event if I recall correctly :)

@avanmeul

I couldn't reproduce the error on my system. Enabled FIPS compliance both in the registry and in app.config, rebooted a few times, still no error when using the managed cryptography. Maybe it's because I'm using Windows N which is EU only and lacks these features? Not sure.

The compiled .dlls with the change is available here, perhaps you could confirm if it fixes the issue?

Thorium commented 5 years ago

@piaste, I thought that initially also, but then I noticed that the stack trace listed in this issue is using the hash codes as dictionary keys so this is some other place in the code.

Thorium commented 5 years ago

Also, the PR is reversing the instructions given here: https://stackoverflow.com/questions/32836532/how-to-check-that-an-asp-net-application-is-fips-ready

piaste commented 5 years ago

Why do you think it's using it as hash keys? In the stack trace I believe TKey is string (the unhashed connection string). There's no other search results for "sha256" in the code in any case.

My understanding of FIPS compliance is that AesManaged is not FIPS-compliant, AesCryptoServiceProvider is, and Aes.Create() can instantiate either one based on .NET's decision (and likewise for SHA1, SHA256, etc.).

So in the answer you linked it's advised to replace the Managed() creation with a .Create()call which should take into account the FIPS settings and use the unmanaged one. But in the OP's case the .Create() call didn't do the right thing for whatever reason, and it created a SHA256Managed object (see stack trace), so forcing use of the CryptoServiceProvider should fix it.

On that note, I actually managed to trigger the exception now by manually instantiating SHA256Managed with the policy enabled; I still get no exception from SHA256.Create() though.

Thorium commented 5 years ago

Yes, you're right. Or we could probably just use .GetHashCode() as it's faster and there are not so many connection strings anyway.

avanmeul commented 5 years ago

You guys rock!

It works!!!

Thorium commented 5 years ago

Merged to version 1.1.54

avanmeul commented 5 years ago

Verified: tests using 1.1.54 via Nuget update work.

I'd like to convey the gratitude of many stakeholders that are overjoyed by how quickly your team responded to this issue, and how fast it was resolved. (Christmas came early this year: this is a wonderful Christmas gift! =:0)

piaste commented 5 years ago

Thanks for the nice words :)

Out of curiosity, are you at liberty to disclose which government/medical/financial organization is running SQLProvider? It could make for a nice name-drop in the readme.

avanmeul commented 5 years ago

While I can't make any promises about outcomes, I will gladly do everything I can to try to fulfill your request.

avanmeul commented 5 years ago

I have an answer for you. I'm allowed to say: SQLProvider was used in conjunction with work I did as a consultant for the US Navy. So, you can say that SQLProvider was used by the US Navy, but (like most government agencies) that is not an official endorsement.