dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.51k stars 4.78k forks source link

Add AES-GCM support to EncryptedXml #34784

Open componentspace opened 4 years ago

componentspace commented 4 years ago

Shibboleth now defaults to AES-GCM for XML encryption (https://wiki.shibboleth.net/confluence/display/IDP4/GCMEncryption) and a number of other platforms include this support.

The System.Security.Cryptography.Xml.EncryptedXml class supports 3DES and AES-CBC for data encryption but not AES-GCM. I would like to see this support added.

ghost commented 4 years ago

Tagging @bartonjs as an area owner

bartonjs commented 4 years ago

For the most part, we consider the crypto XML package to be on minimal maintenance, only present for applications to have an easier time moving to .NET Core from .NET Framework.

Adding new algorithms to it is complicated, since they would only work on newer runtimes, but they're identified from within the payload. So something that works on .NET 5 would fail on .NET Core 2.1 or .NET Framework.

I'm not sure this is something we really have the option to say yes to.

componentspace commented 4 years ago

If AES-GCM were supported on .NET 5 only rather than earlier releases that would be fine.

The AesGcm class introduced in .NET Core 3.0 exposes this algorithm but unfortunately it doesn't follow the earlier model of extending the SymmetricAlgorithm class. If it did, I think EncryptedXml would then support AES-GCM.

I believe the trend will be to move away from AES-CBC to AES-GCM for XML encryption. If AES-GCM isn't supported by .NET this will make compatibility with other platforms that have made the move problematic and severely reduce the usefulness of the EncryptedXml class.

I hope you will give this further consideration. Thanks.

GrabYourPitchforks commented 4 years ago

One reason AesGcm doesn't extend the SymmetricAlgorithm class is that AES-GCM is an authenticated encryption mechanism, as you know. Contrast this against 3DES-CBC / AES-CBC which both lack an authentication mechanism. GCM produces two outputs: the ciphertext and a nonce. If a particular payload format only has a concept of "here's your ciphertext" (as SymmetricAlgorithm implicitly does via CryptoStream), there's no good way for us to communicate through such an API surface "hey, you also need to pay attention to the nonce!" It really does require special handling by the caller.

We debated emitting [ ciphertext || nonce ] as the single "output" from the GCM routine, but this ran into two problems. First, it would mean that we're necessarily injecting an opinion into an API that is supposed to be a foundational building block. Second, it would mean that anybody performing streaming decryption would be able to see + operate on the plaintext before it had been validated. Neither of these were really paths we wanted to go down. Hence why AesGcm doesn't subclass SymmetricAlgorithm and why it can't be plugged in to existing infrastructure which consumes that base type.

componentspace commented 4 years ago

Thanks for sharing the background as to why extending SymmetricAlgorithm was considered and rejected.

I'm still keen to see AES-GCM support included in the EncryptedXml class or a new class supporting the latest XML encryption specification.

klemensurban commented 3 years ago

We have the same problem here; XML Signature Syntax and Processing version 1.1 is a standard that has been around for 7+ years (and actually version 2.0 since 2015). It can't be that current cryptographic methods are ignored for years for "backward compatibility reasons". This causes interoperability problems in products like ADFS; and we all agree that monolithic solutions are no longer state of the art?

kertzi commented 2 years ago

This is really important for us too because we have national suomi.fi authentication which is used by many official situations and they are switching into AES-GCM.

Many external Saml2 libraries also suffers this because they use EncryptedXml underhood. For example Itfoxtec and Sustainsys.Saml2 Link to Issue

At least some workaround for this would be nice.

We are using .Net 5

spaasis commented 10 months ago

Does anyone have a good workaround for this?