Block access to system indices from REST APIs

[andrross]

Access to system indices via the REST APIs has been deprecated since OpenSearch 1.x (and ES 7.10 before that). This issue is to complete the deprecation and block access to system indices via the REST API. This is obviously a breaking change and can only be completed in the next major version (3.0).

[dbwiddis]

What will be the supported replacement method for accessing these indices?

In particular, many plugin integration tests clean up indices at the end of a test class in order to not impact another test class. See https://github.com/search?q=org%3Aopensearch-project+wipeAllODFEIndices&type=code

    @After
    protected void wipeAllODFEIndices() throws IOException {
        Response response = client().performRequest(new Request("GET", "/_cat/indices?format=json&expand_wildcards=all"));
        // parse response
        for (Object index : parser.list()) {
            Map<String, Object> jsonObject = (Map<String, Object>) index;
            String indexName = jsonObject.get("index").toString();
            if (!".opendistro_security".equals(indexName)) {
                Request request = new Request("DELETE", String.format(Locale.getDefault(), "/%s", indexName));
                // process
            }
        }
    }

[cwperks]

@dbwiddis OpenSearch already has system index protection by default in the default distribution of OpenSearch w/ the security plugin installed. (Strong) System Index Protection is provided by the security plugin which only allows system index access in 2 cases:

1. System Calls: As the system making calls (i.e. blocks wrapped by try (ThreadContext.StoredContext ctx = threadContext.stashContext()) { ... } or just running in a fresh ThreadContext in general)
2. Superuser (Admin Cert): Calls as the superuser (making REST calls with the admin certificate)

Many plugins already account for this and in the snippet you provide above the client() would need to be configured with the admin certificate.

W/o the security plugin installed is a different story. W/o encryption, there isn't a foolproof way to secure system indices. At best, OpenSearch could require a certain HTTP Header to be present in the request to declare the intent to make a request on a system index, but since the requests are unencrypted you can't ensure the integrity of any secrets.

Check out this comment from @nibix here: opensearch-project/security#4896 (comment)

I'd agree to this analysis, @cwperks .

Actually, core already contains ground works for a protection of system indices. It works like this:

In case a REST request is issues against a system index, the following message will be logged: this request accesses system indices: [{}], but in a future major version, direct access to system indices will be prevented by default
Exception: The HTTP header X-opensearch-product-origin is present in the REST request
Other exception: The RestHandler implementation overrides allowSystemIndexAccessByDefault() and returns true.
See this commit for details: 5c8b066

These concepts still come from the ES times. If I understand it correctly, the goal was to replace the warning message by an error as a breaking change with a major release.

IMHO, this concept makes totally sense for core. As @cwperks has pointed out, core has no safe means of authentication. What core can do is a kind of cooperative check: If a client assures that they need to perform operations on a system index, they are allowed to. If the client signals no concrete intent to operate on system index, they are not allowed to. This avoids unwanted operations in case of bugs on side of the client - be it an overly broad index pattern or a similar case. Of course, it does not provide real security. But that just takes into account the fact that core has no means to perform a secure authentication.

IMHO, a good strategy would be to build on the present ground works and to complete them. That would be also useful from a code hygiene point of view, as otherwise that existing code would be there without real purpose.

Relevant code pieces:

OpenSearch/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java

Lines 359 to 378 in 57a6605

	private void checkSystemIndexAccess(Context context, Metadata metadata, Set<Index> concreteIndices, String[] originalPatterns) {
	if (context.isSystemIndexAccessAllowed() == false) {
	final List<String> resolvedSystemIndices = concreteIndices.stream()
	.map(metadata::index)
	.filter(IndexMetadata::isSystem)
	.map(i -> i.getIndex().getName())
	.sorted() // reliable order for testing
	.collect(Collectors.toList());
	if (resolvedSystemIndices.isEmpty() == false) {
	resolvedSystemIndices.forEach(
	systemIndexName -> deprecationLogger.deprecate(
	"open_system_index_access_" + systemIndexName,
	"this request accesses system indices: [{}], but in a future major version, direct access to system "
	+ "indices will be prevented by default",
	systemIndexName
	)
	);
	}
	}
	}

https://github.com/opensearch-project/OpenSearch/blame/57a660558f925da44eca18a7eb21983e61a7124a/server/src/main/java/org/opensearch/rest/RestController.java#L374-L381

OpenSearch/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java

Lines 767 to 774 in 57a6605

	/**
	* Determines whether or not system index access should be allowed in the current context.
	*
	* @return True if system index access should be allowed, false otherwise.
	*/
	public boolean isSystemIndexAccessAllowed() {
	return Booleans.parseBoolean(threadContext.getHeader(SYSTEM_INDEX_ACCESS_CONTROL_HEADER_KEY), true);
	}

[dbwiddis]

Many plugins already account for this and in the snippet you provide above the client() would need to be configured with the admin certificate.

The client() in this case is inherited from OpenSearchTestCase. Looking around in there, it already has a wipeAllIndices() call but these workaround patterns are typically designed where we want to exclude one.

But I notice that class uses adminClient() for this indices-wiping step and ignores the warnings. This is built with the same settings by default so seems to not just be an easy fix. But I assume the correct thing is for plugins to override that client and add the X-opensearch-product-origin header?

To be clear, while the above explanation is very useful, my primary goal here is to update my plugin to not break with 3.0, and all my integ tests (run without security plugin) currently produce this deprecation warning on the delete step.

[cwperks]

To be clear, while the above explanation is very useful, my primary goal here is to update my plugin to not break with 3.0, and all my integ tests (run without security plugin) currently produce this deprecation warning on the delete step.

The deprecation message is coming from a commit before the fork. I agree with @nibix that there should be an effort to clean this up bc the message is there in perpetuity otherwise.

IMHO, a good strategy would be to build on the present ground works and to complete them. That would be also useful from a code hygiene point of view, as otherwise that existing code would be there without real purpose.

When using the security plugin, plugins should configure the admin client with the admin certificate (example). Using the demo security config, kirk.pem and kirk-key.pem contain the admin cert and key. Specifically the plugins.security.authcz.admin_dn setting contains a list of distinguished names corresponding to admin certificates. You can find an example curl request on the documentation website about System Indexes.

Plugins sometimes checkin the demo certs to their repositories, or they can obtain them from the security repo like this: https://github.com/opensearch-project/job-scheduler/blob/main/build.gradle#L54-L65