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

Authorize rest requests #2753

Merged
Show file tree
Hide file tree
Changes from 70 commits
Commits
Show all changes
71 commits
Select commit Hold shift + click to select a range
a8a1660
WIP on rest layer authz
cwperks Mar 27, 2023
0fedc03
WIP on rest-layer authz
cwperks Mar 28, 2023
1793a5b
Extension handshake
cwperks Mar 28, 2023
32d8763
Extension TLS
cwperks Mar 28, 2023
a5a44b8
Remove SecurityRestFilterChanges to isolate extension TLS change
cwperks Mar 29, 2023
99bfd94
Remove SecurityRestFilterChanges to isolate extension TLS change
cwperks Mar 29, 2023
f62f590
Remove SecurityRestFilterChanges to isolate extension TLS change
cwperks Mar 29, 2023
7596b7e
Remove SecurityRestFilterChanges to isolate extension TLS change
cwperks Mar 29, 2023
1fb5316
Remove SecurityRestFilterChanges to isolate extension TLS change
cwperks Mar 29, 2023
25fda77
WIP for HelloWorld sample extension role
cwperks Mar 29, 2023
aea8ede
Initial implementation of authz check in REST layer
cwperks Mar 29, 2023
15eb6b9
Remove header
cwperks Mar 29, 2023
f2cba87
Create authorizeRequest method
cwperks Apr 2, 2023
77700e5
small fix
cwperks Apr 22, 2023
d1d9ed1
Change to ProtectedRoute
cwperks Apr 28, 2023
ccab6ce
Remove extension permissions
cwperks May 8, 2023
174a142
Initial implementation of authz check in REST layer
cwperks Mar 29, 2023
2042a60
Extension TLS
cwperks Mar 28, 2023
0c70700
Adds dummy roles for testing rest authorization against legacy permis…
DarshitChanpura May 9, 2023
9837378
Adds support for legacy permissions to perform rest authorization
DarshitChanpura May 9, 2023
a34c3a4
Fixes white-space changes
DarshitChanpura May 9, 2023
f5afd62
Rebases ConfigConstants with main
DarshitChanpura May 24, 2023
918ab51
Implements a new logic for rest permissions check to be more flexible
DarshitChanpura May 24, 2023
98791a2
Fixes spotless errors
DarshitChanpura May 24, 2023
b639544
Adds regex to match against current role permissions when comparing n…
DarshitChanpura May 25, 2023
4288235
Moves legacy permission check logic to ConfigModelV7
DarshitChanpura May 26, 2023
93a2c66
Fixes extra new-lines
DarshitChanpura May 26, 2023
6e9e83d
Fixes unused imports
DarshitChanpura May 26, 2023
5ee7b12
Fixes out-of-scope white space changes
DarshitChanpura May 26, 2023
34092f9
Fixes code-ql errors
DarshitChanpura May 30, 2023
c41023b
Fixes spotless and code-ql errors
DarshitChanpura May 30, 2023
d746435
Fixes variable name and remove references to whitelist in javadoc
DarshitChanpura May 31, 2023
d68c76e
Adds tests for rest layer privilege evaluator
DarshitChanpura May 31, 2023
2814283
Adds license header to the test file
DarshitChanpura May 31, 2023
3b41475
Merge branch 'main' into authorize-rest-requests
DarshitChanpura Jun 1, 2023
d5392e4
Updates zstd dependency to fetch from core version.properties
DarshitChanpura Jun 1, 2023
425c22b
Updates action name in the regex to be dynamic
DarshitChanpura Jun 2, 2023
b1ed481
Adds support for allowing evaluation against multiple actions names f…
DarshitChanpura Jun 13, 2023
1e3efe2
Updates tests
DarshitChanpura Jun 13, 2023
590f55a
Adds null check
DarshitChanpura Jun 15, 2023
6918a80
Merge branch 'main' into authorize-rest-requests
DarshitChanpura Jun 16, 2023
427460c
Makes authorize logic clearer to follow
DarshitChanpura Jun 20, 2023
f3c4a77
Adds extra check to ensure new actions are also evaluated against tra…
DarshitChanpura Jun 20, 2023
8557fce
Fixes spotless errors
DarshitChanpura Jun 20, 2023
d66294d
Fixes security rest filter setup
DarshitChanpura Jun 20, 2023
4a33b33
Removes extension reference
DarshitChanpura Jun 20, 2023
6a1c25a
Merge branch 'main' into authorize-rest-requests
DarshitChanpura Jun 21, 2023
8503358
turn on audit logging
MaciejMierzwa Jun 22, 2023
afc8c81
Adds unit tests for restPathMatches method
DarshitChanpura Jun 22, 2023
14fe152
Cleans up TODOs
DarshitChanpura Jun 22, 2023
5bae5e8
Organizes demo users and roles for extension
DarshitChanpura Jun 22, 2023
24be564
Address PR feedback
DarshitChanpura Jun 22, 2023
2edd319
Adds more comments
DarshitChanpura Jun 22, 2023
3c397e2
add privileges info
MaciejMierzwa Jun 23, 2023
e7d10c7
Audit Logging for NamedRoutes
DarshitChanpura Jun 27, 2023
54cfcd9
Merge remote-tracking branch 'upstream/main' into authorize-rest-requ…
DarshitChanpura Jun 29, 2023
8bfeb25
Makes whoami action a named route and fixes license header check
DarshitChanpura Jun 29, 2023
ccd3d2d
Adds integ tests for whoami route
DarshitChanpura Jul 5, 2023
c68083c
Merge branch 'main' into authorize-rest-requests
DarshitChanpura Jul 6, 2023
dc0c1e2
Change permissions order in roles.yml
DarshitChanpura Jul 6, 2023
39314d7
Adds developer documentation for authorization in REST layer
DarshitChanpura Jul 6, 2023
ebc5486
Fixes broken tests
DarshitChanpura Jul 6, 2023
a78582f
Fixes checkstyle errors
DarshitChanpura Jul 7, 2023
a325d8e
Addresses feedback and cleans up logic for super admin check
DarshitChanpura Jul 7, 2023
0e905a0
Addresses Plugin Install CI failure
DarshitChanpura Jul 7, 2023
25a41b6
Fixes failing citest task
DarshitChanpura Jul 7, 2023
1b1f519
Modifies WhoAmI integ tests
DarshitChanpura Jul 7, 2023
ffdd927
Adds a new endpoint called whoamiprotected and removes changes made t…
DarshitChanpura Jul 7, 2023
9b05d44
Updates documentation to reflect the new API
DarshitChanpura Jul 7, 2023
a21c4c0
Addresses PR feedback
DarshitChanpura Jul 7, 2023
6c5ee8e
Renames action0 to actions
DarshitChanpura Jul 7, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
DarshitChanpura marked this conversation as resolved.
Show resolved Hide resolved

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:
DarshitChanpura marked this conversation as resolved.
Show resolved Hide resolved
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.
DarshitChanpura marked this conversation as resolved.
Show resolved Hide resolved
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.
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 {
DarshitChanpura marked this conversation as resolved.
Show resolved Hide resolved
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
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