Cache empty auth results to reduce db load

[mhenke1]

Cache negative auth responses to avoid going to the DB with every identical call

Description

Since negative auth responses (Identity.get) are not cached,
all requests with a non existing namespace or an invalid authkey will perform queries to the subject db.
A user can influence OW performance by issuing a high number of those calls.

ThIs PR changes that behavior in the way, that negative auth results are cached as None value.
Subsequent Identical calls will only use the cache (for the 5 minutes cache period).
The external interface and behavior of the Identity.get methods stays unchanged.

Related issue and scope

• I opened an issue to propose and discuss this change

My changes affect the following components

• API
• Controller
• Message Bus (e.g., Kafka)
• Loadbalancer
• Invoker
• Intrinsic actions (e.g., sequences, conductors)
• Data stores (e.g., CouchDB)
• Tests
• Deployment
• CLI
• General tooling
• Documentation

Types of changes

• Bug fix (generally a non-breaking change which closes an issue).
• Enhancement or new feature (adds new functionality).
• Breaking change (a bug fix or enhancement which changes existing behavior).

Checklist:

• I signed an Apache CLA.
• I reviewed the style guides and followed the recommendations (Travis CI will check :).
• I added tests to cover my changes.
• My changes require further changes to the documentation.
• I updated the documentation where necessary.

[mhenke1]

[PG3:2985 ok]

[codecov-io]

Codecov Report

Merging #4104 into master will decrease coverage by 3.49%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master    #4104     +/-   ##
=========================================
- Coverage   84.57%   81.07%   -3.5%     
=========================================
  Files         148      148             
  Lines        7110     7118      +8     
  Branches      431      423      -8     
=========================================
- Hits         6013     5771    -242     
- Misses       1097     1347    +250

Impacted Files	Coverage Δ
...re/database/MultipleReadersSingleWriterCache.scala	98% <100%> (+0.32%)	⬆️
...la/org/apache/openwhisk/core/entity/Identity.scala	92.85% <100%> (+0.26%)	⬆️
...core/database/cosmosdb/RxObservableImplicits.scala	0% <0%> (-100%)	⬇️
...core/database/cosmosdb/CosmosDBArtifactStore.scala	0% <0%> (-95.54%)	⬇️
...sk/core/database/cosmosdb/CosmosDBViewMapper.scala	0% <0%> (-92.6%)	⬇️
...whisk/core/database/cosmosdb/CosmosDBSupport.scala	0% <0%> (-83.34%)	⬇️
...abase/cosmosdb/CosmosDBArtifactStoreProvider.scala	0% <0%> (-62.5%)	⬇️
...penwhisk/core/database/cosmosdb/CosmosDBUtil.scala	92% <0%> (-4%)	⬇️
...che/openwhisk/core/database/CouchDbRestStore.scala	73.23% <0%> (-0.51%)	⬇️
... and 22 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 775b757...cccedc2. Read the comment docs.

[markusthoemmes]

Good one, that's indeed an attack vector against the database.

LGTM!

[rabbah]

Agree. Is 5 minutes too high?

[chetanmeh]

Caching such negative lookups may be abused to put lots of of entries in cache and put pressure on heap (causing eviction of valid entries) given our caches are not bounded by size. May be use a separate bounded cache for such entries.

[markusthoemmes]

@chetanmeh the cache is "bounded" as in its entries are garbage collectable.

[chetanmeh]

@markusthoemmes All caches are currently configured with softValues thereby the entries are evicted (apart from expiry) on the basis of jvm garbage collection. Now if an attacker is given a way to add entry to one of the cache in arbitrary manner then it can possibly be abused to add lots of entries in Identity cache. This would put pressure on heap and would possibly trigger evictions of valid data in all other caches also.

If we use a bounded cache in terms of entry count then only identity cache would be impacted but other caches would not get impacted.

[mhenke1]

I have now limited the auth cache hard to 100000 entries. Worth to be configurable ?

[chetanmeh]

I have now limited the auth cache hard to 100000 entries. Worth to be configurable ?

@mhenke1 Should be fine to leave it specified in code for now

[mhenke1]

[PG 2:3878 ok]

[mhenke1]

@chetanmeh I have changed the code as suggested

[chetanmeh]

@rabbah @markusthoemmes can you review cache changes once

[rabbah]

LGTM