pgpainless: From time to time decryption errors for the same input data

Hi @vanitasvitae,

We’ve received some feedback about errors that occurred from time to time for the same input data. Please look at the following code

  fun decryptAndOrVerifyWithResult(
    srcInputStream: InputStream,
    publicKeys: PGPPublicKeyRingCollection? = null,
    secretKeys: PGPSecretKeyRingCollection? = null,
    protector: SecretKeyRingProtector? = null,
    passphrase: Passphrase? = null,
    ignoreMdcErrors: Boolean = false
  ): DecryptionResult {
    srcInputStream.use { srcStream ->
      val destOutputStream = ByteArrayOutputStream()
      destOutputStream.use { outStream ->
        try {
          val decryptionStream = genDecryptionStream(
            srcInputStream = srcStream,
            publicKeys = publicKeys,
            secretKeys = secretKeys,
            protector = protector,
            passphrase = passphrase,
            ignoreMdcErrors = ignoreMdcErrors
          )

          decryptionStream.use { it.copyTo(outStream) }
          return DecryptionResult(
            openPgpMetadata = decryptionStream.result,
            content = destOutputStream
          )
        } catch (e: Exception) {
          return DecryptionResult.withError(
            processDecryptionException(e)
          )
        }
      }
    }
  }

From time to time in the catch block we catch the following errors:

java.util.NoSuchElementException: No direct-key signature and no user-id signature found.
	at org.pgpainless.key.info.KeyRingInfo.getPrimaryKeyExpirationDate(KeyRingInfo.java:701)
at org.pgpainless.key.info.KeyRingInfo.getEncryptionSubkeys(KeyRingInfo.java:905)
at org.pgpainless.decryption_verification.OpenPgpMessageInputStream.getDecryptionKey(OpenPgpMessageInputStream.java:695)
at org.pgpainless.decryption_verification.OpenPgpMessageInputStream.processEncryptedData(OpenPgpMessageInputStream.java:492)
at org.pgpainless.decryption_verification.OpenPgpMessageInputStream.consumePackets(OpenPgpMessageInputStream.java:306)
at org.pgpainless.decryption_verification.OpenPgpMessageInputStream.<init>(OpenPgpMessageInputStream.java:227)
at org.pgpainless.decryption_verification.OpenPgpMessageInputStream.create(OpenPgpMessageInputStream.java:166)
at org.pgpainless.decryption_verification.OpenPgpMessageInputStream.create(OpenPgpMessageInputStream.java:136)
at org.pgpainless.decryption_verification.OpenPgpMessageInputStream.create(OpenPgpMessageInputStream.java:116)
at org.pgpainless.decryption_verification.DecryptionBuilder$DecryptWithImpl.withOptions(DecryptionBuilder.java:39)
at com.flowcrypt.email.security.pgp.PgpDecryptAndOrVerify.genDecryptionStream(PgpDecryptAndOrVerify.kt:135)
at com.flowcrypt.email.security.pgp.PgpDecryptAndOrVerify.decryptAndOrVerifyWithResult(PgpDecryptAndOrVerify.kt:80)
at com.flowcrypt.email.security.pgp.PgpDecryptAndOrVerify.decryptAndOrVerifyWithResult$default(PgpDecryptAndOrVerify.kt:64)
at com.flowcrypt.email.security.pgp.PgpMsg.processPgpMsgRawBlock(PgpMsg.kt:868)
at com.flowcrypt.email.security.pgp.PgpMsg.processRawBlocks(PgpMsg.kt:375)
at com.flowcrypt.email.security.pgp.PgpMsg.extractMsgBlocksFromPart(PgpMsg.kt:336)
at com.flowcrypt.email.security.pgp.PgpMsg.extractMsgBlocksFromPart$default(PgpMsg.kt:234)
at com.flowcrypt.email.security.pgp.PgpMsg.processMimeMessage(PgpMsg.kt:225)
at com.flowcrypt.email.security.pgp.PgpMsg$processMimeMessage$2.invokeSuspend(PgpMsg.kt:199)
at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106)
at kotlinx.coroutines.internal.LimitedDispatcher.run(LimitedDispatcher.kt:42)
at kotlinx.coroutines.scheduling.TaskImpl.run(Tasks.kt:95)
at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:570)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:750)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:677)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:664)

java.lang.ArrayIndexOutOfBoundsException: length=8; index=8
at org.bouncycastle.crypto.digests.LongDigest.update(Unknown Source:8)
at org.bouncycastle.crypto.digests.LongDigest.update(Unknown Source:8)
at org.bouncycastle.openpgp.operator.bc.BcImplProvider$EdDsaSigner.update(Unknown Source:2)
at org.bouncycastle.openpgp.operator.bc.SignerOutputStream.write(Unknown Source:2)
at org.bouncycastle.openpgp.PGPSignature.blockUpdate(Unknown Source:2)
at org.bouncycastle.openpgp.PGPSignature.update(Unknown Source:16)
at org.bouncycastle.openpgp.PGPSignature.update(Unknown Source:2)
at org.bouncycastle.openpgp.PGPSignature.updateWithPublicKey(Unknown Source:21)
at org.bouncycastle.openpgp.PGPSignature.verifyCertification(Unknown Source:4)
at org.pgpainless.signature.consumer.SignatureValidator$19.verify(SignatureValidator.java:612)
at org.pgpainless.signature.consumer.SignaturePicker.pickLatestUserIdCertificationSignature(SignaturePicker.java:259)
at org.pgpainless.key.info.KeyRingInfo$Signatures.<init>(KeyRingInfo.java:1164)
at org.pgpainless.key.info.KeyRingInfo.<init>(KeyRingInfo.java:104)
at org.pgpainless.decryption_verification.OpenPgpMessageInputStream.getDecryptionKey(OpenPgpMessageInputStream.java:694)
at org.pgpainless.decryption_verification.OpenPgpMessageInputStream.processEncryptedData(OpenPgpMessageInputStream.java:492)
at org.pgpainless.decryption_verification.OpenPgpMessageInputStream.consumePackets(OpenPgpMessageInputStream.java:306)
at org.pgpainless.decryption_verification.OpenPgpMessageInputStream.<init>(OpenPgpMessageInputStream.java:227)
at org.pgpainless.decryption_verification.OpenPgpMessageInputStream.create(OpenPgpMessageInputStream.java:166)
at org.pgpainless.decryption_verification.OpenPgpMessageInputStream.create(OpenPgpMessageInputStream.java:136)
at org.pgpainless.decryption_verification.OpenPgpMessageInputStream.create(OpenPgpMessageInputStream.java:116)
at org.pgpainless.decryption_verification.DecryptionBuilder$DecryptWithImpl.withOptions(DecryptionBuilder.java:39)
at com.flowcrypt.email.security.pgp.PgpDecryptAndOrVerify.genDecryptionStream(PgpDecryptAndOrVerify.kt:131)
at com.flowcrypt.email.security.pgp.PgpDecryptAndOrVerify.decryptAndOrVerifyWithResult(PgpDecryptAndOrVerify.kt:76)
at com.flowcrypt.email.security.pgp.PgpDecryptAndOrVerify.decryptAndOrVerifyWithResult$default(PgpDecryptAndOrVerify.kt:64)
at com.flowcrypt.email.security.pgp.PgpMsg.processPgpMsgRawBlock(PgpMsg.kt:868)
at com.flowcrypt.email.security.pgp.PgpMsg.processRawBlocks(PgpMsg.kt:375)
at com.flowcrypt.email.security.pgp.PgpMsg.extractMsgBlocksFromPart(PgpMsg.kt:336)
at com.flowcrypt.email.security.pgp.PgpMsg.extractMsgBlocksFromPart$default(PgpMsg.kt:234)
at com.flowcrypt.email.security.pgp.PgpMsg.processMimeMessage(PgpMsg.kt:225)
at com.flowcrypt.email.security.pgp.PgpMsg$processMimeMessage$2.invokeSuspend(PgpMsg.kt:199)
at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106)
at kotlinx.coroutines.internal.LimitedDispatcher.run(LimitedDispatcher.kt:42)
at kotlinx.coroutines.scheduling.TaskImpl.run(Tasks.kt:95)
at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:570)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:750)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:677)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:664)

The main problem - it happens from time to time. For example, the following code resolves the problem

fun decryptAndOrVerifyWithResult(
    srcInputStream: InputStream,
    publicKeys: PGPPublicKeyRingCollection? = null,
    secretKeys: PGPSecretKeyRingCollection? = null,
    protector: SecretKeyRingProtector? = null,
    passphrase: Passphrase? = null,
    ignoreMdcErrors: Boolean = false
  ): DecryptionResult {
    srcInputStream.use { srcStream ->
      val destOutputStream = ByteArrayOutputStream()
      destOutputStream.use { outStream ->
        try {
          val decryptionStream = genDecryptionStream(
            srcInputStream = srcStream,
            publicKeys = publicKeys,
            secretKeys = secretKeys,
            protector = protector,
            passphrase = passphrase,
            ignoreMdcErrors = ignoreMdcErrors
          )

          decryptionStream.use { it.copyTo(outStream) }
          return DecryptionResult(
            openPgpMetadata = decryptionStream.result,
            content = destOutputStream
          )
        } catch (e: Exception) {
          try {
            if (srcStream.markSupported()) {
              srcStream.reset()
              val decryptionStream = genDecryptionStream(
                srcInputStream = srcStream,
                publicKeys = publicKeys,
                secretKeys = secretKeys,
                protector = protector,
                passphrase = passphrase,
                ignoreMdcErrors = ignoreMdcErrors
              )

              decryptionStream.use { it.copyTo(outStream) }
              return DecryptionResult(
                openPgpMetadata = decryptionStream.result,
                content = destOutputStream
              )
            } else {
              return DecryptionResult.withError(
                processDecryptionException(e)
              )
            }
          } catch (e: Exception) {
            return DecryptionResult.withError(
              processDecryptionException(e)
            )
          }
        }
      }
    }
  }

it means if I do one more attempt to decrypt - it works well and I can’t catch the same errors.

I’ve tried to reproduce this issue in a Junit test but failed. I haven’t received any errors for 100000 iterations for the same data. Maybe you have an idea of what can be wrong.

About this issue

Original URL
State: closed
Created a year ago
Comments: 34 (34 by maintainers)

Commits related to this issue

Add test for multi-threaded signature verification. Reproducer for https://github.com/pgpainless/pgpainless/issues/365 — committed to pgpainless/bc-java by vanitasvitae a year ago
Separate verification state from PGPSignature Previously, PGPSignature had its verification state as member fields. This PR separates the state out of PGPSignature into a separate Verification class,... — committed to pgpainless/bc-java by vanitasvitae a year ago

Most upvoted comments

I drafted a patch for BC 😄 https://github.com/bcgit/bc-java/commit/cb024359b083679ba678ec44c6d32d0864321ddc

Feel free to comment on https://github.com/bcgit/bc-java/issues/1379 to let the BC devs know if you’d appreciate such a patch 😉

vanitasvitae on Apr 20, 2023

I’m just not sure about BcKeyFingerprintCalculator() usage.

BcKeyFingerprintCalculator should be thread safe I believe.

vanitasvitae on Apr 20, 2023

I created a bug report / discussion upstream: https://github.com/bcgit/bc-java/issues/1379

vanitasvitae on Apr 20, 2023

Some more insights:

Since you are effectively working in multiple threads on the same PGPSecretKeyRing, you also work on the same PGPPublicKeyRing. This means you work on the same PGPPublicKey objects, which again means you work on the same PGPSignature (binding signature in this case).

Unfortunately, to verify a PGPSignature object, you change its internal state. The following code is run during a binding verification:

        bindingSig.init(new BcPGPContentVerifierBuilderProvider(), primaryKey);
        boolean isValid = bindingSig.verifyCertification(primaryKey, subKey);

Internally, PGPSignature has a member OutputStream used for verifications. This output stream is written to during the update method when doing verifications.

In your case, this output stream is a SignerOutputStream, which internally updates the EdDsaSigner which in turn updates a LongDigest object.

This digest is apparently not thread safe (well, the whole PGPSignature class probably isn’t 😄).

Long story short, you should not call operations that do signature verification on the same PGPSignature object multi-threaded.

I transformed your test from #370 into a pure BC-based test focussed on binding signature verification: https://github.com/pgpainless/bc-java/commit/5c73f0fa3083faea01eba032fb9fcf1677e84795

vanitasvitae on Apr 20, 2023

Is it a bad idea to have such logic(reuse PGPSecretKeyRing)? Do I need to change the logic on my side? Or this problem can be(should be) fixed in PGPainless?

Huh, I need to do some experiments to be able to answer on that. I would have thought that reusing PGPSecretKeyRings would be safe, since their primary function is just to hold the key material, but apparently I’m wrong.

Ideally we should see if we can propose a fix to BC to make KeyRings reusable.

Good work finding the reason for the bug!

vanitasvitae on Apr 20, 2023

Did you create an Android app?

Yes. I can upload the source code later, if it is of use for you.

vanitasvitae on Apr 15, 2023

That appears to be a good place to start the diagnose. Big thanks!

vanitasvitae on Apr 4, 2023

I will check it out. Thanks for the heads up!

vanitasvitae on Mar 31, 2023