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

Allow Deprecation Info API to check for deprecated security configuration #47714

Open
bizybot opened this issue Oct 8, 2019 · 17 comments
Open
Labels
:Data Management/Stats Statistics tracking and retrieval APIs >enhancement :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC Team:Data Management Meta label for data/management team Team:Security Meta label for security team >upgrade

Comments

@bizybot
Copy link
Contributor

bizybot commented Oct 8, 2019

We are in the process of deprecating few Index privilege names that are currently in use. These may or may not have an alternative that is an exact match and so it could be breaking change when few of the deprecated privilege names are removed in future versions.

The index privilege names are referred to when creating a role for the user. The roles are indexed in the .security index and sometimes defined in the file.

The current deprecation info API captures information for cluster_settings, node_settings, index_settings, ml_settings. This seems to be more tied to Settings deprecation and maybe we could enhance this API to include additional deprecation information other than settings like the use of deprecated index privilege names in the roles from .security index or role file.

This issue exists to find if the current deprecation info API can be used for this purpose or if there are any other suggestions to handle this.

Relates: #47333

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security (:Security/Authorization)

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (:Core/Features/Features)

@jasontedor
Copy link
Member

@bizybot I am making an assumption here that when these role names are used, we would emit a deprecation warning and log it. In which case, see #46106 and #46107. That’s going to be our general mechanism for exposing deprecated functionality without having to work too hard to acquire this information otherwise. Can you clarify if my assumption is correct and if what I propose in #46106 and #46107 would be acceptable as general infrastructure without having to build specific infrastructure for the situation here?

@bizybot
Copy link
Contributor Author

bizybot commented Oct 8, 2019

Hi @jasontedor, Thank you for your comment. Indeed what you propose will help to showcase the deprecated usage of privilege names and that would be helpful for the user.
But the deprecation logging would happen only when the privilege names are referred mostly during authorization, so if the user does not log in and perform any action, the deprecation logs would be missing for this usage.

I assume that the Upgrade assistant acts on the info presented by deprecation info API (I might have the wrong assumption here), for example, if we scan .security index and find the roles that need to be migrated as they are using deprecated privilege names and then Upgrade assistant would look at that and invoke some migrate endpoints to perform migrations.
Would this be possible with the suggestions from #46106? I see that we will be collecting the deprecation logs, but unsure how it will be used by the upgrade assistant.

@jasontedor
Copy link
Member

But the deprecation logging would happen only when the privilege names are referred mostly during authorization, so if the user does not log in and perform any action, the deprecation logs would be missing for this usage.

It's not clear to me why we need to emit a deprecation warning for each user, but rather when we parse the role descriptors. Can you explain?

I assume that the Upgrade assistant acts on the info presented by deprecation info API (I might have the wrong assumption here), for example, if we scan .security index and find the roles that need to be migrated as they are using deprecated privilege names and then Upgrade assistant would look at that and invoke some migrate endpoints to perform migrations.

The upgrade assistant offers suggestions that the user can employ to migrate from deprecated functionality. If it's possible to automatically upgrade instances in the .security index, we should do that internal to Elasticsearch automatically on behalf of the user, rather than expecting that they use the upgrade assistant to do so.

@gwbrown
Copy link
Contributor

gwbrown commented Oct 8, 2019

To clarify the technical aspects of this a bit: Each type of deprecation check (Node, Cluster, Index, ML) has a framework that takes each check as a function object, and passes it the appropriate type of metadata (IndexMetaData, ClusterState, etc.). Because role information is not stored in the cluster state, but rather can be in a combination of the .security index and file-based roles, we would need a new piece of infrastructure to retrieve the appropriate information from Security to allow checks to be written against it.

If we can automatically upgrade roles, we should do that, and ignore the rest of this post.

If we can't handle the necessary changes automatically, I think putting these checks in the Deprecation Info API has a big advantage: it allows users to interactively query the API to make sure they've fixed everything that needs fixing, as opposed to having some uncertainty about that - I mean, I'm not sure what would trigger deprecation logs for deprecated role names, and I'm an ES developer. Is it on node startup? Does it require a full cluster restart? If I was administrating a cluster I would feel very uncertain looking for a lack of deprecation logs to make sure I'd fixed the issue, which would be very frustrating given how much a broken security configuration can ruin your day.

To be clear: I'm absolutely for integrating the deprecation logs into the upgrade assistant. But I think for this case, if it's not too difficult, we should make this possible to check in the deprecation info API directly (assuming we can't just auto-fix it).

@jasontedor
Copy link
Member

@gwbrown I think that there is potential for the roles defined in .security to be automatically upgraded (we need someone from the security team to confirm), but not those that are on disk in the roles file (because we aren't going to mutate those).

@bizybot
Copy link
Contributor Author

bizybot commented Oct 9, 2019

@jasontedor @gwbrown

It's not clear to me why we need to emit a deprecation warning for each user, but rather when we parse the role descriptors. Can you explain?

Sorry I did not elaborate on my comment. Yes, you are correct that the deprecation logging would happen when we parse the role descriptors. The role descriptors are loaded when the user authenticates and the authorization engine loads the role descriptors for the role name(s).
So there is a possibility that the deprecation warning might be missing for some role names if the user with the role never logs in.

If we can automatically upgrade roles

I think we would not be able to automatically upgrade all the role descriptors, the reason being the source for the role descriptors can either be a file, .security index or custom roles provider.
Also, the deprecated role names might not have an exact replacement to take an automatic decision for migration and would need the user's inputs. +1 on the avoiding mutation of the file.

The integration of deprecation logs into the upgrade assistant will definitely help to showcase the warnings though there is an advantage in the deprecation info API as it can always parse the deprecations at one go from the .security and file roles store that allows users to see if there are any warnings related to it, instead of waiting for it be populated in the deprecation logs.

Note that both the ways are not going to emit all the deprecated usage there will be some misses for example if there is a custom roles provider deprecation info check function would not be able to parse the role descriptors generated by it or deprecation logging missing sometimes if the user does not log in.

@tvernum
Copy link
Contributor

tvernum commented Oct 9, 2019

rather when we parse the role descriptors

I think the missing context is that we only load & parse index-based role descriptors when they are referenced in some way. That could be that a user with the role authenticates, or someone calls GET /_security/roles, but it is entirely normal for one or more roles to go for some period of time without being loaded/parsed.
We could forcibly load all role descriptors on node startup (or master change, or ...) but we don't currently do that.

I think that there is potential for the roles defined in .security to be automatically upgraded

I think we can, but I'm not sure we should, or at least I can't decide when we should do that.
My reasoning is:

  • I think we should deprecate the old names as soon as we define their replacements so that it's clear why there are two sets of names and which ones should be used. That is likely to be in a minor release.
  • For completeness, I would expect us to expose that deprecation in a number of places including the Roles UI, the Roles API, and deprecation logs
  • We could automatically upgrade the security index in that release but it has some implications that are potentially confusing:
    1. It would mean that installing a minor upgrade would cause sudden, possibly unexpected, changes to their roles. This is likely to confuse users who are familiar with their existing privilege names and suddenly see that things are different.
    2. We would need to perform this same translation whenever the PUT /_security/role/{name} API was called so that the new/updated role didn't put the index back into a deprecated state. This would mean that PUT + GET would return a different JSON, which might cause problems for tooling.
  • My gut feel is that users would prefer that we didn't apply automatic conversions of security data without them opting-in (especially in a minor).
  • It would be reasonable to automatically upgrade the roles during a major upgrade when we remove the old names.
  • However, giving users deprecation warnings in a minor, but not giving them the tools to fix them automatically until a major seem wrong. Some users will manually fix the roles when they see the warnings, only to find that there was a tool for that later in the release cycle.

Given the above, my thoughts were that the tool would be available, opt-in during the minor and then automatic at the major.

not those that are on disk in the roles file (because we aren't going to mutate those).

Correct.

custom roles provider

I don't know how much we can do for custom role providers. The custom roles extension API doesn't provide a way to list role only to get them by name, so it's impossible to know if an extension defines a role that uses a deprecated privilege name unless that role is referenced somewhere. The Upgrade Assistant won't be much help here since it won't be able to list custom roles that use deprecated privilege names.
My hope is that anyone who has a custom roles provider will test it when the recompile it for a major release, but I'm probably optimistic there.

deprecated role names might not have an exact replacement to take an automatic decision for migration

That's true in theory, but I don't think it is practically true in this case (per #47333 (comment), I think every existing privilege can be replacted with one-or-more of the new privileges).

@bizybot
Copy link
Contributor Author

bizybot commented Oct 10, 2019

We could forcibly load all role descriptors on node startup

The Upgrade Assistant won't be much help here since it won't be able to list custom roles that use deprecated privilege names.

Given that we are considering #46106 which will integrate deprecation logging into upgrade assistant, this could be useful as we would be able to present all the deprecations usage most of the time (except for the custom roles).

We do not then need to modify the deprecation info API to collect the information from file & .security index if we forcibly load all the role descriptors at startup and do the deprecation logging.

@tvernum
Copy link
Contributor

tvernum commented Oct 11, 2019

We do not then need to modify the deprecation info API to collect the information from file & .security index if we forcibly load all the role descriptors at startup and do the deprecation logging.

If we take that approach then it is hard for the user to know whether they have fixed everything (per @gwbrown's earlier comment). They probably won't know what triggers the deprecation log, so won't know whether the lack of logging is definitive evidence that the problem is fixed or simply means that the logs haven't run.

I think we need a way for users to explicitly check that the problem is fixed and get an unambiguous 💚 or 🔴

@bizybot
Copy link
Contributor Author

bizybot commented Oct 20, 2019

I think from the discussions so far it is desirable to get point in time deprecation info for the usage of deprecated index privileges by scanning the file roles store and .security index.
This allows users/upgrade assistant to check whether the deprecation warnings have been addressed before proceeding with the upgrade.

We can provide migration API (this might require another discussion on how to achieve this as the upgrade API is removed) for .security index roles and for file-based roles, the users could go and correct the usage in the file. The deprecation info API invocation allows the user to verify whether the roles have been migrated or not after running the migration step.

It seems like in this case it would be good to add the infrastructure in the deprecation info API to collect this information and report it.

@gwbrown @jasontedor thoughts? Thank you.

@gwbrown gwbrown self-assigned this Oct 22, 2019
@gwbrown
Copy link
Contributor

gwbrown commented Oct 22, 2019

I'll do an in-depth look at what will be required to support this in the Deprecation Info API shortly, and report my findings back here. With luck, it will be relatively straightforward. I may come back with questions about Security infrastructure/classes/etc. as part of that process.

@gwbrown
Copy link
Contributor

gwbrown commented Oct 30, 2019

I've done some initial investigation into this and it doesn't look like it'll be too difficult. There's a bit of a wrinkle in that the file-based roles aren't retrievable via the role management APIs, but that shouldn't be too difficult to handle. It looks straightforward to add an internal transport action that can retrieve the file-based roles from each node, somewhat similar to how the node-specific deprecation checks are executed (although in this case it will ship the role definitions to the coordinating node). Does that sound reasonable to the Security folks on this thread?

I would have preferred having each node do the evaluation itself so that we could leverage the existing node-specific deprecation check infrastructure, but that isn't straightforward due to deprecation checks and the necessary Security classes living in different plugins.

I'll have a POC PR up soon, although at this point it may be after EAH.

@rjernst rjernst added Team:Data Management Meta label for data/management team Team:Security Meta label for security team labels May 4, 2020
@gwbrown gwbrown removed their assignment Feb 9, 2021
@joegallo
Copy link
Contributor

I'm removing the team-discuss label from some older Team:Data Management issues -- we've had plenty of time to discuss them, but we haven't, so the label isn't serving its purpose. Feel free to delete this comment and/or re-add the team-discuss label.

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-security (Team:Security)

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

@dakrone dakrone added :Data Management/Stats Statistics tracking and retrieval APIs and removed :Data Management/Other labels Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/Stats Statistics tracking and retrieval APIs >enhancement :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC Team:Data Management Meta label for data/management team Team:Security Meta label for security team >upgrade
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants