fix: special-case AEAD failure

Fixes #2974
Fixes #2963
Fixes #2921
Fixes #2924
Fixes #2653
Fixes #2461
Fixes #2586
Fixes #2179
This commit is contained in:
Harsh Shandilya 2024-04-14 22:50:59 +05:30
parent 312f92d21a
commit 87738477be
8 changed files with 83 additions and 16 deletions

View file

@ -13,11 +13,14 @@ import androidx.fragment.app.setFragmentResultListener
import androidx.lifecycle.lifecycleScope import androidx.lifecycle.lifecycleScope
import app.passwordstore.R import app.passwordstore.R
import app.passwordstore.crypto.PGPIdentifier import app.passwordstore.crypto.PGPIdentifier
import app.passwordstore.crypto.errors.CryptoHandlerException
import app.passwordstore.crypto.errors.NonStandardAEAD
import app.passwordstore.data.crypto.PGPPassphraseCache import app.passwordstore.data.crypto.PGPPassphraseCache
import app.passwordstore.data.passfile.PasswordEntry import app.passwordstore.data.passfile.PasswordEntry
import app.passwordstore.data.password.FieldItem import app.passwordstore.data.password.FieldItem
import app.passwordstore.databinding.DecryptLayoutBinding import app.passwordstore.databinding.DecryptLayoutBinding
import app.passwordstore.ui.adapters.FieldItemAdapter import app.passwordstore.ui.adapters.FieldItemAdapter
import app.passwordstore.ui.dialogs.BasicBottomSheet
import app.passwordstore.util.auth.BiometricAuthenticator import app.passwordstore.util.auth.BiometricAuthenticator
import app.passwordstore.util.auth.BiometricAuthenticator.Result as BiometricResult import app.passwordstore.util.auth.BiometricAuthenticator.Result as BiometricResult
import app.passwordstore.util.extensions.getString import app.passwordstore.util.extensions.getString
@ -27,7 +30,8 @@ import app.passwordstore.util.features.Feature.EnablePGPPassphraseCache
import app.passwordstore.util.features.Features import app.passwordstore.util.features.Features
import app.passwordstore.util.settings.Constants import app.passwordstore.util.settings.Constants
import app.passwordstore.util.settings.PreferenceKeys import app.passwordstore.util.settings.PreferenceKeys
import com.github.michaelbull.result.runCatching import com.github.michaelbull.result.Result
import com.github.michaelbull.result.map
import dagger.hilt.android.AndroidEntryPoint import dagger.hilt.android.AndroidEntryPoint
import java.io.ByteArrayOutputStream import java.io.ByteArrayOutputStream
import java.io.File import java.io.File
@ -198,7 +202,7 @@ class DecryptActivity : BasePGPActivity() {
passphraseCache.cachePassphrase( passphraseCache.cachePassphrase(
this@DecryptActivity, this@DecryptActivity,
gpgIdentifiers.first(), gpgIdentifiers.first(),
passphrase passphrase,
) )
} }
} }
@ -221,24 +225,36 @@ class DecryptActivity : BasePGPActivity() {
onSuccess() onSuccess()
} else { } else {
logcat(ERROR) { result.error.stackTraceToString() } logcat(ERROR) { result.error.stackTraceToString() }
decrypt(isError = true, authResult = authResult) when (result.error) {
is NonStandardAEAD -> {
BasicBottomSheet.Builder(this)
.setTitle(getString(R.string.aead_detect_title))
.setMessage(getString(R.string.aead_detect_message, result.error.message))
.setPositiveButtonClickListener(getString(R.string.dialog_ok)) {
setResult(RESULT_CANCELED)
finish()
}
.setOnDismissListener {
setResult(RESULT_CANCELED)
finish()
}
.build()
.show(supportFragmentManager, "AEAD_INFO_SHEET")
}
else -> decrypt(isError = true, authResult = authResult)
}
} }
} }
private suspend fun decryptPGPStream( private suspend fun decryptPGPStream(
passphrase: String, passphrase: String,
gpgIdentifiers: List<PGPIdentifier>, gpgIdentifiers: List<PGPIdentifier>,
) = runCatching { ): Result<ByteArrayOutputStream, CryptoHandlerException> {
val message = withContext(dispatcherProvider.io()) { File(fullPath).readBytes().inputStream() } val message = withContext(dispatcherProvider.io()) { File(fullPath).readBytes().inputStream() }
val outputStream = ByteArrayOutputStream() val outputStream = ByteArrayOutputStream()
val result = return repository.decrypt(passphrase, gpgIdentifiers, message, outputStream).map {
repository.decrypt( outputStream
passphrase, }
gpgIdentifiers,
message,
outputStream,
)
if (result.isOk) outputStream else throw result.error
} }
private suspend fun createPasswordUI(entry: PasswordEntry) = private suspend fun createPasswordUI(entry: PasswordEntry) =

View file

@ -6,6 +6,7 @@
package app.passwordstore.ui.dialogs package app.passwordstore.ui.dialogs
import android.content.Context import android.content.Context
import android.content.DialogInterface.OnDismissListener
import android.os.Bundle import android.os.Bundle
import android.view.LayoutInflater import android.view.LayoutInflater
import android.view.View import android.view.View
@ -33,6 +34,7 @@ private constructor(
val negativeButtonLabel: String?, val negativeButtonLabel: String?,
val positiveButtonClickListener: View.OnClickListener?, val positiveButtonClickListener: View.OnClickListener?,
val negativeButtonClickListener: View.OnClickListener?, val negativeButtonClickListener: View.OnClickListener?,
val onDismissListener: OnDismissListener?,
) : BottomSheetDialogFragment() { ) : BottomSheetDialogFragment() {
private val binding by viewBinding(BasicBottomSheetBinding::bind) private val binding by viewBinding(BasicBottomSheetBinding::bind)
@ -94,6 +96,9 @@ private constructor(
dismiss() dismiss()
} }
} }
if (onDismissListener != null) {
dialog.setOnDismissListener(onDismissListener)
}
} }
} }
) )
@ -112,6 +117,7 @@ private constructor(
private var negativeButtonLabel: String? = null private var negativeButtonLabel: String? = null
private var positiveButtonClickListener: View.OnClickListener? = null private var positiveButtonClickListener: View.OnClickListener? = null
private var negativeButtonClickListener: View.OnClickListener? = null private var negativeButtonClickListener: View.OnClickListener? = null
private var onDismissListener: OnDismissListener? = null
fun setTitleRes(@StringRes titleRes: Int): Builder { fun setTitleRes(@StringRes titleRes: Int): Builder {
this.title = context.resources.getString(titleRes) this.title = context.resources.getString(titleRes)
@ -135,7 +141,7 @@ private constructor(
fun setPositiveButtonClickListener( fun setPositiveButtonClickListener(
buttonLabel: String? = null, buttonLabel: String? = null,
listener: View.OnClickListener listener: View.OnClickListener,
): Builder { ): Builder {
this.positiveButtonClickListener = listener this.positiveButtonClickListener = listener
this.positiveButtonLabel = buttonLabel this.positiveButtonLabel = buttonLabel
@ -144,13 +150,20 @@ private constructor(
fun setNegativeButtonClickListener( fun setNegativeButtonClickListener(
buttonLabel: String? = null, buttonLabel: String? = null,
listener: View.OnClickListener listener: View.OnClickListener,
): Builder { ): Builder {
this.negativeButtonClickListener = listener this.negativeButtonClickListener = listener
this.negativeButtonLabel = buttonLabel this.negativeButtonLabel = buttonLabel
return this return this
} }
fun setOnDismissListener(
onDismissListener: OnDismissListener,
): Builder {
this.onDismissListener = onDismissListener
return this
}
fun build(): BasicBottomSheet { fun build(): BasicBottomSheet {
require(message != null) { "Message needs to be set" } require(message != null) { "Message needs to be set" }
return BasicBottomSheet( return BasicBottomSheet(
@ -159,7 +172,8 @@ private constructor(
positiveButtonLabel, positiveButtonLabel,
negativeButtonLabel, negativeButtonLabel,
positiveButtonClickListener, positiveButtonClickListener,
negativeButtonClickListener negativeButtonClickListener,
onDismissListener,
) )
} }
} }

View file

@ -377,4 +377,6 @@
<string name="no_keys_imported_dialog_title">No keys imported</string> <string name="no_keys_imported_dialog_title">No keys imported</string>
<string name="no_keys_imported_dialog_message">There are no PGP keys imported in the app yet, press the button below to pick a key file</string> <string name="no_keys_imported_dialog_message">There are no PGP keys imported in the app yet, press the button below to pick a key file</string>
<string name="biometric_prompt_title_gpg_passphrase_cache">Unlock passphrase cache</string> <string name="biometric_prompt_title_gpg_passphrase_cache">Unlock passphrase cache</string>
<string name="aead_detect_title">AEAD encryption detected</string>
<string name="aead_detect_message">%1$s, see https://passwordstore.app/fix-aead for more information</string>
</resources> </resources>

View file

@ -41,8 +41,13 @@ public sealed class CryptoHandlerException(message: String? = null, cause: Throw
/** The passphrase provided for decryption was incorrect. */ /** The passphrase provided for decryption was incorrect. */
public class IncorrectPassphraseException(cause: Throwable) : CryptoHandlerException(null, cause) public class IncorrectPassphraseException(cause: Throwable) : CryptoHandlerException(null, cause)
/** The encrypted material is using an incompatible variant of PGP's AEAD standard. */
public class NonStandardAEAD(cause: Throwable) :
CryptoHandlerException("GnuPG's AEAD implementation is non-standard and unsupported", cause)
/** No keys were passed to the encrypt/decrypt operation. */ /** No keys were passed to the encrypt/decrypt operation. */
public data object NoKeysProvidedException : CryptoHandlerException(null, null) public data object NoKeysProvidedException : CryptoHandlerException(null, null)
/** An unexpected error that cannot be mapped to a known type. */ /** An unexpected error that cannot be mapped to a known type. */
public class UnknownError(cause: Throwable) : CryptoHandlerException(null, cause) public class UnknownError(cause: Throwable, message: String? = null) :
CryptoHandlerException(message, cause)

View file

@ -8,6 +8,7 @@ package app.passwordstore.crypto
import app.passwordstore.crypto.errors.CryptoHandlerException import app.passwordstore.crypto.errors.CryptoHandlerException
import app.passwordstore.crypto.errors.IncorrectPassphraseException import app.passwordstore.crypto.errors.IncorrectPassphraseException
import app.passwordstore.crypto.errors.NoKeysProvidedException import app.passwordstore.crypto.errors.NoKeysProvidedException
import app.passwordstore.crypto.errors.NonStandardAEAD
import app.passwordstore.crypto.errors.UnknownError import app.passwordstore.crypto.errors.UnknownError
import com.github.michaelbull.result.Result import com.github.michaelbull.result.Result
import com.github.michaelbull.result.mapError import com.github.michaelbull.result.mapError
@ -24,6 +25,7 @@ import org.pgpainless.PGPainless
import org.pgpainless.decryption_verification.ConsumerOptions import org.pgpainless.decryption_verification.ConsumerOptions
import org.pgpainless.encryption_signing.EncryptionOptions import org.pgpainless.encryption_signing.EncryptionOptions
import org.pgpainless.encryption_signing.ProducerOptions import org.pgpainless.encryption_signing.ProducerOptions
import org.pgpainless.exception.MessageNotIntegrityProtectedException
import org.pgpainless.exception.WrongPassphraseException import org.pgpainless.exception.WrongPassphraseException
import org.pgpainless.key.protection.SecretKeyRingProtector import org.pgpainless.key.protection.SecretKeyRingProtector
import org.pgpainless.util.Passphrase import org.pgpainless.util.Passphrase
@ -75,6 +77,13 @@ public class PGPainlessCryptoHandler @Inject constructor() :
when (error) { when (error) {
is WrongPassphraseException -> IncorrectPassphraseException(error) is WrongPassphraseException -> IncorrectPassphraseException(error)
is CryptoHandlerException -> error is CryptoHandlerException -> error
is MessageNotIntegrityProtectedException -> {
if (error.message?.contains("Symmetrically Encrypted Data") == true) {
NonStandardAEAD(error)
} else {
UnknownError(error)
}
}
else -> UnknownError(error) else -> UnknownError(error)
} }
} }

View file

@ -9,6 +9,7 @@ package app.passwordstore.crypto
import app.passwordstore.crypto.CryptoConstants.KEY_PASSPHRASE import app.passwordstore.crypto.CryptoConstants.KEY_PASSPHRASE
import app.passwordstore.crypto.CryptoConstants.PLAIN_TEXT import app.passwordstore.crypto.CryptoConstants.PLAIN_TEXT
import app.passwordstore.crypto.errors.IncorrectPassphraseException import app.passwordstore.crypto.errors.IncorrectPassphraseException
import app.passwordstore.crypto.errors.NonStandardAEAD
import com.github.michaelbull.result.getError import com.github.michaelbull.result.getError
import com.google.testing.junit.testparameterinjector.TestParameter import com.google.testing.junit.testparameterinjector.TestParameter
import com.google.testing.junit.testparameterinjector.TestParameterInjector import com.google.testing.junit.testparameterinjector.TestParameterInjector
@ -137,6 +138,23 @@ class PGPainlessCryptoHandlerTest {
} }
} }
@Test
fun aeadEncryptedMaterialIsSurfacedProperly() {
val secKey = PGPKey(TestUtils.getAEADSecretKey())
val plaintextStream = ByteArrayOutputStream()
val ciphertextStream = TestUtils.getAEADEncryptedFile().inputStream()
val res =
cryptoHandler.decrypt(
listOf(secKey),
"Password",
ciphertextStream,
plaintextStream,
PGPDecryptOptions.Builder().build(),
)
assertTrue(res.isErr)
assertIs<NonStandardAEAD>(res.error, message = "${res.error.cause}")
}
@Test @Test
fun canHandleFiltersFormats() { fun canHandleFiltersFormats() {
assertFalse { cryptoHandler.canHandle("example.com") } assertFalse { cryptoHandler.canHandle("example.com") }

View file

@ -21,6 +21,9 @@ object TestUtils {
fun getAEADSecretKey() = this::class.java.classLoader.getResource("aead_sec").readBytes() fun getAEADSecretKey() = this::class.java.classLoader.getResource("aead_sec").readBytes()
fun getAEADEncryptedFile() =
this::class.java.classLoader.getResource("aead_encrypted_file").readBytes()
enum class AllKeys(val keyMaterial: ByteArray) { enum class AllKeys(val keyMaterial: ByteArray) {
ARMORED_SEC(getArmoredSecretKey()), ARMORED_SEC(getArmoredSecretKey()),
ARMORED_PUB(getArmoredPublicKey()), ARMORED_PUB(getArmoredPublicKey()),