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

Throw exception on AEAD decryption failure #90

Merged
merged 3 commits into from
Jan 18, 2021

Conversation

gnarula
Copy link
Contributor

@gnarula gnarula commented Nov 19, 2020

Adds AEADAuthenticationException which is thrown if any AEAD decryption fails.

Throws javax.crypto.AEADBadTagException in case of AEAD decryption failure.

This is a backwards incompatible change to the API and might require a major version bump.

Fixes #89

@gurpreet-
Copy link
Contributor

Thanks so much for the PR!

The only problem I have with it is addition of a new package 😕 I see you also added a custom exception at one point, may I ask why you used javax.crypto.AEADBadTagException rather than AEADAuthenticationException? 🙂

@gnarula
Copy link
Contributor Author

gnarula commented Dec 2, 2020

The only problem I have with it is addition of a new package 😕 I see you also added a custom exception at one point, may I ask why you used javax.crypto.AEADBadTagException rather than AEADAuthenticationException? 🙂

I thought it's better to use an existing Exception in the Java Cryptography Extension API which denotes the same cause.

I reckon javax.crypto.AEADBadTagException is available in JDK >= 1.7 and Android API >= 19 so there aren't any external dependencies being added. Does the project support JDK/Android targets below this? I'd be happy to change it however you see fit.

@gurpreet-
Copy link
Contributor

OK you have convinced me. It's better to reuse than reinvent the wheel. Let's keep it using javax.crypto.AEADBadTagException 👍

Co-authored-by: Gurpreet Paul <gurpreet@gurpreet.co>
@@ -122,8 +211,22 @@ public void encryptAES() {
}
}

@Test(expected = AEADBadTagException.class)
public void encryptAESMalformedCipher() throws AEADBadTagException {
if (lazySodium.cryptoAeadAES256GCMIsAvailable()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi, because of this if statement, wouldn't this fail if cryptoAeadAES256GCMIsAvailable is false because it expects an exception (@Test(expected = AEADBadTagException.class)) all the time?


@Before
public void doBeforeEverything() {
lazySodium = new LazySodiumJava(new SodiumJava(LibraryLoader.Mode.BUNDLED_ONLY));
encoder = new HexMessageEncoder();
Copy link
Contributor

Choose a reason for hiding this comment

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

This can probably be moved to AEADTest rather than be added to the BaseTest class.

@gurpreet-
Copy link
Contributor

Hey I'll merge but then pickup the changes myself 👍

@gurpreet- gurpreet- merged commit 45975c8 into terl:master Jan 18, 2021
gurpreet- added a commit that referenced this pull request Jan 18, 2021
KevinRoebert added a commit to KevinRoebert/lazysodium-java that referenced this pull request Sep 4, 2021
Added support for ARM 64-bit devices.

Tested under:
```
uname -a
Linux ubuntu-linux-20-04-desktop 5.4.0-80-generic terl#90-Ubuntu SMP Fri Jul 9 17:43:26 UTC 2021 aarch64 aarch64 aarch64 GNU/Linux
```
gurpreet- pushed a commit that referenced this pull request Dec 22, 2021
Added support for ARM 64-bit devices.

Tested under:
```
uname -a
Linux ubuntu-linux-20-04-desktop 5.4.0-80-generic #90-Ubuntu SMP Fri Jul 9 17:43:26 UTC 2021 aarch64 aarch64 aarch64 GNU/Linux
```
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.

AEAD.Lazy does not throw exception for failed authentication
2 participants