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

Add install_demo_configuration Batch script for Windows #2161

Merged
merged 12 commits into from
Oct 27, 2022

Conversation

cwperks
Copy link
Member

@cwperks cwperks commented Oct 12, 2022

Description

As part of the effort to support windows distribution in 2.4.0 (opensearch-project/opensearch-build#33) the security plugin needs a script to install the demo configuration on windows.

This PR introduces install_demo_configuration.bat with the same functionality as the bash script and installs the demo certificates and adds default config lines to opensearch.yml

  • Category (Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation)

Enhancement

Issues Resolved

#2148

Testing

Tested by going through the steps outlined on opensearch-project/opensearch-build#33.

Replace step 6 with cd plugins/opensearch-security/tools & ./install_demo_configuration.bat

Screenshot below shows using https and logging in with admin account credentials:

Screen Shot 2022-10-12 at 3 55 17 PM

Check List

  • New functionality includes testing
  • New functionality has been documented
  • Commits are signed per the DCO using --signoff

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.

@cwperks cwperks requested a review from a team October 12, 2022 20:00
@peterzhuamazon peterzhuamazon added the backport 2.x backport to 2.x branch label Oct 12, 2022
@peterzhuamazon
Copy link
Member

Thanks @cwperks adding this to backport 2.x.

@codecov-commenter
Copy link

codecov-commenter commented Oct 12, 2022

Codecov Report

Merging #2161 (465a117) into main (e5ab646) will increase coverage by 0.13%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##               main    #2161      +/-   ##
============================================
+ Coverage     61.02%   61.15%   +0.13%     
- Complexity     3233     3239       +6     
============================================
  Files           257      258       +1     
  Lines         18110    18110              
  Branches       3229     3224       -5     
============================================
+ Hits          11052    11076      +24     
+ Misses         5476     5456      -20     
+ Partials       1582     1578       -4     
Impacted Files Coverage Δ
...ch/security/privileges/PitPrivilegesEvaluator.java 96.42% <0.00%> (ø)
.../opensearch/security/OpenSearchSecurityPlugin.java 79.95% <0.00%> (+0.08%) ⬆️
...earch/security/dlic/rest/api/MigrateApiAction.java 4.34% <0.00%> (+0.18%) ⬆️
...a/org/opensearch/security/tools/SecurityAdmin.java 36.00% <0.00%> (+0.24%) ⬆️
...earch/security/privileges/PrivilegesEvaluator.java 72.38% <0.00%> (+0.35%) ⬆️
...arch/security/configuration/ClusterInfoHolder.java 66.66% <0.00%> (+0.81%) ⬆️
...ty/configuration/ConfigurationLoaderSecurity7.java 70.43% <0.00%> (+4.04%) ⬆️
...nsearch/security/dlic/rest/api/AuditApiAction.java 68.08% <0.00%> (+4.25%) ⬆️
...ecurity/configuration/StaticResourceException.java 25.00% <0.00%> (+25.00%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@peternied peternied left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets update the plugin_install.yml to deploy OpenSearch windows and test this new script.

@cwperks
Copy link
Member Author

cwperks commented Oct 12, 2022

Lets update the plugin_install.yml to deploy OpenSearch windows and test this new script.

Good idea - I will see what it takes to add this to the github workflow.

Signed-off-by: Stephen Crawford <[email protected]>
@DarshitChanpura
Copy link
Member

Lets update the plugin_install.yml to deploy OpenSearch windows and test this new script.

Good idea - I will see what it takes to add this to the github workflow.

Plugin install currently runs on Ubuntu. In that case we will have to add a new workflow for windows.

@stephen-crawford stephen-crawford mentioned this pull request Oct 24, 2022
3 tasks
@cwperks cwperks force-pushed the demo-config-batch-script branch from c0b6cf8 to 53839ae Compare October 24, 2022 14:35
- name: Download OpenSearch Core
run: |
cd ..
Invoke-WebRequest https://artifacts.opensearch.org/snapshots/core/opensearch/2.4.0-SNAPSHOT/opensearch-min-2.4.0-SNAPSHOT-windows-x64-latest.zip -Outfile opensearch-2.4.0.zip
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@scrawfor99 - Is it possible to get this value from gradle properties like the linux-install?

If not, then I think plugin_install.yml needs to be added to the version increment automation here: https://github.com/opensearch-project/security/blob/main/build.gradle#L602-L605

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently there is not a great option for doing this. It is possible but Powershell is not as friendly about it as Unix. I can work on it if you would like however.

Signed-off-by: Stephen Crawford <[email protected]>
@peternied peternied added the v2.4.0 'Issues and PRs related to version v2.4.0' label Oct 24, 2022
@cwperks
Copy link
Member Author

cwperks commented Oct 24, 2022

@opensearch-project/security This PR is now ready for further review. Thank you @scrawfor99 for figuring out how to automate the validation of this change by adding a new job in plugin_install to install this on a windows running and verify the node is brought up successfully. Great work!

Note: For the merge into main we are disabling this job because the distribution for 3.0.0 is not stable and the node does not get brought up successfully. This job will be enabled in 2.x when this change is backported.

Copy link
Member

@peternied peternied left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! I would prefer we addressed the comments I've added as they would lower the amount of work/changes we need to do after merging and getting this running in the 2.4 branch much cleaner.

Please take a look at them if you think it would be better to handle out of band and file another issue for follow up

$encodedCredentials = [Convert]::ToBase64String($credentialBytes)
$baseCredentials = "Basic $encodedCredentials"
$Headers = @{ Authorization = $baseCredentials }
Invoke-WebRequest -SkipCertificateCheck -Uri 'https://localhost:9200' -Headers $Headers
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Save some code and rewrite as Invoke-WebRequest -SkipCertificateCheck -Uri 'https://admin:admin@localhost:9200' , worked when I tried it locally. More details on this format [1]

[1] https://serverfault.com/questions/371907/can-you-pass-user-pass-for-http-basic-authentication-in-url-parameters

- name: Download OpenSearch Core
run: |
cd ..
Invoke-WebRequest https://artifacts.opensearch.org/snapshots/core/opensearch/3.0.0-SNAPSHOT/opensearch-min-3.0.0-SNAPSHOT-windows-x64-latest.zip -Outfile opensearch-3.0.0.zip
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we replace all the references to 3.0.0 with something like ${{ env.OPENSEARCH_VERSION }}, see how this can be set for the job with the env [1]

[1] https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#env

run: ./gradlew integTestRemote -Dtests.rest.cluster=localhost:9200 -Dtests.cluster=localhost:9200 -Dtests.clustername="opensearch" -Dhttps=true -Duser=admin -Dpassword=admin

windows-install:
if: ${{ false }} # disable for now - no stable distribution for 3.0.0 yet
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a link to the issue associated with this failure

Note; we should create an issue in our repo to re-enable

- name: Run OpenSearch with plugin
run: |
cd ..
start .\Opensearch\bin\opensearch.bat
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should have a way to get the logs from the process in cases it doesn't come up like in the linux version

https://github.com/opensearch-project/security/blob/main/.github/workflows/plugin_install.yml#L60-L62

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note; this is a good candidate for a follow up issue

@cwperks
Copy link
Member Author

cwperks commented Oct 25, 2022

@peterzhuamazon Tagging in case you know someone who can help answer a problem we are facing. As part of this PR to add the batch script for install_demo_configuration.bat this PR adds a new job in the plugin_install.yml Github Action to verify the installation on windows - as we also do with linux. This runs on the windows-latest github runner.

We are currently facing a problem where the opensearch node does not startup (the command .\bin\opensearch.bat) because the windows-latest runner is invoking the command as admin and opensearch does not allow the node to run as admin.

In the linux install we add a user and run with the newly created user. It looks like the corresponding windows commands would be:

net user "username" "<password>" /add
runas /user:username ".\bin\program.bat"

but runas prompts for user input and both passwordless and piping in a password are unsupported.

The github action we created works on 2.x, but not on 3.0. It only works on 2.x because this PR (opensearch-project/OpenSearch#4656) has not yet been backported to 2.x.

@cwperks
Copy link
Member Author

cwperks commented Oct 26, 2022

At one point in time there may have been something similar to:

if (Boolean.parseBoolean(System.getProperty("os.insecure.allow.root"))) {
      logger.warn("running as ROOT user. this is a bad idea!");
}

in Bootstrap.java, but there is no such option.

I'm not sure if it helps, but github's documentation says UAC is disabled on the runners. See https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners#administrative-privileges

Ideally, the workflow creates a user opensearch and uses this user account to bring up a node.

@cwperks
Copy link
Member Author

cwperks commented Oct 27, 2022

@opensearch-project/security The new plugin-install job on windows is working now after the revert from core and new snapshot distribution. This is ready to be reviewed again - there is a link to a snapshot distribution that will need to be updated when an official distro is published.

Copy link
Member

@peternied peternied left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥳

@cliu123
Copy link
Member

cliu123 commented Oct 27, 2022

Removing the old Plugin Install task from required checks and replace it with linux-install and windows-install to unblock the merge.

@cliu123 cliu123 merged commit 45c766f into opensearch-project:main Oct 27, 2022
opensearch-trigger-bot bot pushed a commit that referenced this pull request Oct 27, 2022
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Stephen Crawford <[email protected]>

Co-authored-by: Stephen Crawford <[email protected]>
Co-authored-by: Stephen Crawford <[email protected]>
(cherry picked from commit 45c766f)
peterzhuamazon added a commit that referenced this pull request Oct 28, 2022
#2203)

* Add install_demo_configuration Batch script for Windows (#2161)

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

Co-authored-by: Stephen Crawford <[email protected]>
Co-authored-by: Stephen Crawford <[email protected]>
(cherry picked from commit 45c766f)

* Change to 2.4.0 for the min url

Signed-off-by: Peter Zhu <[email protected]>

Signed-off-by: Peter Zhu <[email protected]>
Co-authored-by: Craig Perkins <[email protected]>
Co-authored-by: Peter Zhu <[email protected]>
@dbwiddis
Copy link
Member

I am going to jot down a running list of the things I tried so that if we run into this problem again in the future we have an idea what is worthwhile:

trustlevel 2000 with relative path trustlevel 2000 with absolute path Start-process runas Normdo -- lacks enough documentation for me to debug running errors explorer.exe does not start

Tried a few more things, documented here

@cwperks
Copy link
Member Author

cwperks commented Oct 31, 2022

@dbwiddis I have also been trying with psexec here: main...cwperks:security:windows-debugging#diff-3e971aab38c741b1cb89584068ce23192213a18b1ecc885ac7b450bf5a5bb86eR85-R140 and it feels like its close to a breakthrough, but the swallowing of the output is causing difficulty. I plan to return to this issue so that we can re-apply your change of enforcing that the node does not start with escalated privileges.

@dbwiddis
Copy link
Member

dbwiddis commented Nov 3, 2022

@cwperks
Copy link
Member Author

cwperks commented Nov 3, 2022

Thank you @dbwiddis. I saw your comment this morning and was trying to apply it on this branch https://github.com/cwperks/security/tree/windows-paexec but have still been unsuccessful in bringing up the node. I am able to use both psexec and paexec on a windows computer, but still struggling within the github runner.

@dbwiddis
Copy link
Member

dbwiddis commented Nov 3, 2022

I found that having the .cmd extension on the "batch file" seemed to do something magical that I can't explain..

Specifically: cmd /c xyz only works with built in commands. But it worked calling xyz.cmd directly. And did not work with some other extensions.

Also set the working directory using -w

@peterzhuamazon
Copy link
Member

https://stackoverflow.com/questions/148968/windows-batch-files-bat-vs-cmd

Seems like not that much differences?
I am also wondering as I indeed see more .cmd back in the day than it is now.

Thanks.

@dbwiddis
Copy link
Member

dbwiddis commented Nov 4, 2022

Seems like not that much differences?

I suspect it was the error value difference that I saw...

stephen-crawford pushed a commit to stephen-crawford/security that referenced this pull request Nov 10, 2022
opensearch-project#2203)

* Add install_demo_configuration Batch script for Windows (opensearch-project#2161)

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

Co-authored-by: Stephen Crawford <[email protected]>
Co-authored-by: Stephen Crawford <[email protected]>
(cherry picked from commit 45c766f)

* Change to 2.4.0 for the min url

Signed-off-by: Peter Zhu <[email protected]>

Signed-off-by: Peter Zhu <[email protected]>
Co-authored-by: Craig Perkins <[email protected]>
Co-authored-by: Peter Zhu <[email protected]>
stephen-crawford added a commit to stephen-crawford/security that referenced this pull request Nov 10, 2022
…roject#2161)

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

Co-authored-by: Stephen Crawford <[email protected]>
Co-authored-by: Stephen Crawford <[email protected]>
Signed-off-by: Stephen Crawford <[email protected]>
stephen-crawford pushed a commit to stephen-crawford/security that referenced this pull request Nov 10, 2022
opensearch-project#2203)

* Add install_demo_configuration Batch script for Windows (opensearch-project#2161)

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

Co-authored-by: Stephen Crawford <[email protected]>
Co-authored-by: Stephen Crawford <[email protected]>
(cherry picked from commit 45c766f)

* Change to 2.4.0 for the min url

Signed-off-by: Peter Zhu <[email protected]>

Signed-off-by: Peter Zhu <[email protected]>
Co-authored-by: Craig Perkins <[email protected]>
Co-authored-by: Peter Zhu <[email protected]>
Signed-off-by: Stephen Crawford <[email protected]>
stephen-crawford pushed a commit to stephen-crawford/security that referenced this pull request Nov 10, 2022
opensearch-project#2203)

* Add install_demo_configuration Batch script for Windows (opensearch-project#2161)

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

Co-authored-by: Stephen Crawford <[email protected]>
Co-authored-by: Stephen Crawford <[email protected]>
(cherry picked from commit 45c766f)

* Change to 2.4.0 for the min url

Signed-off-by: Peter Zhu <[email protected]>

Signed-off-by: Peter Zhu <[email protected]>
Co-authored-by: Craig Perkins <[email protected]>
Co-authored-by: Peter Zhu <[email protected]>
Signed-off-by: Stephen Crawford <[email protected]>
@cwperks cwperks added the backport 1.3 backport to 1.3 branch label Nov 18, 2022
@opensearch-trigger-bot
Copy link
Contributor

The backport to 1.3 failed:

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

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-1.3 1.3
# Navigate to the new working tree
cd .worktrees/backport-1.3
# Create a new branch
git switch --create backport/backport-2161-to-1.3
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 45c766f5b98bb88b642d0c7fbccb32653213c300
# Push it to GitHub
git push --set-upstream origin backport/backport-2161-to-1.3
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-1.3

Then, create a pull request where the base branch is 1.3 and the compare/head branch is backport/backport-2161-to-1.3.

wuychn pushed a commit to ochprince/security that referenced this pull request Mar 16, 2023
opensearch-project#2203)

* Add install_demo_configuration Batch script for Windows (opensearch-project#2161)

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

Co-authored-by: Stephen Crawford <[email protected]>
Co-authored-by: Stephen Crawford <[email protected]>
(cherry picked from commit 45c766f)

* Change to 2.4.0 for the min url

Signed-off-by: Peter Zhu <[email protected]>

Signed-off-by: Peter Zhu <[email protected]>
Co-authored-by: Craig Perkins <[email protected]>
Co-authored-by: Peter Zhu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.3 backport to 1.3 branch backport 2.x backport to 2.x branch v2.4.0 'Issues and PRs related to version v2.4.0'
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants