Skip to content

Commit

Permalink
Authorize rest requests (#2753)
Browse files Browse the repository at this point in the history
* WIP on rest layer authz

Signed-off-by: Craig Perkins <[email protected]>

* WIP on rest-layer authz

Signed-off-by: Craig Perkins <[email protected]>

* Extension handshake

Signed-off-by: Craig Perkins <[email protected]>

* Extension TLS

Signed-off-by: Craig Perkins <[email protected]>

* Remove SecurityRestFilterChanges to isolate extension TLS change

Signed-off-by: Craig Perkins <[email protected]>

* Remove SecurityRestFilterChanges to isolate extension TLS change

Signed-off-by: Craig Perkins <[email protected]>

* Remove SecurityRestFilterChanges to isolate extension TLS change

Signed-off-by: Craig Perkins <[email protected]>

* Remove SecurityRestFilterChanges to isolate extension TLS change

Signed-off-by: Craig Perkins <[email protected]>

* Remove SecurityRestFilterChanges to isolate extension TLS change

Signed-off-by: Craig Perkins <[email protected]>

* WIP for HelloWorld sample extension role

Signed-off-by: Craig Perkins <[email protected]>

* Initial implementation of authz check in REST layer

Signed-off-by: Craig Perkins <[email protected]>

* Remove header

Signed-off-by: Craig Perkins <[email protected]>

* Create authorizeRequest method

Signed-off-by: Craig Perkins <[email protected]>

* small fix

Signed-off-by: Craig Perkins <[email protected]>

* Change to ProtectedRoute

Signed-off-by: Craig Perkins <[email protected]>

* Remove extension permissions

Signed-off-by: Craig Perkins <[email protected]>

* Initial implementation of authz check in REST layer

Signed-off-by: Craig Perkins <[email protected]>

* Extension TLS

Signed-off-by: Craig Perkins <[email protected]>

* Adds dummy roles for testing rest authorization against legacy permissions

Signed-off-by: Darshit Chanpura <[email protected]>

* Adds support for legacy permissions to perform rest authorization

Signed-off-by: Darshit Chanpura <[email protected]>

* Fixes white-space changes

Signed-off-by: Darshit Chanpura <[email protected]>

* Rebases ConfigConstants with main

Signed-off-by: Darshit Chanpura <[email protected]>

* Implements a new logic for rest permissions check to be more flexible

Signed-off-by: Darshit Chanpura <[email protected]>

* Fixes spotless errors

Signed-off-by: Darshit Chanpura <[email protected]>

* Adds regex to match against current role permissions when comparing new permission with legacy ones

Signed-off-by: Darshit Chanpura <[email protected]>

* Moves legacy permission check logic to ConfigModelV7

Signed-off-by: Darshit Chanpura <[email protected]>

* Fixes extra new-lines

Signed-off-by: Darshit Chanpura <[email protected]>

* Fixes unused imports

Signed-off-by: Darshit Chanpura <[email protected]>

* Fixes out-of-scope white space changes

Signed-off-by: Darshit Chanpura <[email protected]>

* Fixes code-ql errors

Signed-off-by: Darshit Chanpura <[email protected]>

* Fixes spotless and code-ql errors

Signed-off-by: Darshit Chanpura <[email protected]>

* Fixes variable name and remove references to whitelist in javadoc

Signed-off-by: Darshit Chanpura <[email protected]>

* Adds tests for rest layer privilege evaluator

Signed-off-by: Darshit Chanpura <[email protected]>

* Adds license header to the test file

Signed-off-by: Darshit Chanpura <[email protected]>

* Updates zstd dependency to fetch from core version.properties

Signed-off-by: Darshit Chanpura <[email protected]>

* Updates action name in the regex to be dynamic

Signed-off-by: Darshit Chanpura <[email protected]>

* Adds support for allowing evaluation against multiple actions names for a registered named route

Signed-off-by: Darshit Chanpura <[email protected]>

* Updates tests

Signed-off-by: Darshit Chanpura <[email protected]>

* Adds null check

Signed-off-by: Darshit Chanpura <[email protected]>

* Makes authorize logic clearer to follow

Signed-off-by: Darshit Chanpura <[email protected]>

* Adds extra check to ensure new actions are also evaluated against transport actions

Signed-off-by: Darshit Chanpura <[email protected]>

* Fixes spotless errors

Signed-off-by: Darshit Chanpura <[email protected]>

* Fixes security rest filter setup

Signed-off-by: Darshit Chanpura <[email protected]>

* Removes extension reference

Signed-off-by: Darshit Chanpura <[email protected]>

* turn on audit logging

Signed-off-by: Maciej Mierzwa <[email protected]>

* Adds unit tests for restPathMatches method

Signed-off-by: Darshit Chanpura <[email protected]>

* Cleans up TODOs

Signed-off-by: Darshit Chanpura <[email protected]>

* Organizes demo users and roles for extension

Signed-off-by: Darshit Chanpura <[email protected]>

* Address PR feedback

Signed-off-by: Darshit Chanpura <[email protected]>

* Adds more comments

Signed-off-by: Darshit Chanpura <[email protected]>

* add privileges info

Signed-off-by: Maciej Mierzwa <[email protected]>

* Makes whoami action a named route and fixes license header check

Signed-off-by: Darshit Chanpura <[email protected]>

* Adds integ tests for whoami route

Signed-off-by: Darshit Chanpura <[email protected]>

* Change permissions order in roles.yml

Signed-off-by: Darshit Chanpura <[email protected]>

* Adds developer documentation for authorization in REST layer

Signed-off-by: Darshit Chanpura <[email protected]>

* Fixes broken tests

Signed-off-by: Darshit Chanpura <[email protected]>

* Fixes checkstyle errors

Signed-off-by: Darshit Chanpura <[email protected]>

* Addresses feedback and cleans up logic for super admin check

Signed-off-by: Darshit Chanpura <[email protected]>

* Addresses Plugin Install CI failure

Signed-off-by: Darshit Chanpura <[email protected]>

* Fixes failing citest task

Signed-off-by: Darshit Chanpura <[email protected]>

* Modifies WhoAmI integ tests

Signed-off-by: Darshit Chanpura <[email protected]>

* Adds a new endpoint called whoamiprotected and removes changes made to whoami route

Signed-off-by: Darshit Chanpura <[email protected]>

* Updates documentation to reflect the new API

Signed-off-by: Darshit Chanpura <[email protected]>

* Addresses PR feedback

Signed-off-by: Darshit Chanpura <[email protected]>

* Renames action0 to actions

Signed-off-by: Darshit Chanpura <[email protected]>

---------

Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Maciej Mierzwa <[email protected]>
Co-authored-by: Craig Perkins <[email protected]>
Co-authored-by: MaciejMierzwa <[email protected]>
  • Loading branch information
3 people authored Jul 7, 2023
1 parent 49cbf52 commit a53a8a6
Show file tree
Hide file tree
Showing 14 changed files with 795 additions and 24 deletions.
51 changes: 51 additions & 0 deletions DEVELOPER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ So you want to contribute code to OpenSearch Security? Excellent! We're glad you
- [Running integration tests](#running-integration-tests)
- [Bulk test runs](#bulk-test-runs)
- [Checkstyle Violations](#checkstyle-violations)
- [Authorization in REST Layer](#authorization-in-rest-layer)
- [Submitting Changes](#submitting-changes)
- [Backports](#backports)

Expand Down Expand Up @@ -78,6 +79,51 @@ mv config/* $OPENSEARCH_HOME/config/opensearch-security/
rm -rf config/
```

### Installing demo extension users and roles

If you are working with an extension and want to set up demo users for the Hello-World extension, append following items to files inside `$OPENSEARCH_HOME/config/opensearch-security/`:
1. In **internal_users.yml**
```yaml
hw-user:
hash: "$2a$12$VcCDgh2NDk07JGN0rjGbM.Ad41qVR/YFJcgHp0UGns5JDymv..TOG"
reserved: true
description: "Demo user for ext-test"
```
2. In **roles.yml**
```yaml
extension_hw_greet:
reserved: true
cluster_permissions:
- 'hw:greet'

extension_hw_full:
reserved: true
cluster_permissions:
- 'hw:goodbye'
- 'hw:greet'
- 'hw:greet_with_adjective'
- 'hw:greet_with_name'

legacy_hw_greet_with_name:
reserved: true
cluster_permissions:
- 'cluster:admin/opensearch/hw/greet_with_name'
```
3. In **roles_mapping.yml**
```yaml
legacy_hw_greet_with_name:
reserved: true
users:
- "hw-user"

extension_hw_greet:
reserved: true
users:
- "hw-user"
```
To install the demo certificates and default configuration, answer `y` to the first two questions and `n` to the last one. The log should look like below:

```bash
Expand Down Expand Up @@ -188,6 +234,11 @@ Checkstyle enforces several rules within this codebase. Sometimes it will be nec
// CS-ENFORCE-ALL
```
## Authorization in REST Layer
See [REST_AUTHZ_FOR_PLUGINS](REST_AUTHZ_FOR_PLUGINS.md).
## Submitting Changes
See [CONTRIBUTING](CONTRIBUTING.md).
Expand Down
136 changes: 136 additions & 0 deletions REST_AUTHZ_FOR_PLUGINS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
# Authorization at REST Layer for plugins

This feature is introduced as an added layer of security on top of existing TransportLayer authorization framework. In order to leverage these feature some core changes need to be made at Route registration level. This document talks about how you can achieve this.

**NOTE:** This doesn't replace Transport Layer Authorization. Plugin developers may choose to skip creating transport actions for APIs that do not need interaction with the Transport Layer.

## Pre-requisites

The security plugin must be installed and operational in your OpenSearch cluster for this feature to work.

### How does NamedRoute authorization work?

Once the routes are defined as NamedRoute, they, along-with their handlers, will be registered the same way as Route objects. When a request comes in, `SecurityRestFilter.java` applies an authorization check which extracts information about the NamedRoute.
Next we get the unique name and actionNames associated with that route and evaluate these against existing `cluster_permissions` across all roles of the requesting user. If the authorization check succeeds, the request chain proceeds as normal. If it fails, a 401 response is returned to the user.

NOTE:
1. The action names defined in roles must exactly match the names of registered routes, or else, the request would be deemed unauthorized.
2. This check will not be implemented for plugins who do not use NamedRoutes.



### How to translate an existing Route to be a NamedRoute?

Here is a sample of an existing route converted to a named route:
Before:
```
public List<Route> routes() {
return ImmutableList.of(
new Route(GET, "/uri")
);
}
```
With new scheme:
```
public List<NamedRoute> routes() {
return ImmutableList.of(
new NamedRoute.Builder().method(GET).path("/uri").uniqueName("plugin:uri").actionNames(Set.of("cluster:admin/opensearch/plugin/uri")).build()
);
}
```

`actionNames()` are optional. They correspond to any current actions defined as permissions in roles.
Ensure that these name-to-route mappings are easily accessible to the cluster admins to allow granting access to these APIs.

### How does authorization in the REST Layer work?

We will continue on the above example of translating `/uri` from Route to NamedRoute.

Consider these roles are defined in the cluster:
```yaml
plugin_role:
reserved: true
cluster_permissions:
- 'plugin:uri'

plugin_role_legacy:
reserved: true
cluster_permissions:
- 'cluster:admin/opensearch/plugin/uri'
```
Successful authz scenarios for a user:
1. The user is mapped either to `plugin_role` OR `plugin_role_legacy`.
2. The user is mapped to both of these roles.
3. The user is mapped to `plugin_role` even if no `actionNames()` were registered for this route.

Unsuccessful authz scenarios for a user:
1. The user is not mapped any roles.
2. The user is mapped to a different role which doesn't grant the cluster permissions: `plugin:uri` OR `cluster:admin/opensearch/plugin/uri`/
3. The user is mapped to a role `plugin_role_other` which has a typo in action name, i.e.`plugin:uuri`.


### Sample API in Security Plugin

As part of this effort a new uri `GET /whoamiprotected` was introduced as a NamedRoute version of `GET /whoami`. Here is how you can test it:

#### roles.yml
```yaml
who_am_i_role:
reserved: true
cluster_permissions:
- 'security:whoamiprotected'
who_am_i_role_legacy:
reserved: true
cluster_permissions:
- 'cluster:admin/opendistro_security/whoamiprotected'
who_am_i_role_no_perm:
reserved: true
cluster_permissions:
- 'some_invalid_perm'
```

#### internal_users.yml
```yaml
who_am_i-user:
hash: "$2a$12$VcCDgh2NDk07JGN0rjGbM.Ad41qVR/YFJcgHp0UGns5JDymv..TOG" #admin
reserved: true
description: "Demo user for ext-test"
who_am_i_legacy-user:
hash: "$2a$12$VcCDgh2NDk07JGN0rjGbM.Ad41qVR/YFJcgHp0UGns5JDymv..TOG"
reserved: true
description: "Demo user for ext-test"
who_am_i_no_perm-user:
hash: "$2a$12$VcCDgh2NDk07JGN0rjGbM.Ad41qVR/YFJcgHp0UGns5JDymv..TOG"
reserved: true
description: "Demo user for ext-test"
```

#### roles_mapping.yml
```yaml
who_am_i_role:
reserved: true
users:
- "who_am_i-user"
who_am_i_role_legacy:
reserved: true
users:
- "who_am_i_legacy-user"
who_am_i_role_no_perm:
reserved: true
users:
- "who_am_i_no_perm-user"
```

Follow [DEVELOPER_GUIDE](DEVELOPER_GUIDE.md) to setup OpenSearch cluster and initialize security plugin. Once you have verified that security plugin is installed correctly and OpenSearch is running, execute following curl requests:
1. `curl -XGET https://who_am_i-user:admin@localhost:9200/_plugins/_security/whoami --insecure` should succeed.
2. `curl -XGET https://who_am_i_legacy-user:admin@localhost:9200/_plugins/_security/whoami --insecure` should succeed.
3. `curl -XGET https://who_am_i_no-perm-user:admin@localhost:9200/_plugins/_security/whoami --insecure` should fail.
4. `curl -XPOST ` to `/whoami` with all 3 users should succeed. This is because POST route is not a NamedRoute and hence no authorization check was made.
107 changes: 107 additions & 0 deletions src/integrationTest/java/org/opensearch/security/rest/WhoAmITests.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*
* Modifications Copyright OpenSearch Contributors. See
* GitHub history for details.
*/

package org.opensearch.security.rest;

import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
import org.apache.hc.core5.http.HttpStatus;
import org.junit.ClassRule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.opensearch.test.framework.TestSecurityConfig;
import org.opensearch.test.framework.TestSecurityConfig.Role;
import org.opensearch.test.framework.cluster.ClusterManager;
import org.opensearch.test.framework.cluster.LocalCluster;
import org.opensearch.test.framework.cluster.TestRestClient;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL;

@RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class)
@ThreadLeakScope(ThreadLeakScope.Scope.NONE)
public class WhoAmITests {
protected final static TestSecurityConfig.User WHO_AM_I = new TestSecurityConfig.User("who_am_i_user").roles(
new Role("who_am_i_role").clusterPermissions("security:whoamiprotected")
);

protected final static TestSecurityConfig.User WHO_AM_I_LEGACY = new TestSecurityConfig.User("who_am_i_user_legacy").roles(
new Role("who_am_i_role_legacy").clusterPermissions("cluster:admin/opendistro_security/whoamiprotected")
);

protected final static TestSecurityConfig.User WHO_AM_I_NO_PERM = new TestSecurityConfig.User("who_am_i_user_no_perm").roles(
new Role("who_am_i_role_no_perm")
);

protected final static TestSecurityConfig.User WHO_AM_I_UNREGISTERED = new TestSecurityConfig.User("who_am_i_user_no_perm");

public static final String WHOAMI_ENDPOINT = "_plugins/_security/whoami";
public static final String WHOAMI_PROTECTED_ENDPOINT = "_plugins/_security/whoamiprotected";

@ClassRule
public static LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.THREE_CLUSTER_MANAGERS)
.authc(AUTHC_HTTPBASIC_INTERNAL)
.users(WHO_AM_I, WHO_AM_I_LEGACY, WHO_AM_I_NO_PERM)
.build();

@Test
public void testWhoAmIWithGetPermissions() throws Exception {
try (TestRestClient client = cluster.getRestClient(WHO_AM_I)) {
assertThat(client.get(WHOAMI_PROTECTED_ENDPOINT).getStatusCode(), equalTo(HttpStatus.SC_OK));
}

try (TestRestClient client = cluster.getRestClient(WHO_AM_I)) {
assertThat(client.get(WHOAMI_ENDPOINT).getStatusCode(), equalTo(HttpStatus.SC_OK));
}
}

@Test
public void testWhoAmIWithGetPermissionsLegacy() throws Exception {
try (TestRestClient client = cluster.getRestClient(WHO_AM_I_LEGACY)) {
assertThat(client.get(WHOAMI_ENDPOINT).getStatusCode(), equalTo(HttpStatus.SC_OK));
}

try (TestRestClient client = cluster.getRestClient(WHO_AM_I_LEGACY)) {
assertThat(client.get(WHOAMI_PROTECTED_ENDPOINT).getStatusCode(), equalTo(HttpStatus.SC_OK));
}
}

@Test
public void testWhoAmIWithoutGetPermissions() throws Exception {
try (TestRestClient client = cluster.getRestClient(WHO_AM_I_NO_PERM)) {
assertThat(client.get(WHOAMI_ENDPOINT).getStatusCode(), equalTo(HttpStatus.SC_OK));
}

try (TestRestClient client = cluster.getRestClient(WHO_AM_I_NO_PERM)) {
assertThat(client.get(WHOAMI_PROTECTED_ENDPOINT).getStatusCode(), equalTo(HttpStatus.SC_UNAUTHORIZED));
}
}

@Test
public void testWhoAmIPost() throws Exception {
try (TestRestClient client = cluster.getRestClient(WHO_AM_I)) {
assertThat(client.post(WHOAMI_ENDPOINT).getStatusCode(), equalTo(HttpStatus.SC_OK));
}

try (TestRestClient client = cluster.getRestClient(WHO_AM_I_LEGACY)) {
assertThat(client.post(WHOAMI_ENDPOINT).getStatusCode(), equalTo(HttpStatus.SC_OK));
}

try (TestRestClient client = cluster.getRestClient(WHO_AM_I_NO_PERM)) {
assertThat(client.post(WHOAMI_ENDPOINT).getStatusCode(), equalTo(HttpStatus.SC_OK));
}

try (TestRestClient client = cluster.getRestClient(WHO_AM_I_UNREGISTERED)) {
assertThat(client.post(WHOAMI_ENDPOINT).getStatusCode(), equalTo(HttpStatus.SC_OK));
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@
import org.opensearch.security.http.XFFResolver;
import org.opensearch.security.privileges.PrivilegesEvaluator;
import org.opensearch.security.privileges.PrivilegesInterceptor;
import org.opensearch.security.privileges.RestLayerPrivilegesEvaluator;
import org.opensearch.security.resolver.IndexResolverReplacer;
import org.opensearch.security.rest.DashboardsInfoAction;
import org.opensearch.security.rest.SecurityConfigUpdateAction;
Expand Down Expand Up @@ -205,6 +206,7 @@ public final class OpenSearchSecurityPlugin extends OpenSearchSecuritySSLPlugin
private volatile SecurityInterceptor si;
private volatile PrivilegesEvaluator evaluator;
private volatile UserService userService;
private volatile RestLayerPrivilegesEvaluator restLayerEvaluator;
private volatile ThreadPool threadPool;
private volatile ConfigurationRepository cr;
private volatile AdminDNs adminDns;
Expand Down Expand Up @@ -1019,8 +1021,11 @@ public Collection<Object> createComponents(
principalExtractor = ReflectionHelper.instantiatePrincipalExtractor(principalExtractorClass);
}

restLayerEvaluator = new RestLayerPrivilegesEvaluator(clusterService, threadPool, auditLog, cih, namedXContentRegistry);

securityRestHandler = new SecurityRestFilter(
backendRegistry,
restLayerEvaluator,
auditLog,
threadPool,
principalExtractor,
Expand All @@ -1035,6 +1040,7 @@ public Collection<Object> createComponents(
dcf.registerDCFListener(irr);
dcf.registerDCFListener(xffResolver);
dcf.registerDCFListener(evaluator);
dcf.registerDCFListener(restLayerEvaluator);
dcf.registerDCFListener(securityRestHandler);
if (!(auditLog instanceof NullAuditLog)) {
// Don't register if advanced modules is disabled in which case auditlog is instance of NullAuditLog
Expand Down Expand Up @@ -1072,6 +1078,7 @@ public Collection<Object> createComponents(
components.add(xffResolver);
components.add(backendRegistry);
components.add(evaluator);
components.add(restLayerEvaluator);
components.add(si);
components.add(dcf);
components.add(userService);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ public void logMissingPrivileges(String privilege, String effectiveUser, RestReq
msg.addRemoteAddress(remoteAddress);
msg.addRestRequestInfo(request, auditConfigFilter);
msg.addEffectiveUser(effectiveUser);
msg.addPrivilege(privilege);
save(msg);
}

Expand Down
15 changes: 12 additions & 3 deletions src/main/java/org/opensearch/security/dlic/rest/support/Utils.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import org.opensearch.core.xcontent.NamedXContentRegistry;
import org.opensearch.core.xcontent.ToXContent;
import org.opensearch.core.xcontent.XContentParser;
import org.opensearch.rest.NamedRoute;
import org.opensearch.rest.RestHandler.DeprecatedRoute;
import org.opensearch.rest.RestHandler.Route;
import org.opensearch.security.DefaultObjectMapper;
Expand Down Expand Up @@ -240,9 +241,17 @@ public static List<Route> addRoutesPrefix(List<Route> routes) {
* Total number of routes will be expanded len(prefixes) as much comparing to the list passed in
*/
public static List<Route> addRoutesPrefix(List<Route> routes, final String... prefixes) {
return routes.stream()
.flatMap(r -> Arrays.stream(prefixes).map(p -> new Route(r.getMethod(), p + r.getPath())))
.collect(ImmutableList.toImmutableList());
return routes.stream().flatMap(r -> Arrays.stream(prefixes).map(p -> {
if (r instanceof NamedRoute) {
NamedRoute nr = (NamedRoute) r;
return new NamedRoute.Builder().method(nr.getMethod())
.path(p + nr.getPath())
.uniqueName(nr.name())
.legacyActionNames(nr.actionNames())
.build();
}
return new Route(r.getMethod(), p + r.getPath());
})).collect(ImmutableList.toImmutableList());
}

/**
Expand Down
Loading

0 comments on commit a53a8a6

Please sign in to comment.