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

Should the In-Reply-To header field be protected? #4226

Open
alejandro-colomar opened this issue Mar 31, 2024 · 0 comments · Fixed by #4227
Open

Should the In-Reply-To header field be protected? #4226

alejandro-colomar opened this issue Mar 31, 2024 · 0 comments · Fixed by #4227
Labels
bug:upstream Also exists in upstream Mutt topic:security Security issue type:bug Bug

Comments

@alejandro-colomar
Copy link
Member

alejandro-colomar commented Mar 31, 2024

Alice sends a signed (non-encrypted) message to Bob, during a public discussion.

The message is sufficiently ambiguous that put out of context could be used for an attack.

Mallory takes that message, and sends it in a different discussion that has the same subject (or in a different subthread of the same main thread), modifying the In-Reply-To header (which isn't currently protected by the signature), leading Bob to believe that Alice sent that message in that specific context.

Since the message has a valid signature, Bob considers it valid. :(

Related: #4223

I think we should protect the In-Reply-To header.

Since there's no need for this header field to correctly deliver the message to its recipients, it should also be allowed to put a dummy ... value, as with the Subject, which would hide the relationship between messages in an encrypted conversation, except for its recipients.

For initial messages, that is messages with no In-Reply-To fields, neomutt(1) should probably include an empty In-Reply-To: header field in the protected part, to avoid Mallory adding one outside of it.

alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue Mar 31, 2024
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>
Signed-off-by: Alejandro Colomar <[email protected]>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue Mar 31, 2024
There are several attacks that are possible if the To or Cc fields are
not protected.  One of them is:

-  The metadata of a signed+encrypted message can be modified (without
   decrypting it), to add a mallicious recipient.  Subsequent replies to
   the thread will likely also encrypt to the mallicious recipient,
   disclosing secret data.

The "From" isn't strictly necessary, as the signature will specify an
address, but it doesn't hurt either, and since the public key may have
several IDs, this makes sure that the message contains the one that the
sender wants to use.

Read the links below for a more detailed explanation of the problem.

To be paranoic, we might want to also protect other address lists,
like "Reply-To".  That would need more careful analysis, and also
implies more noise in the pager, so I'm not convinced.

Link: <neomutt#4223>
Link: <neomutt#4226>
Signed-off-by: Alejandro Colomar <[email protected]>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue Mar 31, 2024
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>
Signed-off-by: Alejandro Colomar <[email protected]>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue Mar 31, 2024
There are several attacks that are possible if the To or Cc fields are
not protected.  One of them is:

-  The metadata of a signed+encrypted message can be modified (without
   decrypting it), to add a mallicious recipient.  Subsequent replies to
   the thread will likely also encrypt to the mallicious recipient,
   disclosing secret data.

The "From" isn't strictly necessary, as the signature will specify an
address, but it doesn't hurt either, and since the public key may have
several IDs, this makes sure that the message contains the one that the
sender wants to use.

Read the links below for a more detailed explanation of the problem.

To be paranoic, we might want to also protect other address lists,
like "Reply-To".  That would need more careful analysis, and also
implies more noise in the pager, so I'm not convinced.

Link: <neomutt#4223>
Link: <neomutt#4226>
Signed-off-by: Alejandro Colomar <[email protected]>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue Mar 31, 2024
There are several attacks that are possible if the To or Cc fields are
not protected.  One of them is:

-  The metadata of a signed+encrypted message can be modified (without
   decrypting it), to add a malicious recipient.  Subsequent replies to
   the thread will likely also encrypt to the mallicious recipient,
   disclosing secret data.

The "From" isn't strictly necessary, as the signature will specify an
address, but it doesn't hurt either, and since the public key may have
several IDs, this makes sure that the message contains the one that the
sender wants to use.

Read the links below for a more detailed explanation of the problem.

To be paranoic, we might want to also protect other address lists,
like "Reply-To".  That would need more careful analysis, and also
implies more noise in the pager, so I'm not convinced.

Link: <neomutt#4223>
Link: <neomutt#4226>
Signed-off-by: Alejandro Colomar <[email protected]>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue Mar 31, 2024
There are several attacks that are possible if the To or Cc fields are
not protected.  One of them is:

-  The metadata of a signed+encrypted message can be modified (without
   decrypting it), to add a malicious recipient.  Subsequent replies to
   the thread will likely also encrypt to the malicious recipient,
   disclosing secret data.

The "From" isn't strictly necessary, as the signature will specify an
address, but it doesn't hurt either, and since the public key may have
several IDs, this makes sure that the message contains the one that the
sender wants to use.

Read the links below for a more detailed explanation of the problem.

To be paranoic, we might want to also protect other address lists,
like "Reply-To".  That would need more careful analysis, and also
implies more noise in the pager, so I'm not convinced.

Link: <neomutt#4223>
Link: <neomutt#4226>
Signed-off-by: Alejandro Colomar <[email protected]>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue Apr 1, 2024
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>
Signed-off-by: Alejandro Colomar <[email protected]>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue Apr 1, 2024
There are several attacks that are possible if the To or Cc fields are
not protected.  One of them is:

-  The metadata of a signed+encrypted message can be modified (without
   decrypting it), to add a malicious recipient.  Subsequent replies to
   the thread will likely also encrypt to the malicious recipient,
   disclosing secret data.

The "From" isn't strictly necessary, as the signature will specify an
address, but it doesn't hurt either, and since the public key may have
several IDs, this makes sure that the message contains the one that the
sender wants to use.

Read the links below for a more detailed explanation of the problem.

To be paranoic, we might want to also protect other address lists,
like "Reply-To".  That would need more careful analysis, and also
implies more noise in the pager, so I'm not convinced.

Link: <neomutt#4223>
Link: <neomutt#4226>
Signed-off-by: Alejandro Colomar <[email protected]>
@alejandro-colomar alejandro-colomar changed the title Should the In-Reply-To header field be protected? [crypto] Should the In-Reply-To header field be protected? Apr 6, 2024
@alejandro-colomar alejandro-colomar changed the title [crypto] Should the In-Reply-To header field be protected? Should the In-Reply-To header field be protected? Apr 6, 2024
@alejandro-colomar alejandro-colomar added the topic:security Security issue label Apr 6, 2024
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue Apr 7, 2024
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>
Signed-off-by: Alejandro Colomar <[email protected]>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue Apr 7, 2024
There are several attacks that are possible if the To or Cc fields are
not protected.  One of them is:

-  The metadata of a signed+encrypted message can be modified (without
   decrypting it), to add a malicious recipient.  Subsequent replies to
   the thread will likely also encrypt to the malicious recipient,
   disclosing secret data.

The "From" isn't strictly necessary, as the signature will specify an
address, but it doesn't hurt either, and since the public key may have
several IDs, this makes sure that the message contains the one that the
sender wants to use.

Read the links below for a more detailed explanation of the problem.

To be paranoic, we might want to also protect other address lists,
like "Reply-To".  That would need more careful analysis, and also
implies more noise in the pager, so I'm not convinced.

Link: <neomutt#4223>
Link: <neomutt#4226>
Signed-off-by: Alejandro Colomar <[email protected]>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue Apr 8, 2024
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>
Cc: Richard Russon <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue Apr 8, 2024
... except Bcc, for obvious reasons, and Return-Path, since it shouldn't
be problematic for what we're fixing (I think).

There are several attacks that are possible if the To, Cc, and similar
header fields are not protected.  One of them is:

-  The metadata of a signed+encrypted message can be modified (without
   decrypting it), to add a malicious recipient.  Subsequent replies to
   the thread will likely also encrypt to the malicious recipient,
   disclosing secret data.

Read the links below for a more detailed explanation of the problem.

Link: <neomutt#4223>
Link: <neomutt#4226>
Cc: Richard Russon <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue Apr 8, 2024
This function copies a src ListHead at the tail of a dst ListHead.

Link: <neomutt#4223>
Link: <neomutt#4226>
Cc: Richard Russon <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue Apr 8, 2024
Link: <neomutt#4223>
Link: <neomutt#4226>
Cc: Richard Russon <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue Apr 8, 2024
Move some code out of a conditional, to add support for other protected
headers in the following commits.

Link: <neomutt#4223>
Link: <neomutt#4226>
Cc: Richard Russon <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue Apr 8, 2024
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue Apr 8, 2024
Link: <neomutt#4223>
Link: <neomutt#4226>
Cc: Richard Russon <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue Apr 8, 2024
@alejandro-colomar alejandro-colomar linked a pull request Apr 8, 2024 that will close this issue
@alejandro-colomar alejandro-colomar added the bug:upstream Also exists in upstream Mutt label Apr 8, 2024
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue Apr 8, 2024
Link: <neomutt#4236>
Link: <neomutt#4223>
Link: <neomutt#4226>
Cc: Richard Russon <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue Apr 8, 2024
Closes: <neomutt#4236>
Link: <neomutt#4223>
Link: <neomutt#4226>
Cc: Richard Russon <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue Apr 8, 2024
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue Apr 8, 2024
Link: <neomutt#4223>
Link: <neomutt#4226>
Cc: Richard Russon <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue May 1, 2024
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue May 1, 2024
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue May 2, 2024
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 <[email protected]>
Reviewed-by: наб <[email protected]>
Cc: Pietro Cerutti <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue May 2, 2024
Link: <neomutt#4236>
Link: <neomutt#4223>
Link: <neomutt#4226>
Reviewed-by: наб <[email protected]>
Cc: Richard Russon <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue May 2, 2024
... except Bcc, for obvious reasons.

There are several attacks that are possible if the To, Cc, and similar
header fields are not protected.  One of them is:

-  The metadata of a signed+encrypted message can be modified (without
   decrypting it), to add a malicious recipient.  Subsequent replies to
   the thread will likely also encrypt to the malicious recipient,
   disclosing secret data.

Read the links below for a more detailed explanation of the problem.

Link: <neomutt#4223>
Link: <neomutt#4226>
Cc: Richard Russon <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue May 2, 2024
This function copies a src ListHead at the tail of a dst ListHead.

This is similar to mutt_addrlist_copy(), but for ListHead.

Link: <neomutt#4223>
Link: <neomutt#4226>
Cc: Richard Russon <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue May 2, 2024
Link: <neomutt#4223>
Link: <neomutt#4226>
Cc: Richard Russon <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue May 2, 2024
Move some code out of a conditional, to add support for other protected
headers in the following commits.

Link: <neomutt#4223>
Link: <neomutt#4226>
Cc: Richard Russon <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue May 2, 2024
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue May 2, 2024
This function writes a list to a buffer.

It is similar to mutt_addrlist_write(), but simpler, and for ListHead.

Link: <neomutt#4223>
Link: <neomutt#4226>
Cc: Richard Russon <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
alejandro-colomar added a commit to alejandro-colomar/neomutt that referenced this issue May 2, 2024
flatcap pushed a commit to alejandro-colomar/neomutt that referenced this issue May 8, 2024
... except Bcc, for obvious reasons.

There are several attacks that are possible if the To, Cc, and similar
header fields are not protected.  One of them is:

-  The metadata of a signed+encrypted message can be modified (without
   decrypting it), to add a malicious recipient.  Subsequent replies to
   the thread will likely also encrypt to the malicious recipient,
   disclosing secret data.

Read the links below for a more detailed explanation of the problem.

Link: <neomutt#4223>
Link: <neomutt#4226>
Cc: Richard Russon <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
flatcap pushed a commit to alejandro-colomar/neomutt that referenced this issue May 8, 2024
This function copies a src ListHead at the tail of a dst ListHead.

This is similar to mutt_addrlist_copy(), but for ListHead.

Link: <neomutt#4223>
Link: <neomutt#4226>
Cc: Richard Russon <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
flatcap pushed a commit to alejandro-colomar/neomutt that referenced this issue May 8, 2024
Link: <neomutt#4223>
Link: <neomutt#4226>
Cc: Richard Russon <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
flatcap pushed a commit to alejandro-colomar/neomutt that referenced this issue May 8, 2024
Move some code out of a conditional, to add support for other protected
headers in the following commits.

Link: <neomutt#4223>
Link: <neomutt#4226>
Cc: Richard Russon <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
flatcap pushed a commit to alejandro-colomar/neomutt that referenced this issue May 8, 2024
flatcap pushed a commit to alejandro-colomar/neomutt that referenced this issue May 8, 2024
This function writes a list to a buffer.

It is similar to mutt_addrlist_write(), but simpler, and for ListHead.

Link: <neomutt#4223>
Link: <neomutt#4226>
Cc: Richard Russon <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
flatcap pushed a commit to alejandro-colomar/neomutt that referenced this issue May 8, 2024
flatcap pushed a commit to alejandro-colomar/neomutt that referenced this issue May 8, 2024
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 <[email protected]>
Reviewed-by: наб <[email protected]>
Cc: Pietro Cerutti <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
flatcap pushed a commit to alejandro-colomar/neomutt that referenced this issue May 8, 2024
Link: <neomutt#4236>
Link: <neomutt#4223>
Link: <neomutt#4226>
Reviewed-by: наб <[email protected]>
Cc: Richard Russon <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
flatcap pushed a commit that referenced this issue May 8, 2024
... except Bcc, for obvious reasons.

There are several attacks that are possible if the To, Cc, and similar
header fields are not protected.  One of them is:

-  The metadata of a signed+encrypted message can be modified (without
   decrypting it), to add a malicious recipient.  Subsequent replies to
   the thread will likely also encrypt to the malicious recipient,
   disclosing secret data.

Read the links below for a more detailed explanation of the problem.

Link: <#4223>
Link: <#4226>
Cc: Richard Russon <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
flatcap pushed a commit that referenced this issue May 8, 2024
This function copies a src ListHead at the tail of a dst ListHead.

This is similar to mutt_addrlist_copy(), but for ListHead.

Link: <#4223>
Link: <#4226>
Cc: Richard Russon <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
flatcap pushed a commit that referenced this issue May 8, 2024
Link: <#4223>
Link: <#4226>
Cc: Richard Russon <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
flatcap pushed a commit that referenced this issue May 8, 2024
Move some code out of a conditional, to add support for other protected
headers in the following commits.

Link: <#4223>
Link: <#4226>
Cc: Richard Russon <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
flatcap pushed a commit that referenced this issue May 8, 2024
…ists

Link: <#4223>
Link: <#4226>
Cc: Richard Russon <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
flatcap pushed a commit that referenced this issue May 8, 2024
This function writes a list to a buffer.

It is similar to mutt_addrlist_write(), but simpler, and for ListHead.

Link: <#4223>
Link: <#4226>
Cc: Richard Russon <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
flatcap pushed a commit that referenced this issue May 8, 2024
…fields

Link: <#4223>
Link: <#4226>
Cc: Richard Russon <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
flatcap pushed a commit that referenced this issue May 8, 2024
Link: <#4236>
Link: <#4223>
Link: <#4226>
Reviewed-by: наб <[email protected]>
Cc: Richard Russon <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
flatcap pushed a commit to alejandro-colomar/neomutt that referenced this issue May 8, 2024
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 <[email protected]>
Reviewed-by: наб <[email protected]>
Cc: Pietro Cerutti <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
flatcap pushed a commit that referenced this issue May 8, 2024
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 <[email protected]>
Reviewed-by: наб <[email protected]>
Cc: Pietro Cerutti <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug:upstream Also exists in upstream Mutt topic:security Security issue type:bug Bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant