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

[BUG] Requested method not exposed: minion_runner #57016

Closed
idontwanttosignin opened this issue Apr 30, 2020 · 10 comments
Closed

[BUG] Requested method not exposed: minion_runner #57016

idontwanttosignin opened this issue Apr 30, 2020 · 10 comments
Assignees
Labels
Bug broken, incorrect, or confusing behavior Core relates to code central or existential to Salt severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around v2019.2.5 unsupported version v3000.3 vulnerable version
Milestone

Comments

@idontwanttosignin
Copy link

So I'm a bit confused on the known issue for 2019.2.4. We are seeing highstate failures on several servers with the patch, changing _minion_runner to minion_runner in expose_methods seems to fix theses issues.

[ERROR ] Requested method not exposed: minion_runner

It seems like this is a know error that won't be fixed till June? This seems like a small fix to have that long of a timeline, is there more to this that I'm not seeing?

https://docs.saltstack.com/en/2019.2/topics/releases/2019.2.4.html

@twangboy
Copy link
Contributor

@idontwanttosignin @dwoz is working on a PR to address this issue. It will be a part of the Sodium release. You will have to manually patch your system if you need it before the Sodium release.

@twangboy twangboy added Core relates to code central or existential to Salt severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around labels Apr 30, 2020
@twangboy twangboy added this to the Approved milestone Apr 30, 2020
@dwoz
Copy link
Contributor

dwoz commented May 1, 2020

@idontwanttosignin

Are you able to make these changes to your salt master(s)?

diff --git a/salt/master.py b/salt/master.py
index aae55f1828..b7e632b04e 100644
--- a/salt/master.py
+++ b/salt/master.py
@@ -1177,10 +1177,11 @@ class AESFuncs(TransportMethods):
         'verify_minion', '_master_tops', '_ext_nodes', '_master_opts',
         '_mine_get', '_mine', '_mine_delete', '_mine_flush', '_file_recv',
         '_pillar', '_minion_event', '_handle_minion_event', '_return',
-        '_syndic_return', '_minion_runner', 'pub_ret', 'minion_pub',
+        '_syndic_return', 'minion_runner', 'pub_ret', 'minion_pub',
         'minion_publish', 'revoke_auth', 'run_func', '_serve_file',
         '_file_find', '_file_hash', '_file_find_and_stat', '_file_list',
         '_file_list_emptydirs', '_dir_list', '_symlink_list', '_file_envs',
+        '_file_hash_and_stat',
     )
 
     def __init__(self, opts):

@thusoy
Copy link
Contributor

thusoy commented May 1, 2020

@dwoz That's the change I made, fixes it on our setup at least. I didn't do that last line of adding the _file_hash_and_state though, what does that do?

I just want to weigh in and say that this is super frustrating as a user. Our options are handpatching our saltmaster (and every new saltmaster we create) or running with a known severe vulnerability since a single character fix won't be published for months, and that fix will only come in a new major release? Seems like a weird bind to put users in.

@finalduty
Copy link

finalduty commented May 1, 2020

Can confirm that @dwoz's patch worked for us on v3000.2 and I want to echo @thusoy's sentiment that it's weird to have to wait ~6 weeks to get this fixed.

For anyone looking to apply this patch on linux, you should be able to find the master.py file under /usr/lib. e.g. find /usr/lib -wholename '*/salt/master.py'

The following worked for me to patch the file. NB: if you're going to run this yourself, make sure you take a backup, test on a non-production system first and don't trust code from strangers on the internet.

cd /usr/lib/python3.6/site-packages/salt
cp master.py master.py.bak
cat << EOF | patch -Np1
--- old/master.py 2020-05-01 10:50:30.308977044 +1000
+++ new/master.py 2020-05-01 10:57:53.652957439 +1000
@@ -1177,10 +1177,11 @@
         'verify_minion', '_master_tops', '_ext_nodes', '_master_opts',
         '_mine_get', '_mine', '_mine_delete', '_mine_flush', '_file_recv',
         '_pillar', '_minion_event', '_handle_minion_event', '_return',
-        '_syndic_return', '_minion_runner', 'pub_ret', 'minion_pub',
+        '_syndic_return', 'minion_runner', 'pub_ret', 'minion_pub',
         'minion_publish', 'revoke_auth', 'run_func', '_serve_file',
         '_file_find', '_file_hash', '_file_find_and_stat', '_file_list',
         '_file_list_emptydirs', '_dir_list', '_symlink_list', '_file_envs',
+        '_file_hash_and_stat',
     )
 
     def __init__(self, opts):
EOF

@khicks
Copy link

khicks commented May 1, 2020

This also affects the integration with Hashicorp Vault, which can't fetch secrets anymore without the manual patch.

@idontwanttosignin
Copy link
Author

@dwoz Yes, we had tried that (minus '_file_hash_and_stat') and I can confirm that it works. Based on the patch being released with the typo as a known issue and having a 2 month time frame to get it properly fixed upstream, we were concerned that it may be more complex and changing the expose list could negate security or stability. Since it sounds like thats not the case, I'll continue forward with the roll out and patch that change in as I go.
Thank you for confirming this is a safe change to make.

@rossengeorgiev
Copy link
Contributor

rossengeorgiev commented May 2, 2020

Hi guys, I've took some time to backport the security patches since upgrading may be tricky if you are behind. I've included minion_runner bugfix as well. https://github.com/rossengeorgiev/salt-security-backports

@sagetherage sagetherage added the ZRelease-Sodium retired label label May 3, 2020
@ymasson
Copy link
Contributor

ymasson commented May 4, 2020

Can we have a release (3000.3/2019.2.5) before the Sodium release in few weeks ?

@danlsgiga
Copy link
Contributor

danlsgiga commented May 4, 2020

Agreed with the sentiment from everyone here... this is a breaking change introduced by a security patch... I think a 3000.3 release fixing these issues should come before Sodium.

This is affecting us on the publish.runner feature

@danlsgiga
Copy link
Contributor

These tips are gold for this kind of situation... https://salt.tips/patching-salt-modules/#self-patching

dwoz added a commit to dwoz/salt that referenced this issue May 5, 2020
- Fix saltstack#57016
- Fix saltstack#57027
- Add tests for exposed methods on AESFuncs and ClearFuncs
- Add response validation for patched ClearFuncs.wheel
- Add release notes template for 2019.2.5
dwoz added a commit to dwoz/salt that referenced this issue May 6, 2020
- Fix saltstack#57016
- Fix saltstack#57027
- Add tests for exposed methods on AESFuncs and ClearFuncs
- Add response validation for patched ClearFuncs.wheel
- Add release notes template for 2019.2.5
dwoz added a commit that referenced this issue May 6, 2020
- Fix #57016
- Fix #57027
- Add tests for exposed methods on AESFuncs and ClearFuncs
- Add response validation for patched ClearFuncs.wheel
- Add release notes template for 2019.2.5
@sagetherage sagetherage removed the ZRelease-Sodium retired label label May 11, 2020
@sagetherage sagetherage added the v2019.2.5 unsupported version label May 11, 2020
@sagetherage sagetherage added the v3000.3 vulnerable version label May 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior Core relates to code central or existential to Salt severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around v2019.2.5 unsupported version v3000.3 vulnerable version
Projects
None yet
Development

No branches or pull requests

10 participants