Running Kibana Upgrade assistant from 6 to 7 makes security index non-restricted

[tvernum]

The Kibana upgrade assistant has the ability to upgrades 5.x format indices by reindexing them into a new name and then aliasing the old name to the new name.

It includes the ability to do this for .security-6 and the intent was that this would be the recommended way to upgrade a security index from 5.x to work on 7.x

However, this has some unintended consequences.

Scenario

• I created a 5.6.15 cluster and created users and roles.
• I ran the 5.6 Elasticsearch migration API to convert that v5 .security index into .security-6 with a .security alias.
• I copied the data and config to a new node running 6.7.0 BC3
• I started Elasticsearch and Kibana (also BC3) and ran the Kibana migration assistant on .watches-6 and .security-6

Result:

GET _cat/aliases
.triggered_watches .triggered_watches-6     - - -
.watches           .reindexed-v6-watches-6  - - -
.watches-6         .reindexed-v6-watches-6  - - -
.security          .reindexed-v6-security-6 - - -
.security-6        .reindexed-v6-security-6 - - -
.kibana            .kibana_1                - - -

This is all in keeping with how the upgrade assistant is designed to work. We now have .reindexed-v6-security-6 index, and the .security and .security-6 aliases point to it.

Issue

The problem is that the protections we have to prevent users from getting accidental access to the security index rely on a strict naming convention.
Since Elasticsearch security knows nothing of the Kibana migration assistant, it does not apply any special access restrictions to the .reindexed-v6-security-6 index.

Consequently a user with this role (or many others like it)

{
    "indices" : [
        { "names" : [ ".re*" ], "privileges" : [ "read" ] }
    ]
}

can read from the security index, and a user with write privileges would be able to do much more (e.g. reset passwords).

Solution

Broadly, there's 2 options

1. Update Elasticsearch to know that .reindexed-v6-security-6 is another possible name for the security index. This is not particularly hard, we just need to make sure we cover everywhere it's needed (e.g. RestrictedIndicesNames, but also XPackUser and maybe others)
2. Change the Kibana assistant to do something special for .security-6. It already has some knowledge about special indices - for example it stops Watcher before reindexing .watches-6

I also have a concern about the risk associated with having more than 1 possible name for the security index. As it stands, clusters that were originally upgraded from 5.x will have the .reindexed-v6-security-6 but clusters that upgraded from 6.x only, and clusters that were fresh 7.x installs will have .security-6.
Experience has shown that having too many variables like this leads to mistakes, which in the case of security can be mean quite significant problems.

Based on that, my preference would be something like:

• Change the Kibana assistant to rename .security-6 to .security-7
• Change Elasticsearch from 6.7 onwards to treat both .security-6 and .security-7 as restricted indices
• Change Elasticsearch from 7.0 onwards to name the internal security index as .security-7 whenever it needs to be created.

That seems to leave us in the lowest-risk end state (but I'm naturally cautious about such matters).

[elasticmachine]

Pinging @elastic/es-security

[tvernum]

Ping: @gwbrown @tylersmalley @joshdover

[droberts195]

This also has implications for 7.0 as (unlike 6.x) 7.x will have a migration assistant from day one to prepare for upgrade to version 8. elastic/kibana#30849 details a vaguely similar problem that affects the ML and Watcher indices in 7.0.

Logically extending the proposal in this issue description:

• Change the Kibana assistant in 7.x to rename .security-7 to .security-8
• Change Elasticsearch from 7.0 onwards to treat security-6, .security-7 and .security-8 as restricted indices

[tvernum]

Thanks @droberts195

That sounds like an issue. The 7.x upgrade assistant breaks the security index (#39018), we shouldn't be shipping with that enabled.

[joshdover]

Special handling for .security-6 to .security-7 in the Kibana Upgrade Assistant seems like a reasonable approach to me. For the Upgrade Assistant shipping in 7.0, do we want to disable reindexing of the .security-7 index to .security-8 for the time being?

I ask due to the discussion about special system indices that may happen in 8.0. Are we just creating more work for ourselves by allowing .security-8 to exist if we are considering an alternative approach to storing this type of data?

[joshdover]

As an aside, should we restrict reindexing of the security index in general? While unlikely, it's possible a user could reindex this index and use aliases and get their cluster into a broken or insecure state.

[tvernum]

Special handling for .security-6 to .security-7 in the Kibana Upgrade Assistant seems like a reasonable approach to me.

Great, I'll work on making the necessary changes to ES to treat .security-7 in a special way.

@albertzaharovits, since you've most recently worked on restrict indices, can you get up to speed on this issue and be ready to review my PR when it's up?

For the Upgrade Assistant shipping in 7.0, do we want to disable reindexing of the .security-7 index to .security-8 for the time being?

Yes, for 2 reasons

1. As we know it stops security feature from working. Sorry, I didn't realise the impact of that when it was first raised.
2. I think this would give users the impression that their security index was "v8 ready", but we don't know what the security index will look like in 8 (or even if it will exist). It is highly likely we will need a special, customised "upgrade security index" step to be ready for ES8, but we can't define that process yet. I don't want users to feel like they're being given a moving target.

As an aside, should we restrict reindexing of the security index in general

Typically we don't, because sometimes it's what's needed in order to fix a broken index, or simply as a backup of their security index.
So far our "Don't do that" approach is mostly successful. What concerns me is if we put a tool like the asistant in front of users and then tell them, "you shouldn't have used that".

[bleskes]

@tvernum thanks for picking this up. To summarize the plan and make sure I don't miss anything:

1. We introduce a new .security-7 naming convention and teach the migration assistant in 6.x to reindex into it.
2. We will not allow upgrading the .security-7 index in the 7.x migration assistant code (no matter what ES version created it). We need to be careful around wording in the kibana side here.
3. The 7.x migration assistant will still allow you to reindex .security-6 into .security-7 if people started with a 6.x cluster and upgrade to 7.x

@tvernum do I miss anything? also, I'm concerned about the type change to _doc being an issue with security. That sounds like security isn't using the typeless API and that, in turn, will likely cause a problem.

@joshdover does this makes sense from your end of things? do you have a kibana issue to track it?

[joshdover]

The .security-6 to .security-7 path should be simple enough to implement in the Assistant. I've created elastic/kibana#31927 to track this.

2. We will not allow upgrading the .security-7 index in the 7.x migration assistant code (no matter what ES version created it). We need to be careful around wording in the kibana side here.

Do we have any problem with just not showing any deprecation warning for .security-7 until we know what we're going to do for 8.0? This could give a false sense of upgradability, but since the assistant already informs the user that they need to upgrade to the last 7.x minor to get the latest warnings, this seems fine to me.

[bleskes]

but since the assistant already informs the user that they need to upgrade to the last 7.x minor to get the latest warnings, this seems fine to me.

good one. works from my perspective.

[tvernum]

@bleskes

We introduce a new .security-7 naming convention and teach the migration assistant in 6.x to reindex into it.

Yes. The ES side of this is #39337 which is about ready to merge (I just want a 2nd review on it given the impact).

We will not allow upgrading the .security-7 index in the 7.x migration assistant code (no matter what ES version created it).

Yes. The issue is not so much the naming issue (though there is one) but that (per your comment below) current security code can't handle a reindex to _doc / type-less APIs.

The 7.x migration assistant will still allow you to reindex .security-6 into .security-7 if people started with a 6.x cluster and upgrade to 7.x

That's fine (and could be helpful) as long as it doesn't change the name of the doc type.

do I miss anything?

No, I don't think so.
But I haven't done any impact assessment for internal indices other than .security
I have concerns about (for example) the fact that none of the ES watcher test suite will ever test running with .watches pointing to .reindexed-v6-watches-6. However, I think that's a separate isuse to consider.

I'm concerned about the type change to _doc being an issue with security. That sounds like security isn't using the typeless API and that, in turn, will likely cause a problem.

It is certainly the case that security isn't using typeless APIs. Do you have specific concerns? We'd like to get rid of the security index (convert it to a special internal index) before ES8, so we haven't prioritised any work on migrating it to typeless APIs.

[tylersmalley]

I have a PR up to make this change in Kibana for 6.7: elastic/kibana#31996

Tomorrow I will take care of excluding security from being upgradeable in 7.0+, unless @gwbrown you think that should be handled in the deprecations API?

[bleskes]

But I haven't done any impact assessment for internal indices other than .security
I have concerns about (for example) the fact that none of the ES watcher test suite will ever test running with .watches pointing to .reindexed-v6-watches-6. However, I think that's a separate isuse to consider.

@gwbrown can you please chase/test/evaluate where we stand on the other internal indices? I would look at everything that starts with a ..

It is certainly the case that security isn't using typeless APIs. Do you have specific concerns?

@tvernum Good question. I need to understand more. I'll reach out to discuss via another channel.

[jakelandis]

@bleskes @gwbrown @tvernum - I have been working through the internal indexes see #38637

WIP PR for Logstash Central monitoring: #38653
WIP PR for Monitoring : #39336
WIP PR for Watcher: #39383

I haven't done anything with the .security templates/indexes yet.

The goal here is to have these ready by next week and asses how much risk merging these into 7.0 would introduce.

I am also working with Kibana to move to the typeless API's. I will to ask for any changes beyond adoption of the typeless API (e.g. changing types or index patterns) that those be merged to a feature branch and assess the risk next week as well. Ping me directly for additional information.

[gwbrown]

After digging through the code and talking to some experts:

• .watches will also need some special handling - the underlying index must have a name starting with .watches (see here primarily - Watcher needs to attach a listener to the .watches index, which it does by matching the name. There may be other places, but this is the biggest one, as it needs that listener to pick up new or changed watches.).
• .triggered-watches should be fine.
• .watcher-history* indices should be fine.
• .monitoring* indices should be fine.
• .logstash should be fine, as far as Elasticsearch is concerned, though I still need to check with the associated features in Kibana and Logstash. I will be a bit surprised if there are issues, but it's possible.
• I need to check with @droberts195 regarding ML's indices.

I still need to do more thorough manual (and if possible, automated) testing - I'll continue to work on that.

One consistent theme I've seen is that by and large, the features that depend on these indices are not ready to use a typeless index. It may be that we want to disable the reindex assistant for all internal indices in 7.0 until we've made some progress on that front. The PRs Jake linked to above should address this for those features, but they may not make it into 7.0, and they require some pretty significant changes.

[gwbrown]

Tomorrow I will take care of excluding security from being upgradeable in 7.0+, unless @gwbrown you think that should be handled in the deprecations API?

Given that we'll already need special cases Kibana side for .security and .watches, it might be a good idea to keep those contained to one place. On the other hand, putting a different message in the Deprecations API would be good to dissuade users from reindexing them manually if they aren't using the Kibana migration assistant.

[tvernum]

The ES changes for .security-7 (#39337) have been merged and backported to 6.7 (#39433)

[bleskes]

Given that we'll already need special cases Kibana side for .security and .watches

@gwbrown, we had a meeting today about it and the decision was that:

1. .watches will require some fixing to make it properly respond to alias changes. @jakelandis is on it.
2. The migration assistant in 7.0 will not allow reindexing of the internal indices in 7.x for now. Once we figure out what we want we can enable that functionality in the right way.
3. We will continue to explore what it means to go typeless internally in 7.0. @jpountz is working on backporting the _doc-matches-any-type functionality to 6.7. That will allow ML to keep their move to the typeless API and open the door to everything else if deemed possible. It will be great if you can continue your research to see if there are other problems.

[tvernum]

I am going to close this issue and I believe the specific problem is now resolved with #39337 and elastic/kibana#31996

The conversation here has crossed into other related issues, but I believe all of them are covered by other tracking issues. If there's something on this thread that hasn't made it to another issue/PR, please open one.