-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Allow generation of other types of SSH CA keys #14008
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall +1 from me, a few little nits on the specific PR and I'll let others discuss/decide about the UI parts of this PR, hence the lack of approval from my part.
f969810
to
282125a
Compare
282125a
to
3d02735
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Needs docs and changelog though.
@cipherboy Thanks! Just wanting to make sure I understand the desired behaviour here: we want to enable a user to select the key type first (from ssh-rsa, ecdsa-sha2-nistp256, ecdsa-sha2-nistp384, ecdsa-sha2-nistp521, or ssh-ed25519) and then, based on that, enter the key bits. Is that correct? Should key bits be an open input, or are there only specific values that are allowed for each key type? |
Hey @ivana-mcconnell! I'd actually love to see something similar to those boxes in the PKI UI RFC doc where the user first selects if they're going to import a SSH CA (first two fields on that form) OR if they're going to generate a key. (Or put them under folding menus, you'd know better than me!). In the former case, they just need the top two fields (private/public key). In the latter case, they just need the two new fields. Yes, ideally a drop-down would be best for the key type; I tried getting that working with a select but failed. Presently key bits is only necessary when |
Once design (@ivana-mcconnell) has put together designs for this I'm happy to review your PR, or pair through implementing in the UI |
3d02735
to
d04f0c2
Compare
fmt errors all in unrelated files. Per discussion with @ivana-mcconnell, removed the fledgling UI :-) |
This adds two new arguments to config/ca, mirroring the values of PKI secrets engine but tailored towards SSH mounts. Key types are specified as x/crypto/ssh KeyAlgo identifiers (e.g., ssh-rsa or ssh-ed25519) and respect current defaults (ssh-rsa/4096). Key bits defaults to 0, which for ssh-rsa then takes a value of 4096. Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
d04f0c2
to
e360b0e
Compare
To bring better parity with the changes of hashicorp#14008, wherein we allowed OpenSSH-style key identifiers during generation. When specifying a list of allowed keys, validate against both OpenSSH-style key identifiers and the usual simplified names as well ("rsa" or "ecdsa"). Notably, the PKI secrets engine prefers "ec" over "ecdsa", so we permit both as well. Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Allow OpenSSH-style key type identifiers To bring better parity with the changes of #14008, wherein we allowed OpenSSH-style key identifiers during generation. When specifying a list of allowed keys, validate against both OpenSSH-style key identifiers and the usual simplified names as well ("rsa" or "ecdsa"). Notably, the PKI secrets engine prefers "ec" over "ecdsa", so we permit both as well. Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> * Fix missing quote in docs
Curious about general sentiment here; if it is positive, I'll add docs and a changelog and we can decide what to do about the UI (leave it as-is or drop it for future work).
See #11035 for context. We derive types from the
x/crypto/ssh
types (which match OpenSSH's constants), thus making key bits only relevant for RSA keys. We're notably short exporting the private key (for parity with PKI secrets engine).Resolves: #11035
This probably needs some additional UI work, but it works 😆
@ivana-mcconnell or @hellobontempo do either you have any thoughts here? It seems like the UI of SSH CA generation isn't at all like the PKI functionality; it looks like there's no dynamic fields (the hbs template needed to be updated manually) and lacks a lot of the
actions
for groups or changes &c. I couldn't figure out how to use a<Select />
component either; it appears to require anonChange
action (which we appear to lack completely) and even without it, theoptions
didn't populate (either manually or from the element's chosen values). Similarly failed via<FormFieldFromModel />
-- that didn't even display at all. :/Edit: Screenshot