Add connectToNodeAsExtension in TransportService #6866

cwperks · 2023-03-29T14:41:32Z

Description

Companion PR in sdk repo: opensearch-project/opensearch-sdk-java#619
Companion PR in security repo: opensearch-project/security#2599

Adds new methods inside TransportService for extension to connect to opensearch as an extension. When performing the handshake for security, the extension needs to identify itself as an extension. This PR is modeled after cross cluster functionality where the node identifies which cluster it belongs to using the _opendistro_security_remotecn ThreadContext header, for this PR I chose to have the extension identify its request using the extension_unique_id thread context header which the security plugin needs to permit the handshake and other transport requests used in extension bootstrap process.

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff
Commit changes are listed out in CHANGELOG.md file (See: Changelog)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

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

github-actions · 2023-03-29T15:18:39Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      2 org.opensearch.backwards.MixedClusterClientYamlTestSuiteIT.test {p0=indices.shrink/30_copy_settings/Copy settings during shrink index}

URL: https://build.ci.opensearch.org/job/gradle-check/13021/
CommitID: de8b8b8
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

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

github-actions · 2023-03-31T16:54:00Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/13148/
CommitID: 0003dcc

codecov-commenter · 2023-03-31T16:57:50Z

Codecov Report

Merging #6866 (5ebed1c) into main (d984f50) will decrease coverage by 0.10%.
The diff coverage is 69.35%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@             Coverage Diff              @@
##               main    #6866      +/-   ##
============================================
- Coverage     70.71%   70.61%   -0.10%     
+ Complexity    59497    59456      -41     
============================================
  Files          4859     4859              
  Lines        285339   285369      +30     
  Branches      41133    41135       +2     
============================================
- Hits         201772   201517     -255     
- Misses        66965    67303     +338     
+ Partials      16602    16549      -53

Impacted Files	Coverage Δ
...ava/org/opensearch/transport/TransportService.java	`79.12% <0.00%> (-3.39%)`	⬇️
...a/org/opensearch/index/mapper/TextFieldMapper.java	`88.62% <100.00%> (+0.27%)`	⬆️

... and 458 files with indirect coverage changes

github-actions · 2023-03-31T17:01:52Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/13149/
CommitID: 4d87934

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

github-actions · 2023-04-03T13:47:57Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.cluster.allocation.AwarenessAllocationIT.testThreeZoneOneReplicaWithForceZoneValueAndLoadAwareness

URL: https://build.ci.opensearch.org/job/gradle-check/13265/
CommitID: 98b1af0
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

saratvemulapalli

Trying to understand. It definitely makes sense for cross cluster use cases. But with connectToNodeAsExtension lying in OpenSearch node, who is the intended user ?

Also curious how security is plugged in for cross cluster. I couldnt map the flow.

cwperks · 2023-04-10T15:19:01Z

Hi @saratvemulapalli, from what I can tell in the code. For cross-cluster use-cases, the security plugin enriches all outgoing transport requests with the cluster name:

https://github.com/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java#L132

When receiving the request, the Security plugin first confirms if the request is coming from a trusted node by comparing the distinguished name from the node to the full list of node_dns:

https://github.com/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/transport/DefaultInterClusterRequestEvaluator.java#L82-L102

If its coming from a trusted node, it then goes on to compare the cluster name that the node belongs to:

https://github.com/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/transport/SecurityRequestHandler.java#L311-L327

With the method (connectToNodeAsExtension ) added here, I proposed adding a threadcontext header extension_unique_id on the handshake with the OpenSearch node.

The call to extensionTransportService.connectToNode(extensionsRunner.opensearchNode); in ExtensionsInitRequestHandler will not succeed with the security plugin installed because it initiates a transport handshake action which the security plugin forbids since it originates from outside the cluster.

The companion Security plugin PR checks for the extensions_unique_id threadcontext header and permits the request as coming from a trusted source even though it is not a node in the cluster.

Thank you for reviewing this! I wanted to get a PR out to show the problem with secure communication between OpenSearch nodes and an extension to get more eyes on the problem.

github-actions · 2023-04-10T16:49:22Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/13758/
CommitID: e1c754b

cwperks · 2023-04-11T16:45:40Z

server/src/main/java/org/opensearch/transport/TransportService.java

+    ) {
+        return (newConnection, actualProfile, listener) -> {
+            // We don't validate cluster names to allow for CCS connections.
+            threadPool.getThreadContext().putHeader("extension_unique_id", extensionUniqueId);


This is the line that makes this different than connectToNode. This injects extension_unique_id into the threadcontext for the handshake which allows the security plugin to identify that the transport request was initiated by an extensions. See companion PR in security repo: opensearch-project/security#2599

stephen-crawford

Great work!

stephen-crawford · 2023-04-14T19:45:00Z

server/src/main/java/org/opensearch/transport/TransportService.java

+    ) {
+        return (newConnection, actualProfile, listener) -> {
+            // We don't validate cluster names to allow for CCS connections.
+            threadPool.getThreadContext().putHeader("extension_unique_id", extensionUniqueId);


I am confused on how using the thread context will work here. The action futures are all going to be working asynchronously so how can we inject to the thread context at this point?

I'm trying to model this like cross-cluster which uses a thread context header to store the cluster name. See: https://github.com/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java#L131-L132

With this, during the handshake it would inject the extension_unique_id into the context so that the security plugin can know which extension the handshake originated from.

This uses the getTransportInterceptors extension point from NetworkPlugin to send and restore the thread context from one node to another. As far as I know, something like this would not be supported with extensions.

github-actions · 2023-04-22T02:04:59Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/14433/
CommitID: 1ef93fc

github-actions · 2023-04-27T18:12:38Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/14604/
CommitID: 38ca9da
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

saratvemulapalli · 2023-04-28T16:40:05Z

Hi @saratvemulapalli, from what I can tell in the code. For cross-cluster use-cases, the security plugin enriches all outgoing transport requests with the cluster name:

https://github.com/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java#L132

When receiving the request, the Security plugin first confirms if the request is coming from a trusted node by comparing the distinguished name from the node to the full list of node_dns:

https://github.com/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/transport/DefaultInterClusterRequestEvaluator.java#L82-L102

If its coming from a trusted node, it then goes on to compare the cluster name that the node belongs to:

https://github.com/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/transport/SecurityRequestHandler.java#L311-L327

With the method (connectToNodeAsExtension ) added here, I proposed adding a threadcontext header extension_unique_id on the handshake with the OpenSearch node.

The call to extensionTransportService.connectToNode(extensionsRunner.opensearchNode); in ExtensionsInitRequestHandler will not succeed with the security plugin installed because it initiates a transport handshake action which the security plugin forbids since it originates from outside the cluster.

The companion Security plugin PR checks for the extensions_unique_id threadcontext header and permits the request as coming from a trusted source even though it is not a node in the cluster.

Thank you for reviewing this! I wanted to get a PR out to show the problem with secure communication between OpenSearch nodes and an extension to get more eyes on the problem.

Thanks @cwperks , this really helps.

saratvemulapalli · 2023-04-28T16:40:44Z

The changes are good, could you take care of gradle check.

github-actions · 2023-04-28T17:37:31Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/14630/
CommitID: 5ebed1c

cwperks · 2023-04-28T17:44:02Z

@saratvemulapalli The CI is passing now.

opensearch-trigger-bot · 2023-05-01T19:47:48Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-6866-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 b752555d38477b7077aacb4b8a15b07877299b0c
# Push it to GitHub
git push --set-upstream origin backport/backport-6866-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-6866-to-2.x.

…6866) * Add connectToNodeAsExtension in TransportService Signed-off-by: Craig Perkins <[email protected]> * Add to CHANGELOG Signed-off-by: Craig Perkins <[email protected]> * Update java docstrings Signed-off-by: Craig Perkins <[email protected]> --------- Signed-off-by: Craig Perkins <[email protected]> (cherry picked from commit b752555)

* Add connectToNodeAsExtension in TransportService (#6866) * Add connectToNodeAsExtension in TransportService Signed-off-by: Craig Perkins <[email protected]> * Add to CHANGELOG Signed-off-by: Craig Perkins <[email protected]> * Update java docstrings Signed-off-by: Craig Perkins <[email protected]> --------- Signed-off-by: Craig Perkins <[email protected]> (cherry picked from commit b752555) * Update CHANGELOG Signed-off-by: Craig Perkins <[email protected]> --------- Signed-off-by: Craig Perkins <[email protected]>

…6866) * Add connectToNodeAsExtension in TransportService Signed-off-by: Craig Perkins <[email protected]> * Add to CHANGELOG Signed-off-by: Craig Perkins <[email protected]> * Update java docstrings Signed-off-by: Craig Perkins <[email protected]> --------- Signed-off-by: Craig Perkins <[email protected]> Signed-off-by: Shivansh Arora <[email protected]>

Add connectToNodeAsExtension in TransportService

de8b8b8

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

github-actions bot added the all-star-contributor label Mar 29, 2023

This was referenced Mar 29, 2023

Setup extension TLS opensearch-project/opensearch-sdk-java#619

Merged

Identify extension Transport requests and permit handshake and extension registration actions opensearch-project/security#2599

Merged

cwperks mentioned this pull request Mar 29, 2023

Create NamedRoute to map extension routes to a shortened name #6870

Merged

6 tasks

cwperks added 2 commits March 31, 2023 12:23

Add to CHANGELOG

0003dcc

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

Merge branch 'main' into extension-handshake

4d87934

Update java docstrings

98b1af0

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

cwperks marked this pull request as ready for review April 3, 2023 13:55

cwperks requested review from reta, anasalkouz, andrross, Bukhtawar, CEHENKLE, dblock, gbbafna, setiah, kartg, kotwanikunal, mch2, nknize, owaiskazi19, Rishikesh1159, ryanbogan and saratvemulapalli as code owners April 3, 2023 13:55

cwperks requested review from dreamer-89, tlfeng, VachaShah and xuezhou25 as code owners April 3, 2023 13:55

saratvemulapalli reviewed Apr 7, 2023

View reviewed changes

Merge branch 'main' into extension-handshake

e1c754b

cwperks commented Apr 11, 2023

View reviewed changes

stephen-crawford reviewed Apr 14, 2023

View reviewed changes

Merge branch 'main' into extension-handshake

1ef93fc

Merge branch 'main' into extension-handshake

38ca9da

Merge branch 'main' into extension-handshake

5ebed1c

saratvemulapalli approved these changes Apr 28, 2023

View reviewed changes

saratvemulapalli merged commit b752555 into opensearch-project:main Apr 28, 2023

owaiskazi19 mentioned this pull request Apr 28, 2023

[Feature/Extensions] Profile detector opensearch-project/anomaly-detection#882

Merged

cwperks mentioned this pull request May 1, 2023

[Backport 1.x] Setup extension TLS opensearch-project/opensearch-sdk-java#718

Merged

owaiskazi19 added the backport 2.x Backport to 2.x branch label May 1, 2023

cwperks mentioned this pull request May 1, 2023

[Backport 2.x] Add connectToNodeAsExtension in TransportService #7346

Merged

cwperks mentioned this pull request May 22, 2023

[BUG] Duplicate Unique ID setting in OpenSearch transport opensearch-project/opensearch-sdk-java#771

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add connectToNodeAsExtension in TransportService #6866

Add connectToNodeAsExtension in TransportService #6866

cwperks commented Mar 29, 2023 •

edited

Loading

github-actions bot commented Mar 29, 2023

github-actions bot commented Mar 31, 2023

codecov-commenter commented Mar 31, 2023 •

edited

Loading

github-actions bot commented Mar 31, 2023

github-actions bot commented Apr 3, 2023

saratvemulapalli left a comment

cwperks commented Apr 10, 2023

github-actions bot commented Apr 10, 2023

cwperks Apr 11, 2023

stephen-crawford left a comment

stephen-crawford Apr 14, 2023

cwperks Apr 27, 2023

cwperks Apr 27, 2023

github-actions bot commented Apr 22, 2023

github-actions bot commented Apr 27, 2023

saratvemulapalli commented Apr 28, 2023

saratvemulapalli commented Apr 28, 2023

github-actions bot commented Apr 28, 2023

cwperks commented Apr 28, 2023

opensearch-trigger-bot bot commented May 1, 2023

Add connectToNodeAsExtension in TransportService #6866

Add connectToNodeAsExtension in TransportService #6866

Conversation

cwperks commented Mar 29, 2023 • edited Loading

Description

Check List

github-actions bot commented Mar 29, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Mar 31, 2023

Gradle Check (Jenkins) Run Completed with:

codecov-commenter commented Mar 31, 2023 • edited Loading

Codecov Report

github-actions bot commented Mar 31, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Apr 3, 2023

Gradle Check (Jenkins) Run Completed with:

saratvemulapalli left a comment

Choose a reason for hiding this comment

cwperks commented Apr 10, 2023

github-actions bot commented Apr 10, 2023

Gradle Check (Jenkins) Run Completed with:

cwperks Apr 11, 2023

Choose a reason for hiding this comment

stephen-crawford left a comment

Choose a reason for hiding this comment

stephen-crawford Apr 14, 2023

Choose a reason for hiding this comment

cwperks Apr 27, 2023

Choose a reason for hiding this comment

cwperks Apr 27, 2023

Choose a reason for hiding this comment

github-actions bot commented Apr 22, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Apr 27, 2023

Gradle Check (Jenkins) Run Completed with:

saratvemulapalli commented Apr 28, 2023

saratvemulapalli commented Apr 28, 2023

github-actions bot commented Apr 28, 2023

Gradle Check (Jenkins) Run Completed with:

cwperks commented Apr 28, 2023

opensearch-trigger-bot bot commented May 1, 2023

cwperks commented Mar 29, 2023 •

edited

Loading

codecov-commenter commented Mar 31, 2023 •

edited

Loading