pgpainless: Question: how to catch MDC errors

I am decrypting message with code like this:

      val decryptionStream = PGPainless.decryptAndOrVerify()
          .onInputStream(input)
          .decryptWith(protector, PGPSecretKeyRingCollection(listOf(pgpSecretKeyRing)))
          .verifyWith(pgpPublicKeyRingCollection)
          .ignoreMissingPublicKeys()
          .build()

      val output = ByteArrayOutputStream()
      decryptionStream.use { it.copyTo(output) }

How do I catch two following situations:

There is no MDC
MDC doesn’t match

With the “plain BouncyCastle” I’ve found something like this:

         val data: PGPLiteral = ....
         // Do this after finished reading from the PGPLiteral data stream
         var mdcAbsent = false
          var mdcError: Boolean
          try {
            mdcError =data.verify()
          } catch (ex: PGPException) {
            mdcAbsent = ex.message?.contains("data not integrity protected") ?: false
            mdcError = !mdcAbsent
          }

But how to achieve the same with PGPainless?

About this issue

Original URL
State: closed
Created 3 years ago
Comments: 21 (21 by maintainers)

Most upvoted comments

I imagine this should work for our purposes - at least for now. @IvanPizhenko did you notice a message without MDC (or wrong MDC) get decrypted without throwing? If so then the message could be used as a test case to implement against to improve MDC handling.

I’ve found one such test, at least by name it is declared to miss MDC, and there is no exception, it decrypts, but it sets isIntegrityProtected to false. Attaching here a file with test data. mdctest.txt

@vanitasvitae please examine this

IvanPizhenko on Apr 27, 2021

Anyway - it would be far better if missing and incorrect MDCs could be detected somehow explicitly, so we do not rely on those PGP exception, with a bit cryptic error messages, which can potentially happen due to some other reasons than missing MDC

I agree with this - It would be great if PGPainless could throw an explicit MdcException("MDC missing, message is unsafe to render") or MdcException("MDC does not match, message is unsafe to render") where MDC exception could inherit from PGP exception for example. If not, then throw a PGPexception with a clear message that this is about MDC.

tomholub on Apr 27, 2021

I imagine this should work for our purposes - at least for now. @IvanPizhenko did you notice a message without MDC (or wrong MDC) get decrypted without throwing? If so then the message could be used as a test case to implement against to improve MDC handling.

tomholub on Apr 26, 2021

I also like this approach, but it makes is more insecure because other library users may not realize they have to deal with MDC.

Therefore I think the default should be to throw, but make it configurable to not throw and instead return an enum. Then this should be documented. Therefore the library user has to take an extra action to have a chance of insecure usage, and hopefully there will be a big warning in the docs telling him he absolutely still has to check the enum, and don’t render the message as is.

tomholub on Apr 25, 2021