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

[5.5] Add GCM support to encrypter #21963

Closed
wants to merge 2 commits into from

Conversation

bukka
Copy link

@bukka bukka commented Nov 5, 2017

This PR adds support for AES GCM ciphers with 128 or 256 bit key. The default is still the same (CBC) so it should be fine for 5.5 I guess (not sure what the rules are thought so happy to create a PR for 5.6 only).

I added GCM support to openssl ext in 7.1 so it's available only for PHP version higher or equal to 7.1.

@bukka bukka force-pushed the encrypter-gcm-support-5-5 branch 2 times, most recently from 432329b to 95f2ce5 Compare November 5, 2017 18:27
@@ -44,9 +44,19 @@ public function testWithCustomCipher()
$this->assertEquals('foo', $e->decrypt($encrypted));
}

public function testAeadCipher()
{
$this->onlyForAead();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just use the @requires PHP 7.1 annotation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And drop the extra method. :)

@taylorotwell
Copy link
Member

No plans to add this to the core.

@GrahamCampbell
Copy link
Member

What are the benefits of GCM? Maybe make a package, and see if i gets traction?

@bukka
Copy link
Author

bukka commented Nov 6, 2017

@GrahamCampbell I think that the main benefit is a better performance in this context. The GCM doesn't need an extra MAC creation / validation as it's all done when applying mode during encryption / decryption. All is fully handled by openssl_encrypt / openssl_decrypt.

@taylorotwell I'd be interested to know the reason why you prefer not to have it in the core. Is it just because you prefer to have a very thin encrypter with a single alg supported? Or would you prefer a different more extendable implementation? I'm also wondering what are your thoughts about more standard output format like JWE which would also allow additional key protection?

@davidstanley01
Copy link
Contributor

@taylorotwell why no plans for this?

@Krisell
Copy link
Contributor

Krisell commented Aug 20, 2021

@bukka I'm sorry this discussion didn't move on. GCM support was recently added to Laravel but with some minor issues, that your PR solved. I used your PR as inspiration for a new to fix the issues: #38475

Thank you for this work back in 2017!

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

Successfully merging this pull request may close these issues.

5 participants