Skip to content

Allow to import unarmored S/MIME certificate#3568

Merged
tomholub merged 4 commits intomasterfrom
issue-3559-improved-cert-import
Apr 18, 2021
Merged

Allow to import unarmored S/MIME certificate#3568
tomholub merged 4 commits intomasterfrom
issue-3559-improved-cert-import

Conversation

@rrrooommmaaa
Copy link
Contributor

@rrrooommmaaa rrrooommmaaa commented Apr 16, 2021

This PR allows to import a certificate in DER format
in addition to already implemented PEM and PKCS#12 PFX formats

issue #3559


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

@rrrooommmaaa
Copy link
Contributor Author

I'll also add advanced email extraction to completely support the provided certificate.
The e-mail there isn't stored in "Subject", rather in "Subject Alternative Name" in the format
"RFC822 Name=..."

@tomholub
Copy link
Collaborator

Excellent!

const emails = SmimeKey.getNormalizedEmailsFromCertificate(certificate);
const key = {
type: 'x509',
id: certificate.serialNumber.toUpperCase(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm afraid we can't use serialNumber as fingerprint.
Should we open a new task for this as well as include migration to re-calculate ids?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Created an issue to discuss

@rrrooommmaaa rrrooommmaaa marked this pull request as ready for review April 18, 2021 09:56
@rrrooommmaaa
Copy link
Contributor Author

We can merge this, but the original issue is still unresolved due to serialNumber vulnerability

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 tomholub merged commit fc3904f into master Apr 18, 2021
@tomholub tomholub deleted the issue-3559-improved-cert-import branch April 18, 2021 19:11
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.

2 participants