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

MSC2134: Identity Hash Lookups #2134

Merged
merged 73 commits into from
Aug 8, 2019
Merged
Changes from 12 commits
Commits
Show all changes
73 commits
Select commit Hold shift + click to select a range
3eff76b
MSC 2134
Half-Shot Jun 15, 2019
a8c26d2
Wrap
Half-Shot Jun 15, 2019
8b92df7
s/medium/address
Half-Shot Jun 15, 2019
12431f1
Base64 potential issue
Half-Shot Jun 15, 2019
f8dbf2b
Update proposals/2134-identity-hash-lookup.md
Half-Shot Jun 17, 2019
d2b47a5
Allow for changing the hashing algo and add at-rest details
anoadragon453 Jun 18, 2019
063b9f6
Require a salt to defend against rainbow tables
anoadragon453 Jun 18, 2019
bc9b6c3
Add salt to example and signal link
anoadragon453 Jun 18, 2019
5049e55
Drop /api from the new endpoint
anoadragon453 Jun 18, 2019
6bb4a9e
Add per-is salt consideration
anoadragon453 Jun 18, 2019
f41ed02
remove sec concerns
anoadragon453 Jun 18, 2019
3ee27d3
salt->pepper. 1 pepper/is. add multi-hash idea
anoadragon453 Jun 19, 2019
f28476f
line wrap and fix wording
anoadragon453 Jun 19, 2019
1343e19
Specify hash algorithm and fallback considerations
anoadragon453 Jun 20, 2019
1fea604
Don't define error message
anoadragon453 Jun 21, 2019
e3b2ad3
pepper -> lookup_pepper
anoadragon453 Jun 21, 2019
c63edc7
Clean up wording around peppers and hashes
anoadragon453 Jun 21, 2019
2383a55
404 for deprecated endpoint
anoadragon453 Jun 21, 2019
53f025e
Specify optional pepper rotation period
anoadragon453 Jun 21, 2019
21e93a1
Naming and capitalization
turt2live Jun 21, 2019
e3ff802
http err codes and hash wording fixes
anoadragon453 Jun 24, 2019
acdb2b1
Merge branch 'hs/hash-identity' of github.com:matrix-org/matrix-doc i…
anoadragon453 Jun 24, 2019
02ac0f3
Give the user control!
anoadragon453 Jun 24, 2019
ee10576
Update with feedback
anoadragon453 Jun 24, 2019
36a35a3
Clarify how the spec defines hashing algs
anoadragon453 Jun 24, 2019
0a4c83d
no plural. 3pid -> 3PID
anoadragon453 Jun 24, 2019
fae6883
Update with review comments
anoadragon453 Jun 25, 2019
f951f31
Fix terrible wording
anoadragon453 Jun 25, 2019
96e43aa
Define what characters lookup_pepper can consist of
anoadragon453 Jun 25, 2019
df88b13
Update proposals/2134-identity-hash-lookup.md
anoadragon453 Jun 25, 2019
dfb37fc
update with feedback
anoadragon453 Jun 25, 2019
0fd4fe2
Add algo/pepper to err resp
anoadragon453 Jun 26, 2019
7549c5d
Merge branch 'hs/hash-identity' of github.com:matrix-org/matrix-doc i…
anoadragon453 Jun 26, 2019
6f81d37
New hashing method
anoadragon453 Jul 1, 2019
922a20b
small fixes
anoadragon453 Jul 1, 2019
53bd384
Clarify salting
anoadragon453 Jul 3, 2019
f4a1e02
simple method once more
anoadragon453 Jul 4, 2019
3702669
update from comments
anoadragon453 Jul 5, 2019
dd8a654
Address review comments
anoadragon453 Jul 8, 2019
1963a24
fix attacks paragraph
anoadragon453 Jul 8, 2019
ed67e26
pepper must not be an empty string, append medium
anoadragon453 Jul 8, 2019
3514437
Ability for client/server to decide on no hashing
anoadragon453 Jul 12, 2019
36cb8ed
none -> m.none
anoadragon453 Jul 16, 2019
0444c80
review comments
anoadragon453 Jul 22, 2019
887cd5e
I really hope someone doesn't invest none-hash
anoadragon453 Jul 22, 2019
577021f
resolve some comments
anoadragon453 Jul 23, 2019
b26a9ed
Expand on why we can't trust dirty homeservers
anoadragon453 Jul 23, 2019
9fd6bd3
Add details about why this proposal should exist
anoadragon453 Jul 23, 2019
3031df7
Add example for none algo
anoadragon453 Jul 23, 2019
3b8c57e
Don't require servers/clients to support "none"
anoadragon453 Jul 23, 2019
8f3e588
pepper is not a secret val. Still needs to be around.
anoadragon453 Jul 24, 2019
c6dd595
Clients can cache the hash details if they want to
anoadragon453 Jul 25, 2019
da876bb
missing word
anoadragon453 Jul 25, 2019
0ac70b2
Clarify peppering should not happen on none algo
anoadragon453 Jul 25, 2019
20c72a3
Update proposals/2134-identity-hash-lookup.md
anoadragon453 Jul 25, 2019
6119b9a
*@hobnobbob.com is unlikely to be guessed
anoadragon453 Jul 25, 2019
87a54e8
Merge branch 'hs/hash-identity' of github.com:matrix-org/matrix-doc i…
anoadragon453 Jul 25, 2019
ffbfde8
Update proposals/2134-identity-hash-lookup.md
anoadragon453 Jul 26, 2019
5580a2a
Update proposals/2134-identity-hash-lookup.md
anoadragon453 Jul 26, 2019
a17c74f
switch medium and address around, space between address and pepper
anoadragon453 Jul 26, 2019
027c2d7
Merge branch 'hs/hash-identity' of github.com:matrix-org/matrix-doc i…
anoadragon453 Jul 26, 2019
6660768
Don't repeat fast hash bit
anoadragon453 Jul 26, 2019
4d1f2ea
Apply suggestions from code review
anoadragon453 Jul 26, 2019
c8527b7
Merge branch 'hs/hash-identity' of github.com:matrix-org/matrix-doc i…
anoadragon453 Jul 26, 2019
57de107
Move medium back behind the address
anoadragon453 Jul 31, 2019
9913f5b
Slightly clarify pepper value
anoadragon453 Jul 31, 2019
33d22c3
hashes are not stream ciphers
anoadragon453 Jul 31, 2019
3789d82
Incorporate solution analysis from the context of attacks
anoadragon453 Aug 1, 2019
acf8d34
Merge branch 'hs/hash-identity' of github.com:matrix-org/matrix-doc i…
anoadragon453 Aug 1, 2019
c401a4d
punctuation
anoadragon453 Aug 1, 2019
3877724
fix speeling
anoadragon453 Aug 1, 2019
96e06b6
Add line, britishise
anoadragon453 Aug 1, 2019
3edf5e3
Make hashes real values
anoadragon453 Aug 2, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
160 changes: 160 additions & 0 deletions proposals/2134-identity-hash-lookup.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
# MSC2134: Identity Hash Lookups
Copy link
Member

@ara4n ara4n Jul 31, 2019

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.

Copy link
Member

@anoadragon453 anoadragon453 Aug 1, 2019

Choose a reason for hiding this comment

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

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?

On it, thanks!

instead we pass around a hash salted with a static salt for that IS

This is something we could append on to /hash_details, or even use the lookup_pepper from it for this purpose? Perhaps renaming it to something more generic in the process? We don't want to reuse 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).

Copy link
Member

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.

Copy link
Member

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)

Copy link
Member

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.


[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 independent
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved
party. To summarise the issue, lookups (of matrix user ids) are performed using
turt2live marked this conversation as resolved.
Show resolved Hide resolved
non-hashed 3pids (third-party IDs) which means that the identity server can
turt2live marked this conversation as resolved.
Show resolved Hide resolved
identify and record every 3pid that the user wants to check, whether that
address is already known by the identity server or not.

If the 3pid is hashed, the identity service could not determine the address
turt2live marked this conversation as resolved.
Show resolved Hide resolved
unless it has already seen that address in plain-text during a previous call of
the /bind mechanism.

Note that in terms of privacy, this proposal does not stop an identity service
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved
from mapping hashed 3pids to users, resulting in a social graph. However, the
identity of the 3pid will at least remain a mystery until /bind is used.

This proposal thus calls for the Identity Service’s /lookup API to use hashed
3pids instead of their plain-text counterparts.

## Proposal
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved

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 (we also drop the `/api` in order to preserve consistency across
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved
other endpoints):

- `/_matrix/identity/v2/lookup`
- `/_matrix/identity/v2/bulk_lookup`

`address` should no longer be in a plain-text format, but will now take a hash
value, and the resulting digest should be encoded in unpadded base64. For
example:
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved

```python
address = "user@example.org"
pepper = "matrix"
digest = hashlib.sha256((pepper + address).encode()).digest()
result_address = unpaddedbase64.encode_base64(digest)
print(result_address)
CpvOgBf0hFzdqZD4ASvWW0DAefErRRX5y8IegMBO98w
```

SHA-256 has been chosen as it is [currently used
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved
elsewhere](https://matrix.org/docs/spec/server_server/r0.1.2#adding-hashes-and-signatures-to-outgoing-events)
in the Matrix protocol. Additionally a
[pepper](https://en.wikipedia.org/wiki/Pepper_(cryptography)) must be prepended
to the data before hashing in order to serve as a weak defense against existing
rainbow tables. This pepper will be specified by the identity server in order
to prevent a single rainbow table being generated for all identity servers. As
time goes on, this algorithm may be changed provided a spec bump is performed.
Then, clients making a request to `/lookup` must use the hashing algorithm
defined in whichever version of the CS spec they and the IS have agreed to
speaking.
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved

anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved
Identity servers can specify their own peppers, which can be handy if a rainbow table is released for their current one. Identity servers could also set a timer for rotating this value to further impede rainbow table publishing. As such, it must be possible for clients to be able to query what pepper an identity server requires before sending it hashes. Thus a new endpoint must be added:
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved

```
GET /_matrix/identity/v2/lookup_pepper
```

This endpoint takes no parameters, and simply returns the current pepper as a JSON object:
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved

```
{
"pepper": "matrixrocks"
}
```

In addition, the pepper the client used must be appended as a parameter to the
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved
new `/lookup` and `/bulk_lookup` endpoints, ensuring that the client is using
the right one. If it does not match what the server has on file (which may be
the case is it rotated right after the client's request for it), then client
will know to query the pepper again instead of just getting a response saying
no contacts are registered on that identity server.

Thus, a call to `/bulk_lookup` would look like the following:

```
{
"threepids": [
[
"email",
"user@example.org"
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved
],
[
"msisdn",
"123456789"
],
[
"email",
"user2@example.org"
]
],
"pepper": "matrixrocks"
}
```

If the pepper does not match the server's, the client should receive a `400
M_INVALID_PARAM` with the error `Provided pepper value does not match
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved
'$server_pepper'`. Clients should ensure they don't enter an infinite loop if
they receive this error more than once even after changing to the correct
pepper.

No parameter changes will be made to /bind, but identity servers should keep a
hashed value for each address it knows about in order to process lookups
quicker. It is the recommendation that this is done during the act of binding.

`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


anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved
## Tradeoffs
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 plain-text addresses.

## Potential issues
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved

This proposal does not force an identity service to stop handling plain-text
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

Unpadded base64 has been chosen to encode the value due to its ubiquitous
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved
support in many languages, however it does mean that special characters in the
address will have to be encoded when used as a parameter value.
Copy link
Member

Choose a reason for hiding this comment

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

errrm.... there are no special characters to encode in base64?

Copy link
Member

@anoadragon453 anoadragon453 Jun 24, 2019

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

so surely we should use url-safe base64 then?

Copy link
Member

Choose a reason for hiding this comment

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

This is a non-issue given the hashes appear in the request body. Would advocate for taking out the special characters sentence entirely.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

oh. In that case, we should use URL safe unpadded base64 to avoid the vast majority of the concerns in MSC1884 (see the paragraphs explaining how software failing 50% of the time is bad)


## Other considered solutions

Ideally identity servers would never receive plain-text addresses, however it
is necessary for the identity server to send an email/sms message during a
bind, as it cannot trust a homeserver to do so as the homeserver may be lying.
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved
Additionally, only storing 3pid hashes at rest instead of the plain-text
versions is impractical if the hashing algorithm ever needs to be changed.

Bloom filters are an alternative method of providing private contact discovery,
however does not scale well due to clients needing to download a large filter
that needs updating every time a new bind is made. Further considered solutions
are explored in https://signal.org/blog/contact-discovery/ Signal's eventual
Copy link
Member

Choose a reason for hiding this comment

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

this sentence no sense makes

Copy link
Member

Choose a reason for hiding this comment

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

it still makes no sense - reopened. You have the URL twice, and once in the middle of the sentence.

Copy link
Member

Choose a reason for hiding this comment

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

It's two different URLs guys.

solution of using SGX is considered impractical for a Matrix-style setup.

Bit out of scope for this MSC, but there was an argument for not keeping all
IDs as hashed on disk in the identity server, that being if a hashing algorithm
was broken, we couldn't update the hashing algorithm without having the
plaintext 3PIDs. Well @toml helpfully said that we could just take the old
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved
hashes and rehash them in the more secure hashing algorithm, thus transforming
the algo from ex. SHA256 to SHA256+SomeBetterAlg. This may spur an MSC in the
future that supports this, unless it is just an implementation detail.

## Conclusion

This proposal outlines an effective method to stop bulk collection of user's
contact lists and their social graphs without any disastrous side effects. All
turt2live marked this conversation as resolved.
Show resolved Hide resolved
functionality which depends on the lookup service should continue to function
unhindered by the use of hashes.