Skip to content
This repository has been archived by the owner on Oct 15, 2024. It is now read-only.

Switch new PGP backend to use PGPainless #1522

Merged
merged 12 commits into from
Oct 23, 2021
Merged

Switch new PGP backend to use PGPainless #1522

merged 12 commits into from
Oct 23, 2021

Conversation

msfjarvis
Copy link
Member

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates

📜 Description

Implements a new crypto backend based on PGPainless, a BouncyCastle-backed, pure-Java library for OpenPGP that irons out a lot of the warts with other PGP libraries and is in active maintenance.

💡 Motivation and Context

While Gopenpgp is great in of itself, it has certain characteristics that make it rather undesirable for us.

  1. It uses JNI, which incurs a library loading runtime penalty on the app as well as overhead on each call to a Gopenpgp method.
  2. It's impossible to test without an emulator, which significantly slows down our CI.
  3. It adds maintenance overhead for us, since we need to maintain a fork of the library to add convenience methods for our use case.

PGPainless gets rid of all these problems:

  1. It's a pure Java library built on top of BouncyCastle, just like OpenKeychain
  2. Since it does not use JNI, we can unit test it.
  3. It is actively maintained and the development of it is sponsored by a corporate entity.

Additionally, it leverages strong typing to entirely prevent multiple bug classes such as using a private and public key interchangeably.

Once this PR is merged I will archive our fork of Gopenpgp.

💚 How did you test it?

Inserted hard-coded credentials and verified decryption/encryption works perfectly.

📝 Checklist

  • I formatted the code ./gradlew spotlessApply
  • I reviewed submitted code
  • I added a CHANGELOG entry if applicable

🔮 Next steps

  • Improve the logic for finding matching keys
  • Add key import flow
  • Add UI to request passphrase at runtime

@msfjarvis msfjarvis added C-technical-debt Category: This makes the code harder to read and modify, but has no impact on end users P-high Priority: high, must be resolved before next major release S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed A-PGP Area: OpenKeychain-backed PGP labels Oct 23, 2021
@msfjarvis msfjarvis added this to the v2.0.0 milestone Oct 23, 2021
@msfjarvis msfjarvis requested a review from a team as a code owner October 23, 2021 11:04
Copy link
Member

@Skrilltrax Skrilltrax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@msfjarvis msfjarvis merged commit aac74ae into android-password-store:develop Oct 23, 2021
@msfjarvis msfjarvis deleted the pgpainless-backend branch October 24, 2021 17:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-PGP Area: OpenKeychain-backed PGP C-technical-debt Category: This makes the code harder to read and modify, but has no impact on end users P-high Priority: high, must be resolved before next major release S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants