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

Introducing passive_intertransport_auth to facilitate communication b… #1156

Merged

Conversation

dhiAmzn
Copy link
Contributor

@dhiAmzn dhiAmzn commented May 3, 2021

…etween nodes with adv sec enabled and nodes without adv sec enabled.

Changes to ssl_dual_mode_enabled to support the same.

Signed-off-by: Dhiresh Jain dhireshj@amazon.com

opendistro-for-elasticsearch/security pull request intake form

Please provide as much details as possible to get feedback/acceptance on your PR quickly

NOTE: This commit has been moved from opendistro repo to opensearch. The manual test was done using docker in opendistro. Docker testing is currently unavailable in opensearch so did not perform a manual test here yet.

  1. Category: New feature

  2. Github Issue # or road-map entry, if available:

  3. Description of changes: Introducing opendistro security dynamic config feature flag "passive_intertransport_auth".
    Additionally, making changes to ssl_dual_node behaviour.
    The changes allow transport communication between nodes with advanced security enabled and nodes without advanced security enabled (ssl only or plugin disabled).
    The feature flag should be used transiently to facilitate migration of cluster to using advanced security.

  4. Why these changes are required?
    Currently, migrating to advanced security requires a full cluster restart as any other kind of migration process causes transport communication to break between old and newer nodes. These changes will allow a cluster to be migrated to use advanced security without any downtime, by allowing old and new nodes to successfully communicate at the transport layer.
    NOTE: There is an existing feature disable_intertransport_auth that if enabled will cause transport level authorization to be skipped. However, the side affect of this is that any non-super admin user will be able to update opendistro_security during migration.
    passive_intertransport_auth does not skip authorization, instead it injects a default user, which you can map to the permissions you want to give during this transient phase of migration.

  5. What is the old behavior before changes and new behavior after changes? (Please add any example/logs/screen-shot if available)
    Old Behaviour: Transport communication from old node to a new node fails at the new nodes with the error: No user found for...
    New Behaviour: Transport communication from old node to a new node succeeds. When a user is not found and this feature flag is set to true, we inject a default user. Note that this user should have all index and cluster permissions.

  6. Testing done: (Please provide details of testing done: Unit testing, integration testing and manual testing)
    Extensive UTs to cover all cases.

__With passive_intertransport_auth disabled:__
dhireshj@f45c899db61b ~/w/d/dhi_security (transport_passive_mode)> curl "https://localhost:9200/_cat/nodes" -k
192.168.240.3 18 22 44 2.01 1.88 1.04 dimr * elasticsearch1
192.168.240.2 55 20 49 2.01 1.88 1.04 dir  - elasticsearch2
dhireshj@f45c899db61b ~/w/d/dhi_security (transport_passive_mode)> curl -X PUT "https://localhost:9200/test5?pretty" -H 'Content-Type: application/json' -d'
                                                                   {
                                                                       "settings": {
                                                                           "index": {
                                                                             "number_of_shards": 1,
                                                                             "auto_expand_replicas": "0-all",
                                                                             "routing.allocation.exclude._name": "elasticsearch1"
                                                                           }
                                                                       }
                                                                   }' -k
{
  "acknowledged" : true,
  "shards_acknowledged" : true,
  "index" : "test5"
}
dhireshj@f45c899db61b ~/w/d/dhi_security (transport_passive_mode)> curl -XPOST "https://localhost:9200/test5/_doc/1" -k -H 'Content-Type: application/json' -d'{}' -k
{"error":{"root_cause":[{"type":"security_exception","reason":"No user found for indices:admin/mapping/auto_put"}],"type":"security_exception","reason":"No user found for indices:admin/mapping/auto_put"},"status":500}⏎


__With passive_intertransport_auth enabled:__

dhireshj@f45c899db61b ~/w/d/dhi_security (transport_passive_mode) [60]> curl "https://localhost:9200/_cat/nodes" -k
192.168.240.3 17 21 48 2.84 1.97 0.95 dimr * elasticsearch1
192.168.240.2 26 21 49 2.84 1.97 0.95 dir  - elasticsearch2
dhireshj@f45c899db61b ~/w/d/dhi_security (transport_passive_mode)> curl -X PUT "https://localhost:9200/test4?pretty" -H 'Content-Type: application/json' -d'
                                                                   {
                                                                       "settings": {
                                                                           "index": {
                                                                             "number_of_shards": 1,
                                                                             "auto_expand_replicas": "0-all",
                                                                             "routing.allocation.exclude._name": "elasticsearch1"
                                                                           }
                                                                       }
                                                                   }' -k
{
  "acknowledged" : true,
  "shards_acknowledged" : true,
  "index" : "test4"
}
dhireshj@f45c899db61b ~/w/d/dhi_security (transport_passive_mode) [52]> curl -XPOST "https://localhost:9200/test4/_doc/1" -k -H 'Content-Type: application/json' -d'{}' -k
{"_index":"test4","_type":"_doc","_id":"1","_version":1,"result":"created","_shards":{"total":1,"successful":1,"failed":0},"_seq_no":0,"_primary_term":1}⏎

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or

(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.

(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.

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.

@dhiAmzn dhiAmzn requested a review from a team May 27, 2021 17:53
@dhiAmzn dhiAmzn force-pushed the transport_passive_mode branch 2 times, most recently from c7853af to 974087a Compare May 28, 2021 21:19
@codecov-commenter
Copy link

codecov-commenter commented May 28, 2021

Codecov Report

Merging #1156 (ada9ef5) into main (2ecb088) will increase coverage by 0.04%.
The diff coverage is 75.38%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #1156      +/-   ##
============================================
+ Coverage     64.61%   64.65%   +0.04%     
- Complexity     3178     3193      +15     
============================================
  Files           245      247       +2     
  Lines         17140    17191      +51     
  Branches       3034     3042       +8     
============================================
+ Hits          11075    11115      +40     
- Misses         4519     4526       +7     
- Partials       1546     1550       +4     
Impacted Files Coverage Δ
...arch/security/ssl/OpenSearchSecuritySSLPlugin.java 83.97% <0.00%> (ø)
...ssl/transport/SecuritySSLTransportInterceptor.java 0.00% <0.00%> (ø)
...g/opensearch/security/support/ConfigConstants.java 94.44% <ø> (ø)
...rch/security/setting/OpensearchDynamicSetting.java 60.00% <60.00%> (ø)
...pensearch/security/configuration/CompatConfig.java 41.66% <66.66%> (+12.63%) ⬆️
.../security/setting/TransportPassiveAuthSetting.java 80.00% <80.00%> (ø)
...rch/security/transport/SecurityRequestHandler.java 75.71% <83.33%> (+0.52%) ⬆️
...org/opensearch/security/filter/SecurityFilter.java 78.03% <87.50%> (+2.43%) ⬆️
.../opensearch/security/OpenSearchSecurityPlugin.java 81.34% <100.00%> (+0.11%) ⬆️
...g/opensearch/security/ssl/transport/SSLConfig.java 100.00% <100.00%> (+10.00%) ⬆️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2ecb088...ada9ef5. Read the comment docs.

vengadanathan-s
vengadanathan-s previously approved these changes Jun 4, 2021
…etween nodes with adv sec enabled and nodes without adv sec enabled.

Changes to ssl_dual_mode_enabled to support the same.

Signed-off-by: Dhiresh Jain <dhireshj@amazon.com>
Signed-off-by: Dhiresh Jain <dhireshj@amazon.com>
Signed-off-by: Dhiresh Jain <dhireshj@amazon.com>
…r setting instead of opendistro config

Signed-off-by: Dhiresh Jain <dhireshj@amazon.com>
Signed-off-by: Dhiresh Jain <dhireshj@amazon.com>
Signed-off-by: Dhiresh Jain <dhireshj@amazon.com>
Signed-off-by: Dhiresh Jain <dhireshj@amazon.com>
Signed-off-by: Dhiresh Jain <dhireshj@amazon.com>
Signed-off-by: Dhiresh Jain <dhireshj@amazon.com>
niravpi pushed a commit to niravpi/security that referenced this pull request Jun 18, 2021
…etween nodes with adv sec enabled and nodes without adv sec enabled.(opensearch-project#1156)

(Cherry picked from commit 9adcd20)
niravpi pushed a commit to niravpi/security that referenced this pull request Jun 18, 2021
…etween nodes with adv sec enabled and nodes without adv sec enabled.(opensearch-project#1156)

(Cherry picked from commit 9adcd20)
niravpi pushed a commit to niravpi/security that referenced this pull request Jun 18, 2021
…etween nodes with adv sec enabled and nodes without adv sec enabled.(opensearch-project#1156)

(Cherry picked from commit 9adcd20)
niravpi pushed a commit to niravpi/security that referenced this pull request Jun 18, 2021
…etween nodes with adv sec enabled and nodes without adv sec enabled.(opensearch-project#1156)

(Cherry picked from commit 9adcd20)
niravpi pushed a commit to niravpi/security that referenced this pull request Jun 18, 2021
…etween nodes with adv sec enabled and nodes without adv sec enabled.(opensearch-project#1156)

(Cherry picked from commit 9adcd20)
niravpi pushed a commit to niravpi/security that referenced this pull request Jun 18, 2021
…etween nodes with adv sec enabled and nodes without adv sec enabled.(opensearch-project#1156)

(Cherry picked from commit 9adcd20)
niravpi pushed a commit to niravpi/security that referenced this pull request Jun 18, 2021
…etween nodes with adv sec enabled and nodes without adv sec enabled.(opensearch-project#1156)

(Cherry picked from commit 9adcd20)
niravpi pushed a commit to niravpi/security that referenced this pull request Jun 18, 2021
…etween nodes with adv sec enabled and nodes without adv sec enabled.(opensearch-project#1156)

(Cherry picked from commit 9adcd20)
niravpi pushed a commit to niravpi/security that referenced this pull request Jun 21, 2021
…etween nodes with adv sec enabled and nodes without adv sec enabled.(opensearch-project#1156)

(Cherry picked from commit 9adcd20)
niravpi pushed a commit to niravpi/security that referenced this pull request Jun 21, 2021
…etween nodes with adv sec enabled and nodes without adv sec enabled.(opensearch-project#1156)

(Cherry picked from commit 9adcd20)
vrozov pushed a commit that referenced this pull request Jun 21, 2021
…etween nodes with adv sec enabled and nodes without adv sec enabled.(#1156)

(Cherry picked from commit 9adcd20)
vrozov pushed a commit that referenced this pull request Jun 21, 2021
…etween nodes with adv sec enabled and nodes without adv sec enabled.(#1156)

(Cherry picked from commit 9adcd20)
vrozov pushed a commit that referenced this pull request Jun 21, 2021
…etween nodes with adv sec enabled and nodes without adv sec enabled.(#1156)

(Cherry picked from commit 9adcd20)
vrozov pushed a commit that referenced this pull request Jun 21, 2021
…etween nodes with adv sec enabled and nodes without adv sec enabled.(#1156)

(Cherry picked from commit 9adcd20)
vrozov pushed a commit that referenced this pull request Jun 21, 2021
…etween nodes with adv sec enabled and nodes without adv sec enabled.(#1156)

(Cherry picked from commit 9adcd20)
vrozov pushed a commit that referenced this pull request Jun 21, 2021
…etween nodes with adv sec enabled and nodes without adv sec enabled.(#1156)

(Cherry picked from commit 9adcd20)
vrozov pushed a commit that referenced this pull request Jun 21, 2021
…etween nodes with adv sec enabled and nodes without adv sec enabled.(#1156)

(Cherry picked from commit 9adcd20)
vrozov pushed a commit that referenced this pull request Jun 21, 2021
…etween nodes with adv sec enabled and nodes without adv sec enabled.(#1156)

(Cherry picked from commit 9adcd20)
vrozov pushed a commit that referenced this pull request Jun 21, 2021
…etween nodes with adv sec enabled and nodes without adv sec enabled.(#1156)

(Cherry picked from commit 9adcd20)
vrozov pushed a commit that referenced this pull request Jun 22, 2021
…etween nodes with adv sec enabled and nodes without adv sec enabled.(#1156)

(Cherry picked from commit 9adcd20)
andy840314 pushed a commit that referenced this pull request Jun 28, 2021
…etween nodes with adv sec enabled and nodes without adv sec enabled.(#1156)

(Cherry picked from commit 9adcd20)
andy840314 pushed a commit that referenced this pull request Jun 28, 2021
…etween nodes with adv sec enabled and nodes without adv sec enabled.(#1156)

(Cherry picked from commit 9adcd20)
@cliu123 cliu123 added the enhancement New feature or request label Jun 29, 2021
cliu123 pushed a commit that referenced this pull request Jul 15, 2021
* Correcting setupSslOnlyMode to use AbstractSecurityUnitTest.hasCustomTransportSettings() (#1057)
(cherry picked from commit 70a4f70)

* Introducing passive_intertransport_auth to facilitate communication between nodes with adv sec enabled and nodes without adv sec enabled.(#1156)
(Cherry picked from commit 9adcd20)

Co-authored-by: Debjani Banerjee <56744681+debjanibnrj@users.noreply.github.com>
Co-authored-by: dhiAmzn <81139246+dhiAmzn@users.noreply.github.com>
cliu123 pushed a commit that referenced this pull request Jul 15, 2021
* Correcting setupSslOnlyMode to use AbstractSecurityUnitTest.hasCustomTransportSettings() (#1057)
(cherry picked from commit 70a4f70)

* Introducing passive_intertransport_auth to facilitate communication between nodes with adv sec enabled and nodes without adv sec enabled.(#1156)
(Cherry picked from commit 9adcd20)

Co-authored-by: Debjani Banerjee <56744681+debjanibnrj@users.noreply.github.com>
Co-authored-by: dhiAmzn <81139246+dhiAmzn@users.noreply.github.com>
cliu123 pushed a commit that referenced this pull request Jul 16, 2021
* Correcting setupSslOnlyMode to use AbstractSecurityUnitTest.hasCustomTransportSettings() (#1057)
(cherry picked from commit 70a4f70)

* Introducing passive_intertransport_auth to facilitate communication between nodes with adv sec enabled and nodes without adv sec enabled.(#1156)
(Cherry picked from commit 9adcd20)

Co-authored-by: Debjani Banerjee <56744681+debjanibnrj@users.noreply.github.com>
Co-authored-by: dhiAmzn <81139246+dhiAmzn@users.noreply.github.com>
lbreinig pushed a commit to lbreinig/security that referenced this pull request Dec 23, 2021
opensearch-project#1156)

* Introducing passive_intertransport_auth to facilitate communication between nodes with adv sec enabled and nodes without adv sec enabled.

Changes to ssl_dual_mode_enabled to support the same.

Signed-off-by: Dhiresh Jain <dhireshj@amazon.com>

* Addressed comments

Signed-off-by: Dhiresh Jain <dhireshj@amazon.com>

* Minor fix

Signed-off-by: Dhiresh Jain <dhireshj@amazon.com>

* Modifying transportInterClusterPassiveAuth to be controlled by cluster setting instead of opendistro config

Signed-off-by: Dhiresh Jain <dhireshj@amazon.com>

* Fixing unit tests

Signed-off-by: Dhiresh Jain <dhireshj@amazon.com>

* Resolving conflicts with mainline

Signed-off-by: Dhiresh Jain <dhireshj@amazon.com>

* Removing Filtered property from new setting, improving code readability

Signed-off-by: Dhiresh Jain <dhireshj@amazon.com>

* Fixing UTs where node space causing shards to be unassigned

Signed-off-by: Dhiresh Jain <dhireshj@amazon.com>

* Reverting cluster setting in UT

Signed-off-by: Dhiresh Jain <dhireshj@amazon.com>

* Addressing minor comment on UT

Signed-off-by: Dhiresh Jain <dhireshj@amazon.com>
wuychn pushed a commit to ochprince/security that referenced this pull request Mar 16, 2023
opensearch-project#1156)

* Introducing passive_intertransport_auth to facilitate communication between nodes with adv sec enabled and nodes without adv sec enabled.

Changes to ssl_dual_mode_enabled to support the same.

Signed-off-by: Dhiresh Jain <dhireshj@amazon.com>

* Addressed comments

Signed-off-by: Dhiresh Jain <dhireshj@amazon.com>

* Minor fix

Signed-off-by: Dhiresh Jain <dhireshj@amazon.com>

* Modifying transportInterClusterPassiveAuth to be controlled by cluster setting instead of opendistro config

Signed-off-by: Dhiresh Jain <dhireshj@amazon.com>

* Fixing unit tests

Signed-off-by: Dhiresh Jain <dhireshj@amazon.com>

* Resolving conflicts with mainline

Signed-off-by: Dhiresh Jain <dhireshj@amazon.com>

* Removing Filtered property from new setting, improving code readability

Signed-off-by: Dhiresh Jain <dhireshj@amazon.com>

* Fixing UTs where node space causing shards to be unassigned

Signed-off-by: Dhiresh Jain <dhireshj@amazon.com>

* Reverting cluster setting in UT

Signed-off-by: Dhiresh Jain <dhireshj@amazon.com>

* Addressing minor comment on UT

Signed-off-by: Dhiresh Jain <dhireshj@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants