neuhalje / bouncy-gpg

Make using Bouncy Castle with OpenPGP fun again!
https://neuhalje.github.io/bouncy-gpg/
Other
205 stars 58 forks source link

NullPointerException when trying to decrypt non-encrypted data (v2.3.0) #58

Open lclement57 opened 3 years ago

lclement57 commented 3 years ago

Describe the bug When we call the method decryptAndVerifyStream with an input stream that contains data that is not encrypted (for example only signed), there is a NullPointerException when reading the returned InputStream later thrown by name.neuhalfen.projects.crypto.bouncycastle.openpgp.decrypting.MDCValidatingInputStream.validateMDC, where the local variable pbe is null.

In v2.2.0, there was no exception, trying to decrypt a non-encrypted stream just returned the original stream. The same thing happens when trying to read an input stream obtained after trying to decrypt an input stream that was (incorrectly) encrypted twice (which is a unit test we are running).

The issue seems to be introduced by the following commit: https://github.com/neuhalje/bouncy-gpg/commit/986d43609d52e841e608d66b7c8157f321d7e760#diff-f7bfd613f0ac7d22e02bd9c1727a319dad532f787e9de5ae702da71b252580b4

Here the new MDCValidatingInputStream class is introduced, and we see that the value of pbe comes from DecryptionStreamFactory, where it is initialized only if the method nextDecryptedStream finds an instance of PGPEncryptedDataList, otherwise it stays null and is passed to the MDCValidatingInputStream where there is no null check.

To Reproduce In v2.3.0, execute a decryption with:

BouncyGPG
                        .decryptAndVerifyStream()
                        .withConfig(keyringConfig)
                        .andIgnoreSignatures()
                        .fromEncryptedInputStream(in)

where in is an input stream containing not encrypted, but signed data.

Expected behavior No NullPointerException thrown.

System (please complete the following information):

neuhalje commented 3 years ago

Thanks for the very detailed error report! This seems to be a bug. I'll try to get into that during the weekend (or ideally review a PR ;-) )

miks commented 3 years ago

Same here. Spent a couple of hours trying to understand what critical part is missing or I'm doing something completely wrong way. Until I searched "validateMDC" in this repo and get came upon this issue. Changed the version to 2.2.0 and it's working as expected. I'm not sure about PR in which cases MDCValidatingInputStream needs to be bypassed.

Sauhardstark commented 3 years ago

It seems to be linked to my changes related to MDC verification

@neuhalje Should we look into adding a null check at the MDC verification or should we check which streams should be wrapped with the MDCVerificationInputStream in the first place ?

SandeepLakka commented 10 months ago

@neuhalje / @Sauhardstark , can this be fixed asap?. we cannot downgrade to 2.2.0 as we had #56 issue in that release. Could you please check and let us know when can this be fixed. Thanks!

Sauhardstark commented 9 months ago

Hi, I'll raise a PR for this immediately

Sauhardstark commented 9 months ago

@SandeepLakka / @neuhalje - I've raised a PR for this. Please take a look