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][RC1 Sodium] Vault module cannot fetch more than one secret #57561

Closed
dkacar-oradian opened this issue Jun 5, 2020 · 6 comments · Fixed by #62684
Closed

[BUG][RC1 Sodium] Vault module cannot fetch more than one secret #57561

dkacar-oradian opened this issue Jun 5, 2020 · 6 comments · Fixed by #62684
Labels
expected-behavior intended functionality
Milestone

Comments

@dkacar-oradian
Copy link

Description
The vault module in Sodium RC can get only one secret in a run. Attempting to get the second secret results in an error.

Setup
Master configuration:

vault:
  url: https://xdcvault.oradian.ord:8200
  auth:
    uses: 1
    method: token
    token: xxx-redacted-xxx
  policies:
    - salt-policy

The same effect happens with or without uses directive. The same configuration (without uses) works perfectly fine on 3000.3 (and it's the same token).

pillar/top.sls:

base:
  stest-app-01.xdc:
    - testvault

pillar/testvault.sls:

test:
  foo: bar
  jvmdump_pubkey: {{ salt.vault.read_secret("secretsv1/infra/services/jvmdump/ssh_key", "public_key") }}
  jenkins_pubkey: {{ salt.vault.read_secret("secretsv1/infra/services/jenkins/master/ssh_key", "public_key") }}

Those secrets use Vault V1 protocol because that is the only option with the current release (3000.3).

Steps to Reproduce the behavior

This is all happening on master. First demonstration that the secrets from Vault can be obtained one by one:

# salt-run salt.cmd vault.read_secret secretsv1/infra/services/jenkins/master/ssh_key public_key
[INFO    ] Runner completed: 20200605113856409157
ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAx/M8W5yvaxtYaBwnZSH1mSUoiYCWtjMhUNPX1a9GlV8QEGdCu+CoEEn++HLC67gtOUJVmlZfxNJ1AqPp9Iuog86erKZKnd+mFLQNNyBGeWX8J6cUiwVu7U9xk8g5NXU6kIO+JmxvIToslJla7Rl5BiHaEEViPxwwxKD3drT0KhHHb/9G/xAuTRowgaYJxad7id41R0TH4u2xMAMgTLe+xHaRk+tFem6PcUMf0Fkr3164zsHNngGWST/JQpgz/h1HXiw4wslEke3yvU2ED6HEXRglD/pmyLZVKGVP8m8EJdnBCeWaBlWqe8rTx9O3BaDrTa8J/kwU+aiFHHSM1E3RlQ== Jenkins access key
[INFO    ] Runner completed: 20200605113853756613
# salt-run salt.cmd vault.read_secret secretsv1/infra/services/jvmdump/ssh_key public_key
[INFO    ] Runner completed: 20200605113911398018
ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAtM09Yx+6xdOm3yVUWD/OoQLeEtVFENm/wdzTMhUajr4xdf9kGGv7K1fQV7KNyPAFj6ygmtO0N0VbkaqwkGyTFIaho9US3pqVx4KPr8vIf8TAOkJLXEsLfX10x+wMdBaalJ7EV59HeoCk0E0GcZjmNaUtpcIzzJWNNIB2Vs1hNajZycrQM5r7H20/Nh3m49rjO5PvfgkuLVYlY8HOihNxIZXqvegQER/dQJu7ssDIgvKWK5w0wO3UK1vaaWNCNGrIdo/x03lNkcaMOZl9om0IqIQt1tJf5tugx/OoXLPlP8ZE0mZzQW2EIbnl8rMPUPSHXIo/oFjRVcF0ccFttd8X+Q== jvmdump@xdcdevops-01
[INFO    ] Runner completed: 20200605113908767397

But if I try to get pillars for the above server (which require two secrets from Vault), I get an error.

# salt-run pillar.show_pillar stest-app-01.xdc

[INFO    ] Runner completed: 20200605114209000586
[INFO    ] Permission denied from vault
[INFO    ] Permission denied from vault
[ERROR   ] Unable to connect to vault server: {"errors":["permission denied"]}

[ERROR   ] Failed to get secret metadata TypeError: exceptions must derive from BaseException
[INFO    ] Permission denied from vault
[INFO    ] Permission denied from vault
[ERROR   ] Unable to connect to vault server: {"errors":["permission denied"]}

[ERROR   ] Rendering exception occurred
Traceback (most recent call last):
  File "/usr/lib/python3.6/site-packages/salt/modules/vault.py", line 211, in read_secret
    response.raise_for_status()
...
  File "/usr/lib/python3.6/site-packages/salt/modules/vault.py", line 229, in read_secret
    "Failed to read secret! {0}: {1}".format(type(err).__name__, err)
salt.exceptions.CommandExecutionError: Failed to read secret! HTTPError: 403 Client Error: Forbidden for url: https://xdcvault.oradian.ord:8200/v1/secretsv1/infra/services/jenkins/master/ssh_key

During handling of the above exception, another exception occurred:
...
salt.exceptions.SaltRenderError: Problem running salt function in Jinja template: Failed to read secret! HTTPError: 403 Client Error: Forbidden for url: https://xdcvault.oradian.ord:8200/v1/secretsv1/infra/services/jenkins/master/ssh_key; line 4

---
test:
  foo: bar
  jvmdump_pubkey: {{ salt.vault.read_secret("secretsv1/infra/services/jvmdump/ssh_key", "public_key") }}
  jenkins_pubkey: {{ salt.vault.read_secret("secretsv1/infra/services/jenkins/master/ssh_key", "public_key") }}    <======================
---
[CRITICAL] Pillar render error: Rendering SLS 'testvault' failed. Please see master log for details.
_errors:
    - Rendering SLS 'testvault' failed. Please see master log for details.
[INFO    ] Runner completed: 20200605114207131400

On the Vault server logs contain the same thing as reported here: {"errors":["permission denied"]}

If I comment out one pillar which accesses Vault (doesn't matter which one) then the command completes without problems and I get the correct output. But it's no go with more than one.

I suspect there's something wrong with token caching. This is from master log at the debug level:

[DEBUG   ][58728] Using session storage for vault credentials
[DEBUG   ][58728] Not caching vault single use token
[DEBUG   ][58728] Got metadata for secretsv1/infra/services/jvmdump/ssh_key
][DEBUG   ][58728] Reading Vault secret for stest-app-01.xdc at secretsv1/infra/services/jvmdump/ssh_key
[DEBUG   ][58728] Fetching metadata for secretsv1/infra/services/jenkins/master/ssh_key
[INFO    ][58728] Permission denied from vault
[DEBUG   ][58728] Deleting cache file
[DEBUG   ][58728] Attempted to delete vault cache file, but it does not exist.
[DEBUG   ][58728] Retrying with new credentials
[INFO    ][58728] Permission denied from vault
[DEBUG   ][58728] Deleting cache file
[DEBUG   ][58728] Attempted to delete vault cache file, but it does not exist.
[ERROR   ][58728] Unable to connect to vault server: {"errors":["permission denied"]}

[ERROR   ][58728] Failed to get secret metadata TypeError: exceptions must derive from BaseException
][DEBUG   ][58728] Reading Vault secret for stest-app-01.xdc at secretsv1/infra/services/jenkins/master/ssh_key
[INFO    ][58728] Permission denied from vault
[DEBUG   ][58728] Deleting cache file
[DEBUG   ][58728] Attempted to delete vault cache file, but it does not exist.
[DEBUG   ][58728] Retrying with new credentials
[INFO    ][58728] Permission denied from vault
[DEBUG   ][58728] Deleting cache file
[DEBUG   ][58728] Attempted to delete vault cache file, but it does not exist.
[ERROR   ][58728] Unable to connect to vault server: {"errors":["permission denied"]}

I'm attaching full debug log from the master (file: master_vault_error.log.gz).

I'm also attaching master debug log for the case when the second vault-accessing pillar was commented out (file: master_vault_single.log.gz) and there was no error reported.

Expected behavior

Pillars should be rendered without errors.

I would expect things to just work without the uses directive because that's what will be inherited after the upgrade from the current stable version. But the release notes aren't clear about that case. It isn't explicitly mentioned at all.

Versions Report

salt --versions-report
           Salt: 3001rc1
 
Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: Not Installed
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
         Jinja2: 2.11.1
        libgit2: Not Installed
       M2Crypto: 0.35.2
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.6.2
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: Not Installed
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 3.6.8 (default, Apr  2 2020, 13:34:55)
   python-gnupg: Not Installed
         PyYAML: 3.12
          PyZMQ: 17.0.0
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.1.4

System Versions:
           dist: centos 7 Core
         locale: ISO-8859-2
        machine: x86_64
        release: 3.10.0-1127.el7.x86_64
         system: Linux
        version: CentOS Linux 7 Core

Salt minion is also at the RC version.

Additional context

I am using https://github.com/saltstack/salt/blob/v3001rc1/doc/topics/releases/3001.rst as a reference documentation for the new vault module features.

master_vault_error.log.gz
master_vault_single.log.gz

@dkacar-oradian dkacar-oradian added the Bug broken, incorrect, or confusing behavior label Jun 5, 2020
@dkacar-oradian dkacar-oradian changed the title [Sodium RC] [BUG][RC1 Sodium] Jun 5, 2020
@dkacar-oradian dkacar-oradian changed the title [BUG][RC1 Sodium] [BUG][RC1 Sodium] Vault cannot fetch more than one secret Jun 5, 2020
@dkacar-oradian dkacar-oradian changed the title [BUG][RC1 Sodium] Vault cannot fetch more than one secret [BUG][RC1 Sodium] Vault module cannot fetch more than one secret Jun 5, 2020
@dkacar-oradian
Copy link
Author

I have this working fine if I set uses: 0 in the master configuration file.

But I'm really confused with the documentation in the release notes. That effect wasn't obvious to me.

@frogunder
Copy link
Contributor

@dkacar-oradian Thank you for reporting this issue.

With the new updates to the Vault module in 3001 this seems to be expected behavior.
https://docs.saltstack.com/en/master/topics/releases/3001.html#execution-module-updates

@saltstack/team-core Does that seems correct that is is expected behavior?

Thanks.

@frogunder frogunder added this to the Blocked milestone Jun 8, 2020
@frogunder frogunder added expected-behavior intended functionality and removed Bug broken, incorrect, or confusing behavior labels Jun 8, 2020
@dkacar-oradian
Copy link
Author

OK, it is an expected behavior, but the documentation is rather confusing and should be improved.

  1. Release notes say that uses: 0 creates unlimited use token, but that information is not written in the vault module documentation. It should be there.
  2. Neither release notes nor module documentation say what the defaults for uses and ttl are. That should be mentioned. Module documentation says: "Defining uses will cause the salt master to generate a token with that number of uses rather than a single use token". That is an implicit declaration of the default for uses only. It would be better if it was explicit for both.
  3. It seems that the default is uses: 1 (I don't know about ttl). So if one upgrades salt packages but doesn't change the configuration things are likely to fail. This should be mentioned in the release notes, along with the advice to add uses: 0 to get the same behavior as in the previous release. (If that is what would do the trick, I'm still not 100% sure about that.)
  4. It should be mentioned that running over defined uses (and/or ttl) would produce an error. It wasn't obvious (to me, at least) that it would. I thought that in that case another token would be generated and no error would be raised. That should save you from dealing with bug reports like this one.

lkubb added a commit to lkubb/salt-vault-formula that referenced this issue Oct 5, 2022
This commit represents a fundamental rewrite in how Salt interacts with
Vault. The master should still be compatible with minions running the
old code. There should be no breaking changes to public interfaces and
the old configuration format should still apply.

Core:
- Issue AppRoles to minions
- Manage entities with templatable metadata for minions
- Use inbuilt Salt cache
- Separate config cache from token cache
- Cache: introduce connection-scope vs global scope

Utility module:
- Support being imported (__utils__ deprecation)
- Raise exceptions on queries to simplify response handling
- Add classes to wrap complexity, especially regarding KV v2
- Lay some groundwork for renewing tokens

Execution module:
- Add patch_secret
- Add version support to delete_secret
- Allow returning listed keys only in list_secret
- Add policy_[fetch/write/delete] and policies_list
- Add query for arbitrary API queries

State module:
- Make use of execution module
- Change output format

Docs:
- Update for new configuration format
- Correct examples
- Add configuration examples
- Add required policies

Fixes:
saltstack/salt#62552
saltstack/salt#59827
saltstack/salt#62380
saltstack/salt#58174

Probably fixes:
saltstack/salt#60779
saltstack/salt#57561

Might fix:
saltstack/salt#59846
@BlackMetalz
Copy link

Still bug in 3006.1

@mdschmitt
Copy link

I think you gotta wait until #62684 gets mainlined

@BlackMetalz
Copy link

I think you gotta wait until #62684 gets mainlined

Thanks for the idea, but no idea why after upgrading to onedir, a lot of bugs appears which is not happening before 😢

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
expected-behavior intended functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants