Use `requires[security]` in the requirements instead of just `requires` #1563

cyli · 2017-04-14T00:39:43Z

This installs a few more packages, but guarantees that at least on Mac OS a newer version of openssl is used instead of the default, which doesn't support TLS 1.2. I think the openssl is provided for Windows wheels too.

See https://stackoverflow.com/questions/31811949/pip-install-requestssecurity-vs-pip-install-requests-difference.

An alternative is to specify a "security" option similar to requests: see https://github.com/kennethreitz/requests/blob/master/setup.py#L98.

Also since the requirements.txt is pinning specific versions, I just pinned everything (including deps of deps) for a consistent build.

shin- · 2017-05-17T22:12:28Z

I think we should leave it up to users to figure out if they need those additional packages. A sizable amount of users don't require TLS at all when communicating with the engine over a UNIX socket / Windows npipe.

cyli · 2017-05-19T21:37:32Z

@shin How would you feel about a tls option in the setup.py? So people would have to pin or pip install docker[tls]?

… also installs: pyOpenSSL, cryptography, idna and installs cryptography's version of openssl in Mac OS (which by default has an ancient version of openssl that doesn't support TLS 1.2). Signed-off-by: cyli <cyli@twistedmatrix.com>

cyli · 2017-06-06T21:21:36Z

(I'm not sure what is causing the windows failure)

shin- · 2017-06-16T19:14:43Z

Looks like building the cryptography module on AppVeyor fails because opensslv.h is missing.

Failed to build cryptography
Installing collected packages: mock, colorama, py, pytest, coverage, pytest-cov, mccabe, pyflakes, pep8, flake8, appdirs, asn1crypto, backports.ssl-match-hostname, pycparser, cffi, idna, six, pyparsing, packaging, cryptography, docker-pycreds, enum34, ipaddress, pyOpenSSL, requests, websocket-client
  Running setup.py install for cryptography
    Complete output from command c:\projects\docker-py\.tox\py35\scripts\python.exe -c "import setuptools, tokenize;__file__='C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\pip-build-mitiwy39\\cryptography\\setup.py';exec(compile(getattr(tokenize, 'open', open)(__file__).read().replace('\r\n', '\n'), __file__, 'exec'))" install --record C:\Users\appveyor\AppData\Local\Temp\1\pip-ltw8vmfe-record\install-record.txt --single-version-externally-managed --compile --install-headers c:\projects\docker-py\.tox\py35\include\site\python3.5\cryptography:
    running install
    running build
    running build_py
    running egg_info
    writing dependency_links to src\cryptography.egg-info\dependency_links.txt
    writing requirements to src\cryptography.egg-info\requires.txt
    writing top-level names to src\cryptography.egg-info\top_level.txt
    writing src\cryptography.egg-info\PKG-INFO
    writing entry points to src\cryptography.egg-info\entry_points.txt
    warning: manifest_maker: standard file '-c' not found
    
    reading manifest file 'src\cryptography.egg-info\SOURCES.txt'
    reading manifest template 'MANIFEST.in'
    no previously-included directories found matching 'docs\_build'
    warning: no previously-included files matching '*' found under directory 'vectors'
    writing manifest file 'src\cryptography.egg-info\SOURCES.txt'
    running build_ext
    generating cffi module 'build\\temp.win32-3.5\\Release\\_padding.c'
    already up-to-date
    generating cffi module 'build\\temp.win32-3.5\\Release\\_constant_time.c'
    already up-to-date
    generating cffi module 'build\\temp.win32-3.5\\Release\\_openssl.c'
    already up-to-date
    building '_openssl' extension
    C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\cl.exe /c /nologo /Ox /W3 /GL /DNDEBUG /MD -Ic:\python35\include -Ic:\python35\include "-IC:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\INCLUDE" "-IC:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\ATLMFC\INCLUDE" "-IC:\Program Files (x86)\Windows Kits\10\include\10.0.14393.0\ucrt" "-IC:\Program Files (x86)\Windows Kits\NETFXSDK\4.6.1\include\um" "-IC:\Program Files (x86)\Windows Kits\10\include\10.0.14393.0\shared" "-IC:\Program Files (x86)\Windows Kits\10\include\10.0.14393.0\um" "-IC:\Program Files (x86)\Windows Kits\10\include\10.0.14393.0\winrt" /Tcbuild\temp.win32-3.5\Release\_openssl.c /Fobuild\temp.win32-3.5\Release\build\temp.win32-3.5\Release\_openssl.obj
    _openssl.c
    build\temp.win32-3.5\Release\_openssl.c(434): fatal error C1083: Cannot open include file: 'openssl/opensslv.h': No such file or directory
    error: command 'C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\BIN\\cl.exe' failed with exit status 2

Do we need those changes to requirements.txt?

shin- · 2017-06-16T19:17:44Z

pyca/cryptography#3028

Not sure what version of pip AppVeyor is using.

shin- · 2017-06-16T20:38:48Z

pip is up to date on Appveyor, seems like the solution is to force pip to install the wheel using --only-binary.

This should work: shin-@eeb29b9

cyli · 2017-06-16T23:09:10Z

@shin- Ah ok, thank you! Will update.

shin- · 2017-06-16T23:39:22Z

Collecting cryptography==1.8.1 (from -r C:\projects\docker-py/requirements.txt (line 5))
  Could not find a version that satisfies the requirement cryptography==1.8.1 (from -r C:\projects\docker-py/requirements.txt (line 5)) (from versions: 0.2, 0.2.1, 0.2.2, 0.3, 0.4, 0.5, 0.5.1, 0.5.2, 0.5.3, 0.5.4, 0.6, 0.6.1, 0.7, 0.7.1, 0.7.2, 0.8, 0.8.1, 0.8.2, 0.9, 0.9.1, 0.9.2, 0.9.3, 1.0, 1.0.1, 1.0.2, 1.1, 1.1.1, 1.1.2, 1.2, 1.2.1, 1.2.2, 1.2.3, 1.3, 1.3.1, 1.3.2, 1.3.3, 1.3.4, 1.4)
No matching distribution found for cryptography==1.8.1 (from -r C:\projects\docker-py/requirements.txt (line 5))

I guess 1.8.1 is only available on UNIX platforms?

cyli · 2017-06-16T23:44:30Z

@shin- I believe there a wheels for windows: https://pypi.org/project/cryptography/1.8.1/#files

moshez · 2017-06-17T00:19:03Z

tox.ini

 skipsdist=True

 [testenv]
 usedevelop=True
 commands =
    py.test -v --cov=docker {posargs:tests/unit}
+platform =
+    windows: windows


This should be

{py27,py33,py34,py35}-windows: windows

and ditto for the other relevant lines.

If you're asking whether I feel bad already for giving you this advice: yes but I have no better advice :(

ah, good point, thanks!

moshez · 2017-06-17T00:20:51Z

tox.ini

+    linux: linux
+    darwin: darwin
+install_command = 
+    {py27,py33,py34,py35}-windows: pip install --only-binary=cryptography {opts} {packages}


you probably want to put an install command here too, because otherwise things will break :(

{py27,...}-{darwin,linux}: pip install {opts} {packages}

all dependencies of dependencies as well so we can get a consistent build. Signed-off-by: cyli <cyli@twistedmatrix.com>

cyli · 2017-06-17T01:41:36Z

@shin- Looks like we were using an older version of tox/virtualenv in appveyor, which installed an older version of pip, which for some reason did not find a matching wheel.

…t pip. Signed-off-by: Ying <ying.li@docker.com>

cyli · 2017-06-17T01:52:38Z

Do we need those changes to requirements.txt?

I wasn't sure if this was in reference to the appveyor failure or a more general question about the extra packages in the requirements.txt in this particular PR. If the latter, I have mainly been following the advice in https://caremad.io/posts/2013/07/setup-vs-requirement/ and https://packaging.python.org/discussions/install-requires-vs-requirements/#requirements-files, where the setup.py has the dependencies required to install the package, generally, and requirements.txt pins the exact versions of both dependencies and dependencies of dependencies, to make sure that CI is repeatable.

Happy to dump that in something else, like requirements.dev.txt or something like that though if you'd prefer.

shin- · 2017-08-15T22:45:46Z

Sorry for neglecting this for so long, I thought I had merged it. Thank you!

cyli · 2017-08-15T23:21:21Z

No worries, thank you!

cyli mentioned this pull request Apr 14, 2017

TLS minimum version can be specified through configuration and command line options moby/moby#32056

Closed

moshez reviewed Jun 17, 2017

View reviewed changes

If we're pinning exact versions of things for requirements.txt, pin

09f50bd

all dependencies of dependencies as well so we can get a consistent build. Signed-off-by: cyli <cyli@twistedmatrix.com>

Upgrade tox and virtualenv in appveyor to make sure we have the lates…

5823bc9

…t pip. Signed-off-by: Ying <ying.li@docker.com>

shin- added this to the 2.5.0 milestone Aug 15, 2017

shin- merged commit 35f29d0 into docker:master Aug 15, 2017

shin- mentioned this pull request Aug 15, 2017

Support SOCKS proxy from HTTP[S]_PROXY env vars #1055

Closed

cyli deleted the use-requires-security branch August 15, 2017 23:21

cyli mentioned this pull request Feb 27, 2018

Remove server support for TLS 1.0 and 1.1 docker/go-connections#50

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use `requires[security]` in the requirements instead of just `requires` #1563

Use `requires[security]` in the requirements instead of just `requires` #1563

cyli commented Apr 14, 2017 •

edited

Loading

shin- commented May 17, 2017

cyli commented May 19, 2017

cyli commented Jun 6, 2017

shin- commented Jun 16, 2017

shin- commented Jun 16, 2017

shin- commented Jun 16, 2017

cyli commented Jun 16, 2017

shin- commented Jun 16, 2017

cyli commented Jun 16, 2017 •

edited

Loading

moshez Jun 17, 2017

cyli Jun 17, 2017

moshez Jun 17, 2017

cyli commented Jun 17, 2017

cyli commented Jun 17, 2017 •

edited

Loading

shin- commented Aug 15, 2017

cyli commented Aug 15, 2017

Use requires[security] in the requirements instead of just requires #1563

Use requires[security] in the requirements instead of just requires #1563

Conversation

cyli commented Apr 14, 2017 • edited Loading

shin- commented May 17, 2017

cyli commented May 19, 2017

cyli commented Jun 6, 2017

shin- commented Jun 16, 2017

shin- commented Jun 16, 2017

shin- commented Jun 16, 2017

cyli commented Jun 16, 2017

shin- commented Jun 16, 2017

cyli commented Jun 16, 2017 • edited Loading

moshez Jun 17, 2017

Choose a reason for hiding this comment

cyli Jun 17, 2017

Choose a reason for hiding this comment

moshez Jun 17, 2017

Choose a reason for hiding this comment

cyli commented Jun 17, 2017

cyli commented Jun 17, 2017 • edited Loading

shin- commented Aug 15, 2017

cyli commented Aug 15, 2017

Use `requires[security]` in the requirements instead of just `requires` #1563

Use `requires[security]` in the requirements instead of just `requires` #1563

cyli commented Apr 14, 2017 •

edited

Loading

cyli commented Jun 16, 2017 •

edited

Loading

cyli commented Jun 17, 2017 •

edited

Loading