Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A few pub keys for a recipient. Creating an encrypted message #1540

Closed
DenBond7 opened this issue Nov 8, 2021 · 12 comments · Fixed by #1523
Closed

A few pub keys for a recipient. Creating an encrypted message #1540

DenBond7 opened this issue Nov 8, 2021 · 12 comments · Fixed by #1523
Assignees

Comments

@DenBond7
Copy link
Collaborator

DenBond7 commented Nov 8, 2021

Creating an encrypted message

When we create a new message we check if a user has a pub key. But before we had one user <-> one pub key.
After changes a user can have a few pub keys with different states, for example:

  1. one good + one expired
  2. one good + one good
  3. one expired + one expired

How to handle such cases?
What color should I use for chips in these cases?

Originally posted by @DenBond7 in #1188 (comment)

@DenBond7 DenBond7 self-assigned this Nov 8, 2021
@tomholub
Copy link
Collaborator

tomholub commented Nov 8, 2021

For color choice, see public key ordering + coloring: #1188 (comment) . As before, if color of any recipient is non-green, then they don't have any good public key and therefore message cannot be sent - they should see a modal.

Here are also some test emails for manual testing: FlowCrypt/flowcrypt-ios#744

Here are modal messages listed: FlowCrypt/flowcrypt-ios#732

For public key choice during encryption, filter recipient keys to only choose the valid ones.

Let's say that your example are three recipients:

A: one good + one expired
B: one good + one good
C: one expired + one expired

These happen to be correctly ordered public keys as per #1188 (comment) (valid first). Based on that, first user is green (per first already ordered key), second key also green, third is orange. Because of C, the user will se an error modal.

For this example:

A: one good + one expired
B: one good + one good

The message will be encrypted for 3 public keys for recipients which is [A-good-1, B-good-1, B-good-2] plus also all of my own public keys that are valid & match by the matching logic based on sender email that we already have. So if I have two such matching keys of my own, then in this situation I could be encrypting for 5 pubkeys total to send to these two recipients.

@DenBond7
Copy link
Collaborator Author

DenBond7 commented Nov 9, 2021

Based on all info that I read I'd like to add more details. I see the following(if not first, then second, and so on)

  • GREEN -> a recipient has at least one usable key (non-revoked, non-expired, well parsed. To be short - a good key)
  • ORANGE -> a recipient has at least one usable key, but it's expired or revoked
  • RED -> a recipient has at least one pub key, but it's not usable(wrongly parsed and so on)
  • GRAY -> a recipient has no pub keys

on this stage, I don't need to sort keys. Just need to check conditions to define a color.

@tomholub
Copy link
Collaborator

tomholub commented Nov 9, 2021

By sorting and then evaluating the first (best) key, it makes the logic simple and fool-proof. Also consistent with other platforms.

Your list doesn't mention revoked keys. Is the current code checking if the key is revoked?

@tomholub
Copy link
Collaborator

tomholub commented Nov 9, 2021

You can test manually with this email. Can also load this public key into your mocks / automated tests.

image

@tomholub
Copy link
Collaborator

tomholub commented Nov 9, 2021

image

@DenBond7
Copy link
Collaborator Author

DenBond7 commented Nov 9, 2021

Is the current code checking if the key is revoked?

No, we don't support that on Android. Could you share details how to do that?

@tomholub
Copy link
Collaborator

tomholub commented Nov 9, 2021

PGPainless should give you information about whether a public key is revoked or not. If not sure how, ask Paul. Maybe the PGPainless readme needs a section about parsing keys and getting details out of them?

@DenBond7
Copy link
Collaborator Author

DenBond7 commented Nov 9, 2021

Got it, thanks. I will check.

@IvanPizhenko
Copy link
Contributor

IvanPizhenko commented Nov 9, 2021

What about revoked keys? As I remember, browser extension takes them into account too. Do they fall into category of keys marked with RED?

@DenBond7
Copy link
Collaborator Author

image
image

@tomholub
Copy link
Collaborator

very good

@tomholub
Copy link
Collaborator

Here's a few more test emails for you, that have combinations of keys. There are no google accounts behind them, so you can send emails and they will bounce back. Which should be enough for manual testing. All @flowcrypt.com

valid.expired
valid.revoked
valid.expired.revoked

tomholub pushed a commit that referenced this issue Nov 25, 2021
… keys per recipient (#1523)

* Added PublicKeyEntity. Refactored code. WIP.| #1188

* Fixed using ContactsDao.getAllContactsWithPgpLD().| #1188

* Fixed using ContactsDao.getAllContactsWithPgp()/getAllContactsWithPgpWhichMatched().| #1188

* Renamed ContactEntity to RecipientEntity.| #1188

* Renamed ContactsDao to RecipientDao.| #1188

* Refactored code.| #1188

* Renamed ContactsViewModel to RecipientsViewModel.| #1188

* Fixed a few methods in RecipientDao. Refactored code.| #1188

* Fixed RecipientDao.getRecipientByEmail().| #1188

* Fixed some moments with adding/updating a recipient with a pub key.| #1188

* Made a workable version after switching to use 'recipients' and 'pub_keys' tables.| #1188

* Fixed tests. Refactored code.| #1188

* Modified code to use a new logic to save contacts on the compose screen.| #1539

* Modified code to use a new logic to apply colors to recipients(chips).| #1540

* Added changes to use all recipient's pub keys for ecnryption + use all sender's mathing pub keys.| #1541

* Fixed signing option.| #1541

* Import recipients manually. Refactored code. Added LookUpPubKeysDialogFragment and ImportRecipientsFromSourceFragment..| #1542

* Added a base realization of importing pub keys manually.| #1542

* Added ImportAllPubKeysFromSourceDialogFragment.| #1542

* Removed unused source.| #1542

* Improved some classes.| #1566

* Added RecipientDetailsFragment.| #1566

* Fixed opening PublicKeyDetailsFragment. Refactored code.| #1566

* Fixed exporting pub key.| #1566

* Fixed deleting pub key.| #1566

* Fixed editing pub key.| #1566

* Fixed database migration bug.| #1188

* Fixed nav_graph.xml for test.| #1188

* Temporary disabled some tests.| #1188

* Temporary disabled some tests(step 2).| #1188

* Fixed Junit tests.| #1188

* Fixed some UI tests.| #1188

* Fixed merge conflicts.| #1188

* Fixed some PR comments. Refactored code.| #1188

* Fixed some PR comments. Refactored code. Step 2.| #1188

* Restored some code.| #1188

* Renamed a file.| #1188

* Fixed importing/updating pub keys for PublicKeyMsgBlock.| #1188
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants