[API Proposal]: Authenticated ciphers should distigush between mismatched tag and failure

[carlreinke]

Background and motivation

When decrypting authenticated data, AesCcm, AesGcm, and ChaCha20Poly1305 throw CryptographicException in the case when the tag does not match (indicating that either the key is wrong or the data is inauthentic) and also in the case when "the decryption operation otherwise failed". A program may want to take a different action in these two scenarios but currently cannot.

API Proposal

namespace System.Security.Cryptography
{
    // Note for discussion: Should this be sealed or unsealed?
    public sealed class AuthenticationTagMismatchException : CryptographicException
    {
        public AuthenticationTagMismatchException();
        public AuthenticationTagMismatchException(string? message);
        public AuthenticationTagMismatchException(string? message, Exception? innerException);
    }
}

API Usage

// User has provided password, and PBKDF was used to derive key.
using (var aesGcm = new AesGcm(key))
{
    try
    {
        aesGcm.Decrypt(iv, ciphertext, tag, plaintext);
    }
    catch (AuthenticationTagMismatchException)
    {
        // Notify user that password was incorrect or data was corrupt.
        // Prompt user for password again in case user typo'd password.
    }
    catch (CryptographicException)
    {
        // Notify the user that decryption failed.
    }
}

Alternative Designs

No response

Risks

No response

[ghost]

Tagging subscribers to this area: @dotnet/area-system-security, @vcsjones
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and motivation

When decrypting authenticated data, AesGcm and ChaCha20Poly1305 throw CryptographicException in the case when the tag does not match (indicating that either the key is wrong or the data is inauthentic) and also in the case when "the decryption operation otherwise failed". A program may want to take a different action in these two scenarios but currently cannot.

API Proposal

namespace System.Cryptography
{
    public partial class AesGcm
    {
        // existing: public void Decrypt(byte[] nonce, byte[] ciphertext, byte[] tag, byte[] plaintext, byte[] associatedData = null);
        // existing: public void Decrypt(ReadOnlySpan<byte> nonce, ReadOnlySpan<byte> ciphertext, ReadOnlySpan<byte> tag, Span<byte> plaintext, ReadOnlySpan<byte> associatedData = default);

        // Returns false only when tag is mismatched.
        public bool TryDecrypt(byte[] nonce, byte[] ciphertext, byte[] tag, byte[] plaintext, byte[] associatedData = null);
        public bool TryDecrypt(ReadOnlySpan<byte> nonce, ReadOnlySpan<byte> ciphertext, ReadOnlySpan<byte> tag, Span<byte> plaintext, ReadOnlySpan<byte> associatedData = default);
    }
    public partial class ChaCha20Poly1305
    {
        // existing: public void Decrypt(byte[] nonce, byte[] ciphertext, byte[] tag, byte[] plaintext, byte[]? associatedData = null);
        // existing: public void Decrypt(ReadOnlySpan<byte> nonce, ReadOnlySpan<byte> ciphertext, ReadOnlySpan<byte> tag, Span<byte> plaintext, ReadOnlySpan<byte> associatedData = default);

        // Returns false only when tag is mismatched.
        public bool TryDecrypt(byte[] nonce, byte[] ciphertext, byte[] tag, byte[] plaintext, byte[]? associatedData = null);
        public bool TryDecrypt(ReadOnlySpan<byte> nonce, ReadOnlySpan<byte> ciphertext, ReadOnlySpan<byte> tag, Span<byte> plaintext, ReadOnlySpan<byte> associatedData = default);
    }
}

API Usage

// User has provided password, and PBKDF was used to derive key.
using (var aesGcm = new AesGcm(key))
{
    try
    {
        if (!aesGcm.TryDecrypt(iv, ciphertext, tag, plaintext))
        {
            // Notify user that password was incorrect or data was corrupt.
            // Prompt user for password again in case user typo'd password.
        }
    }
    catch (CryptographicException)
    {
        // Something went wrong.  Notify user that decryption will not be possible.
    }
}

Alternative Designs

No response

Risks

No response

Author:	carlreinke
Assignees:	-
Labels:	api-suggestion, area-System.Security, untriaged
Milestone:	-

[teo-tsirpanis]

Hello @carlreinke, the .NET team has decided not to give access to the decrypted plaintext in case of an authentication tag mismatch because processing this plaintext leads to security vulnerabilities and defeats the reason to use authenticated encryption in the first place.

Apart from incorrect buffer sizes which are a user code bug that must be fixed, the only other reason the decryption operation would otherwise fail is if the underlying platform does not support this algorithm and you can check it by each class' IsSupported property. There's also the possibility of a spontaneous OS failure but you can't do anything about it.

Since the API you are proposing is very unlikely to be accepted, I am going to close this issue. Let me know if you have any further questions.

[bartonjs]

The 99.9999% case is that tag verification failed (whether that be from tampered data or a mismatched ciphertext+tag+key). The 0.0001% case is that something weird went on during the decryption operation, such as the underlying library doing malloc in low memory conditions and getting a failure. Neither indicate a permanent inability to decrypt.

It's also not something we can necessarily guarantee that we can even detect a difference from across all of our underlying providers forever.

(Apparently I didn't hit comment 2 hours ago)

[carlreinke]

the .NET team has decided not to give access to the decrypted plaintext in case of an authentication tag mismatch

The proposed API does not assume or imply that decrypted plaintext would be made available in case of a tag mismatch. I would expect that the decrypted plaintext would be zeroed before returning false, just as it is zeroed before throwing today.

Neither indicate a permanent inability to decrypt.

Okay, but it is still very much a distinct failure mode from a tag mismatch.

It's also not something we can necessarily guarantee that we can even [implement this feature] across all of our underlying providers forever.

Is this the bar for new .NET APIs? I'm not sure how you can implement any new feature if that's the case. If there's a planned-to-be-supported platform that does not provide a means of implementing the proposed API, I would understand.

[carlreinke]

Other languages/runtimes that support distinguishing a mismatched tag from other failures include Java and Swift.

[vcsjones]

@carlreinke to help me better understand:

A program may want to take a different action in these two scenarios but currently cannot.

Can you give a few examples here? How is distinguishing a bad authentication tag useful to you, as opposed to "any failure" (CryptographicException)? I'm trying to better understand the use case which may help drive any API changes that get made.

My 2 cents: rather than introduce new APIs that indicate if the tag mismatched, instead I would change the existing APIs to throw an AuthenticationTagMismatchException which derives from CryptographicException.

Proposalified:

namespace System.Security.Cryptography {
    public sealed class AuthenticationTagMismatchException : CryptographicException {
        public AuthenticationTagMismatchException() { }
        public AuthenticationTagMismatchException(string? message) : base(message) { }
        public AuthenticationTagMismatchException(string? message, System.Exception? innerException) { }
    }
}

[carlreinke]

The only use case I have is basically the one described in the issue description.

AuthenticationTagMismatchException would be an acceptable solution.

I only went with a try-pattern in my proposal because in my use case the key is derived from user input, and "bad user input is not exceptional, it happens all the time." I don't feel particularly strongly about that for this API though.

[bartonjs]

It's also not something we can necessarily guarantee that we can even [implement this feature] across all of our underlying providers forever.

Is this the bar for new .NET APIs? I'm not sure how you can implement any new feature if that's the case.

That particular concern is most apparent in cryptography, since (as a design goal / security principle) we use platform-provided cryptographic libraries rather than writing our own. I was most concerned about Android (a current platform) and our eventual move to Apple's CryptoKit library. I sensed a pretty high Spock-eyebrow through @vcsjones' message to me when suggested that we have the data available to us.

[bartonjs]

I prefer the distinct exception type over the new method, for two reasons:

1. Try[Something](ReadOnlySpan, Span, ...) is usually a Span Try-Write method, where the false return means only "the destination was too small". In this case it isn't, but that requires domain knowledge.
2. I really don't like the idea of someone ignoring the return value.

As for the perf impact of the exception (the generalized Try vs the Span-specific one), I don't think it really matters here. If the main source of failure is "the user typed some bad input" that means we're in an interactive session and the user probably won't notice the timing difference between Try->false and try->catch. There's not a big-data / fast-path / loop concern (at least, not one big enough to offset the above concerns (for me)).

[carlreinke]

I updated the proposal to be what @vcsjones proposed.

[vcsjones]

Just to enumerate all of the platforms:

1. Android has AEADBadTagException

2. CryptoKit (Apple platforms), if / when we get around to supporting it has authenticationFailure as case for the error enum.

3. Windows has an NTSTATUS of STATUS_AUTH_TAG_MISMATCH.

4. OpenSSL says check the return value of EVP_DecryptFinal_ex (or cipher final). This what we are currently doing to throw the CryptographicException with the tag-mismatch error text.

   runtime/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AesGcm.OpenSsl.cs

   Line 109 in 899bf97

   throw new CryptographicException(SR.Cryptography_AuthTagMismatch);

   and says as much in their demo code. https://github.com/openssl/openssl/blob/3068a183ae3a6eba858aa23b2a4d9f7d2ba8a4c1/demos/cipher/aesgcm.c#L201-L206

[GrabYourPitchforks]

We should consider unsealing the exception. It's not all that common for exception types to be sealed outside of a very small population which represent something very special to the runtime.

[bartonjs]

Video

Looks good as proposed

namespace System.Security.Cryptography
{
    // Note for discussion: Should this be sealed or unsealed?
    public sealed class AuthenticationTagMismatchException : CryptographicException
    {
        public AuthenticationTagMismatchException();
        public AuthenticationTagMismatchException(string? message);
        public AuthenticationTagMismatchException(string? message, Exception? innerException);
    }
}

[vcsjones]

@bartonjs i have a branch for this, so removed [help wanted].