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 role-based health checks to PKI Health Check #17877

Merged
merged 10 commits into from
Nov 17, 2022

Conversation

cipherboy
Copy link
Contributor

@cipherboy cipherboy commented Nov 10, 2022

Based on #17750 and #17865; will be rebased once they merge.

This adds the three role-based health checks:

  • role_allows_localhost, for whether or not a role has both a non-empty allowed_domains list and allow_localhost=true.
  • role_allows_glob_wildcards, for whether or not a role allows both glob domains and wildcard certs, which is potentially unsafe.
  • role_no_store_false, for whether or not a role has roles with no_store=false and no auto-rebuild of CRLs, as this can cause performance issues.

@cipherboy cipherboy force-pushed the cipherboy-add-role-health-checks branch from f341ce1 to a1e7c14 Compare November 16, 2022 20:49
@cipherboy cipherboy requested review from stevendpclark, kitography and a team November 16, 2022 20:49
@cipherboy cipherboy added this to the 1.13.0-rc1 milestone Nov 16, 2022
@cipherboy cipherboy marked this pull request as ready for review November 16, 2022 20:49
@cipherboy
Copy link
Contributor Author

cipherboy commented Nov 16, 2022

Will be rebased once #17984 is merged. #17984 no longer has invasive changes so this should be stand-alone now, though semgrep will fail with the two findings #17984 will fix, I've added fixes for the remainder in-tree.

Signed-off-by: Alexander Scheel <[email protected]>
Signed-off-by: Alexander Scheel <[email protected]>
Copy link
Contributor

@stevendpclark stevendpclark left a comment

Choose a reason for hiding this comment

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

Overall +1, just a few nits and a question or two to validate my understanding.

Signed-off-by: Alexander Scheel <[email protected]>
Signed-off-by: Alexander Scheel <[email protected]>
Copy link
Contributor

@stevendpclark stevendpclark left a comment

Choose a reason for hiding this comment

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

lgtm, thanks for the tweaks.

@cipherboy
Copy link
Contributor Author

Thanks! Merging...

@cipherboy cipherboy merged commit ba73453 into main Nov 17, 2022
@cipherboy cipherboy deleted the cipherboy-add-role-health-checks branch December 1, 2022 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants