Skip to content

Implements support for ECDSA keys. Fixes #2163.#8431

Merged
adferrand merged 1 commit intocertbot:ecdsafrom
atombrella:ec_dsa_2163
Nov 4, 2020
Merged

Implements support for ECDSA keys. Fixes #2163.#8431
adferrand merged 1 commit intocertbot:ecdsafrom
atombrella:ec_dsa_2163

Conversation

@atombrella
Copy link
Copy Markdown
Contributor

@atombrella atombrella commented Nov 4, 2020 •
edited
Loading

Continued from #8254

I'm not sure about the conflict with run on existing certificates. I'm willing to have a look at it, but a few hints are appreciated.

I added some code as NamespaceConfig.key_type, but I had to wrap an try-except around it, since the tests were failing without it. Again, please hint if there's a better way to deal with this.

I'm hoping this will eventually be in a release of certbot.

Thanks to @pahrohfit and @Tomoyuki-GH for previous efforts to implement
suport for this.

Co-Authored-By: Robert Dailey rob@wargam.es
Co-Authored-By: Tomoyuki-GH 55397638+Tomoyuki-GH@users.noreply.github.com

Pull Request Checklist

  • If the change being made is to a distributed component, edit the master section of certbot/CHANGELOG.md to include a description of the change being made.
  • Include your name in AUTHORS.md if you like.

@adferrand
Copy link
Copy Markdown
Collaborator

adferrand commented Nov 4, 2020

I commit myself to keep the target branch ecdsa in sync with master until this PR is merged.

@atombrella
Copy link
Copy Markdown
Contributor Author

atombrella commented Nov 4, 2020

I commit myself to keep the target branch ecdsa in sync with master until this PR is merged.

Thanks. I think I addressed your comments from the original PR :) If not, we can do another round. I think there are also 1-2 issues from the ecdsa area that need to be handled before this lands in master.

adferrand
adferrand requested changes Nov 4, 2020
View reviewed changes
Copy link
Copy Markdown
Collaborator

@adferrand adferrand left a comment

Choose a reason for hiding this comment

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

I think that from my last review (#8254 (review)) the only remaining point is below.

Beside this, what is the problem of conflict with run on existing certificates you are referring to?

@atombrella
Copy link
Copy Markdown
Contributor Author

atombrella commented Nov 4, 2020

Beside this, what is the problem of conflict with run on existing certificates you are referring to?

Maybe I misunderstood the extend of this issue.
#8365

@adferrand
Copy link
Copy Markdown
Collaborator

adferrand commented Nov 4, 2020

Beside this, what is the problem of conflict with run on existing certificates you are referring to?

Maybe I misunderstood the extend of this issue.
#8365

Ok this was this issue. No problem, we can tackle that after this PR.

@atombrella @Tomoyuki-GH
Implements support for ECDSA keys. Fixes certbot#2163.
47c1045 
Thanks to @pahrohfit and @Tomoyuki-GH for previous efforts to implement
suport for this.

Co-Authored-By: Robert Dailey <rob@wargam.es>
Co-Authored-By: Tomoyuki-GH <55397638+Tomoyuki-GH@users.noreply.github.com>
@ivandeex ivandeex mentioned this pull request Nov 4, 2020
Closed
2 tasks
adferrand
adferrand approved these changes Nov 4, 2020
View reviewed changes
Copy link
Copy Markdown
Collaborator

@adferrand adferrand left a comment

Choose a reason for hiding this comment

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

And this LGTM!

@atombrella, thanks you a lot for your patience and your dedication to make ECDSA certificates live in Certbot.

(I merge this PR instead of squashing to ensure authorship is preserved. It will need to be the same for ecdsa to master).

@adferrand adferrand merged commit 198f5a9 into certbot:ecdsa Nov 4, 2020
This was referenced Nov 4, 2020
Closed
Closed
@atombrella atombrella deleted the ec_dsa_2163 branch November 5, 2020 07:30
This was referenced Nov 11, 2020
Closed
Merged
@HOSTED-POWER
Copy link
Copy Markdown

HOSTED-POWER commented Nov 17, 2020

In which certbot version can we expect ECDSA support? :)

@adferrand
Copy link
Copy Markdown
Collaborator

adferrand commented Nov 17, 2020

I have big hopes for the next one.

@adferrand
Copy link
Copy Markdown
Collaborator

adferrand commented Nov 19, 2020

The feature has been merged to master. It will be available for the next Certbot release!

@bmw bmw mentioned this pull request Nov 19, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adferrand adferrand adferrand approved these changes

Assignees

@adferrand adferrand

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@atombrella @adferrand @HOSTED-POWER