android-password-store / Android-Password-Store

Android application compatible with ZX2C4's Pass command line application
https://passwordstore.app
GNU General Public License v3.0
2.53k stars 251 forks source link

fix null pointer exception when using public only keys (#3143) #3144

Closed gregrenda closed 1 month ago

gregrenda commented 1 month ago

Don't call PGPSecretKeyRingCollection for public only keys

msfjarvis commented 1 month ago

Please add a test for it here https://github.com/android-password-store/Android-Password-Store/blob/develop/crypto/pgpainless/src/test/kotlin/app/passwordstore/crypto/PGPainlessCryptoHandlerTest.kt

msfjarvis commented 1 month ago

I had some time so I tried to adapt an existing test to reproduce this but couldn't get it to fail without your fix, feel free to build upon this or write your own test. It should fail on develop and pass on this branch.

  @Ignore("Should fail when decrypting but completes successfully")
  @Test
  fun decryptWithPublicKeys() {
    val aliceSecKeyRing =
      PGPainless.generateKeyRing().modernKeyRing("Alice <owner@example.com>", KEY_PASSPHRASE)
    val bobSecKeyRing = PGPainless.generateKeyRing().modernKeyRing("Bob <owner@example.com>", KEY_PASSPHRASE)
    val aliceCert = PGPainless.extractCertificate(aliceSecKeyRing)
    val aliceKey = PGPKey(PGPainless.asciiArmor(aliceCert).encodeToByteArray())
    val bobKey = PGPKey(PGPainless.asciiArmor(bobSecKeyRing).encodeToByteArray())

    val ciphertextStream = ByteArrayOutputStream()
    val encryptRes =
      cryptoHandler.encrypt(
        listOf(aliceKey, bobKey),
        PLAIN_TEXT.byteInputStream(Charsets.UTF_8),
        ciphertextStream,
        PGPEncryptOptions.Builder().withAsciiArmor(true).build(),
      )
    assertTrue(encryptRes.isOk)

    val message = ciphertextStream.toByteArray().decodeToString()
    val info = MessageInspector.determineEncryptionInfoForMessage(message)
    assertTrue(info.isEncrypted)
    assertEquals(2, info.keyIds.size)
    assertFalse(info.isSignedOnly)

    assertFails {
      val ciphertextStreamCopy = message.byteInputStream()
      val plaintextStream = ByteArrayOutputStream()
      cryptoHandler.decrypt(
        listOf(aliceKey, bobKey),
        KEY_PASSPHRASE,
        ciphertextStreamCopy,
        plaintextStream,
        PGPDecryptOptions.Builder().build(),
      )
    }
  }
gregrenda commented 1 month ago

Added a unit test. You have to pass both private and public keys to decrypt simultaneously to trigger the bug. You probably shouldn't be passing public keys to decrypt but that's likely a bigger change.