When using x509.managed_certs with a specified minions
attribute the globbing fails to match the minion correctly. It displays an error like:
hostname.net is not permitted to use signing policy signing-policy-name
https://github.com/oliverisaac/salt/commit/bd5b20ab705f619fbde2b0a734d5e0fd8b060856
@oliverisaac Thanks for the report. Can you provide the output from salt --versions-report
so we know what version this issue is being reported against. Thanks!
Sure:
Salt Version:
Salt: 2019.2.1
Dependency Versions:
cffi: 1.9.1
cherrypy: Not Installed
dateutil: 2.5.3
docker-py: Not Installed
gitdb: 2.0.0
gitpython: 2.1.1
ioflo: Not Installed
Jinja2: 2.9.4
libgit2: Not Installed
libnacl: Not Installed
M2Crypto: 0.24.0
Mako: Not Installed
msgpack-pure: Not Installed
msgpack-python: 0.4.8
mysql-python: 1.3.7
pycparser: 2.17
pycrypto: 2.6.1
pycryptodome: Not Installed
pygit2: Not Installed
Python: 2.7.13 (default, Sep 26 2018, 18:42:22)
python-gnupg: Not Installed
PyYAML: 3.12
PyZMQ: 16.0.2
RAET: Not Installed
smmap: 2.0.1
timelib: Not Installed
Tornado: 4.4.3
ZMQ: 4.2.1
System Versions:
dist: debian 9.11
locale: UTF-8
machine: x86_64
release: 4.9.0-11-amd64
system: Linux
version: debian 9.11
The issue is present in the current code base: https://github.com/saltstack/salt/blob/master/salt/modules/match.py#L311
The glob
function defines an opts
local variable but then doesn't actually use it when it calls the matcher.
This causes problems if the minion we're trying to glob against is different than the minion the code is running on. salt mysql* match.glob mysql*
works fine if run on host example.host
@oliverisaac Thanks for this update. Can you provide an example where the above call to match.glob
does not work? So far I haven't been able to reproduce a situation where it fails when I would expect it to succeed.
I can't easily describe our exact setup because it's a nesting of a nesting of config/formulas. In short, we are using salt as the CA authority for our db clusters. When we try to generate the cert, we specify that the signing_policy is, for example: mysql-prod-*
. Then, when our minion needs a cert, that signing_policy is globbed against the minion's hostname.
However, this happens on the salt master. So when the globbing happens, match.glob is called with a specific hostname.
If you look at the code in my commit linked above ( https://github.com/oliverisaac/salt/commit/bd5b20ab705f619fbde2b0a734d5e0fd8b060856#diff-889b4ad7294d8f45dfd1c3a87b9ba427L286-R314 ) you'll see that if you call match.glob("mysql-*", minion_id="mysql-prod-01")
while running on a minion with a non-matching hostname (e.g. salt01
) then the function will fail.
Lines 302 and 307 both copy __opts__
into the local variable opts
but then that local opts
isn't passed onto the final matchers
function. Instead the original __opts__
is passed.
Hello,
I have the same problem since I upgraded to 2019.2.2. And I confirm that the fix provided by @oliverisaac do resolve this problem.
I manage to reproduce this problem by following the documentation regarding the X509 state (https://docs.saltstack.com/en/latest/ref/states/all/salt.states.x509.html).
(I just had to create the ca.key by hand)
8af5f2682576 is the "client"
311f1c083f10 is the salt master and ca
root@311f1c083f10:/# cat /srv/salt/signing_policies.conf
x509_signing_policies:
www:
- minions: '8af5f2682576'
- signing_private_key: /etc/pki/ca.key
- signing_cert: /etc/pki/ca.crt
- C: US
- ST: Utah
- L: Salt Lake City
- basicConstraints: "critical CA:false"
- keyUsage: "critical keyEncipherment"
- subjectKeyIdentifier: hash
- authorityKeyIdentifier: keyid,issuer:always
- days_valid: 90
- copypath: /etc/pki/issued_certs/
root@311f1c083f10:/# salt 8af5f2682576 state.apply www
8af5f2682576:
----------
ID: /etc/pki/www.crt
Function: x509.certificate_managed
Result: False
Comment: An exception occurred in this state: Traceback (most recent call last):
File "/usr/lib/python2.7/dist-packages/salt/state.py", line 1933, in call
**cdata['kwargs'])
File "/usr/lib/python2.7/dist-packages/salt/loader.py", line 1951, in wrapper
return f(*args, **kwargs)
File "/usr/lib/python2.7/dist-packages/salt/states/x509.py", line 576, in certificate_managed
'New': __salt__['x509.read_certificate'](certificate=certificate)}
File "/usr/lib/python2.7/dist-packages/salt/modules/x509.py", line 557, in read_certificate
cert = _get_certificate_obj(certificate)
File "/usr/lib/python2.7/dist-packages/salt/modules/x509.py", line 371, in _get_certificate_obj
text = get_pem_entry(text, pem_type='CERTIFICATE')
File "/usr/lib/python2.7/dist-packages/salt/modules/x509.py", line 493, in get_pem_entry
raise salt.exceptions.SaltInvocationError(errmsg)
SaltInvocationError: PEM does not contain a single entry of type CERTIFICATE:
8af5f2682576 not permitted to use signing policy www
Started: 20:58:31.875648
Duration: 698.192 ms
Changes:
Summary for 8af5f2682576
------------
Succeeded: 0
Failed: 1
------------
Total states run: 1
Total run time: 698.192 ms
ERROR: Minions returned with non-zero exit code
And by adding some debug logs there https://github.com/saltstack/salt/blob/v2019.2.2/salt/modules/match.py#L311 I got :
[...]
[DEBUG ] LazyLoaded x509.sign_remote_certificate
[DEBUG ] LazyLoaded pillar.get
[DEBUG ] LazyLoaded config.get
[DEBUG ] LazyLoaded match.glob
[DEBUG ] opts['id'] : 8af5f2682576
[DEBUG ] __opts__['id'] : 311f1c083f10
[DEBUG ] LazyLoaded glob_match.match
[DEBUG ] Minion return retry timer set to 10 seconds (randomized)
[...]
Hope this can help.
Best regards
I'm running into the same problem as described in this issue in my Salt 2019.2.2
setup. @oliverisaac's patch seems to work perfectly!
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.
Please don't close this stale-bot. :-)
Thank you for updating this issue. It is no longer marked as stale.
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.
Please don't close this stale-bot - this still appears to be an issue in 2019.2.3
(and @oliverisaac 's patch continues to work just fine too).
Thank you for updating this issue. It is no longer marked as stale.
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.
Hello
Please remove the stale tag.
I ran into this problem again after upgrading to 3000.
Any chance to get this fixed for 3000.1 ?
Thank you for updating this issue. It is no longer marked as stale.
@oliverisaac We're facing same issue here, is there a PR ready to merge for this.
@garethgreenaway Any chances to see this fix in 3000.1
?
@sticky-note , no I didn't make a PR as they require that I submit tests cases as well and I'm not familiar enough with the code base to do that.
Seriously Salt, We have to update to 2019.2.4 but this issue appear, and the PR is lock until test !
I known test is important but sometime people don't know the test stack to wrote test.
@garethgreenaway can you fix this issue introduce by you ?
All, apologies for the delay in responding on this one.
@oliverisaac Can you point me at code that would fix this issue and we'll get some tests written for it. If you want to put in a PR and tag me, I'll make sure some tests get added.
Maybe I missed something, but I just applied the patch listed in the bug report to both my master and a test minion and restarted all the services. It's still bombing out with the same error that the signing policy doesn't exist.
@garethgreenaway : I've rebased off master and created a PR: https://github.com/saltstack/salt/pull/57441
@oliverisaac Thanks! I was finally able to reproduce this and finally see the root of the issue. Will check your PR and get some tests written for it.
PR with the changes from @oliverisaac have been merged and the fix will be available in the upcoming release.
Most helpful comment
@oliverisaac We're facing same issue here, is there a PR ready to merge for this.
@garethgreenaway Any chances to see this fix in
3000.1
?