Problem with latest Symfony update and signed_cookie feature

[Flo-JB]

After the latest Symfony update with this pull https://github.com/symfony/symfony/pull/46249 Symfony refreshes the session on each page view because the implemented regex filter from the pull reequest fails because of the "." stored in the session cookie with signed_cookie feature enabled.

It would be easier to fix this one on Symfony side but maybe they keep the restricted check and an update in this library is needed...

[peter17]

@Flo-JB may I ask which value you use for signed_cookie.names? Thanks!

For other people with similar problem, it may be possible to exclude the session cookie from the signing process and sign only other cookies, see #154

[Flo-JB]

@peter17 sure - in this specific project all cookies are included by setting the value to ['*']. Excluding the session cookie is a possible fix right now. But aren't there any disadvantages regarding security in this case?

I don't see a good generic solution right now without breaking the signing meachism (for example: storing the hash in a separate cookie file instead of the same one) Adjusting your regex validation isn't a good solution either because this would be just related to this library and basically "wrong" because the validation itself is correct for the session_id

[peter17]

I am thinking of an ugly solution that might work:

on src/EventListener/SignedCookieListener.php

replace the following block:

        $names = true === $this->signedCookieNames ? $request->cookies->keys() : $this->signedCookieNames;
        foreach ($names as $name) {
            if ($request->cookies->has($name)) {
                $cookie = $request->cookies->get($name);
                if ($this->signer->verifySignedValue($cookie)) {
                    $request->cookies->set($name, $this->signer->getVerifiedRawValue($cookie));
                } else {
                    $request->cookies->remove($name);
                }
            }
        }

by

        $names = true === $this->signedCookieNames ? $request->cookies->keys() : $this->signedCookieNames;
        foreach ($names as $name) {
            if ($request->cookies->has($name)) {
                $cookie = $request->cookies->get($name);
                if ($this->signer->verifySignedValue($cookie)) {
                    $rawValue = $this->signer->getVerifiedRawValue($cookie);
                    $request->cookies->set($name, $rawValue);
                    if ($name === session_name()) {
                        $_COOKIE[session_name()] = $rawValue;
                    }
                } else {
                    $request->cookies->remove($name);
                    if ($name === session_name()) {
                        unset($_COOKIE[session_name()]);
                    }
                }
            }
        }

I believe it would work because I think this happens before the session_start() call of the NativeSessionStorage. If it didn't then an exception would have been thrown by NativeSessionStorage before I made my fix there.

@Flo-JB Do you have a minimum code to reproduce the issue? Or can you try this on your project? I don't have time now to set up a project and test this... Thanks!

[Flo-JB]

@peter17 I tested with your code and it works. You are right - you can use the time between onKernelRequest and onKernelResponse to fix the cookie value for your regex check.

With that fix the cookie will be rewritten two times on each page call (surely not the perfect solution) but I think better than the other options.

Another alternative would be more complex - I think of an overridable session_id check service instead of a hardcoded one which could be injected to NativeSessionStorage and could be replaced by the NelmioSecurityBundle.

Big thanks for investigating this issue

[peter17]

Good! I'd like the opinion of a maintainer of NelmioSecurityBundle before I make a PR with this change. Maybe they have better ideas or can comment the code above?

Thanks in advance