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 signed key constraints to SSH CA #4468

Closed
wants to merge 2 commits into from
Closed

Add signed key constraints to SSH CA #4468

wants to merge 2 commits into from

Conversation

broamski
Copy link
Contributor

Adds the ability to enforce particular ssh key types and minimum key lengths when using Signed SSH Certificates via the SSH Secret Engine.

e.g.

  "allowed_user_key_specs": {
      "rsa": 4096,
      "ecdsa": 521,
  }

@jefferai jefferai added this to the 0.10.2 milestone May 19, 2018
@jefferai
Copy link
Member

Maps are super unfriendly for CLI users so this needs to be rethought. Probably it should follow the key_type/key_bits values from the PKI backend.

@jefferai jefferai modified the milestones: 0.10.2, 0.10.3 May 25, 2018
@jefferai jefferai modified the milestones: 0.10.3, 0.10.4 Jun 11, 2018
@chrishoffman chrishoffman modified the milestones: 0.10.4, 0.10.5 Jul 19, 2018
@jefferai jefferai modified the milestones: 0.10.5 , 0.11, next-release Aug 13, 2018
@chrishoffman chrishoffman modified the milestones: next-release, 0.12 Oct 9, 2018
@vishalnayak
Copy link
Contributor

@broamski

We discussed about this internally and agreed that accepting maps in this case is okay, given that there are 2 other fields that accept map inputs. As Jeff mentioned, it is a pain to work with map inputs in the CLI. However, the docs provide some examples to work with map inputs in CLI (https://www.vaultproject.io/docs/secrets/ssh/signed-ssh-certificates.html).

SignedKeyConstraints is broader in scope for the functionality it is doing. Assuming that it is named so to accommodate newer constraints within itself, it is unclear as of yet how other kinds of constraints look. Hence, we concluded that it is better to rename the field as allowed_user_key_lengths and confine the behavior to just that. Newer constraints, if they show up, can go under respective newer fields.

Currently the values specified impose length limits. We may want to change that to impose exact lengths (by changing >= to =). This way, it is easier to reason about the role specifications and the behavior will be consistent with other knobs.

@briankassouf briankassouf modified the milestones: 1.0, 1.0.1 Nov 28, 2018
@jefferai jefferai modified the milestones: 1.0.1, 1.0.2 Dec 12, 2018
@chrishoffman chrishoffman requested a review from catsby January 7, 2019 17:30
@jefferai jefferai modified the milestones: 1.0.2, 1.0.3 Jan 14, 2019
@hashicorp-cla
Copy link

hashicorp-cla commented Jan 15, 2019

CLA assistant check
All committers have signed the CLA.

@jefferai jefferai modified the milestones: 1.0.3, 1.0.4 Feb 1, 2019
@catsby
Copy link
Contributor

catsby commented Feb 7, 2019

Closing in favor of #6030

@catsby catsby closed this Feb 7, 2019
@briankassouf briankassouf modified the milestones: 1.0.4, 1.0.3 Mar 14, 2019
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.

7 participants