-
Notifications
You must be signed in to change notification settings - Fork 380
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
MSC2134: Identity Hash Lookups #2134
Changes from 4 commits
3eff76b
a8c26d2
8b92df7
12431f1
f8dbf2b
d2b47a5
063b9f6
bc9b6c3
5049e55
6bb4a9e
f41ed02
3ee27d3
f28476f
1343e19
1fea604
e3b2ad3
c63edc7
2383a55
53f025e
21e93a1
e3ff802
acdb2b1
02ac0f3
ee10576
36a35a3
0a4c83d
fae6883
f951f31
96e43aa
df88b13
dfb37fc
0fd4fe2
7549c5d
6f81d37
922a20b
53bd384
f4a1e02
3702669
dd8a654
1963a24
ed67e26
3514437
36cb8ed
0444c80
887cd5e
577021f
b26a9ed
9fd6bd3
3031df7
3b8c57e
8f3e588
c6dd595
da876bb
0ac70b2
20c72a3
6119b9a
87a54e8
ffbfde8
5580a2a
a17c74f
027c2d7
6660768
4d1f2ea
c8527b7
57de107
9913f5b
33d22c3
3789d82
acf8d34
c401a4d
3877724
96e06b6
3edf5e3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
# MSC2134: Identity Hash Lookups | ||
|
||
[Issue #2130](https://github.com/matrix-org/matrix-doc/issues/2130) has been recently | ||
created in response to a security issue brought up by an independant party. To summarise | ||
the issue, lookups (of matrix user ids) are performed using non-hashed 3pids which means | ||
that the 3pid is identifiable to anyone who can see the payload (e.g. [email protected] | ||
can be identified). | ||
|
||
The problem with this, is that a malicious identity service could then store the plaintext | ||
3pid and make an assumption that the requesting entity knows the holder of the 3pid, even | ||
if the identity service does not know of the 3pid beforehand. | ||
|
||
If the 3pid is hashed, the identity service could not determine the owner of the 3pid | ||
unless the identity service has already been made aware of the 3pid by the owner | ||
themselves (using the /bind mechanism). | ||
|
||
Note that this proposal does not stop a identity service from mapping hashed 3pids to many | ||
users, in an attempt to form a social graph. However the identity of the 3pid will remain | ||
a mystery until /bind is used. | ||
|
||
It should be clear that there is a need to hide any address from the identity service that | ||
has not been explicitly bound to it, and this proposal aims to solve that for the lookup API. | ||
|
||
## Proposal | ||
|
||
This proposal suggests making changes to the Identity Service API's lookup endpoints. Due | ||
to the nature of this proposal, the new endpoints should be on a `v2` path: | ||
|
||
- `/_matrix/identity/api/v2/lookup` | ||
turt2live marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- `/_matrix/identity/api/v2/bulk_lookup` | ||
|
||
The parameters will remain the same, but `address` should no longer be in a plain-text | ||
format. `address` will now take a SHA-256 format hash value, and the resulting digest should | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not a truncated hash for extra security + smaller network load? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would do this, but I'm not sure what's safe to truncate by... |
||
be encoded in base64 format. For example: | ||
|
||
```python | ||
address = "[email protected]" | ||
digest = hashlib.sha256(address.encode()).digest() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should prolly be {H(email + salt), salt} to make rainbow table attacks slightly harder There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (although salt would have to be constant for everyone, so it doesn’t buy much) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would this require us to do a negotiation step where the server sends it's unique salt, and then hash it using that salt? Bit more involved but would ensure uniqueness? (Alternatively, use the server's server_name as the salt?) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the question is how bound 3pids are stored. if they are stored as hashes, then we have no choice but have a predictable salt in the hash, which doesn’t achieve much other than stop arbitrary pregenerated hash tables of mail addresses from working. instead, you’d have to pregenerate H(email + “foo”) tables specifically for matrix, where foo is defined in this MSC.. We can’t use server_name as when you’re doing a lookup you don’t know any server_names for the target. if we store the 3pids as plain text equivalent in the IS, then the salt could be random per lookup, as the server can calculate its own hashes to compare against the uploaded ones. however, this would be pretty inefficient, plus we probably don’t want to be storing unhashed 3pids. it would mean an attacker would have to calculate hash tables for every salt (ie per lookup item) to crack the hash. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Specifying a salt in the spec does at least drive away lazy attackers who want to quickly use their existing tables. It's a bit of a weak argument, but if we can force people to do even a little bit of Matrix-specific logic to exploit a vulnerability then we are in a slightly safer position. We can also specify a SHOULD for servers to hash and salt the hashes of 3PIDs when persisting them, making rainbow tables much harder to use at the persistence level. It does a look a bit dirty ( |
||
result_address = base64.encodebytes(digest).decode() | ||
print(result_address) | ||
CpvOgBf0hFzdqZD4ASvWW0DAefErRRX5y8IegMBO98w= | ||
``` | ||
|
||
### Example request | ||
|
||
anoadragon453 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
SHA-256 has been chosen as it is [currently used elsewhere](https://matrix.org/docs/spec/server_server/r0.1.2#adding-hashes-and-signatures-to-outgoing-events) in the Matrix protocol, and the only | ||
requirement for the hashing algorithm is that it cannot be used to guess the real value of the address | ||
anoadragon453 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
No parameter changes will be made to /bind, but identity services should keep a hashed value | ||
for each address it knows about in order to process lookups quicker and it is the recommendation | ||
that this is done at the time of bind. | ||
|
||
`v1` versions of these endpoints may be disabled at the discretion of the implementation, and | ||
should return a `M_FORBIDDEN` `errcode` if so. | ||
|
||
|
||
anoadragon453 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
## Tradeoffs | ||
anoadragon453 marked this conversation as resolved.
Show resolved
Hide resolved
anoadragon453 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
* This approach means that the client now needs to calculate a hash by itself, but the belief | ||
is that most languages provide a mechanism for doing so. | ||
* There is a small cost incurred by doing hashes before requests, but this is outweighed by | ||
the privacy implications of sending plaintext addresses. | ||
|
||
|
||
## Potential issues | ||
anoadragon453 marked this conversation as resolved.
Show resolved
Hide resolved
anoadragon453 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
This proposal does not force a identity service to stop handling plaintext requests, because | ||
a large amount of the matrix ecosystem relies upon this behavior. However, a conscious effort | ||
should be made by all users to use the privacy respecting endpoints outlined above. Identity | ||
services may disallow use of the v1 endpoint. | ||
anoadragon453 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Base64 has been chosen to encode the value due to it's ubiquitous support in many languages, | ||
anoadragon453 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
however it does mean that special characters in the address will have to be encoded when used | ||
as a parameter value. | ||
|
||
|
||
## Security considerations | ||
|
||
None | ||
anoadragon453 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
## Conclusion | ||
|
||
This proposal outlines a quick and effective method to stop bulk collection of users contact | ||
Half-Shot marked this conversation as resolved.
Show resolved
Hide resolved
|
||
lists and their social graphs without any disasterous side effects. All functionality which | ||
depends on the lookup service should continue to function unhindered by the use of hashes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is almost there - thank you for the epic. The current draft reads really well.
My main remaining concerns are that we need to spell out the attacks and tradeoffs and conclusion rationale between the solutions more clearly. I’ve tried to do this in note form at https://gist.github.com/ara4n/8d5fe3030d9fad00111f9ec343e86feb - would it be possible to try to incorporate this?
Meanwhile, I agree that a rotating pepper hash lookups is the best approach here (having reasoned it through).
Otherwise, my only other remaining concern is that we should be protecting the IS db better by storing 3pids in hashed form (and thus also 3pid invites and other bindings). ie wherever we currently pass around 3pids instead we pass around a hash salted with a static salt for that IS. i don’t think we even need the raw 3pid for validation purposes, as we can validate using a nonce instead? I’d much rather we spent the time to figure out protecting the db rather than figuring out k-anon further. This could be a separate MSC though, but it feels like we should have thought it through enough to ensure that this MSC doesn’t design it out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On it, thanks!
This is something we could append on to
/hash_details
,or even use theWe don't want to reuselookup_pepper
from it for this purpose? Perhaps renaming it to something more generic in the process?lookup_pepper
of course. The salt shouldn't rotate, while the pepper should.Looking at the IS API docs, the following would need to be changed to enable storing hashed IDs at rest.
Endpoints that would already work are:
There's still the GDPR concern that if we do get compromised, we're obligated to notify everyone that hashes were taken. Either we use matrix as the communication medium (does the law disallow this?) or we send a message to Homeservers who do have the plaintext 3PIDs that they should send an email (this could be horribly abused by an evil IS though).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right - thanks for doing the storing hashed ID analysis, this is excellent. i suggest we copy-paste this verbatim as a starting point for a new MSC so as to not block this one further.
I've asked @lampholder whether we can do data breach notifications via Matrix or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(i've given this the FCP ✅on the assumption that the spelling-out-the-attack and the more concrete tradeoff comparison makes it into the MSC)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one other gotcha sprang to mind which we should note: having written out a basic threat model, it becomes clear that a malicious IS could just fail to rotate the pepper (or reuse the same pepper). So the rotating pepper really buys us very little indeed unless clients check for pepper reuse, which seems onerous and also useless given they can’t tell about pepper reuse from before they connected.
So while we might as well keep the ability of the server to specify the pepper it uses for the hashes, in think there is limited use in bothering to rotate it.