-
Notifications
You must be signed in to change notification settings - Fork 24.4k
[8.x] Use new stronger AES-256-GCM encryption by default #5674
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
Conversation
Do we need to bump the minimum framework version here too? |
@GrahamCampbell the current one is already the minimum for this version. |
🚢 |
I like this change. Quick question, Will this work with data already encrypted in other 8.x projects? Or would I have to decrypt existing data with AES-256-CBC and re-encrypt with AES-256-GCM? Thank you. |
@alteredchimera as with any encryption you'll need to decrypt/re-encrypt data that was already encrypted using a different algorithm. Note that you're in no way required to move to the new algorithm. Existing apps will continue to work as is. |
@driesvints Thanks for clarification. I like to keep my 8.x projects up to the latest releases. So I'm happy to do this, I just wanted the added clarification, so I appreciate it. For you, the team and collaborators, keep up the sensational work. :) |
@alteredchimera I'll just add here that since cookies are encrypted, switching from CBC to GCM will invalidate all sessions (sign all users out or cause 419), so a migration-strategy would be needed there as well if this is a live application (something like, for a transition period, try decrypting with both, but only encrypt with the new). |
@taylorotwell Why did we move back to Main reasons why we should NOT use CBC?
Is your main reason that using the same IV twice can compromise your key? |
@Tklaversma The change was reverted due to a bug in older versions of OpenSSL, see here for details: laravel/framework#38594 This bug has since been patched in Laravel (by always using lowercase cipher names), so I agree GCM should be the default now. TLS support has nothing to do with the encryption used in the application layer, and the IV is never reused. Regarding authenticated encryption, Laravel adds this via an |
@Krisell thanks for clearing this up. |
@Tklaversma feel free to attempt a new pr |
Implements laravel/framework#38190 as a default.