murmur128 allows key hashes to be reversed

[jakub-bochenski]

Describe the bug

MurmurHash is a non-cryptographic hash function suitable for general hash-based lookup.
[..]
Unlike cryptographic hash functions, it is not specifically designed to be difficult to reverse by an adversary, making it unsuitable for cryptographic purposes.

https://en.wikipedia.org/wiki/MurmurHash

Reproduction steps

We would like to show key hashes in reports etc. for identification.
If I understand correctly using murmur128 to hash keys makes the hashes as sensitive as the actual keys.

[buger]

It always about the ballance between security and performance. Key hashing is an operation which performed on each API call. So yes, if you need add it to public reports, you may consider use sha256 hashing instead, or "hash" tyk hashes before showing them in reports.

[jakub-bochenski]

Thanks for confirming this.

tyk hashes before showing them in reports.

Actually, I think even more important is that keys are not stored in Redis, and instead only hashes are there for security reasons.

If the Redis DB is compromised, then the attacker could recreate all the keys if murmur is used.

[buger]

The whole point of hashing, in any way, is to be used for DB lookups, so you can turn any kind of content, to some predictable hash value, which you can use as DB identifier. So it depends on your security requirments, and if you ready to sacrifice some speed for less security. For example FIPS security framework (required for gov agencies) put some strict guidelines on crypto and hashing algorithms, and will force your company to use algorithms like SHA256. But when using SHA256 key hashing algorithm, as mentioned above, it will be stored in redis as sha256 keys which is crypto algorithm,.

[andyo-tyk]

Hi @jakub-bochenski,

As @buger has explained, you can use SHA256 with Tyk for cryptographic hashing of keys, for info please see the docs here: https://tyk.io/docs/basic-config-and-security/security/key-hashing/#custom-key-hash-algorithms

I'll close this issue as resolved, but if you have further questions please don't hesitate to reply and I can reopen it, or you could open a new issue.

Thanks for supporting Tyk!

[jakub-bochenski]

@andyo-tyk well I can't if I use the official Helm Chart

Can you act on this PR? TykTechnologies/tyk-charts#110

[andyo-tyk]

Hi @jakub-bochenski, I've highlighted the PR to my colleague who looks after that repo - they will take a look and pick it up there.

[buger]

@jakub-bochenski in meantime while it gets reviewed, you can unblock yourself just by setting his env var like this:

gateway:
  extraEnvs: 
    - name: MY_CUSTOM_ENV
      value: "foo"

This example assumes tyk-single-dc umbrella chart. But key here is this MY_CUSTOM_ENV env option.

[jakub-bochenski]

Nice idea

So if an ENV entry is specified multiple times in k8s config the last value is the effective one? I didn't know that

[buger]

@jakub-bochenski When the same environment variable is set in multiple places in a Kubernetes deployment, the last entry will take precedence and override any previous settings.

So in the case of TYK_GW_HASHKEYFUNCTION, if it is already set as part of the base Tyk Gateway chart values, and you also specify it in extraEnvs, the extraEnvs value will override the base chart value.

The order of precedence works like this:

1. Default value in Gateway chart values.yaml
2. Value overridden in umbrella chart (e.g. tyk-single-dc values.yaml)
3. extraEnvs value in Gateway or umbrella chart

So the extraEnvs value is always the last one applied and will override anything set in the base chart or umbrella chart values.
The only exception is if you explicitly set a valueFrom entry in extraEnvs that references a secret or configMap, that would take precedence over a plain value.

But in summary - yes, extraEnvs would override a value like TYK_GW_HASHKEYFUNCTION that is already set in the base Gateway chart. The extraEnvs value would win.

[jakub-bochenski]

@buger I don't think overriding envs like that is a good idea.
This causes unexpected errors during deployment The order in patch list … doesn't match $setElementOrder list: e.g. https://stackoverflow.com/q/60727150/1237617