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

Improve CRD fields for specifying key requirements #203

Open
SgtCoDFish opened this issue Feb 20, 2023 · 0 comments
Open

Improve CRD fields for specifying key requirements #203

SgtCoDFish opened this issue Feb 20, 2023 · 0 comments

Comments

@SgtCoDFish
Copy link
Member

We discussed curve parameters and checking for curves in standup earlier, and I said I'd look into approver-policy because checking curves is an important part of that. This issue documents what I found.

approver-policy uses just the field size when checking ECDSA curves, which isn't ideal since it opens the possibility of a cert using a bizarre non-standard curve over a field of the same size yet being accepted.

As a user of approver-policy, 99.999999% of the time I don't want to allow this. I want to ensure that certs use one of the standard curves.

More than that, though, the whole API for policy around key types isn't ideal.

It makes sense to allow "minSize" and "maxSize" for RSA, but those are largely meaningless for the other 2 key types.

For Ed25519 there's no meaningful other parameter to check, and for ECDSA it would be better to check for exact named curves for reasons given above.

Checking named curves obviously means that if a new curve is added we'd need to update approver-policy to support it, but that seems a worthwhile tradeoff.

This section of the CRD might be better if it had different properties for each curve type, e.g.:

RSA

    privateKey:
      algorithm: RSA # unchanged, nothing wrong with this
      minSize: 2048
      maxSize: 4096

ECDSA

    privateKey:
      algorithm: ECDSA
      allowedCurves: ["P-256", "P-384"] # don't allow P521 or any possible future curves

EdDSA

    privateKey:
      algorithm: Ed25519
      # nothing else makes sense here
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

No branches or pull requests

1 participant