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

Amavis skips signature checks #681

Closed
lyfenwebos opened this issue Jun 9, 2021 · 2 comments
Closed

Amavis skips signature checks #681

lyfenwebos opened this issue Jun 9, 2021 · 2 comments
Labels
bug Something isn't working compatibility Compatibility with existing software

Comments

@lyfenwebos
Copy link

Describe the bug
MimeKit's part with signature has wrong open/closing lines. This causes troubles for our Zimbra Antivirus.
It tries to scan attachment(signature) and fails with error "Encrypted"

If I sign my message with Gpg4Win, everything is working flawlessly

I compared 2 messages (Gpg4Win and MimeKit)

Differences in signature part I have found:

Gpg4Win:

Signature markers -----BEGIN PGP SIGNATURE----- / -----END PGP SIGNATURE-----
Filename openpgp-digital-signature.asc

MimeKit:

Signature markers -----BEGIN PGP MESSAGE----- / -----END PGP MESSAGE-----
Filename signature.pgp

Platform (please complete the following information):

  • OS: Windows
  • .NET Runtime: CoreCLR
  • .NET Framework: .Net Core 5
  • MimeKit Version: 2.12.0

To Reproduce
Steps to reproduce the behavior:

  1. Compose and Sign message using Mimekit
  2. Send it to any Mail server with amavis( I can reproduce it on Zimbra Mail Server )
  3. User will receive mail with subject UNCHECKED and in amavis logs you will see error UNCHECKED-ENCRYPTED.
    This is valid for encrypted messages, but not for signed.

Expected behavior
Message passes amavis checks and user gets mail without notices from Mail server, that message is unchecked and user should be careful with it

Additional context
Probably the main issue is in signature markers, but would be nice to have signature filename openpgp-digital-signature.asc or any other in .asc format.

@jstedfast jstedfast added bug Something isn't working compatibility Compatibility with existing software labels Jun 9, 2021
@jstedfast
Copy link
Owner

The .asc vs .pgp file extension doesn’t change the format, but I should probably be using the .asc extension anyway so I will make that change.

The existence of the Content-Disposition header shouldn’t prevent things from working and in some cases might help make it possible to save the signature as an attachment, so I think I’ll keep that unless I discover that it is actually breaking things for some mailers.

As you noted, I agree with you that I think the real issue here is the BEGIN/END PGP MESSAGE marker. It should be BEGIN/END PGP SIGNATURE, I think, and the fact that Gpg4Win uses that marker as well suggests that MimeKit is getting it wrong.

MimeKIt doesn’t write those markers itself, it uses BouncyCastle’s ArmoredOutputStream which is supposed to correctly write a marker, but something is clearly going wrong.

This appears to be where BouncyCastle decides which marker to use: https://github.com/bcgit/bc-csharp/blob/master/crypto/src/bcpg/ArmoredOutputStream.cs#L283

Okay, so my theory is that it’s because MimeKit compresses the PgpSignature packet and writes the resulting PgpCompressed packet to the ArmoredOutputStream and therefore it uses BEGIN/END PGP MESSAGE marker (that's the default).

https://github.com/jstedfast/MimeKit/blob/master/MimeKit/Cryptography/OpenPgpContextBase.cs#L833

jstedfast added a commit that referenced this issue Jun 9, 2021
Due to the fact that MimeKit was compressing the PGP signature, the
armored output ended up using -----BEGIN PGP MESSAGE----- instead of
-----BEGIN PGP SIGNATURE-----, which is what it should have been using.

Also switched the filename extension used with the application/pgp-signature
MIME part to be .asc instead of .pgp for consistency with other PGP/MIME
implementations like Gpg4Win.

Fixes issue #681
@jstedfast
Copy link
Owner

MimeKit 2.13.0 has been released with this fix.

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

No branches or pull requests

2 participants