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

Allow publish.publish wrapper from salt-ssh to contact minions connected to the master instead of only salt-ssh Minions #40943

Closed
jccomputing opened this issue Apr 27, 2017 · 13 comments · Fixed by #65646 · May be fixed by #65654
Labels
Core relates to code central or existential to Salt Feature new functionality including changes to functionality and code refactors, etc.
Milestone

Comments

@jccomputing
Copy link
Contributor

Description of Issue/Question

Setup

With a salt-vagrant-demo setup, I use this simple configuration:

caserver.sls

salt-minion-service:
  service.running:
    - name: salt-minion
    - enable: True
    - listen:
      - file: /etc/salt/minion.d/signing_policies.conf

/etc/salt/minion.d/signing_policies.conf:
  file.managed:
    - source: salt://signing_policies.conf

python-m2crypto:
  pkg.installed

/etc/pki:
  file.directory: []

/etc/pki/issued_certs:
  file.directory: []

/etc/pki/ca.key:
  x509.private_key_managed:
    - bits: 4096
    - require:
      - file: /etc/pki

/etc/pki/ca.crt:
  x509.certificate_managed:
    - signing_private_key: /etc/pki/ca.key
    - CN: test.local
    - C: US
    - ST: NY
    - L: NY
    - basicConstraints: "critical CA:true"
    - keyUsage: "critical cRLSign, keyCertSign"
    - subjectKeyIdentifier: hash
    - authorityKeyIdentifier: keyid,issuer:always
    - days_valid: 3650
    - days_remaining: 0
    - backup: True
    - require:
      - x509: /etc/pki/ca.key

mine.send:
  module.run:
    - func: x509.get_pem_entries
    - kwargs:
        glob_path: /etc/pki/ca.crt
    - onchanges:
        - x509: /etc/pki/ca.crt

caclient.sls

python-m2crypto:
  pkg.installed

/etc/pki:
  file.directory: []

/etc/pki/{{ grains['id'] }}.key:
  x509.private_key_managed:
    - name: /etc/pki/{{ grains['id'] }}.key
    - bits: 2048

/etc/pki/{{ grains['id'] }}.crt:
  x509.certificate_managed:
    - ca_server: minion2
    - signing_policy: default
    - public_key: /etc/pki/{{ grains['id'] }}.key
    - CN: {{ grains['fqdn'] }}
    - days_remaining: 30
    - backup: True
    - watch:
      - x509: /etc/pki/{{ grains['id'] }}.key

roster file should be properly configured.

Steps to Reproduce Issue

Apply caserver.sls to minion2.
salt 'minion2' state.apply caserver

Then apply caclient.sls to minion1 with salt-ssh:
salt-ssh -i 'minion1' state.apply caclient

I get the following error:

          ID: /etc/pki/minion1.crt
    Function: x509.certificate_managed
      Result: False
     Comment: An exception occurred in this state: Traceback (most recent call last):
                File "/var/tmp/.cloud_0a7c24_salt/py2/salt/state.py", line 1744, in call
                  **cdata['kwargs'])
                File "/var/tmp/.cloud_0a7c24_salt/py2/salt/loader.py", line 1702, in wrapper
                  return f(*args, **kwargs)
                File "/var/tmp/.cloud_0a7c24_salt/py2/salt/states/x509.py", line 442, in certificate_managed
                  new = __salt__['x509.create_certificate'](testrun=True, **kwargs)
                File "/var/tmp/.cloud_0a7c24_salt/py2/salt/modules/x509.py", line 1324, in create_certificate
                  arg=str(kwargs))[ca_server]
              KeyError: 'minion2'
     Started: 22:45:05.039768
    Duration: 1.308 ms
     Changes:   

If I apply the caclient.sls state to minion1 with salt it works (salt 'minion1' state.apply caclient).

I can reproduce the error with salt if I disallow call to x509.sign_remote_certificate by commenting contents of /etc/salt/master.d/peer.conf, so I guess that salt-ssh is unable to properly call publish.publish.

Is it the expected behavior?

Versions Report

Salt Version:
           Salt: 2016.11.4
 
Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 1.5
      docker-py: Not Installed
          gitdb: 0.5.4
      gitpython: 0.3.2 RC1
          ioflo: Not Installed
         Jinja2: 2.7.2
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: 0.9.1
   msgpack-pure: Not Installed
 msgpack-python: 0.4.6
   mysql-python: 1.2.3
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.6 (default, Oct 26 2016, 20:30:19)
   python-gnupg: Not Installed
         PyYAML: 3.10
          PyZMQ: 14.0.1
           RAET: Not Installed
          smmap: 0.8.2
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.0.5
 
System Versions:
           dist: Ubuntu 14.04 trusty
        machine: x86_64
        release: 3.13.0-117-generic
         system: Linux
        version: Ubuntu 14.04 trusty
@gtmanfred
Copy link
Contributor

This should be the expected behavior, the x509.certified_managed requires making calls back to the salt master to trigger commands on the signing machine.

Because salt-ssh runs without a zeromq back to the salt master and no event stream, it would not be possible to use this feature.

Thanks,
Daniel

@gtmanfred gtmanfred added the expected-behavior intended functionality label Apr 28, 2017
@gtmanfred gtmanfred added this to the Blocked milestone Apr 28, 2017
@gtmanfred
Copy link
Contributor

We would have to provide a publish module in salt.client.ssh.wrappers, and I am not sure that this would even be possible to make a module like that work.

@gtmanfred
Copy link
Contributor

Actually, hrm it looks like we have a publish.publish wrapper already

@gtmanfred
Copy link
Contributor

Ahh, I see the error.

So, with salt-ssh it expects everything to be salt-ssh, if minion2 was in your roster config, this might work.

@gtmanfred gtmanfred added Feature new functionality including changes to functionality and code refactors, etc. Core relates to code central or existential to Salt and removed expected-behavior intended functionality labels Apr 28, 2017
@gtmanfred gtmanfred changed the title x509.certificate_managed is not working with salt-ssh Allow publish.publish wrapper from salt-ssh to contact minions connected to the master instead of only salt-ssh Minions Apr 28, 2017
@gtmanfred gtmanfred modified the milestones: Approved, Blocked Apr 28, 2017
@gtmanfred
Copy link
Contributor

I have updated the title and tagged this as a feature request.

This would be the change that needs to be made for this to work.

https://github.com/saltstack/salt/blob/develop/salt/client/ssh/wrapper/publish.py#L88

Thanks,
Daniel

@stale
Copy link

stale bot commented Oct 10, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

@stale stale bot added the stale label Oct 10, 2018
@gtmanfred
Copy link
Contributor

Still useful

@stale
Copy link

stale bot commented Oct 10, 2018

Thank you for updating this issue. It is no longer marked as stale.

@stale stale bot removed the stale label Oct 10, 2018
@volfco
Copy link

volfco commented Feb 10, 2019

Throwing my hat into this issue. I've run into the exact same issue as the initial issue posted. Well similar issue.

I have my salt-ssh initiating host acting as the CA, but other than that the configuration is the same.

          ID: /root/www.crt
    Function: x509.certificate_managed
      Result: False
     Comment: An exception occurred in this state: Traceback (most recent call last):
                File "/var/tmp/.root_a7a25f_salt/py2/salt/state.py", line 1913, in call
                  **cdata['kwargs'])
                File "/var/tmp/.root_a7a25f_salt/py2/salt/loader.py", line 1898, in wrapper
                  return f(*args, **kwargs)
                File "/var/tmp/.root_a7a25f_salt/py2/salt/states/x509.py", line 502, in certificate_managed
                  new = __salt__['x509.create_certificate'](testrun=True, **kwargs)
                File "/var/tmp/.root_a7a25f_salt/py2/salt/modules/x509.py", line 1384, in create_certificate
                  'ca_server did not respond'
              SaltInvocationError: ca_server did not respond salt master must permit peers to call the sign_remote_certificate function.
     Started: 05:04:31.312658
    Duration: 8.813 ms
     Changes:   

It might be derailing the root issue, but it looks like I'm unable to correctly reference the salt-ssh host as the host that the minion should publish the create_certificate request to.

@stale
Copy link

stale bot commented Jan 9, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

@stale stale bot added the stale label Jan 9, 2020
@volfco
Copy link

volfco commented Jan 9, 2020

still useful

@stale
Copy link

stale bot commented Jan 9, 2020

Thank you for updating this issue. It is no longer marked as stale.

@stale stale bot removed the stale label Jan 9, 2020
@lkubb
Copy link
Contributor

lkubb commented Oct 30, 2023

(Referring to x509.certificate_managed specifically:) I don't think this will work with salt-ssh in its current form since the state modules are run on the SSH minion (which currently does not have a communication channel back to the master during execution, unlike in heist-salt), while the wrappers are used during state rendering on the master. x509.create_certificate on the other hand would work if it had a wrapper module and might currently somewhat work if called during pillar rendering, not sure if that's useful though.

If salt-ssh was more reliable I assume it would be possible to run the state modules on the master with wrapped execution modules, as long as the states were guaranteed to solely rely on the execution modules and would not circumvent the loader for any local operation. (This would likely be terribly slow though).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core relates to code central or existential to Salt Feature new functionality including changes to functionality and code refactors, etc.
Projects
None yet
4 participants