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

Salt Minion Windows Installer uses openssl and own cert chain #46644

Open
mruepp opened this issue Mar 21, 2018 · 26 comments
Open

Salt Minion Windows Installer uses openssl and own cert chain #46644

mruepp opened this issue Mar 21, 2018 · 26 comments
Assignees
Labels
Feature new functionality including changes to functionality and code refactors, etc. Windows ZD The issue is related to a Zendesk customer support ticket.
Milestone

Comments

@mruepp
Copy link

mruepp commented Mar 21, 2018

Description of Issue/Question

In Windows we have to add self signed ca chain to the salt minion installer cert.pem
We want salt to use the windows truststore certs which are delivered by active directory, basically use schannel transport or be able to configure this on a case to case base.
This is a problem with all self signed ca environments. Often on Linux we deploy cert chains with salt or other mechanisms, in Windows the cachains are usually as point of truth in AD and deployed with AD

Setup

(Please provide relevant configs and/or SLS files (Be sure to remove sensitive info).)
Default salt minion python 3 amd64 installer

Steps to Reproduce Issue

(Include debug logs if possible and relevant.)
use file.managed from https source with self signed cert will throw ssl not verified error, also archive.extracted

Versions Report

`
Salt Version:
Salt: 2017.7.4

Dependency Versions:
cffi: 1.6.0
cherrypy: Not Installed
dateutil: Not Installed
docker-py: Not Installed
gitdb: Not Installed
gitpython: Not Installed
ioflo: Not Installed
Jinja2: 2.7.2
libgit2: 0.24.6
libnacl: Not Installed
M2Crypto: Not Installed
Mako: Not Installed
msgpack-pure: Not Installed
msgpack-python: 0.4.6
mysql-python: Not Installed
pycparser: 2.14
pycrypto: 2.6.1
pycryptodome: Not Installed
pygit2: 0.24.2
Python: 2.7.5 (default, Aug 4 2017, 00:39:18)
python-gnupg: Not Installed
PyYAML: 3.11
PyZMQ: 15.3.0
RAET: Not Installed
smmap: Not Installed
timelib: Not Installed
Tornado: 4.2.1
ZMQ: 4.1.4

System Versions:
dist: centos 7.4.1708 Core
locale: UTF-8
machine: x86_64
release: 3.10.0-693.21.1.el7.x86_64
system: Linux
version: CentOS Linux 7.4.1708 Core
`

@garethgreenaway garethgreenaway added this to the Blocked milestone Mar 22, 2018
@garethgreenaway garethgreenaway added the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Mar 22, 2018
@garethgreenaway
Copy link
Contributor

@twangboy Thoughts on this one?

@twangboy
Copy link
Contributor

We currently bundle OpenSSL 1.0.2n and we package certifi from pypi for the certificate store. Not sure how we would make it use the Windows certificate store.

@dwoz ?

@twangboy
Copy link
Contributor

@mruepp Could you try installing python-certifi-win32 on your minion to see if that fixes your issue? You can do that from the minion with:

salt-call --local pip.install python-certifi-win32 cwd=C:\\salt\\bin\\Scripts bin_env=C:\\salt\\bin\\Scripts\\pip.exe

or from the master with:

salt <minion-name> pip.install python-certifi-win32 cwd=C:\\salt\\bin\\Scripts bin_env=C:\\salt\\bin\\Scripts\\pip.exe

If that works, we'll add that dependency to the Salt installation for future versions.

@whytewolf whytewolf added the ZD The issue is related to a Zendesk customer support ticket. label Jun 27, 2018
@whytewolf
Copy link
Collaborator

ZD-2644

@whytewolf
Copy link
Collaborator

I can confirm that installing python-certifi-win32 fixes the issue.

@twangboy
Copy link
Contributor

The only thing I can think of to do here is a separate Salt build that uses the system certificate store using python-certifi-win32. @rallytime @UtahDave ?

@cachedout
Copy link
Contributor

Can you expand on why this would be (or should be) a separate build?

@rallytime
Copy link
Contributor

Fixed with #48476

@twangboy
Copy link
Contributor

twangboy commented Jul 11, 2018

This fix is causing issues with our tests. We'll need to do a deep dive into the python-certifi-win32 code to see what's going on there. To replicate the issue:

On a Windows system without ioflo installed...

  1. pip install ioflo --no-cache-dir
  2. Observe successful installation
  3. pip uninstall ioflo
  4. pip install python-certifi-win32
  5. pip install ioflo --no-cache-dir
  6. Observe Stacktrace
Collecting ioflo
  Downloading https://files.pythonhosted.org/packages/52/87/14967c110d44c82db520f95e4b5a419835c550ee47b0c961c7d5297e9f88/ioflo-1.7.4.tar.gz (831kB)
    100% |################################| 839kB 728kB/s
    Complete output from command python setup.py egg_info:
    Download error on https://pypi.python.org/simple/setuptools_git/: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed (_ssl.c:720) -- Some packages may not be found!
    Download error on https://pypi.python.org/simple/setuptools-git/: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed (_ssl.c:720) -- Some packages may not be found!
    Couldn't find index page for 'setuptools_git' (maybe misspelled?)
    Download error on https://pypi.python.org/simple/: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed (_ssl.c:720) -- Some packages may not be found!
    No local packages or working download links found for setuptools_git>=1.1
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "C:\Users\ADMINI~1\AppData\Local\Temp\2\pip-build-l3skp269\ioflo\setup.py", line 78, in <module>
        scripts=PYTHON_SCRIPTS,)
      File "c:\python35\lib\distutils\core.py", line 108, in setup
        _setup_distribution = dist = klass(attrs)
      File "c:\python35\lib\site-packages\setuptools\dist.py", line 318, in __init__
        self.fetch_build_eggs(attrs['setup_requires'])
      File "c:\python35\lib\site-packages\setuptools\dist.py", line 375, in fetch_build_eggs
        replace_conflicting=True,
      File "c:\python35\lib\site-packages\pkg_resources\__init__.py", line 851, in resolve
        dist = best[req.key] = env.best_match(req, ws, installer)
      File "c:\python35\lib\site-packages\pkg_resources\__init__.py", line 1123, in best_match
        return self.obtain(req, installer)
      File "c:\python35\lib\site-packages\pkg_resources\__init__.py", line 1135, in obtain
        return installer(requirement)
      File "c:\python35\lib\site-packages\setuptools\dist.py", line 443, in fetch_build_egg
        return cmd.easy_install(req)
      File "c:\python35\lib\site-packages\setuptools\command\easy_install.py", line 667, in easy_install
        raise DistutilsError(msg)
    distutils.errors.DistutilsError: Could not find suitable distribution for Requirement.parse('setuptools_git>=1.1')

@twangboy twangboy reopened this Jul 11, 2018
@twangboy twangboy self-assigned this Jul 11, 2018
@twangboy
Copy link
Contributor

NOTE: If you install setuptools-git before ioflo then it will install correctly.

@twangboy
Copy link
Contributor

twangboy commented Aug 15, 2018

So, Python has been using the Windows Certificate Store since 2.7.9 and 3.4. (see here) The problem is another dependency that Salt is using; Tornado. certifi is a requirement for Tornado versions <5. Starting with Tornado 5.0 they dropped the certifi requirement. (see here) However, Salt on Py3 is pinned to using versions of Tornado greater than 4.2.1 and less than 5 (see here). Support for Tornado 5 will be implemented in the Fluorine release (see here).

Salt on Py2 does support Tornado 5. So, theoretically, you could install the Py2 version of Salt, uninstall certifi and upgrade Tornado to version 5.1 in the salt installation to work around this issue.

Bottom line, this issue should be fixed in Fluorine by updating support for Tornado 5 on Py3.

@twangboy
Copy link
Contributor

@rickh563 ^^^

@rallytime rallytime added ZRELEASED - Fluorine reitred label and removed Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged labels Aug 24, 2018
@rallytime rallytime modified the milestones: Blocked, Approved Aug 24, 2018
@dwoz
Copy link
Contributor

dwoz commented Aug 24, 2018

This is dependent on the work being done here:

saltstack/salt-ci-images#995

@rallytime
Copy link
Contributor

@dwoz Yes, correct. Once that is done, we'll need to change the deps for the Py3 windows package to use tornado 5.

@rallytime rallytime added ZRELEASED - Neon retired label and removed ZRELEASED - Fluorine reitred label labels Sep 26, 2018
@KChandrashekhar
Copy link

Tornado 5.0 support #51883 should fix this; Pending testing and validation post Tornado 5.0 fix

@KChandrashekhar KChandrashekhar removed this from the Approved milestone Apr 29, 2019
@sagetherage sagetherage added this to the Magnesium milestone Jul 29, 2020
@sagetherage sagetherage assigned dwoz and unassigned twangboy Aug 17, 2020
@sagetherage sagetherage modified the milestones: Magnesium, Aluminium Sep 14, 2020
@sagetherage sagetherage added Aluminium Release Post Mg and Pre Si and removed Magnesium Mg release after Na prior to Al labels Sep 14, 2020
@sagetherage sagetherage assigned cmcmarrow and unassigned dwoz Sep 14, 2020
@sagetherage sagetherage assigned twangboy and unassigned cmcmarrow Jan 4, 2021
@sagetherage sagetherage added Silicon v3004.0 Release code name and removed Aluminium Release Post Mg and Pre Si labels Mar 24, 2021
@sagetherage sagetherage modified the milestones: Aluminium, Silicon Mar 24, 2021
@sagetherage sagetherage removed the Silicon v3004.0 Release code name label May 24, 2021
@sagetherage sagetherage modified the milestones: Silicon, Approved May 24, 2021
@sagetherage
Copy link
Contributor

Descoping from Silicon for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature new functionality including changes to functionality and code refactors, etc. Windows ZD The issue is related to a Zendesk customer support ticket.
Projects
None yet
Development

No branches or pull requests