imazen / imageflow-dotnet-server

A super-fast image server to speed up your site - deploy as a microservice, serverless, or embeddable.
https://docs.imageflow.io
GNU Affero General Public License v3.0
252 stars 33 forks source link

FIPS problem with SHA1 Hash in BucketCounter and maybe elsewhere #71

Closed iJungleboy closed 1 year ago

iJungleboy commented 1 year ago

2sxc is an open source CMS plugin for DNN. It uses ImageFlow for image resizing.

It appears that recently it was installed on a government website which enabled FIPS compliance in the registry. This seems to tell Windows to complain whenever encryption or hashes are created which are deemed insecure.

Here's the original issue on 2sxc https://github.com/2sic/2sxc/issues/2988

After some research we discovered that certain APIs are not FIPS compliant, such as

2sxc itself had to be updated to match these requirements, but it appears that ImageFlow on .net uses some non-FIPS APIs such as SHA1.

Here's an image from the current release:

Load-Images-with-Params-Fail-(20230130)

According to my quick review of the API, there are certain obvious places where this should (and could easily) be fixed:

  1. https://github.com/imazen/imageflow-dotnet-server/blob/71865b556c810a4d8436a2a1f3c2d9b9453ce2af/src/Imazen.HybridCache/BucketCounter.cs#L105-L113 should be changed to not use the Managed class, and use SHA256
  2. https://github.com/imazen/resizer/blob/92bbf3a275801a41a331137243dec461ec4eceea/plugins/Security/SimpleSecureEncryption.cs#L70-L81 should change from RijndaelManaged to the AesCrytpoServiceProvider - probably https://learn.microsoft.com/en-us/dotnet/api/system.security.cryptography.aescryptoserviceprovider?view=net-7.0

I'm not sure if I caught all the use cases. @lilith What's your take on this? Would it be possible to get this fixed soon?

lilith commented 1 year ago

It's been a while since I've seen this kind of issue arise - FIPS validation is a tad bogus and not used very often anymore, not recommended by MSFT either. For hash algorithms it's a patently absurd concept... Mathematically identical algorithms with identical output in all cases... yet it doesn’t matter.

That said, we can certainly switch to factory methods that use FIPS validated algorithms for customers still using those versions of windows and wanting to check that legacy compliance box.

I'd want to make sure that the changes are .NET Standard 2.0 compatible for Imageflow projects, though, and that if the default implementation changes that the startup costs don't incur a performance penalty compared to the C# implementation on any OS platform.

If you want to submit PRs for these I will prioritize them.

https://techcommunity.microsoft.com/t5/microsoft-security-baselines/why-we-amp-8217-re-not-recommending-amp-8220-fips-mode-amp-8221/ba-p/701037

On Tue, Jan 31, 2023, 2:57 AM iJungleboy @.***> wrote:

2sxc https://2sxc.org/ is an open source CMS plugin for DNN. It uses ImageFlow for image resizing.

It appears that recently it was installed on a government website which enabled FIPS compliance in the registry. This seems to tell Windows to complain whenever encryption or hashes are created which are deemed insecure.

Here's the original issue on 2sxc 2sic/2sxc#2988 https://github.com/2sic/2sxc/issues/2988

After some research we discovered that certain APIs are not FIPS compliant, such as

  • SHA1
  • AesManaged, RjindalManaged etc. - basically any .net cryto class with ...Managed as they also allow API uses which are not compliant. Usually switching to ...CrytpoServiceProvider should do the trick

2sxc itself had to be updated to match these requirements, but it appears that ImageFlow on .net uses some non-FIPS APIs such as SHA1.

Here's an image from the current release:

[image: Load-Images-with-Params-Fail-(20230130)] https://user-images.githubusercontent.com/4568451/215554572-aefca366-3a5b-4dfe-a60a-f7e0e65c60b1.jpg

According to my quick review of the API, there are certain obvious places where this should (and could easily) be fixed:

1. https://github.com/imazen/imageflow-dotnet-server/blob/71865b556c810a4d8436a2a1f3c2d9b9453ce2af/src/Imazen.HybridCache/BucketCounter.cs#L105-L113 should be changed to not use the Managed class, and use SHA256 2. https://github.com/imazen/resizer/blob/92bbf3a275801a41a331137243dec461ec4eceea/plugins/Security/SimpleSecureEncryption.cs#L70-L81 should change from RijndaelManaged to the AesCrytpoServiceProvider - probably https://learn.microsoft.com/en-us/dotnet/api/system.security.cryptography.aescryptoserviceprovider?view=net-7.0

I'm not sure if I caught all the use cases. @lilith https://github.com/lilith What's your take on this? Would it be possible to get this fixed soon?

— Reply to this email directly, view it on GitHub https://github.com/imazen/imageflow-dotnet-server/issues/71, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA2LHYDR75OBT62MWZRIJDWVDOYRANCNFSM6AAAAAAUMG4HXQ . You are receiving this because you were mentioned.Message ID: @.***>

iJungleboy commented 1 year ago

@lilith Thanks for the insights and for being open to this change.

I've forwarded this issue to @david-poindexter so their team can choose how to proceed.

david-poindexter commented 1 year ago

@iJungleboy we are not familiar enough with this project to be able to submit a PR for this. Hopefully, someone will be able to. We are happy to sponsor this change as long as it is at a reasonable level.

lilith commented 1 year ago

I can do it under either the business or platinum support contracts: https://docs.google.com/document/d/1NWU6xDJChHUbsUrLPL_I7sCR2p-htG7Ip-qU2NO8oEs/edit?usp=drivesdk

On Tue, Jan 31, 2023, 2:56 PM David Poindexter @.***> wrote:

@iJungleboy https://github.com/iJungleboy we are not familiar enough with this project to be able to submit a PR for this. Hopefully, someone will be able to. We are happy to sponsor this change as long as it is at a reasonable level.

— Reply to this email directly, view it on GitHub https://github.com/imazen/imageflow-dotnet-server/issues/71#issuecomment-1411121976, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA2LH5HCYY663MN7Y34BFLWVGDA7ANCNFSM6AAAAAAUMG4HXQ . You are receiving this because you were mentioned.Message ID: @.***>

david-poindexter commented 1 year ago

@lilith thanks - I'll send you an email.

iJungleboy commented 1 year ago

@lilith cool ChatGPT chat :) https://docs.google.com/document/d/1MPJ6hoV9hVHJRh89fy87NngbVXcjCtP4sB8ezGL6P0A/edit

I would like to blog that - ist it ok for you?

lilith commented 1 year ago

Absolutely!

On Wed, Feb 1, 2023, 1:01 AM iJungleboy @.***> wrote:

@lilith https://github.com/lilith cool ChatGPT chat :) - I would like to blog that - ok?

— Reply to this email directly, view it on GitHub https://github.com/imazen/imageflow-dotnet-server/issues/71#issuecomment-1411613265, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA2LHYQFOK6X7J3IWEHSCLWVIJ4DANCNFSM6AAAAAAUMG4HXQ . You are receiving this because you were mentioned.Message ID: @.***>

iJungleboy commented 1 year ago

@lilith thanks :) - blogged about it here: https://blazor-cms.org/blog?details=chatgpt-s-imageflow-the-resizer-in-2sxc Corrected: https://blazor-cms.org/blog?details=chatgpt-loves-imageflow-the-resizer-in-2sxc

lilith commented 1 year ago

Awesome! Thank you so much!

On Thu, Feb 2, 2023, 10:31 AM iJungleboy @.***> wrote:

@lilith https://github.com/lilith thanks :) - blogged about it here: https://blazor-cms.org/blog?details=chatgpt-s-imageflow-the-resizer-in-2sxc

— Reply to this email directly, view it on GitHub https://github.com/imazen/imageflow-dotnet-server/issues/71#issuecomment-1414113568, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA2LH7X6M7XTVIEZSGNTDLWVPVPPANCNFSM6AAAAAAUMG4HXQ . You are receiving this because you were mentioned.Message ID: @.***>

lilith commented 1 year ago

This issue should be resolved for Imageflow Server. Let me know if you run into anything else related, closing this for now :)

iJungleboy commented 1 year ago

@lilith this is awesome news! thank you so much!