ncrypt/crypt_gpgme.c: Show encryption information by alejandro-colomar · Pull Request #4221 · neomutt/neomutt

alejandro-colomar · 2024-03-29T00:36:21Z

Show a block of information about the encryption of a message, which includes the recipients that can decrypt the message.

This info isn't necessarily complete: the sender could hide some recipients, so that they are able to decrypt the message, but aren't listed. See gpg(1) options --recipient and --hidden-recipient. I'm not sure, but I suspect a malicious sender could also add fake recipients, which are listed, but cannot really decrypt the message.

Therefore, this should be taken only as an informative block, and with a pinch of salt.

Closes: #4163
Cc: @flatcap
Cc: @dd9jn

What does this PR do?

Show encryption information about a message.

Screenshots (if relevant)

Unencrypted mail:

Message-ID: <ZgU0xhvp82jUPsy1@debian>

[-- Begin signature information --]
Good signature from: Alejandro Colomar <[email protected]>
                aka: Alejandro Colomar <[email protected]>
                aka: Alejandro Colomar Andres <[email protected]>
            created: Thu Mar 28 10:13:42 2024
[-- End signature information --]

[-- The following data is signed --]

Encrypted mail:

Message-ID: <ZgA4K7hxzbGPTTGY@debian>

[-- Begin encryption information --]
Recipient: (RSA)      C527609C7E1875BD
Recipient: (RSA)      00C14A7DBBDD521C
[-- End encryption information --]

[-- Begin signature information --]
Good signature from: Alejandro Colomar <[email protected]>
                aka: Alejandro Colomar <[email protected]>
                aka: Alejandro Colomar Andres <[email protected]>
            created: Sun Mar 24 15:26:50 2024
[-- End signature information --]

[-- The following data is PGP/MIME signed and encrypted --]

Does this PR meet the acceptance criteria? (This is just a reminder for you,
this section can be removed if you fulfill it.)
```
I need to test a few things still:

*  Try to fake a recipient.
*  Hide a recipient.
*  Test the application handler.
```
- Documentation created/updated (you have to edit
  docs/manual.xml.head
  for that)
  
  Not yet.
- All builds and tests are passing
  
  It builds (and runs) on my PC.
- Added doxygen code documentation
  syntax
  
  Small, but yeah. I may need to expand those a little bit.
- Code follows the style guide
  
  I think so.
What are the relevant issue numbers?
Show information about the encryption of a message #4163

Revisions

v9c

v9c changes (by @flatcap ):

Rebase

$ git range-diff devel/security..gpg neomutt/main..gh/gpg 
1:  149652003 = 1:  0ea051657 ncrypt/crypt_gpgme.c: Split conditional
2:  18fff6841 = 2:  2f629e658 ncrypt/crypt_gpgme.c: Show encryption information
3:  9e50d8358 = 3:  7d5f76dfb docs: Add feature page for the encryption information block
4:  c95200e83 = 4:  ba9a785dd $crypt_encryption_info: Add variable

Merged

Merged:

$ git range-diff neomutt/main..gh/gpg neomutt/main..12d9d5a75
1:  0ea051657 = 1:  d181b5e97 ncrypt/crypt_gpgme.c: Split conditional
2:  2f629e658 = 2:  a7c37625e ncrypt/crypt_gpgme.c: Show encryption information
3:  7d5f76dfb = 3:  8320798b4 docs: Add feature page for the encryption information block
4:  ba9a785dd = 4:  12d9d5a75 $crypt_encryption_info: Add variable

alejandro-colomar · 2024-03-29T13:27:22Z

v1b (by @flatcap) changes:

Rebase on main

$ git range-diff 2dde10ea0^..2dde10ea0 neomutt/main..gh/gpg 
1:  2dde10ea0 = 1:  f09907fe9 ncrypt/crypt_gpgme.c: Show encryption information

alejandro-colomar · 2024-03-29T13:29:13Z

v1c changes:

Rebase on main
Add some links to the commit message

$ git range-diff gh/gpg^..gh/gpg neomutt/main..gpg
1:  f09907fe9 ! 1:  7ed3c312c ncrypt/crypt_gpgme.c: Show encryption information
    @@ Commit message
         pinch of salt.
     
         Closes: <https://github.com/neomutt/neomutt/issues/4163>
    +    Link: <https://www.gnupg.org/documentation/manuals/gpgme.pdf#Public%20Key%20Algorithms>
    +    Link: <https://www.gnupg.org/documentation/manuals/gpgme.pdf#Decrypt>
    +    Link: <https://linux.codidact.com/posts/291205>
         Cc: Richard Russon <[email protected]>
         Cc: Werner Koch <[email protected]>
         Signed-off-by: Alejandro Colomar <[email protected]>

alejandro-colomar · 2024-03-29T21:22:28Z

v2 changes:

Call a gpgme function to get the algo string.
Use a format similar to gpg-agent(1): "%s key, ID %s\n", algo, keyid
Use "?" for an unknown algo (like gpg does in several places).

$ git range-diff main gh/gpg gpg 
1:  7ed3c312c ! 1:  28d469fb7 ncrypt/crypt_gpgme.c: Show encryption information
    @@ ncrypt/crypt_gpgme.c: static void print_smime_keyinfo(const char *msg, gpgme_sig
     +{
     +  const char  *algo;
     +
    ++  algo = gpgme_pubkey_algo_name(r->pubkey_algo);
    ++  if (algo == NULL)
    ++    algo = "?";
    ++
     +  state_puts(state, "Recipient: ");
    -+  switch (r->pubkey_algo) {
    -+  case GPGME_PK_RSA:
    -+  case GPGME_PK_RSA_E:
    -+  case GPGME_PK_RSA_S:
    -+    algo = "(RSA)      ";
    -+    break;
    -+  case GPGME_PK_DSA:
    -+    algo = "(DSA)      ";
    -+    break;
    -+  case GPGME_PK_ELG:
    -+  case GPGME_PK_ELG_E:
    -+    algo = "(ElGamal)  ";
    -+    break;
    -+  case GPGME_PK_ECC:
    -+    algo = "(Elliptic) ";
    -+    break;
    -+  case GPGME_PK_ECDSA:
    -+    algo = "(ECDSA)    ";
    -+    break;
    -+  case GPGME_PK_ECDH:
    -+    algo = "(ECDH)     ";
    -+    break;
    -+  case GPGME_PK_EDDSA:
    -+    algo = "(EdDSA)    ";
    -+    break;
    -+  default:
    -+    algo = "           ";
    -+    break;
    -+  }
     +  state_puts(state, algo);
    ++  state_puts(state, " key, ID ");
     +  state_puts(state, r->keyid);
     +  state_puts(state, "\n");
     +}

alejandro-colomar · 2024-03-29T22:07:30Z

Hmmm, interesting thing:

If the recipient is hidden (which I've tested by manually editing a real message, first decrypting it with with gpg(1), editing a few things, then re-encrypting it with different recipients), gpgme reports a key ID consisting of all 0s. The following is an experiment encrypted to me (non-hidden), and two other recipients (both with --hidden-recipient).

[-- Begin encryption information --]
Recipient: RSA key, ID 00C14A7DBBDD521C
Recipient: RSA key, ID 0000000000000000
Recipient: RSA key, ID 0000000000000000
[-- End encryption information --]

I like that behavior for now.

flatcap · 2024-03-29T22:37:31Z

I like that behavior for now.

Yeah, it looks good.

A possible alternative is

Recipient: RSA key, ID [hidden]

ncrypt/crypt_gpgme.c

alejandro-colomar · 2024-03-29T22:42:08Z

I like that behavior for now.

Yeah, it looks good.

A possible alternative is
Recipient: RSA key, ID [hidden]

Yup, that looks good. For now, I'll be lazy and not code that. :)
But we can definitely do that if 0s confuse anyone, or if anyone dislikes 0s.

flatcap · 2024-03-30T00:08:54Z

A quick look at the crash (it's late here)...
I see:

(gdb) p	*state
$2 = {fp_in = 0x615000016600, fp_out = 0x0, prefix = 0x0, flags = 0, wraplen = 0}

It's writing to state->fp_out == NULL

Update:
Though decrypt_part(..., FILE *fp_out, ...) is valid.

alejandro-colomar · 2024-03-30T09:50:11Z

A quick look at the crash (it's late here)... I see:
(gdb) p	*state
$2 = {fp_in = 0x615000016600, fp_out = 0x0, prefix = 0x0, flags = 0, wraplen = 0}
It's writing to state->fp_out == NULL

Update: Though decrypt_part(..., FILE *fp_out, ...) is valid.

Those two seem to always have different values. Without triggering the crash, I see that
state->fp_out=0x559e77af3c30
but
fp_out=0x559e77afe2f0

I don't understand what those two pointers are used for. :|

alejandro-colomar · 2024-03-30T10:26:45Z

A quick look at the crash (it's late here)... I see:
(gdb) p	*state
$2 = {fp_in = 0x615000016600, fp_out = 0x0, prefix = 0x0, flags = 0, wraplen = 0}
It's writing to state->fp_out == NULL
Update: Though decrypt_part(..., FILE *fp_out, ...) is valid.
Those two seem to always have different values. Without triggering the crash, I see that state->fp_out=0x559e77af3c30 but fp_out=0x559e77afe2f0

I don't understand what those two pointers are used for. :|

Hmmm, I checked that the time that state->fp_out is NULL, state->flags is 0, so a check (state->flags & STATE_DISPLAY) would prevent the crash, I suspect.

I've also made sense in my head of the difference between state->fp_out and fp_out: the latter seems to be (just) the message, while the former is the entire screen, which includes the informative blocks (crypto, sig) and of course the message. When viewing attachments, the messages exist, but there's no wrapper screen, so a NULL might make sense. This is all supposition, though; don't trust me much.

I still have no clue what _attach_ is.

flatcap · 2024-03-30T10:27:11Z

My best guess of how it works, is...

An email might need several steps of processing to display it
Each State is one of those steps
The States are maybe chained together

These "handlers" originate in mutt_body_handler()
mutt_body_handler() may be called recursively.

Where does the extra fp_out fit in? No idea, yet :-)

alejandro-colomar · 2024-03-30T10:34:05Z

v3 changes:

Fix crash (make the print conditional)
Explicitly test != NULL (@flatcap , is that OK? I see mixed style in the project)
Add feature page

$ git range-diff main gh/gpg gpg 
1:  28d469fb7 ! 1:  cb4c830dc ncrypt/crypt_gpgme.c: Show encryption information
    @@ ncrypt/crypt_gpgme.c: static void print_smime_keyinfo(const char *msg, gpgme_sig
     + */
     +static void show_encryption_info(struct State *state, gpgme_decrypt_result_t result)
     +{
    -+    if (state->flags & STATE_DISPLAY)
    -+      state_attach_puts(state, _("[-- Begin encryption information --]\n"));
    ++    state_attach_puts(state, _("[-- Begin encryption information --]\n"));
     +
     +    for (gpgme_recipient_t r = result->recipients; r; r = r->next)
     +      show_one_recipient(state, r);
     +
    -+    if (state->flags & STATE_DISPLAY)
    -+      state_attach_puts(state, _("[-- End encryption information --]\n\n"));
    ++    state_attach_puts(state, _("[-- End encryption information --]\n\n"));
     +}
     +
     +/**
    @@ ncrypt/crypt_gpgme.c: restart:
        ciphertext = NULL;
     +
     +  result = gpgme_op_decrypt_result(ctx);
    -+  if (result)
    ++  if (result != NULL && (state->flags & STATE_DISPLAY))
     +    show_encryption_info(state, result);
     +
        if (err != 0)
    @@ ncrypt/crypt_gpgme.c: int pgp_gpgme_application_handler(struct Body *b, struct S
     +          gpgme_decrypt_result_t result = NULL;
     +
     +          result = gpgme_op_decrypt_result(ctx);
    -+          if (result)
    ++          if (result != NULL && (state->flags & STATE_DISPLAY))
     +            show_encryption_info(state, result);
      
                /* Check whether signatures have been verified.  */
-:  --------- > 2:  869884246 docs: Add feature page for the encryption information block

alejandro-colomar · 2024-03-30T10:36:07Z

v3b changes:

Fix indentation.
Explicitly test != NULL in loop controlling expression

$ git range-diff main gh/gpg gpg 
1:  cb4c830dc ! 1:  a3274d3bf ncrypt/crypt_gpgme.c: Show encryption information
    @@ ncrypt/crypt_gpgme.c: static void print_smime_keyinfo(const char *msg, gpgme_sig
     + */
     +static void show_encryption_info(struct State *state, gpgme_decrypt_result_t result)
     +{
    -+    state_attach_puts(state, _("[-- Begin encryption information --]\n"));
    ++  state_attach_puts(state, _("[-- Begin encryption information --]\n"));
     +
    -+    for (gpgme_recipient_t r = result->recipients; r; r = r->next)
    -+      show_one_recipient(state, r);
    ++  for (gpgme_recipient_t r = result->recipients; r != NULL; r = r->next)
    ++    show_one_recipient(state, r);
     +
    -+    state_attach_puts(state, _("[-- End encryption information --]\n\n"));
    ++  state_attach_puts(state, _("[-- End encryption information --]\n\n"));
     +}
     +
     +/**
2:  869884246 = 2:  adeebfdbb docs: Add feature page for the encryption information block

alejandro-colomar · 2024-03-30T10:39:40Z

When does the pgp_gpgme_application_handler() handler run? I don't know how to test it. :|

flatcap · 2024-03-30T11:05:51Z

(This is from a quick trawl of the code, not from understanding :-)

An email with an attachment of application/pgp (or something like that) will trigger it.

In mutt_body_handler(),

neomutt/handler.c

Lines 1666 to 1670 in 37d021f

    
           if (((WithCrypto & APPLICATION_PGP) != 0) && mutt_is_application_pgp(b)) 
        
           { 
        
             encrypted_handler = crypt_pgp_application_handler; 
        
             handler = encrypted_handler; 
        
           }

Then we set the handler to crypt_pgp_application_handler.
This calls the Crypto API function application_handler().
GPGME implements the API, here

neomutt/ncrypt/crypt_mod_pgp_gpgme.c

Lines 58 to 67 in 37d021f

    
           const struct CryptModuleSpecs CryptModPgpGpgme = { 
        
             // clang-format off 
        
             APPLICATION_PGP, 
        
             pgp_gpgme_init, 
        
             NULL, /* cleanup */ 
        
             pgp_gpgme_void_passphrase, 
        
             pgp_gpgme_valid_passphrase, 
        
             pgp_gpgme_decrypt_mime, 
        
             pgp_gpgme_application_handler,

Leading us to

neomutt/ncrypt/crypt_gpgme.c

Line 2427 in 37d021f

int pgp_gpgme_application_handler(struct Body *b, struct State *state)

alejandro-colomar · 2024-03-30T11:26:22Z

application/pgp

Would you mind sending me an email with such an attachment? I don't know how to write one. ;)

alejandro-colomar · 2024-03-30T11:37:19Z

application/pgp

Would you mind sending me an email with such an attachment? I don't know how to write one. ;)

Ahh, this worked: attaching an encrypted file (I'm the hidden recipient in the test below).

[-- Attachment #2: bar.asc --]
[-- Type: application/pgp-keys, Encoding: 7bit, Size: 1.5K --]

[-- Begin encryption information --]
Recipient: RSA key, ID 4A83A79DA571C41C
Recipient: RSA key, ID 0000000000000000
[-- End encryption information --]

[-- BEGIN PGP MESSAGE --]

foo

[-- END PGP MESSAGE --]

The entire mail is shown as:

Date: Sat, 30 Mar 2024 12:22:13 +0100
From: Alejandro Colomar <[email protected]>
To: [email protected]
Subject: ...
Message-ID: <Zgf15gQlPaqFldYi@debian>

[-- Begin encryption information --]
Recipient: RSA key, ID 00C14A7DBBDD521C
[-- End encryption information --]

[-- Begin signature information --]
Good signature from: Alejandro Colomar <[email protected]>
                aka: Alejandro Colomar <[email protected]>
                aka: Alejandro Colomar Andres <[email protected]>
            created: Sat Mar 30 12:22:13 2024
[-- End signature information --]

[-- The following data is PGP/MIME signed and encrypted --]

Subject: Test application handler

[-- Attachment #1 --]
[-- Type: text/plain; charset=utf-8, Encoding: quoted-printable, Size: 0.1K --]

Some test

--
<https://www.alejandro-colomar.es/>
 
[-- Attachment #2: bar.asc --]
[-- Type: application/pgp-keys, Encoding: 7bit, Size: 1.5K --]

[-- Begin encryption information --]
Recipient: RSA key, ID 4A83A79DA571C41C
Recipient: RSA key, ID 0000000000000000
[-- End encryption information --]

[-- BEGIN PGP MESSAGE --]

foo

[-- END PGP MESSAGE --]

And viewing just the attachment:

[-- Begin encryption information --]
Recipient: RSA key, ID 4A83A79DA571C41C
Recipient: RSA key, ID 0000000000000000
[-- End encryption information --]

[-- BEGIN PGP MESSAGE --]

foo

[-- END PGP MESSAGE --]

And the multipart/mixed (all):

[-- Attachment #1 --]
[-- Type: text/plain; charset=utf-8, Encoding: quoted-printable, Size: 0.1K --]

Some test

--
<https://www.alejandro-colomar.es/>

[-- Attachment #2: bar.asc --]
[-- Type: application/pgp-keys, Encoding: 7bit, Size: 1.5K --]

[-- Begin encryption information --]
Recipient: RSA key, ID 4A83A79DA571C41C
Recipient: RSA key, ID 0000000000000000
[-- End encryption information --]

[-- BEGIN PGP MESSAGE --]

foo

[-- END PGP MESSAGE --]

alejandro-colomar · 2024-04-25T21:28:15Z

v7 changes:

_("") (except for the newline, which I guess doesn't need to be i18n'd. [@nabijaczleweli ]

$ git range-diff devel/security gh/gpg gpg 
1:  55be3699b = 1:  55be3699b ncrypt/crypt_gpgme.c: Split conditional
2:  76180da27 ! 2:  f0a0767ca ncrypt/crypt_gpgme.c: Show encryption information
    @@ Commit message
         Link: <https://github.com/neomutt/neomutt/issues/4223>
         Co-developed-by: Richard Russon <[email protected]>
         Cc: Werner Koch <[email protected]>
    +    Cc: наб <[email protected]>
         Signed-off-by: Alejandro Colomar <[email protected]>
     
      ## ncrypt/crypt_gpgme.c ##
    @@ ncrypt/crypt_gpgme.c: static void print_smime_keyinfo(const char *msg, gpgme_sig
     +  if (!algo)
     +    algo = "?";
     +
    -+  state_puts(state, "Recipient: ");
    ++  state_puts(state, _("Recipient: "));
     +  state_puts(state, algo);
    -+  state_puts(state, " key, ID ");
    ++  state_puts(state, _(" key, ID "));
     +  state_puts(state, r->keyid);
     +  state_puts(state, "\n");
     +}
3:  582879f10 = 3:  4ba79c5a0 docs: Add feature page for the encryption information block
4:  7b29638ef = 4:  a60ecaa89 $crypt_encryption_info: Add variable

nabijaczleweli · 2024-04-26T11:50:52Z

This is still wrong, it needs to be (if git grep isn't leading me astray) state_printf(_("Recipient: %s key, ID %s\n"), algo, r->keyid) and probably annotated with // L10N: %s=RSA/ECDSA/..., %s=4A83A79DA571C41C

alejandro-colomar · 2024-04-26T11:55:15Z

This is still wrong, it needs to be (if git grep isn't leading me astray) state_printf(_("Recipient: %s key, ID %s\n"), algo, r->keyid) and probably annotated with // L10N: %s=RSA/ECDSA/..., %s=4A83A79DA571C41C

Hmmm! I didn't know of that function. Yep, that looks much nicer. Thanks!

Some users want to be able to turn this feature off. But default to 'yes', since it's better to not hide crypto-related info, unless the user explicitly says they don't care about it. Link: <neomutt#4221> Suggested-by: наб <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]>

alejandro-colomar · 2024-04-26T12:03:42Z

v8 changes:

Compact into a single state_printf() call, and comment L10N. [@nabijaczleweli ]

$ git range-diff devel/security gh/gpg gpg 
1:  55be3699b = 1:  55be3699b ncrypt/crypt_gpgme.c: Split conditional
2:  f0a0767ca ! 2:  c4738ad68 ncrypt/crypt_gpgme.c: Show encryption information
    @@ ncrypt/crypt_gpgme.c: static void print_smime_keyinfo(const char *msg, gpgme_sig
     +  if (!algo)
     +    algo = "?";
     +
    -+  state_puts(state, _("Recipient: "));
    -+  state_puts(state, algo);
    -+  state_puts(state, _(" key, ID "));
    -+  state_puts(state, r->keyid);
    -+  state_puts(state, "\n");
    ++  // L10N: %s (algo) = RSA/ECDSA/..., %s (r->keyid) = 1111111111111111
    ++  state_printf(state, _("Recipient: %s key, ID %s\n"), algo, r->keyid);
     +}
     +
     +/**
3:  4ba79c5a0 = 3:  d9f1eecb3 docs: Add feature page for the encryption information block
4:  a60ecaa89 = 4:  c268086e6 $crypt_encryption_info: Add variable

TL;DR: These blank lines are not part of the signed and/or encrypted message. neomutt(1) was claiming so, which was incorrect. As наб suggested (see links below), I think we were abusing blank lines a bit too much. Long story: A MIME part doesn't include the trailing blank line. That blank line belongs to the structure of the whole message, and doesn't belong to the body part. A typical PGP/MIME email looks like this: Date: Mon, 1 Apr 2024 21:40:04 +0200 From: Alejandro Colomar <[email protected]> To: [email protected] Subject: After Message-ID: <ZgsNlP1MtLAsGBA3@debian> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="bHQR9+uNWFqbSKv/" Content-Disposition: inline --bHQR9+uNWFqbSKv/ Content-Type: text/plain; protected-headers=v1; charset=utf-8 Content-Disposition: inline Date: Mon, 1 Apr 2024 21:40:04 +0200 From: Alejandro Colomar <[email protected]> To: [email protected] Subject: After Supposedly, migadu is fixed now. Let's see --bHQR9+uNWFqbSKv/ Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEE6jqH8KTroDDkXfJAnowa+77/2zIFAmYLDZQACgkQnowa+77/ 2zKXvBAApT2XkOxpPRRmhjawh/axbHG3NXBldiYe9DAupE4eVJLxLnv8/tf+WWHU oru0vdOBaO1H8VwNi22P+aXo+29HARKawlwhVmgd2auSYzrR+UaSTSwLTN0UO8e/ KCG2LHPMrTnZjPmlWdVRjOq44hkwes32yByjGrVz6cuwtss7XAdDmxuRGpRjHB47 EVBstReybfepSXEhPJTB+yUfUpnVZ9G4nUBDlBODHHY8T2FPB8AMQmXAUGSIn96Y V33lIyiQ92gBrp3UmppfegOU+Clt18UD9tGs00UmIRlagclS9gUcUrY8CeKKMOJ5 PYjsyqjiBLTVO58lKAjXEcAOMs0us+psxmSgzqBw52bLhxlwUDytwMUJUcQiERme zG6lz2lbZ45PMzrtKcm4fAxoJyQgwyf5zrQG5iyqvkmngQoo3ONlqZsZPz9GQffi C6uB8lmRvJM4FcPs/w2Ee6bXA3xk9ClGWryFGwdJM49nGMPXyzDx3/nc5KlOAXAn QPqFSy+kEuzXGJXQZcYwj0o/xNU3B+EecZH148xuAzBHpk8yQd07ZOGhwJFnUq+N r/cr0pr4A74t+J38Bb4vLKcWgg2pqISqX6zf/0VyYEa6o7dvMxG31gRw+z/Tv+Ln 2WKY/wWvJHbfFuhED7uNtMfiKq8JA2ZxyKi9z/V5G780/89soG0= =BrzD -----END PGP SIGNATURE----- --bHQR9+uNWFqbSKv/-- Of which the main body part is precisely this: ``` Content-Type: text/plain; protected-headers=v1; charset=utf-8 Content-Disposition: inline Date: Mon, 1 Apr 2024 21:40:04 +0200 From: Alejandro Colomar <[email protected]> To: [email protected] Subject: After Supposedly, migadu is fixed now. Let's see ``` Note no trailing blank. When verifying the signature, that blank line belongs to the encapsulation boundary, and if included, it would fail the verification. See <#4229> for that. Then, a MIME part can be further subdivided. Quoting RFC1521: > Each part starts with an encapsulation boundary, and then contains a > body part consisting of header area, a blank line, and a body area. This RFC has been obsoleted by RFC2045, but the new one is less clear in the wording, and probably nothing really changed, so don't worry about it. This says that the blank line between the headers and the body is *not* part of the body. This is very much like with HTTP messages. [neo]mutt(1), however, when displaying signed/encrypted email, printed both the separating blank and the trailing blank (or it invented a blank line before and after the body, however it is; it might be that, since the blank goes even before the protected subject, if there is one). These blanks were spurious, and may lead to think that the original body actually has those blanks. That's a lie. Not a big one, but a lie. Trimming those 2 blanks makes the output more correct, faithful to the original message as it was sent, and less consuming of non-renewable resources (think of 24-rows here). Closes: <#4242> Link: <#4221 (comment)> Link: <#4229> Link: <https://datatracker.ietf.org/doc/html/rfc1521#section-7.2> Link: <https://datatracker.ietf.org/doc/html/rfc2045#section-2.4> Cc: наб <[email protected]> Cc: Pietro Cerutti <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]>

Some users want to be able to turn this feature off. But default to 'yes', since it's better to not hide crypto-related info, unless the user explicitly says they don't care about it. Link: <neomutt#4221> Suggested-by: наб <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]>

alejandro-colomar · 2024-04-29T09:55:02Z

v8b (by @flatcap ) changes:

Rebase

$ git range-diff devel/security..gpg neomutt/devel/security..gh/gpg 
1:  55be3699b = 1:  db58c6335 ncrypt/crypt_gpgme.c: Split conditional
2:  c4738ad68 = 2:  4df67334a ncrypt/crypt_gpgme.c: Show encryption information
3:  d9f1eecb3 = 3:  4d4644abd docs: Add feature page for the encryption information block
4:  c268086e6 = 4:  3066f1f63 $crypt_encryption_info: Add variable

ncrypt/crypt_gpgme.c

alejandro-colomar · 2024-04-29T12:31:33Z

v9 changes:

Fixup with @flatcap 's suggested patch for L10N comment.

$ git range-diff devel/security gh/gpg gpg 
1:  db58c6335 = 1:  db58c6335 ncrypt/crypt_gpgme.c: Split conditional
2:  4df67334a ! 2:  640f64465 ncrypt/crypt_gpgme.c: Show encryption information
    @@ ncrypt/crypt_gpgme.c: static void print_smime_keyinfo(const char *msg, gpgme_sig
     +  if (!algo)
     +    algo = "?";
     +
    -+  // L10N: %s (algo) = RSA/ECDSA/..., %s (r->keyid) = 1111111111111111
    ++  // L10N: Show the algorithm and key ID of the encryption recipients, e.g
    ++  //       Recipient: RSA key, ID 1111111111111111
     +  state_printf(state, _("Recipient: %s key, ID %s\n"), algo, r->keyid);
     +}
     +
3:  4d4644abd = 3:  6fc8c64fd docs: Add feature page for the encryption information block
4:  3066f1f63 = 4:  c63186d66 $crypt_encryption_info: Add variable
5:  4cdf7c3d0 < -:  --------- ncrypt/crypt_gpgme.c: @flatcap's suggested comment

Some users want to be able to turn this feature off. But default to 'yes', since it's better to not hide crypto-related info, unless the user explicitly says they don't care about it. Link: <neomutt#4221> Suggested-by: наб <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]>

alejandro-colomar · 2024-05-02T10:41:18Z

v9b changes:

Rebase

$ git range-diff main..gh/gpg devel/security..gpg 
1:  db58c6335 = 1:  149652003 ncrypt/crypt_gpgme.c: Split conditional
2:  640f64465 = 2:  18fff6841 ncrypt/crypt_gpgme.c: Show encryption information
3:  6fc8c64fd = 3:  9e50d8358 docs: Add feature page for the encryption information block
4:  c63186d66 = 4:  c95200e83 $crypt_encryption_info: Add variable

This is a small refactor in preparation for the next commit. Signed-off-by: Alejandro Colomar <[email protected]>

Show a block of information about the encryption of a message, which includes the recipients that can decrypt the message. This info isn't necessarily complete: the sender could hide some recipients, so that they are able to decrypt the message, but aren't listed. See gpg(1) options --recipient and --hidden-recipient. I'm not sure, but I suspect a malicious sender could also add fake recipients, which are listed, but cannot really decrypt the message. Therefore, this should be taken only as an informative block, and with a pinch of salt. It would be good to protect the To and Cc header fields, to provide a guarantee to the recipient that the listed recipients match the original recipients as intended by the sender. See the bug neomutt#4223 linked below. Closes: <neomutt#4163> Link: <https://www.gnupg.org/documentation/manuals/gpgme.pdf#Public%20Key%20Algorithms> Link: <https://www.gnupg.org/documentation/manuals/gpgme.pdf#Decrypt> Link: <https://linux.codidact.com/posts/291205> Link: <neomutt#4223> Co-developed-by: Richard Russon <[email protected]> Cc: Werner Koch <[email protected]> Cc: наб <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]>

Signed-off-by: Alejandro Colomar <[email protected]>

Some users want to be able to turn this feature off. But default to 'yes', since it's better to not hide crypto-related info, unless the user explicitly says they don't care about it. Link: <neomutt#4221> Suggested-by: наб <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]>

flatcap · 2024-05-08T08:08:49Z

rebased over [devel/security]

alejandro-colomar · 2024-05-08T09:26:26Z

Thanks! :-)

alejandro-colomar requested a review from a team as a code owner March 29, 2024 00:36

flatcap force-pushed the gpg branch from 2dde10e to f09907f Compare March 29, 2024 01:46

flatcap assigned alejandro-colomar Mar 29, 2024

flatcap added type:discuss Your views/opinions are requested type:enhancement Feature Request labels Mar 29, 2024

alejandro-colomar force-pushed the gpg branch from f09907f to 7ed3c31 Compare March 29, 2024 13:28

This comment was marked as resolved.

Sign in to view

alejandro-colomar mentioned this pull request Mar 29, 2024

Should To and Cc header fields be protected? #4223

Closed

This comment was marked as resolved.

Sign in to view

alejandro-colomar force-pushed the gpg branch from 7ed3c31 to 28d469f Compare March 29, 2024 21:20

alejandro-colomar commented Mar 29, 2024

View reviewed changes

ncrypt/crypt_gpgme.c Outdated Show resolved Hide resolved

alejandro-colomar force-pushed the gpg branch from 28d469f to 8698842 Compare March 30, 2024 10:32

alejandro-colomar force-pushed the gpg branch from 8698842 to adeebfd Compare March 30, 2024 10:35

alejandro-colomar force-pushed the gpg branch from 7b29638 to a60ecaa Compare April 25, 2024 21:27

alejandro-colomar force-pushed the gpg branch from a60ecaa to c268086 Compare April 26, 2024 12:03

This comment was marked as resolved.

Sign in to view

flatcap force-pushed the devel/security branch from 3225fba to 93d7352 Compare April 29, 2024 07:50

flatcap force-pushed the gpg branch from c268086 to 3066f1f Compare April 29, 2024 08:10

flatcap requested changes Apr 29, 2024

View reviewed changes

ncrypt/crypt_gpgme.c Outdated Show resolved Hide resolved

alejandro-colomar force-pushed the gpg branch from 4cdf7c3 to c63186d Compare April 29, 2024 12:32

alejandro-colomar force-pushed the gpg branch from c63186d to c95200e Compare May 2, 2024 10:40

alejandro-colomar added 4 commits May 8, 2024 08:22

ncrypt/crypt_gpgme.c: Split conditional

0ea0516

This is a small refactor in preparation for the next commit. Signed-off-by: Alejandro Colomar <[email protected]>

docs: Add feature page for the encryption information block

7d5f76d

Signed-off-by: Alejandro Colomar <[email protected]>

flatcap force-pushed the gpg branch from c95200e to ba9a785 Compare May 8, 2024 08:06

flatcap approved these changes May 8, 2024

View reviewed changes

flatcap merged commit 12d9d5a into neomutt:devel/security May 8, 2024

alejandro-colomar deleted the gpg branch May 8, 2024 09:49

Uh oh!

Conversation

alejandro-colomar commented Mar 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alejandro-colomar commented Mar 29, 2024

Uh oh!

alejandro-colomar commented Mar 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

alejandro-colomar commented Mar 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alejandro-colomar commented Mar 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flatcap commented Mar 29, 2024

Uh oh!

Uh oh!

alejandro-colomar commented Mar 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flatcap commented Mar 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alejandro-colomar commented Mar 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alejandro-colomar commented Mar 30, 2024

Uh oh!

flatcap commented Mar 30, 2024

Uh oh!

alejandro-colomar commented Mar 30, 2024

Uh oh!

alejandro-colomar commented Mar 30, 2024

Uh oh!

alejandro-colomar commented Mar 30, 2024

Uh oh!

flatcap commented Mar 30, 2024

Uh oh!

alejandro-colomar commented Mar 30, 2024

Uh oh!

alejandro-colomar commented Mar 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alejandro-colomar commented Apr 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nabijaczleweli commented Apr 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alejandro-colomar commented Apr 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alejandro-colomar commented Apr 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as resolved.

alejandro-colomar commented Apr 29, 2024

Uh oh!

Uh oh!

alejandro-colomar commented Apr 29, 2024

Uh oh!

alejandro-colomar commented May 2, 2024

Uh oh!

flatcap commented May 8, 2024

Uh oh!

alejandro-colomar commented May 8, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

alejandro-colomar commented Mar 29, 2024 •

edited

Loading

alejandro-colomar commented Mar 29, 2024 •

edited

Loading

alejandro-colomar commented Mar 29, 2024 •

edited

Loading

alejandro-colomar commented Mar 29, 2024 •

edited

Loading

alejandro-colomar commented Mar 29, 2024 •

edited

Loading

flatcap commented Mar 30, 2024 •

edited

Loading

alejandro-colomar commented Mar 30, 2024 •

edited

Loading

alejandro-colomar commented Mar 30, 2024 •

edited

Loading

alejandro-colomar commented Apr 25, 2024 •

edited

Loading

nabijaczleweli commented Apr 26, 2024 •

edited

Loading

alejandro-colomar commented Apr 26, 2024 •

edited

Loading

alejandro-colomar commented Apr 26, 2024 •

edited

Loading