Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WindowsSecureMimeContext uses first RecipientInfo only #530

Closed
ydhondt opened this issue Dec 23, 2019 · 3 comments
Closed

WindowsSecureMimeContext uses first RecipientInfo only #530

ydhondt opened this issue Dec 23, 2019 · 3 comments
Labels
bug Something isn't working

Comments

@ydhondt
Copy link

ydhondt commented Dec 23, 2019

Describe the bug
Using the WindowsSecureMimeContext decrypting an encrypted mail sometimes fails. The reason for this is that it might find multiple RecipientInfos and only tries to find the key for the first one while it should also check the second, third, ...
After discovering the issue, I found it also documented at https://www.limilabs.com/blog/envelopedcms-decrypt-problem.

Platform:

  • OS: Windows
  • .NET Framework: .Net Core 3.1, .NET 4.8
  • MimeKit Version: all

Proposed Solution
Rather than using the EnvelopedCms.Decrypt() method, use the EnvelopedCms.Decrypt(System.Security.Cryptography.Pkcs.RecipientInfo recipientInfo) for each recipient info. The below code loops over them and stops as soon as we had a successful decryption.

public override MimeEntity Decrypt (Stream encryptedData, CancellationToken cancellationToken = default (CancellationToken))
{
    if (encryptedData == null)
        throw new ArgumentNullException (nameof (encryptedData));

    var enveloped = new EnvelopedCms ();

    enveloped.Decode (ReadAllBytes (encryptedData));
    
    var success = false;

    foreach (var recipientInfo in enveloped.RecipientInfos)
    {
        try
        {
            enveloped.Decrypt(recipientInfo);
            success = true;
            break;
        }
        catch (CryptographicException)
        {
        }
    }

    if (!success)
    {
        // No key was found for any of the recipients, so throw an exception.
        // Probably should add some info on which recipients we tried to look for.
        throw new CryptographicException("No key to decrypt was found");
    }    

    var decryptedData = enveloped.Encode ();

    var memory = new MemoryStream (decryptedData, false);

    return MimeEntity.Load (memory, true, cancellationToken);
}
jstedfast added a commit that referenced this issue Dec 23, 2019
@jstedfast
Copy link
Owner

Thanks, I've fixed this in git master now.

@jstedfast jstedfast added the bug Something isn't working label Dec 23, 2019
@ydhondt
Copy link
Author

ydhondt commented Dec 29, 2019

@jstedfast I added a comment to your code fix last week but I have the idea it went overlooked. So I'm adding it here as well:

I am not sure this will work. I think you should break after the first successful attempt.
With this code, if there are multiple RecipientInfos and for at least one there is a failure to decrypt the EnvelopedCms, it will result in a complete failure while it should not.
As soon as there is one RecipientInfo for which the decryption is successful, the decryption should be considered successful. Hence the boolean and the break in my suggested code.

@jstedfast
Copy link
Owner

I already fixed that :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants