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.

Sign up for GitHub

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

Jump to bottom

[GDS Client] Fix Certificate Request when private Key of existing Certificate is not exportable #607

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

[GDS Client] Fix Certificate Request when private Key of existing Certificate is not exportable #607

wants to merge 8 commits into from

Conversation

romanett
Copy link
Contributor

Proposed changes

The GDS Client uses a Certificate Signing Request (CSR) when a new CA signed Certificate is requested from the GDS and an older cert exists.

The CSR can be executed however applying the existing private key to the new Certificate fails when the old Certificates private Key is not exportable. This is the case e.g. for many certs in the Windows X509 Certificate Store.

With this fix the GDS Client uses a KeyPair Request instead of a Certificate Signing Request to request a new CA signed Cert from the GDS, when the private key of the existing cert is not exportable.

Related Issues

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • Enhancement (non-breaking change which adds functionality)
  • Test enhancement (non-breaking change to increase test coverage)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected, requires version increase of Nuget packages)
  • Documentation Update (if none of the other choices apply)

Checklist

  • I have read the CONTRIBUTING doc.
  • I have signed the CLA.
  • I ran tests locally with my changes, all passed.
  • I fixed all failing tests in the CI pipelines.
  • I fixed all introduced issues with CodeQL and LGTM.
  • I have added tests that prove my fix is effective or that my feature works and increased code coverage.
  • I have added necessary documentation (if appropriate).
  • Any dependent changes have been merged and published in downstream modules.

Further comments

@romanett romanett requested a review from mregen May 12, 2024 10:32
@romanett romanett self-assigned this May 12, 2024
@sveinfolkeson sveinfolkeson mentioned this pull request Jun 5, 2024
Open
5 tasks
@romanett romanett requested review from mregen and removed request for mregen July 2, 2024 15:44
@sveinfolkeson
Copy link

If the configuration file specifies the store for the ApplicationCertificate to be StoreType=Directory, then the public and private key are stored in the certs and private folder. If I recreate an existing certificate with this configuration the public and private key will be added to theses folders. Would it not be better that the existing public and private key are replaced with the new ones?

@romanett
Copy link
Contributor Author

@mregen i am not shure about replacing existing certs, what is your opinion?

@mregen
Copy link
Contributor

mregen commented Sep 13, 2024

If the configuration file specifies the store for the ApplicationCertificate to be StoreType=Directory, then the public and private key are stored in the certs and private folder. If I recreate an existing certificate with this configuration the public and private key will be added to theses folders. Would it not be better that the existing public and private key are replaced with the new ones?

It depends, it could be that the cert was signed. Then it should not be updated. But we should definitely consider the option to delete the old certificate. Problem for self signed certs is that the client/server may loose trust with our applications.

@mregen
Copy link
Contributor

mregen commented Sep 13, 2024

The preference is always to use a CSR to avoid that a private key must be sent over the wire.
In this case when a private key is not exportable, I would create a new public/private key pair for the X509Store certificate, create the CSR with it, when it comes back merge it with the signed certificate and store it as not exportable in the X509Store.
Since the process for signing the cert may take some time, the question is how to save the private key in the meantim, e.g. keep it in memory until the cert is Updated. If the application is restarted the signing process must be repeated.

@romanett
Copy link
Contributor Author

@mregen @sveinfolkeson please see my updated PR that now creates a tempoary cert in memory to create a csr from that. I still need to test if a certificate update is handled correctly, but this will take some more time.

@romanett romanett marked this pull request as draft October 22, 2024 15:37
@romanett romanett marked this pull request as ready for review October 26, 2024 07:49
@romanett
Copy link
Contributor Author

romanett commented Oct 26, 2024 •
edited
Loading

Test is successful. GDS Client can now sucessfully handle non exportable Cert in X509 store and recreate the private key:
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mregen mregen Awaiting requested review from mregen

Assignees

@romanett romanett

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CertificateFactory.CreateCertificateWithPrivateKey(newCert, OldCertificate) returns error in private key
3 participants
@romanett @sveinfolkeson @mregen