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

Drop specific point version pins in requirements.txt #134

Closed
konklone opened this issue Nov 6, 2017 · 16 comments
Closed

Drop specific point version pins in requirements.txt #134

konklone opened this issue Nov 6, 2017 · 16 comments

Comments

@konklone
Copy link
Collaborator

konklone commented Nov 6, 2017

We're falling behind, and currently are effectively pinned to cryptography 1.9, and are not able to go to 2.1. I think it's worth considering a dependency strategy that relies on semantic versioning, since this comes up a lot.

@konklone
Copy link
Collaborator Author

Unfortunately, it's sslyze that's stuck on cryptography 1.9:

https://github.com/nabla-c0d3/sslyze/blob/master/requirements.txt

@egyptiankarim
Copy link
Contributor

egyptiankarim commented Dec 2, 2017

Has this finally caught up to us in a big bad way? I think so.

> docker run --rm -it --name pshtt -v /Users/karim/Code/pshtt:/home/pshtt pshtt/cli itcd.hq.nasa.gov
Traceback (most recent call last):
[...]
  File "/usr/local/lib/python3.6/site-packages/pkg_resources/__init__.py", line 872, in resolve
    raise VersionConflict(dist, req).with_context(dependent_req)
pkg_resources.ContextualVersionConflict: (cryptography 1.9 (/usr/local/lib/python3.6/site-packages), Requirement.parse('cryptography>=2.1.4'), {'pyopenssl'})

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/bin/pshtt", line 6, in <module>
    from pkg_resources import load_entry_point
  [...]
  File "/usr/local/lib/python3.6/site-packages/pkg_resources/__init__.py", line 867, in resolve
    raise DistributionNotFound(req, requirers)
pkg_resources.DistributionNotFound: The 'cryptography>=2.1.4' distribution was not found and is required by pyopenssl

From pyOpenSSL:

Release Information

17.5.0 (2017-11-30)
Backward-incompatible changes:
The minimum cryptography version is now 2.1.4.

Do we want to modify setup.py to lock to an older version of pyOpenSSL until we work out a better way to manage dependencies?

@egyptiankarim
Copy link
Contributor

Pertinent SSLyze issue: Update cryptography to latest version #262

@egyptiankarim
Copy link
Contributor

egyptiankarim commented Dec 2, 2017

Okay. This should fix our immediate issues: Version bumped cryptography dependency. #263

Although, we may still need to think about how we handle dependencies in general.

@jsf9k
Copy link
Member

jsf9k commented Dec 2, 2017

Nice work, @egyptiankarim!

@jsf9k
Copy link
Member

jsf9k commented Dec 6, 2017

@egyptiankarim, has your fix for sslyze hit PyPI yet? I think I may be seeing a related bug in a Docker container I have that installs both domain-scan and pshtt from their respective GitHub repos. I do not see this problem with an analogous container that installs both domain-scan and trustymail. Here is where the problematic container is trying to call domain-scan/gather url --suffix=.gov --url=https://analytics.usa.gov/data/live/sites.csv:

scan_1        | ---gathering hostnames from analytics.usa.gov...---
scan_1        | From cffi callback <function _verify_callback at 0x7f93bdb62620>:
scan_1        | Traceback (most recent call last):
scan_1        |   File "/opt/pyenv/versions/3.6.1/lib/python3.6/site-packages/OpenSSL/SSL.py", line 313, in wrapper
scan_1        |     _lib.X509_up_ref(x509)
scan_1        | AttributeError: module 'lib' has no attribute 'X509_up_ref'
scan_1        | Remote URL not downloaded successfully.
scan_1        | Traceback (most recent call last):
scan_1        | 
scan_1        |   File "/opt/pyenv/versions/3.6.1/lib/python3.6/site-packages/urllib3/contrib/pyopenssl.py", line 438, in wrap_socket
scan_1        |     cnx.do_handshake()
scan_1        | 
scan_1        |   File "/opt/pyenv/versions/3.6.1/lib/python3.6/site-packages/OpenSSL/SSL.py", line 1806, in do_handshake
scan_1        |     self._raise_ssl_error(self._ssl, result)
scan_1        | 
scan_1        |   File "/opt/pyenv/versions/3.6.1/lib/python3.6/site-packages/OpenSSL/SSL.py", line 1546, in _raise_ssl_error
scan_1        |     _raise_current_error()
scan_1        | 
scan_1        |   File "/opt/pyenv/versions/3.6.1/lib/python3.6/site-packages/OpenSSL/_util.py", line 54, in exception_from_error_queue
scan_1        |     raise exception_type(errors)
scan_1        | 
scan_1        | OpenSSL.SSL.Error: [('SSL routines', 'ssl3_get_server_certificate', 'certificate verify failed')]

I tried using wget instead in this case, but then the same error just pops up later when domain-scan tries to download the PSL and the preload lists. 😭

Does this look like the same error you chased down?

@egyptiankarim
Copy link
Contributor

egyptiankarim commented Dec 7, 2017

has your fix for sslyze hit PyPI yet?

I don't think it has yet. The last package was uploaded a few days before I made my pull request. I'm not sure what the release schedule looks like for SSLyze, but I've asked the question.

Does this look like the same error you chased down?

This looks like a different error than what I was seeing before, but I think it is related to version discrepancies in cryptography.

From the changelog, it doesn't look like the x509_up_ref method was supported until 2.1.4, which went live on 2017-11-29, in preparation for updates to pyOpenSSL, which came out like a day later.

So, until SSLyze publishes my pull request, we're locked to a version of cryptography that has downstream impacts on pyOpenSSL 😝

@egyptiankarim
Copy link
Contributor

egyptiankarim commented Dec 7, 2017

For the record: To manage my local work, I've just stashed a set of version locks in setup.py that allow me to build and run clean with Docker. I'm not sure if that'll work for your use cases, but it's what I had to do.

What does the team think about maybe setting up master in that way? We can tightly version lock that branch for operations in terms of dependencies, and then leave develop free to break as things in the ecosystem update and such.

@jsf9k
Copy link
Member

jsf9k commented Dec 7, 2017

Thanks for the information, @egyptiankarim. Can you paste your setup.py in this issue (or create a new one) so I can see what is involved? I'm probably OK with locking master down.

@conorsch
Copy link

conorsch commented Dec 7, 2017

Great idea on locking master and switching to a git-flow-like branching strategy.

As for managing the dependencies, consider pip-compile (from the pip-tools) package. You'd create a requirements.in file that tracks version pinning, then generate a requirements.txt with dependencies fully resolved and explicit versions pinned. Something like:

pip-compile --output-file requirements.txt requirements.in

Switching to pip-compile doesn't solve the cryptography conflict described above, but it will help to clearly identify which packages are causing conflicts going forward.

@egyptiankarim
Copy link
Contributor

@conorsch I like that idea in theory. But does that conflict with the way we have setup.py working, though? I'm trying to follow the conversation on jazzband/pip-tools#418 and I'm struggling to understand how these pieces are intended to fit together. Rough sketch, I guess the idiomatic argument is that "applications" are meant to do dependency management via requirements.txt and that "libraries" are meant to use setup.py. So, were we on the wrong track with #141?

@jsf9k I'll work on a quick pull that includes the version-pins. I don't know if there's a more pythonic approach to doing this sort of thing, but I just searched for the latest versions of each dependency that I knew were compatible and slapped in ==.

Excuse all my ignorance, please. I'm a Ruby punk 🔴 💎 ❤️ 😄

@egyptiankarim
Copy link
Contributor

As promised:

@jsf9k
Copy link
Member

jsf9k commented Dec 8, 2017

Really, if sslyze would just release a new version to PyPI I think that would fix my problem. 😐

I still think there is value in setting versions appropriately in master and develop, but the dependency_links stuff I did in 0a1c2c0 is just a hack until a new version of sslyze hits PyPI.

@jsf9k jsf9k self-assigned this Dec 8, 2017
@jsf9k
Copy link
Member

jsf9k commented Dec 8, 2017

I was eventually able to fix my particular problem by installing the master branch of sslyze in my Docker container before I install domain-scan or pshtt:

pip3 install --upgrade git+https://github.com/nabla-c0d3/sslyze.git

@jsf9k jsf9k removed their assignment Dec 8, 2017
@egyptiankarim
Copy link
Contributor

I lost track of this for a while, but it looks like the last few published releases of sslyze (>= 1.2.0) all include the version bumped cryptography dependencies that originally broke things for us.

@jsf9k
Copy link
Member

jsf9k commented Dec 27, 2017

Thanks for the update, @egyptiankarim. Since this discussion I've been forcing pip to install sslyze from the GitHub develop branch, so it's good to know I don't need to do that any longer.

cisagovbot pushed a commit that referenced this issue Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants