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

x509 Documentation -> mine.send not working #56142

Closed
svenseeberg opened this issue Feb 12, 2020 · 13 comments · Fixed by #56162
Closed

x509 Documentation -> mine.send not working #56142

svenseeberg opened this issue Feb 12, 2020 · 13 comments · Fixed by #56162
Labels
Bug broken, incorrect, or confusing behavior ZRelease-Sodium retired label
Milestone

Comments

@svenseeberg
Copy link
Contributor

Description of Issue

I'm trying to set up a CA based on the example provided in https://docs.saltstack.com/en/latest/ref/states/all/salt.states.x509.html and also using https://backbeat.tech/blog/using-saltstack-for-internal-ssl-certificates/ as a reference.

AFAICT the documented mine.send for the ca.crt is not working. I find two issues:

  • The onchange does not trigger if the ca.crt already exists
  • After removing the onchange, I get a The following arguments are missing: m_name

Setup

The setup is basically the example provided in the documentation with no changes.

Steps to Reproduce Issue

Follow the documentation.

Versions Report

Salt Version:
           Salt: 3000
 
Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.7.3
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
         Jinja2: 2.10
        libgit2: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 3.7.3 (default, Dec 20 2019, 18:57:59)
   python-gnupg: Not Installed
         PyYAML: 3.13
          PyZMQ: 17.1.2
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.3.1
 
System Versions:
           dist: debian 10.3 
         locale: UTF-8
        machine: x86_64
        release: 4.19.0-8-amd64
         system: Linux
        version: debian 10.3 
@DmitryKuzmenko
Copy link
Contributor

@svenseeberg First I could see in the doc is: depends M2Crypto. And your version report says: M2Crypto: Not Installed. Could you please confirm it isn't the case?

@DmitryKuzmenko DmitryKuzmenko added the info-needed waiting for more info label Feb 12, 2020
@DmitryKuzmenko DmitryKuzmenko added this to the Blocked milestone Feb 12, 2020
@OrangeDog
Copy link
Contributor

OrangeDog commented Feb 12, 2020

The onchange does not trigger if the ca.crt already exists

That is expected. There was no change.

The following arguments are missing: m_name

It looks like the example is using the "legacy" version of module.run, where kwarg names can collide and get confused. The example is probably wrong, and should be rewritten to the new style anyway.

The previous state also uses managed_private_key, which doesn't work properly: #39608

@svenseeberg
Copy link
Contributor Author

@svenseeberg First I could see in the doc is: depends M2Crypto. And your version report says: M2Crypto: Not Installed. Could you please confirm it isn't the case?

Ah sorry, M2Crypto is installed on the minion. The states that require M2Crypto work well. That means the CA is being generated. Just to be sure I now installed M2crypto on the Salt master as well, but that does not change the outcome.

I guess what @OrangeDog is saying is correct. That is also what the blog entry on backbeat.tech says about module.run.

@svenseeberg
Copy link
Contributor Author

svenseeberg commented Feb 13, 2020

After looking at https://docs.saltstack.com/en/develop/topics/releases/2017.7.0.html#state-module-changes, I figure that module.run should be looking like this:

x509.get_pem_entries:
  module.run:
    - glob_path: /etc/pki/ca.crt

However, I can't figure out how to combine it with mine.send.

@OrangeDog
Copy link
Contributor

It think it should be

sent ca to salt mine:
  module.run:
    - mine.send:
      - name: x509.get_pem_entries
      - glob_path: /etc/pki/ca.crt
    - onchanges:
      - x509: /etc/pki/ca.crt

@svenseeberg
Copy link
Contributor Author

svenseeberg commented Feb 13, 2020

It think it should be

sent ca to salt mine:
  module.run:
    - mine.send:
      - name: x509.get_pem_entries
      - glob_path: /etc/pki/ca.crt
    - onchanges:
      - x509: /etc/pki/ca.crt

Sadly I cannot get your example to work.

What does work currently is this: I set the minion config to

use_superseded:
  - module.run

And add the state

foo:
  module.run:
    - x509.get_pem_entries:
      - glob_path: /etc/pki/ca.crt

which then returns

          ID: foo
    Function: module.run
      Result: True
     Comment: x509.get_pem_entries: Success
     Started: 17:51:11.107280
    Duration: 0.378 ms
     Changes:   
              ----------
              x509.get_pem_entries:
                  ----------
                  /etc/pki/ca.crt:
                      -----BEGIN CERTIFICATE-----
                      [CERT_CONTENT]
                      -----END CERTIFICATE-----

But combining this with mine.send in any fashion I can come up with does not yield any results.

So I was thinking of a different approach. Should it be possible to add the x509.get_pem_entries function to the minion mine_functions? Like

mine_functions:
  x509.get_pem_entries:
    - glob_path: /etc/pki/ca.crt

But the minion log says

2020-02-13 18:00:38,804 [salt.loaded.int.module.mine:194 ][ERROR   ][15629] Function x509.get_pem_entries in mine.update failed to execute

@svenseeberg
Copy link
Contributor Author

svenseeberg commented Feb 13, 2020

So I finally solved the issue. However, I guess there is room for improvement.

I'm now setting the mine functions via Pillar:

mine_functions:
  x509.get_pem_entries: [/etc/pki/ca.crt]

After changing the Pillar, I run systemctl restart salt-minion.

Then I can actually see that the function is there:

# salt 'ca*' config.get mine_functions                        
ca.example.com:
    ----------
    x509.get_pem_entries:
        - /etc/pki/ca.crt

and returns the desired result:

# salt 'ca*' mine.get 'ca.example.com' 'x509.get_pem_entry'
ca.example.com:
    ----------
    ca.example.com:
        -----BEGIN CERTIFICATE-----
        [CONTENT]
        -----END CERTIFICATE-----

AFAICT the important takeaway is x509.get_pem_entries: [/etc/pki/ca.crt] and the minion restart(?).

Is there any better way of refreshing the mine_functions on the minion than restarting the daemon?

@svenseeberg
Copy link
Contributor Author

svenseeberg commented Feb 13, 2020

I guess the best solution is this one (based on the example on https://docs.saltstack.com/en/latest/ref/states/all/salt.states.x509.html):

/srv/salt/signing_policies.conf:

mine_functions:
  x509.get_pem_entries: [/etc/pki/ca.crt]

x509_signing_policies:
  internal:
    - minions: '*'
    - signing_private_key: /etc/pki/salt-ca.key
    - signing_cert: /etc/pki/salt-ca.crt
    - basicConstraints: "critical CA:false"
    - keyUsage: "critical keyEncipherment"
    - subjectKeyIdentifier: hash
    - authorityKeyIdentifier: keyid,issuer:always
    - O: 'Example Ltd'
    - copypath: /etc/pki/issued_certs/

and for /srv/salt/ca.sls:

x509_signing_policies:
  file.managed:
    - name: /etc/salt/minion.d/signing_policies.conf
    - source: salt://signing_policies.conf
  service.running:
    - name: salt-minion
    - watch:
      - file: /etc/salt/minion.d/signing_policies.conf

If this is suitable, I can create a pull request for the documentation.

svenseeberg added a commit to svenseeberg/salt that referenced this issue Feb 14, 2020
* The used module.run syntax is outdated. Replace with
  minion.d configuration that now includes mine_functions.
* Rename the minion.d/signing_policies.conf to x509.conf
  to represent the broader scope of the file.
@stale
Copy link

stale bot commented Mar 14, 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 Mar 14, 2020
@svenseeberg
Copy link
Contributor Author

The issue still exists, a suggestion for a fix is still open as a pull request.

@stale
Copy link

stale bot commented Mar 16, 2020

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

@stale stale bot removed the stale label Mar 16, 2020
@sagetherage sagetherage added Bug broken, incorrect, or confusing behavior and removed info-needed waiting for more info labels Mar 17, 2020
@sagetherage sagetherage modified the milestones: Blocked, Approved Mar 17, 2020
@sagetherage
Copy link
Contributor

Working on getting this issue updated and reviewed as well as the PR, it may take a day to do that, or longer, but working to get 👀 on both. Thank you!

dwoz pushed a commit that referenced this issue Apr 11, 2020
* The used module.run syntax is outdated. Replace with
  minion.d configuration that now includes mine_functions.
* Rename the minion.d/signing_policies.conf to x509.conf
  to represent the broader scope of the file.
@sagetherage sagetherage added the ZRelease-Sodium retired label label May 19, 2020
@Gmasquelier
Copy link

Got the same issue following this aswell, but managed to make it work by replacing func by m_name.

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

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 ZRelease-Sodium retired label
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants