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 missing verify option for unwrap request #32

Merged
merged 1 commit into from
Jan 9, 2024

Conversation

Syoc
Copy link
Contributor

@Syoc Syoc commented Jan 3, 2024

I might be mistaken, but I think the unwrap request is missing the verify option.
Users are as a result unable to use unwrap with a custom verify setting.

What issues does this PR fix or reference?

I can create an associated issue if required.

Previous Behavior

Traceback (most recent call last):
File "/usr/bin/salt-call", line 11, in
sys.exit(salt_call())
File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/scripts.py", line 443, in salt_call
client.run()
File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/cli/call.py", line 50, in run
caller.run()
File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/cli/caller.py", line 95, in run
ret = self.call()
File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/cli/caller.py", line 202, in call ret["return"] = self.minion.executors[fname](
File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 159, in call
ret = self.loader.run(run_func, *args, **kwargs)
File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 1245, in run
return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs)
File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 1260, in _run_as
return _func_or_method(*args, **kwargs)
File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/executors/direct_call.py", line 10, in execute
return func(*args, **kwargs)
File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 159, in call
ret = self.loader.run(run_func, *args, **kwargs)
File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 1245, in run
return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs)
File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 1260, in _run_as
return _func_or_method(*args, **kwargs)
File "/opt/saltstack/salt/lib/python3.10/site-packages/saltext/vault/modules/vault.py", line 1306, in get_server_config
client = vault.get_authd_client(opts, context)
File "/opt/saltstack/salt/lib/python3.10/site-packages/saltext/vault/utils/vault/factory.py", line 101, in get_authd_client
client, config = _build_authd_client(opts, context, force_local=force_local)
File "/opt/saltstack/salt/lib/python3.10/site-packages/saltext/vault/utils/vault/factory.py", line 275, in _build_authd_client
config, embedded_token, unauthd_client = _get_connection_config(
File "/opt/saltstack/salt/lib/python3.10/site-packages/saltext/vault/utils/vault/factory.py", line 430, in _get_connection_config
new_config, unwrap_client = _query_master(
File "/opt/saltstack/salt/lib/python3.10/site-packages/saltext/vault/utils/vault/factory.py", line 767, in _query_master
return check_result(
File "/opt/saltstack/salt/lib/python3.10/site-packages/saltext/vault/utils/vault/factory.py", line 692, in check_result
unwrapped_response = unwrap_client.unwrap(
File "/opt/saltstack/salt/lib/python3.10/site-packages/saltext/vault/utils/vault/client.py", line 240, in unwrap
res = self.session.request("POST", url, headers=headers, json=payload)
File "/opt/saltstack/salt/lib/python3.10/site-packages/requests/sessions.py", line 589, in request
resp = self.send(prep, **send_kwargs)
File "/opt/saltstack/salt/lib/python3.10/site-packages/requests/sessions.py", line 703, in send r = adapter.send(request, **kwargs)
File "/opt/saltstack/salt/lib/python3.10/site-packages/requests/adapters.py", line 517, in send raise SSLError(e, request=request)
requests.exceptions.SSLError: HTTPSConnectionPool(host='vault', port=8200): Max retries exceeded with url: /v1/sys/wrapping/unwrap (Caused by SSLError(SSLCertVerificationError(1, '[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: self-signed certificate in certificate chain (_ssl.c:1007)')))

New Behavior

IIt works.

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

No

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

Copy link
Member

@lkubb lkubb left a comment

Choose a reason for hiding this comment

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

Thanks for submitting this fix!

Could you create an issue (can be very basic, mostly for the changelog) and add a changelog entry? Something like echo Fixed verify parameter for unwrap requests > changelog/<issue#>.fixed.md. Otherwise LGTM.

Note that pinning the trusted root certificate still works since it's implemented on top of requests, but setting verify to anything else should not work with wrapping currently, good catch!

Edit: Also the lint check is failing, maybe you can get that fixed up as well.

Copy link
Member

@lkubb lkubb left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! 🚀

@lkubb lkubb merged commit 918e9e9 into salt-extensions:main Jan 9, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Verify parameter is not honored by unwrap client
2 participants