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

more than one public key per recipient #1188

Closed
tomholub opened this issue Apr 22, 2021 · 19 comments · Fixed by #1523
Closed

more than one public key per recipient #1188

tomholub opened this issue Apr 22, 2021 · 19 comments · Fixed by #1523
Assignees

Comments

@tomholub
Copy link
Collaborator

(I'm filing a separate issue to discuss from #480 )

Right now, we import one public key per recipient email. So for [email protected] I can only have one public key. Would it be possible to allow more than one public key per each recipient email? Then we can import any keys, but have to change logic on how to choose the right key when we send out encrypted message, for example.

@DenBond7
Copy link
Collaborator

So for [email protected] I can only have one public key.

That's true for now

Would it be possible to allow more than one public key per each recipient email?

I think yes. For example, we can store a few pub keys as a single source in the database record. In that case, a user will have a list of pub keys(in most cases with a single pub key) instead of a single key.

Then we can import any keys, but have to change logic on how to choose the right key when we send out encrypted message, for example.

It's possible. For now, we just create a data class that contains only recipient emails. But we also can store for example email<->fingerprint pair. After that during the creation raw MIME message, we can choose the right key. Of course, we should create UI for that.

Anyway, it's possible.

@tomholub
Copy link
Collaborator Author

After that during the creation raw MIME message, we can choose the right key. Of course, we should create UI for that.

This would be automatic - we'd have rules about how to choose the right one. No user interaction.

I think yes. For example, we can store a few pub keys as a single source in the database record. In that case, a user will have a list of pub keys(in most cases with a single pub key) instead of a single key.

Would this end up being three db rows with three public keys, or one row with several public keys in it?

@DenBond7
Copy link
Collaborator

Would this end up being three db rows with three public keys, or one row with several public keys in it?

only one row as before. Just public_key column will contain a list of keys. Also, we can add a new column, for example, default_key, and store a fingerprint of the default key.

@tomholub
Copy link
Collaborator Author

Would it alternatively be possible to do a migration and be able to store more than one row per recipient, with one key per row? It's more work but it would be the most flexible.

@DenBond7
Copy link
Collaborator

Would it alternatively be possible to do a migration and be able to store more than one row per recipient, with one key per row?

yes, we can do that. I don't see any problems with that.

@tomholub tomholub added this to the soon milestone Apr 22, 2021
@IvanPizhenko
Copy link
Contributor

IvanPizhenko commented Apr 22, 2021

why not store additional keys in the additional table? one record per key, referencing recipient as foreign key?

@tomholub
Copy link
Collaborator Author

That's roughly what we do on browser extension - a list of recipients in one table with references to public keys, and a public key table next to it.

@DenBond7
Copy link
Collaborator

why not store additional keys in the additional table? one record per key, referencing recipient as foreign key?

That's roughly what we do on browser extension - a list of recipients in one table with references to public keys, and a public key table next to it.

@tomholub I'd like to clarify the relationships between contacts and pub_keys tables. Will it be one-to-many or many-to-many?

Is there a situation when we can have a single pub key for two different email addresses? (a pub key that contains a few user ids). What we are going to support?

I'm going to add the following

@Entity(
  tableName = "pub_keys",
  indices = [
    Index(
      name = "email_fingerprint_in_pub_keys",
      value = ["email", "fingerprint"],
      unique = true
    )
  ],
  foreignKeys = [
    ForeignKey(
      entity = ContactEntity::class,
      parentColumns = ["email"],
      childColumns = ["email"],
      onDelete = ForeignKey.CASCADE
    )
  ]
)
class PubKeysEntity(
  @PrimaryKey(autoGenerate = true) @ColumnInfo(name = BaseColumns._ID) val id: Long? = null,
  val email: String,
  @ColumnInfo(name = "fingerprint") val fingerprint: String,
  @ColumnInfo(name = "public_key") val publicKey: ByteArray
)

@DenBond7
Copy link
Collaborator

DenBond7 commented Oct 20, 2021

  • Discuss all actions with recipients and pub keys
  • Add a testable version
  • Add migration
  • add tests
  • test that deleting a contact also deletes dependent keys

@tomholub
Copy link
Collaborator Author

tomholub commented Oct 20, 2021

On other platforms (browser and iOS) we have renamed Contact to RecipientWithPubKeys as follows:

struct RecipientWithPubKeys {
    let email: String
    /// name if known
    let name: String?
    /// last time an email was sent to this contact, update when email is sent
    let lastUsed: Date?
    /// public keys
    var pubKeys: [PubKey]
}

So instead of Contact that has one pubkey in it, it's a recipient which has an array of keys in it. I recommend to do it the same, it works well. Then when pulling keys from database, most of the time, you will pull the RecipientWithPubKeys as a whole.

When ordering the public keys that come from the database, please use this logic: FlowCrypt/flowcrypt-ios#714 it will save us trouble later.

After that, when deciding color of the recipient now that several keys are involved, use this logic (after sorting methodically, use the first key for the color) - FlowCrypt/flowcrypt-ios#712

@tomholub I'd like to clarify the relationships between contacts and pub_keys tables. Will it be one-to-many or many-to-many?
Is there a situation when we can have a single pub key for two different email addresses? (a pub key that contains a few user ids). What we are going to support?

To give you some examples:

Tom may have a key for Tom + Human + Security
Human may have a key for Human
Mart may have a key for Mart + Security

So you can definitely have a situation where Tom has more than one key (expected) but also where one key is shared by Tom and Human or Tom and Mart (I think this answers your question).

In this situation, it would be best to de-duplicate keys by their fingerprints. So that you don't have to store the shared key twice, and when you update it on one end, then it updates on both.

I suppose that makes it many-to-many?

@DenBond7
Copy link
Collaborator

Thank you for the clarification 👍

Tom may have a key for Tom + Human + Security
Human may have a key for Human
Mart may have a key for Mart + Security

So you can definitely have a situation where Tom has more than one key (expected) but also where one key is shared by Tom and Human or Tom and Mart (I think this answers your question).

Some moments are worrying me. Let me explain.
As for me specifiyng a pub key for a particular user will be more flexible. I mean it would be better to have a separate copy of a key for each recipient.
I see the following

  1. For example on a server we have a pub key with the following userIds: [email protected], [email protected] and [email protected]. One key that relates to three recipients.
  2. A user opens a compose screen and types '[email protected]'. we fetch a pub key from the server and save it to the local database. We will have a row with a unique recipient email and the key fingerprint. Based on that data we will be able to fetch all available keys for a particular user (including deleting or modifying)
  3. If a user deletes some recipient, keys that relates to that recipient will be deleted automatically. (Cascade deleting).
  4. This logic will help us keep recipients independent of each other. It's very similar to what we have with account private keys. We had to migrate to that logic to be more flexible and clear.
  5. In that case we will have one-to-many case. It's simpler.

@tomholub
Copy link
Collaborator Author

It's ok to do it this way, if you prefer it. It's true it will probably make it simpler for you.

But you can no longer de-duplicate by key fingerprint, and it cannot be a unique index. So your unique index will probably be recipient_id+fingerprint?

@DenBond7
Copy link
Collaborator

It's ok to do it this way, if you prefer it. It's true it will probably make it simpler for you.

Got it. I will use that logic

So your unique index will probably be recipient_id+fingerprint?

Correct

@DenBond7
Copy link
Collaborator

DenBond7 commented Nov 8, 2021

Encryption and signing. NEW messages

Imagine that I have the following records in the local database

1| FINGERPRINT| [email protected] | KEY
2| FINGERPRINT| [email protected] | KEY1
3| FINGERPRINT| [email protected] | KEY2

I'm a sender. I'd like to encrypt and sign a message for a recipient. What combination of keys should I use?

  1. KEY + KEY1
  2. KEY + KEY2
  3. KEY + KEY1 + KEY2

@DenBond7
Copy link
Collaborator

Also need to add an ability to see the public key details. Before we used to show the contact details that have only one key. But now we need to change that screen

Before
Contacts list -> public key info

now
Recipients list -> Recipients details with pub key list -> public key info

@tomholub
Copy link
Collaborator Author

yes - correct

DenBond7 added a commit that referenced this issue Nov 23, 2021
DenBond7 added a commit that referenced this issue Nov 23, 2021
DenBond7 added a commit that referenced this issue Nov 23, 2021
DenBond7 added a commit that referenced this issue Nov 23, 2021
DenBond7 added a commit that referenced this issue Nov 23, 2021
DenBond7 added a commit that referenced this issue Nov 23, 2021
DenBond7 added a commit that referenced this issue Nov 23, 2021
DenBond7 added a commit that referenced this issue Nov 23, 2021
DenBond7 added a commit that referenced this issue Nov 23, 2021
DenBond7 added a commit that referenced this issue Nov 24, 2021
DenBond7 added a commit that referenced this issue Nov 24, 2021
DenBond7 added a commit that referenced this issue Nov 25, 2021
DenBond7 added a commit that referenced this issue Nov 25, 2021
DenBond7 added a commit that referenced this issue Nov 25, 2021
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