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.

Sign up for GitHub

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

Jump to bottom

We shouldn't trust unprotected header fields when replying #4237

Open
alejandro-colomar opened this issue Apr 8, 2024 · 1 comment
Open

We shouldn't trust unprotected header fields when replying #4237

alejandro-colomar opened this issue Apr 8, 2024 · 1 comment
Labels
bug:upstream Also exists in upstream Mutt topic:security Security issue type:bug Bug type:discuss Your views/opinions are requested

Comments

@alejandro-colomar
Copy link
Member

alejandro-colomar commented Apr 8, 2024 •
edited

This is not a new bug report, but a branch of #4223.

I'd like to discuss how to behave when replying to messages with security, i.e., messages that are signed and/or encrypted.

I would discard any headers that are not protected, and only reply to protected From and similar fields.

If a sender doesn't sign nor encrypt their messages, of course, neomutt(1) will continue to reply as always, because there's nothing we can do.

But when a sender signs or encrypts their messages, when replying with neomutt(1), we will stop accepting unprotected address lists. So, for a message sent with an old version of neomutt(1), or with some other vulnerable MUA, like thunderbird(1), it will have empty To and Cc, and the user will have to manually specify it. This will be inconvenient. But it also means we will not blindly use insecure recipients, avoiding accidental secret-information leaks and similar issues. Hopefully, all MUAs would take these vulnerabilities seriously, and fix their programs soon.

Does that sound reasonable?
@alejandro-colomar alejandro-colomar added type:discuss Your views/opinions are requested bug:upstream Also exists in upstream Mutt topic:security Security issue type:bug Bug labels Apr 8, 2024
@alejandro-colomar alejandro-colomar linked a pull request Apr 8, 2024 that will close this issue
Protect more header fields #4227
Merged
@alejandro-colomar alejandro-colomar removed a link to a pull request Apr 9, 2024
Protect more header fields #4227
Merged
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue Apr 11, 2024
@alejandro-colomar
ncrypt/crypt.c: Don't weed protected headers
cb6ab8a 
They are part of the crypto message, which means the sender considers
them part of the important data, and should not be carelessly weeded.

Link: <neomutt#4223>
Link: <neomutt#4226>
Link: <neomutt#4227>
Link: <neomutt#4236>
Link: <neomutt#4237>
Cc: Richard Russon <rich@flatcap.org>
Cc: наб <nabijaczleweli@nabijaczleweli.xyz>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
Copy link
Member Author

alejandro-colomar commented Apr 12, 2024 •
edited

As mentioned in #4221 (comment), we could add some warnings if the headers have been tampered (they don't match the protected ones).

I propose:

[-- The header field '%s' has been tampered --]

(Please review syntax; not native English here. I'm especially bad with prepositions.)

flatcap pushed a commit to alejandro-colomar/neomutt that referenced this issue Apr 12, 2024
@alejandro-colomar @flatcap
ncrypt/crypt.c: Don't weed protected headers
75ca2e1 
They are part of the crypto message, which means the sender considers
them part of the important data, and should not be carelessly weeded.

Link: <neomutt#4223>
Link: <neomutt#4226>
Link: <neomutt#4227>
Link: <neomutt#4236>
Link: <neomutt#4237>
Cc: Richard Russon <rich@flatcap.org>
Cc: наб <nabijaczleweli@nabijaczleweli.xyz>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue Apr 12, 2024
@alejandro-colomar
$crypt_protected_headers_weed: Add variable
78e1bcf 
Protected header fields are part of the crypto message, which means the
sender considers them part of the important data, and should not be
carelessly weeded.

If the user want to do it, allow them via this variable, but default to
not weeding them.

Link: <neomutt#4223>
Link: <neomutt#4226>
Link: <neomutt#4227>
Link: <neomutt#4236>
Link: <neomutt#4237>
Cc: Richard Russon <rich@flatcap.org>
Cc: наб <nabijaczleweli@nabijaczleweli.xyz>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
flatcap pushed a commit to alejandro-colomar/neomutt that referenced this issue Apr 16, 2024
@alejandro-colomar @flatcap
$crypt_protected_headers_weed: Add variable
a5a4540 
Protected header fields are part of the crypto message, which means the
sender considers them part of the important data, and should not be
carelessly weeded.

If the user want to do it, allow them via this variable, but default to
not weeding them.

Link: <neomutt#4223>
Link: <neomutt#4226>
Link: <neomutt#4227>
Link: <neomutt#4236>
Link: <neomutt#4237>
Cc: Richard Russon <rich@flatcap.org>
Cc: наб <nabijaczleweli@nabijaczleweli.xyz>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue Apr 17, 2024
@alejandro-colomar
$crypt_protected_headers_weed: Add variable
ed1fc00 
Protected header fields are part of the crypto message, which means the
sender considers them part of the important data, and should not be
carelessly weeded.

If the user want to do it, allow them via this variable, but default to
not weeding them.

Link: <neomutt#4223>
Link: <neomutt#4226>
Link: <neomutt#4227>
Link: <neomutt#4236>
Link: <neomutt#4237>
Cc: Richard Russon <rich@flatcap.org>
Cc: наб <nabijaczleweli@nabijaczleweli.xyz>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue Apr 17, 2024
@alejandro-colomar
$crypt_protected_headers_weed: Add variable
055d442 
Protected header fields are part of the crypto message, which means the
sender considers them part of the important data, and should not be
carelessly weeded.

If the user want to do it, allow them via this variable, but default to
not weeding them.

Link: <neomutt#4223>
Link: <neomutt#4226>
Link: <neomutt#4227>
Link: <neomutt#4236>
Link: <neomutt#4237>
Cc: Richard Russon <rich@flatcap.org>
Cc: наб <nabijaczleweli@nabijaczleweli.xyz>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue Apr 22, 2024
@alejandro-colomar
$crypt_protected_headers_weed: Add variable
66625aa 
Protected header fields are part of the crypto message, which means the
sender considers them part of the important data, and should not be
carelessly weeded.

If the user want to do it, allow them via this variable, but default to
not weeding them.

Link: <neomutt#4223>
Link: <neomutt#4226>
Link: <neomutt#4227>
Link: <neomutt#4236>
Link: <neomutt#4237>
Cc: Richard Russon <rich@flatcap.org>
Reviewed-by: наб <nabijaczleweli@nabijaczleweli.xyz>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue Apr 22, 2024
@alejandro-colomar
$crypt_protected_headers_weed: Add variable
1e23dbe 
Protected header fields are part of the crypto message, which means the
sender considers them part of the important data, and should not be
carelessly weeded.

If the user want to do it, allow them via this variable, but default to
not weeding them.

Link: <neomutt#4223>
Link: <neomutt#4226>
Link: <neomutt#4227>
Link: <neomutt#4236>
Link: <neomutt#4237>
Cc: Richard Russon <rich@flatcap.org>
Reviewed-by: наб <nabijaczleweli@nabijaczleweli.xyz>
Cc: Pietro Cerutti <gahr@gahr.ch>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
flatcap pushed a commit to alejandro-colomar/neomutt that referenced this issue Apr 29, 2024
@alejandro-colomar @flatcap
$crypt_protected_headers_weed: Add variable
a59ee02 
Protected header fields are part of the crypto message, which means the
sender considers them part of the important data, and should not be
carelessly weeded.

If the user want to do it, allow them via this variable, but default to
not weeding them.

Link: <neomutt#4223>
Link: <neomutt#4226>
Link: <neomutt#4227>
Link: <neomutt#4236>
Link: <neomutt#4237>
Cc: Richard Russon <rich@flatcap.org>
Reviewed-by: наб <nabijaczleweli@nabijaczleweli.xyz>
Cc: Pietro Cerutti <gahr@gahr.ch>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue May 2, 2024
@alejandro-colomar
$crypt_protected_headers_weed: Add variable
41e0f12 
Protected header fields are part of the crypto message, which means the
sender considers them part of the important data, and should not be
carelessly weeded.

If the user want to do it, allow them via this variable, but default to
not weeding them.

Link: <neomutt#4223>
Link: <neomutt#4226>
Link: <neomutt#4227>
Link: <neomutt#4236>
Link: <neomutt#4237>
Cc: Richard Russon <rich@flatcap.org>
Reviewed-by: наб <nabijaczleweli@nabijaczleweli.xyz>
Cc: Pietro Cerutti <gahr@gahr.ch>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
flatcap pushed a commit to alejandro-colomar/neomutt that referenced this issue May 8, 2024
@alejandro-colomar @flatcap
$crypt_protected_headers_weed: Add variable
818381a 
Protected header fields are part of the crypto message, which means the
sender considers them part of the important data, and should not be
carelessly weeded.

If the user want to do it, allow them via this variable, but default to
not weeding them.

Link: <neomutt#4223>
Link: <neomutt#4226>
Link: <neomutt#4227>
Link: <neomutt#4236>
Link: <neomutt#4237>
Cc: Richard Russon <rich@flatcap.org>
Reviewed-by: наб <nabijaczleweli@nabijaczleweli.xyz>
Cc: Pietro Cerutti <gahr@gahr.ch>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
flatcap pushed a commit to alejandro-colomar/neomutt that referenced this issue May 8, 2024
@alejandro-colomar @flatcap
$crypt_protected_headers_weed: Add variable
0a97c12 
Protected header fields are part of the crypto message, which means the
sender considers them part of the important data, and should not be
carelessly weeded.

If the user want to do it, allow them via this variable, but default to
not weeding them.

Link: <neomutt#4223>
Link: <neomutt#4226>
Link: <neomutt#4227>
Link: <neomutt#4236>
Link: <neomutt#4237>
Cc: Richard Russon <rich@flatcap.org>
Reviewed-by: наб <nabijaczleweli@nabijaczleweli.xyz>
Cc: Pietro Cerutti <gahr@gahr.ch>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
flatcap pushed a commit that referenced this issue May 8, 2024
@alejandro-colomar @flatcap
$crypt_protected_headers_weed: Add variable
2251dc9 
Protected header fields are part of the crypto message, which means the
sender considers them part of the important data, and should not be
carelessly weeded.

If the user want to do it, allow them via this variable, but default to
not weeding them.

Link: <#4223>
Link: <#4226>
Link: <#4227>
Link: <#4236>
Link: <#4237>
Cc: Richard Russon <rich@flatcap.org>
Reviewed-by: наб <nabijaczleweli@nabijaczleweli.xyz>
Cc: Pietro Cerutti <gahr@gahr.ch>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug:upstream Also exists in upstream Mutt topic:security Security issue type:bug Bug type:discuss Your views/opinions are requested
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@alejandro-colomar