search

envoyproxy / envoy

Cloud-native high-performance edge/middle/service proxy
https://www.envoyproxy.io
Apache License 2.0
25.08k stars 4.82k forks source link

Crypto-grade hashing for resource sharing #11967

Open htuch opened 4 years ago

htuch commented 4 years ago

Currently, we use MesasgeUtil::hash in various places, e.g. SDS, to hash a config source (https://github.com/envoyproxy/envoy/blob/master/source/common/secret/secret_manager_impl.h#L82).

Since our threat model has the control plane as trusted, this isn't a huge issue. But if we make the control plane untrusted in the future, this can be somewhat scary. As an example, imagine an Envoy that has delegated listener config (with SDS) to TrustedControlPlane and another listener config (with SDS) to SuperScaryControlPlane. It's plausible that SuperScaryControlPlane is able to engineer a collision and steal secrets via the dynamic prover's dedupe algorithm, since we're only using the weak xxhash.

This is something we probably should fix before considering Envoy robust to untrusted control plane or ready for arbitrary federation.

CC @antoniovicente @kyessenov

antoniovicente commented 4 years ago

Is use of any form of hash appropriate in this case? Could we use the actual config provider entity as the map key?

kyessenov commented 4 years ago

It has to be some structured hash since proto bytes are not deterministic. Alternatively, there could be another (string?) representation for a config source identifier.

lizan commented 4 years ago

It no longer use proto bytes for hashing, it is using text format which is stable and expands any recursively. But if there's a structured hash it will be better.

htuch commented 4 years ago

The config source, as currently constituted, can be extremely large, it's a proto, which can include things like credentials. So, I think a hash or some abbreviated representation is the only thing that can work.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or other activity occurs. Thank you for your contributions.

htuch commented 3 years ago

FWIW, we're starting to see the "convert the proto to text and then hash it" show up as non-trivial in config update profiles.