Skip to content

Conversation

@invidian
Copy link
Contributor

@invidian invidian commented Oct 3, 2019

This PR adds support for generating private keys using ed25519 algorithm.

NOTE: I wasn't sure what PEM header to use for this certificate. Since my usecase is to generate SSH keys, I used OPENSSH PRIVATE KEY for now and also the output of PEM private key is currently in SSH-compatible format. Please guide me how this should be handled, as I think this output is valuable. Maybe we could have new computed property private_key_openssh with properly formatted all keys for SSH? BTW this problem does not occur with RSA and ECDSA keys, since sshd accepts those keys with output provided by this resource.

Also included some basic unit tests, please let me know if more conditions should be added.

Refs #26

Copy link

@jacobwgillespie jacobwgillespie left a comment

Choose a reason for hiding this comment

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

I'm excited about this PR and the potential for generating all necessary host SSH keys with Terraform! 🎉 I'm not a repo committer, but had one comment when reading the diff.

@ghost ghost added size/L and removed size/XXL labels Oct 4, 2019
@invidian
Copy link
Contributor Author

invidian commented Nov 5, 2019

Any feedback on this one?

@Andor
Copy link

Andor commented Dec 3, 2019

@invidian I'm also not a committer, but I have a suggestion: maybe you can also update the documentation? To be more specific: website/docs/r/private_key.html.md file.

@invidian
Copy link
Contributor Author

invidian commented Dec 4, 2019

Good idea, I'll add it. Though still there is not much activity in this repository it seems...

@ghost ghost added the documentation label Dec 4, 2019
@Andor
Copy link

Andor commented Dec 5, 2019

@invidian maybe add a remark about ec25519 for public_key_openssh attribute also?

@invidian
Copy link
Contributor Author

invidian commented Dec 5, 2019

@Andor thanks for suggestion. I also added a mention to private_key_pem.

@invidian
Copy link
Contributor Author

After going trough the whole X.509, PEM, DER topics, I looked into this again and I changed the PR a bit. This provider it focused on generating certificates for TLS communication (so X.509) and OpenSSH support is somehow an addition. In this case private_key_pem should always produce OpenSSL-compatible output, which is not the case.

I pushed one more commit, which adds private_key_openssh attribute, which always contains the private key in OpenSSH-compatible format. That means RSA and ECDSA keys are still written in PEM format, while ED25519 is written in OpenSSH private key format, as OpenSSH does not support ED25519 private key in PEM format (I didn't confirm in the code and I couldn't find it in OpenSSH documentation though...).

This way both use-cases are covered: generating private keys for TLS and for OpenSSH.

@jackivanov
Copy link

Any insights on when this PR might be merged? cc @apparentlymart

@RobCannon
Copy link

@appilon Can you merge this commit and release please?

@PascalBourdier
Copy link

@wjam @kmoe this PR seems legit ? no ?

@invidian
Copy link
Contributor Author

Now that Terraform 0.13 is here with Terraform Registry, I'm going to fork and push my copy there, stay tuned 😄

@invidian
Copy link
Contributor Author

terraform {
  required_providers {
    tls = {
      source = "invidian/tls"
      version = "2.2.1"
    }
  }
}

💥

This commit add support for ED25519 algorithm when generating
tls_private_key resource.

Refs #26

Signed-off-by: Mateusz Gozdek <[email protected]>
This commit adds private_key_openssh attribute, which always contains
private key in format, which is compatible with OpenSSH.

This allows to produce ED25519 private key in OpenSSL compatible format
in private_key_pem attribute and OpenSSH-compatible format in this new
attribute.

Other key types are the same in private_key_pem and private_key_openssh,
as OpenSSH can read them. In the future, this could be changed to produce
all private keys OpenSSH native format.

Refs #26

Signed-off-by: Mateusz Gozdek <[email protected]>
Base automatically changed from master to main February 1, 2021 17:28
@RobCannon
Copy link

This PR still hasn't been picked up. I have been using invidian's provider for a while, but there is no reason why this PR hasn't been added to the official provider. @kmoe are you the person to look at this?

@1ace
Copy link

1ace commented Jun 29, 2021

@bflad & @kmoe you guys have merged all the PRs that have been merged in the last year, so I'm assuming you are the ones to ping for this one that seems to have fallen through the cracks?

@grahamc
Copy link

grahamc commented Dec 9, 2021

@invidian Thank you for this PR! Would you be up for rebasing it and releasing an updated version?

@invidian
Copy link
Contributor Author

invidian commented Dec 9, 2021

@invidian Thank you for this PR! Would you be up for rebasing it and releasing an updated version?

You mean in my fork, right? I've just enabled issues for my fork's repository, maybe you can create an issue there, but I'll try to follow up myself. Any particular reason why a new release would be needed?

@grahamc
Copy link

grahamc commented Dec 9, 2021

Ah, it looks like a rebase really isn't getting anything, sorry -- I thought I remembered more being in 3.1.0. A release would be great :).

@invidian
Copy link
Contributor Author

invidian commented Jan 3, 2022

Released 3.1.0-ed25519, rebased on latest version of this repo, for everyone interested :)

@detro
Copy link
Contributor

detro commented Feb 17, 2022

Thank you for your time and contribution, we really appreciate it. As part of a bigger effort to add complete support for ED25519 key algorithm, I’m closing this in favor of issue #150. Please refer to the new issue for what will be included and how work will proceed.

@detro detro closed this Feb 17, 2022
@invidian invidian deleted the ed25519-support branch February 17, 2022 11:26
@invidian invidian restored the ed25519-support branch February 17, 2022 11:27
@detro detro self-assigned this Mar 24, 2022
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 28, 2024
@invidian invidian deleted the ed25519-support branch May 28, 2024 09:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants