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

"Hard-Invalidation" required for UserContextInvalidator::invalidateContext() #593

Open
das-peter opened this issue Jun 14, 2023 · 1 comment

Comments

@das-peter
Copy link

The standard UserContextInvalidator::invalidateContext() might be prone to short-term information disclosure issues if used in a Varnish xkey-softpurge setup with hash caching enabled (hash_cache_ttl).
This because even after the invalidation the deprecated context will be returned for the next request before it will be updated in the varnish cache.
So while a logout / user switch will invalidate the context, the context hash will serve the invalidated context once more before it is updated. This could be abused to fetch information only available for the user with the "invalidated" context.

In my current project I've worked around this by hacking together a hard-purge version:

        if ($this->cacheProxyClient instanceof Varnish) {
            $instanceReflection = new \ReflectionObject($this->cacheProxyClient);
            $optionsProperty = $instanceReflection->getProperty('options');
            $optionsProperty->setAccessible(true);
            $optionsBackup = $invalidatorOptions = $optionsProperty->getValue($this->cacheProxyClient);
            if (stripos($optionsBackup['tags_header'] ?? '', 'xkey') !== false) {
                $invalidatorOptions['tags_header'] = 'xkey-purge';
                $optionsProperty->setValue($this->cacheProxyClient, $invalidatorOptions);
            }
        }
        try {
            $this->invalidator->invalidateContext($sessionId);
        } finally {
            if (isset($optionsProperty) && isset($optionsBackup)) {
                $optionsProperty->setValue($this->cacheProxyClient, $optionsBackup);
            }
        }

This ensures that the tags_header is set to xkey-purge for invalidation if xkey is used.

This is a pretty smelly approach and makes me think we might should have a general approach for soft- / hard-invalidations. Or at least a way to change tags_header at runtime rather than in config only.
I'm more tempted by a general approach because I use the above hack not only for context invalidation but also for cache items that have to be purged immediately - e.g. generic content pages vs. "real-time" info.

Before I start fiddling with code - any objections / ideas?

My current proposal would be to introduce a second parameter to TagCapable::invalidateTags to indicate the type of invalidation (default , hard, soft).
Caches that implement soft / hard-purge then can switch between these modes regardless of the configured default mode.
E.g. ProxyClient\Varnish::invalidateByPurgekeys() could toggle between the appropriate tags_header setting.

@dbu
Copy link
Contributor

dbu commented Sep 18, 2023

hi @das-peter , sorry, this issue went under... i read what you propose. i agree with your reasoning that we want a way to decide hard/soft at runtime.

the issue belongs into FOSHttpCache and not here in the bundle, because tag invalidation is implemented in FOSHttpCache.

for BC reasons, we can't add a second parameter to the TagCapable interface.

i would propose to instead create interface TagExplicitCapable extends TagCapable with invalidateTagsSoft and invalidateTagsHard. that would also avoid having to pass around flags. all clients in FOSHttpCache would implement the new interface (and ignore the choice if they don't support it.
i wonder if we could find better names than "soft" and "hard" because that is not self-explanatory. the problem is that invalidateTagsImmediately would be misleading - you still have to flush. any ideas?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants