Repository navigation
fix: skip duplicate key-auth credentials at reconcile time - #2900
shreemaan-abhishek wants to merge 1 commit into
Conversation
The Consumer webhook checks key-auth keys only at admission, and skips a credential whose Secret does not exist yet. A Secret created after its Consumer therefore published a key another Consumer on the same Gateway already used. The reconciler now drops a key-auth credential whose key an older Consumer on the same Gateway publishes, and reports it on the status. Keys are found through hashed field indexes on Consumers and Secrets, and Consumer and Secret changes requeue Consumers sharing a key so a skipped credential returns once the key is free. Signed-off-by: Abhishek Choudhary <shreemaan.abhishek@gmail.com>
|
Paired with api7/api7-ingress-controller#491. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| // GenKeyAuthKeyIndex hashes a key-auth key so the index never holds plaintext. | ||
| func GenKeyAuthKeyIndex(key string) string { | ||
| sum := sha256.Sum256([]byte(key)) |
There was a problem hiding this comment.
False positive. The hash is only an in-memory lookup key for a field index. It is never stored, logged, or sent anywhere, and the plaintext key already sits in the same process cache, so there is nothing to crack offline. SHA-256 here only keeps plaintext out of the index.
|
Please check CI. |
There was a problem hiding this comment.
A Consumer can lose a key it is already using to another Consumer that edits its own Secret afterwards.
When two Consumers share a key, this PR lets the Consumer created earlier keep it. But "created earlier" is not "used the key first":
- Consumer
buses keyKand is published. - Consumer
a, created beforeb, changes its own Secret'sdata.keytoK. Secret updates go through no webhook. - On the next reconcile,
ais older, soakeepsKandb's credential is dropped.
b used K first but loses it, and requests with K now authenticate as a. The webhook would reject the same change if a made it inline, so this is not the webhook's first-come behavior. On master, b is not removed by the controller.
The controller does not record when a key was first used, so it cannot pick the right owner. Suggestion: when a key is shared, skip it on every Consumer that uses it and report the conflict in each Consumer's status.
3 participants
Type of change:
What this PR does / why we need it:
The Consumer webhook rejects a key-auth key that another Consumer on the same Gateway already uses, but only at admission. When a credential's
secretRefpoints to a Secret that does not exist yet, the key is empty and the check skips it. Creating the Secret afterwards fires no Consumer webhook, and the reconciler published the key with no check, so two Consumers on one Gateway could end up sharing a key-auth key.This PR moves the check into the reconciler, which sees every change:
Provider.Update, the reconciler drops any key-auth credential whose key an older Consumer on the same Gateway publishes. "Older" is by creation time, with namespace/name as the tie-break, so exactly one Consumer keeps the key, matching the webhook's first-come behavior. The Consumer stays published with its other credentials, and its status reportsN key-auth credential(s) skipped: key already used by another Consumer.data.key). Index values are SHA-256 hashes of the key, so plaintext keys never land in the index.Consumers with unique keys reconcile exactly as before.
Pre-submission checklist: