Skip to content

Cluster privilege for proxied PKI#44862

Merged
albertzaharovits merged 15 commits intoelastic:proxied-pkifrom
albertzaharovits:security-pki-delegation-cluster-privilege
Aug 7, 2019
Merged

Cluster privilege for proxied PKI#44862
albertzaharovits merged 15 commits intoelastic:proxied-pkifrom
albertzaharovits:security-pki-delegation-cluster-privilege

Conversation

@albertzaharovits
Copy link
Copy Markdown
Contributor

This adds the delegate_pki cluster privilege that grants permissions to run the delegate-pki and invalidate-token APIs. The delegate_pki is not part of the manage, manage_security or any other cluster privilege apart from all.

It also adds the privilege to the kibana_system role.

Please review #44561 first, as this needs tests that rely on the REST handler to exist.

@albertzaharovits albertzaharovits added >enhancement WIP :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) labels Jul 25, 2019
@albertzaharovits albertzaharovits self-assigned this Jul 25, 2019
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-security

@albertzaharovits
Copy link
Copy Markdown
Contributor Author

@tvernum @bizybot this is ready for review as I have added tests. There will be merge conflicts with #44561 but I'll deal with them later.

private static final Automaton ALL_CLUSTER_AUTOMATON = patterns("cluster:*", "indices:admin/template/*");
private static final Automaton MANAGE_AUTOMATON = minusAndMinimize(ALL_CLUSTER_AUTOMATON, MANAGE_SECURITY_AUTOMATON);
private static final Automaton MANAGE_AUTOMATON = minusAndMinimize(minusAndMinimize(ALL_CLUSTER_AUTOMATON, MANAGE_SECURITY_AUTOMATON),
DELEGATE_PKI_AUTOMATON);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd prefer we have something like:

Automaton ALL_SECURITY_ACTIONS = patterns("cluster:admin/xpack/security/*");
Automaton MANAGE_SECURITY_AUTOMATON = minusAndMinimize(ALL_SECURITY_ACTIONS, DELEGATE_PKI_AUTOMATON);
Automaton MANAGE_AUTOMATON = minusAndMinimize(ALL_CLUSTER_AUTOMATON, ALL_SECURITY_ACTIONS); 

Copy link
Copy Markdown
Contributor Author

@albertzaharovits albertzaharovits Aug 5, 2019

Choose a reason for hiding this comment

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

I went with your suggestion. Thanks for the details!

One idea I had in mind was to have this cluster action be named completely different, for example cluster:authenticate/pki/delegate. The reason is that it doesn't fit under the admin namespace (the user doing delegation could very well not be an "admin"), and it avoids possible misconfiguration when users define privileges by patterns. But I backtracked because of a lack of a good name. If you feel this is a good idea and have a name suggestion, we could give it a try.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't have a good suggestion. I think putting it under admin is OK (but I agree with your attempts to find an even better place for it).

@albertzaharovits
Copy link
Copy Markdown
Contributor Author

Thanks for the review @tvernum ! This is ready for another look.

Copy link
Copy Markdown
Contributor

@tvernum tvernum left a comment

Choose a reason for hiding this comment

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

LGTM

@albertzaharovits
Copy link
Copy Markdown
Contributor Author

@elasticmachine test this please

@albertzaharovits
Copy link
Copy Markdown
Contributor Author

ping @bizybot

@albertzaharovits
Copy link
Copy Markdown
Contributor Author

@elasticmachine test this please

1 similar comment
@albertzaharovits
Copy link
Copy Markdown
Contributor Author

@elasticmachine test this please

@albertzaharovits
Copy link
Copy Markdown
Contributor Author

@elasticmachine run elasticsearch-ci/2

Copy link
Copy Markdown
Contributor

@bizybot bizybot left a comment

Choose a reason for hiding this comment

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

LGTM, Thank you.

@albertzaharovits albertzaharovits merged commit 1d6e31b into elastic:proxied-pki Aug 7, 2019
@albertzaharovits albertzaharovits deleted the security-pki-delegation-cluster-privilege branch August 7, 2019 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>enhancement :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants