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

Always return CSR as part of Resource #1593

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

relvira
Copy link

@relvira relvira commented Feb 17, 2022

Return the generated CSR in the certificate Resource after an Obtain or ObtainForCSR.

There are cases where users of this library will like to perform some additional operations with the generated CSR after an obtain was run, for example: submitting the CSR to an external tool that monitors certificate-transparency logs, in order to make sure the certificate issued by the library is legitimate.

I am not very familiar with this codebase so I might've missed something. I updated existing tests to make sure the CSR in Resource exists.

@ldez Please let me know if there's anything else I can do to speed up this work (or if there's something terribly wrong with my approach).

Thank you.

@ldez
Copy link
Member

ldez commented Feb 18, 2022

Hello,

I think you already have all the information to re-create the CSR:

certcrypto.GenerateCSR(privateKey, commonName, san, mustStaple)
  • privateKey is your private key
  • commonName is the first domain
  • san is your domains
  • mustStaple is a user choice

The only case that I see for your PR is when you don't provide explicitly a private key.

Am I missing something? 🤔

@relvira
Copy link
Author

relvira commented Feb 21, 2022

thanks @ldez for your comment, I will definitely give that a try and report back. Wouldn't it make sense to have the CSR returned as part of the certificates struct anyway? a CSR is always generated and it was slightly confusing to have the attribute available in the struct but it was empty after all.

Looking forward to hear your thoughts!

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

Successfully merging this pull request may close these issues.

2 participants