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

Security considerations editorial #229

Merged
merged 12 commits into from
Jan 19, 2024

Conversation

ekr
Copy link
Collaborator

@ekr ekr commented Nov 8, 2023

This is intended to tighten up the security considerations section a bit. It's mostly editorial but somewhat substantial changes.

@ekr ekr requested a review from bifurcation November 8, 2023 04:25
Copy link
Member

@beurdouche beurdouche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modulo some small changes this is fine.

Comment on lines 1409 to 1422
MLS provides strong authentication within a group, such that a group member
cannot send a message that appears to be from another group member.
Additionally, some services require that a recipient be able to prove to the
service provider that a message was sent by a given client, in order to report
abuse. MLS supports both of these use cases. In some deployments, these services
are provided by mechanisms which allow the receiver to prove a message's origin
to a third party. This is often called "non-repudiation".

Roughly speaking, "deniability" is the opposite of "non-repudiation", i.e., the
property that it is impossible to prove to a third party that a message was sent
by a given sender. MLS does not make any claims with regard to deniability. It
may be possible to operate MLS in ways that provide certain deniability
properties, but defining the specific requirements and resulting notions of
deniability requires further analysis.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block has been deleted without being dispatched and the title of the next section disappeared as well which seems to be a mistake.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. This is just pilot error. Thanks for catching it.

public-key encrypted to subgroups with HPKE.
generated from Group Secrets. Specifically, each epoch establishes
a per-sender "Ratchet Secret", which is then used to generate an
AEAD key, which is used to protect MLS Plaintext messages.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
AEAD key, which is used to protect MLS Plaintext messages.
AEAD key to protect application messages.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that this is gramatically correct. You don't generate the key to protect the messages.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please discard : )

Comment on lines 1460 to 1461
Each time a message is sent, the Ratchet Secret is used
to create a new Ratchet Secret and a new corresponding AEAD key.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Each time a message is sent, the Ratchet Secret is used
to create a new Ratchet Secret and a new corresponding AEAD key.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementations can work this way but don't have to. Some implementations derive these in batch.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I think we need to say something in order to explain the security properties. How about

"A new Ratchet Secret is used to generate the AEAD keys for each new message"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That works, yes !

Comment on lines 1639 to 1625
> MLS, the application should delete plaintext messages and ciphertexts
> as soon as practical after encryption or decryption.

Even though, from the strict point of view of the security formalization, a
ciphertext is always public and will forever be, there is no loss in trying to
erase ciphertexts as much as possible.
> MLS, the application should delete plaintext messages.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure I agree with removing this. If I was the adversary I would try to collect the ciphertexts and attack some device that has been excluded from the group for not updating enough.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I can live with keeping this.

draft-ietf-mls-architecture.md Outdated Show resolved Hide resolved
draft-ietf-mls-architecture.md Outdated Show resolved Hide resolved
Copy link
Contributor

@rohan-wire rohan-wire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a few comments inline that I'd like addressed, especially towards the end of the section. The earlier part of this PR does greatly improve the readability of the section.

draft-ietf-mls-architecture.md Outdated Show resolved Hide resolved
draft-ietf-mls-architecture.md Outdated Show resolved Hide resolved
@@ -1700,7 +1701,7 @@ the IP address and depending on the protocol the MAC address of the device.
Similar concerns exist in the peer-to-peer use cases of MLS.

> **RECOMMENDATION:** In the case where privacy or anonymity is important, using
> adequate protection such as TOR or a VPN can improve metadata protection.
> adequate protection such as MASQUE, ToR, or a VPN can improve metadata protection.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add reference for MASQUE

draft-ietf-mls-architecture.md Outdated Show resolved Hide resolved
Comment on lines 1840 to 1848
If the AS is compromised, it could validate a (or generate a new)
signature keypair for an attacker. Because a user can have many MLS
clients running the MLS protocol, it possibly has many signature
keypairs for multiple devices. These attacks could be very difficult
to detect, especially in large groups where the UI might not reflect
all the changes back to the users. If the application participates in
a key transparency mechanism in which it is possible to determine
every key for a given user, then this then this would allow for the
detection of surreptitiously created false credentials.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please restore or rewrite the text about injecting a malicious client. I realize that earlier text alludes to that attack, but this is a very practical attack which has been exploited in the wild and we should explicitly call it out.

Comment on lines +1852 to 1853
> number of notifications is too high, the client should provide a log of
> state of the device so that the user can examine it.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the notifications are about new clients, which "the client" may not have access to. The "public log" might be a ledger generated by the AS for example.

Copy link
Collaborator Author

@ekr ekr Nov 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm..... ISTM that there are two potential things this might be talking about:

  1. Changes to the set of clients in a group.
  2. Changes to the set of clients for a user, whether in a group or not.

I assumed that this text was about the former because of the text "This is especially the case in large groups where the UI might not reflect all the changes back to the users." which is clearly about groups, not the set of clients for a user.

Note that the next recommendation talks about transparency.

Comment on lines -1869 to -1872
been published in the transparency log. Another benefit of this mechanism is for
revocation. The users of a group could check for revoked keys (in case of
compromise detection) using a mechanism such as CRLite or some more advanced
privacy preserving technology.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add back some mention of revocation. Maybe:

"When the credential type support revocation, the users of a group could check for revoked keys, for example for x509 credentials, using a mechanism such as CRLite."

(I don't particularly care if we mention CRLite or some other mechanism, but I think we should say something about revocation.)

@beurdouche beurdouche merged commit e99caf0 into mlswg:main Jan 19, 2024
1 check passed
@beurdouche
Copy link
Member

Approved. Thanks Ekr and Rohan !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants