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

Add regenerate-keys option to Issue command to always regenerate key pair #65

Closed
wants to merge 1 commit into from
Closed

Conversation

Ayesh
Copy link

@Ayesh Ayesh commented Nov 13, 2017

Hallo Niklas,
Thanks for this great acme client. I use it almost every day in many servers I manage, and it's working quite well!

Currently, acme-client reuses existing private key if one is present in the pointed storage directory. This is one difference between EFF's certbot; By default, certbot always regenerates a new private key on each renewal. With acme-client, one has to delete the existing private key or point to a different directory to make acme-client generate a new key. None of these options are convenient as a command line option.

I added one, and it seems to work OK at my end. I'm sending a PR hoping you could take a look, and perhaps merge if you think it suits.

Thank you 👍🎉

@kelunik
Copy link
Owner

kelunik commented Nov 13, 2017

Looks pretty fine. Do you want to add a corresponding option to auto as well?

$keyPair = (new OpenSSLKeyGenerator)->generate($bits);
$keyPair = (yield $keyStore->put($path, $keyPair));
}
else {
Copy link
Owner

Choose a reason for hiding this comment

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

Please move that to the previous line (} else {) and indent the code with four spaces.

@Ayesh
Copy link
Author

Ayesh commented Nov 13, 2017

Thanks for replying! I have force-pushed that commit with code styling changes. I'm sorry for not paying attention to code styling.

I'll also work on changes in the auto command.

@kelunik
Copy link
Owner

kelunik commented Dec 16, 2017

@Ayesh Do you still plan to work on auto?

@Ayesh
Copy link
Author

Ayesh commented Dec 16, 2017

I'm so sorry about the delay on my side. I have made some progress and will commit them later today. Thank you for the follow-up.

@kelunik kelunik closed this in 51acff5 Apr 15, 2018
@kelunik
Copy link
Owner

kelunik commented Apr 15, 2018

@Ayesh Implemented it in 51acff5 slightly differently. I deferred writing the new key until the certificate is successfully issued, so things won't break if issuance fails.

Thanks for your contribution.

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