Skip to content

Revocation handling #3579#3631

Merged
tomholub merged 7 commits intomasterfrom
issue-3579-revocations
May 15, 2021
Merged

Revocation handling #3579#3631
tomholub merged 7 commits intomasterfrom
issue-3579-revocations

Conversation

@rrrooommmaaa
Copy link
Contributor

@rrrooommmaaa rrrooommmaaa commented May 11, 2021

This PR handles revocations.

Introduces a separate store to keep fingerprints of revoked keys (as well as some arbitrary additional info).
As OpenPGP and X.509 can potentially have same cryptographic material,
revocation of abcdef also "revokes" abcdef-X509, as well as revocation abcdef-X509 also "revokes" abcdef.
Given that, it's not to safe to load OpenPGP key if there is revocation for its X.509 sibling, even setting revoked flag manually,
because it will be lost during armor/parse. So the storage simply doesn't return such keys as if they are missing in the storage.

close #3579


Tests (delete all except exactly one):

  • Tests added or updated

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

  • addresses the issue it closes (if any)
  • code is readable and understandable
  • is accompanied with tests, or tests are not needed
  • is free of vulnerabilities
  • is documented clearly and usefully, or doesn't need documentation

Copy link
Collaborator

@tomholub tomholub left a comment

Choose a reason for hiding this comment

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

Looks good :)

@rrrooommmaaa rrrooommmaaa marked this pull request as ready for review May 15, 2021 06:43
@rrrooommmaaa
Copy link
Contributor Author

Migration may be a bit slow (it loads and parses all the OpenPGP keys in a single batch), but it will only happen if the user migrated to new 'emails' and 'pubkeys' storagewith older version of the extension

tomholub
tomholub previously approved these changes May 15, 2021
Copy link
Collaborator

@tomholub tomholub left a comment

Choose a reason for hiding this comment

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

Thanks!

@tomholub
Copy link
Collaborator

We have added a requirement for authentication on the backend where there wasn't any. Some mock tests were forwarding requests to real backend which no longer works. I'm updating those tests.

tomholub
tomholub previously approved these changes May 15, 2021
tomholub
tomholub previously approved these changes May 15, 2021
@tomholub tomholub enabled auto-merge (squash) May 15, 2021 17:55
@tomholub tomholub merged commit 71bfc79 into master May 15, 2021
@tomholub tomholub deleted the issue-3579-revocations branch May 15, 2021 18:19
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 this pull request may close these issues.

Make sure the key remains revoked when trying to update with older version of it

2 participants