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

[FEATURE REQUEST] match.compound runner #63278

Closed
lkubb opened this issue Dec 9, 2022 · 2 comments · Fixed by #64302
Closed

[FEATURE REQUEST] match.compound runner #63278

lkubb opened this issue Dec 9, 2022 · 2 comments · Fixed by #64302
Labels
Bug broken, incorrect, or confusing behavior Feature new functionality including changes to functionality and code refactors, etc. needs-triage security issues and PRs for the Security Working Group

Comments

@lkubb
Copy link
Contributor

lkubb commented Dec 9, 2022

Is your feature request related to a problem? Please describe.
The x509 modules support signing policies that can be restricted to specific minions. These restrictions can be specified either by globbing on the minion ID or as a compound matcher expression. If you opt for the latter, the CA server queries the minion itself if it matches the expression. This can be a security risk if the minion is compromised.

If someone called you, saying: "Hey, I'm your bank, please send me a copy of your passport!" and you would verify their claim by calling back on the same number and asking them, "Are you really my bank?", you will be vulnerable to scammers. "Sure, mate. Now gib ID."

Describe the solution you'd like
Implement a match.compound runner that the CA server can query to verify the minion (the master is trusted by definition).

Describe alternatives you've considered
Not use compound matcher expressions for restricting signing policies.

@lkubb lkubb added Feature new functionality including changes to functionality and code refactors, etc. needs-triage labels Dec 9, 2022
@OrangeDog
Copy link
Contributor

I would categorise this as a security issue, rather than a feature request.

Though it's not generally possible to have "secure" compound matchers, because minions control what their grains are,.

@OrangeDog OrangeDog added Bug broken, incorrect, or confusing behavior security issues and PRs for the Security Working Group labels Dec 9, 2022
@lkubb
Copy link
Contributor Author

lkubb commented Dec 9, 2022

I would categorise this as a security issue, rather than a feature request.

Fine with me. It definitely is a security issue, but you would have to opt in by a) choosing to use compound expressions and b) allowing peer communication from the CA minion. Not sure if there is a warning in the docs currently, but my x509_v2 PR includes one. I would like to remedy that situation with this feature.

Though it's not generally possible to have "secure" compound matchers, because minions control what their grains are,.

Agreed, but this is also opt-in and should probably be warned against as well after this security issue has been remedied.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior Feature new functionality including changes to functionality and code refactors, etc. needs-triage security issues and PRs for the Security Working Group
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants