etingof / pysnmp

Python SNMP library
http://snmplabs.com/pysnmp/
BSD 2-Clause "Simplified" License
584 stars 201 forks source link

PyCrypto transition #130

Open mattsb42-aws opened 6 years ago

mattsb42-aws commented 6 years ago

We're working on transitioning away from PyCrypto(|dome) for all of the software that we use, and PySNMP is currently a large contributor to PyCryptodome use. We believe that there is a strong security case to move from PyCrypto(|dome) to pyca/cryptography, but before wading into that I wanted to test the waters here to see if this path would be agreeable to PySNMP or if we need to look into maintaining a patched fork.

The upsides of this move would be replacing single-use cryptographic implementations with widely used and heavily scrutinized implementations and aligning PySNMP with the ongoing direction that the PyCA is setting for Python cryptography.

The (arguable) downside would be dropping official support for some legacy CPython minor versions. pyca/cryptography supports 2.7 and 3.4+. Currently, PySNMP claims support for 2.4-2.7 and 3.2+, though I see that the test suite is currently broken for 2.6 and I have been so far unable in my test setup to get 2.4, 2.5, and 3.2 working with the current code. Given that 2.4-2.6 and 3.2-3.3 are all past their official end of life, I think this is a reasonable (and arguably good) tradeoff.

I have put together the necessary code changes here[1], and you can see the successful test runs here[2]. If this is a path that PySNMP is interested in following, I'll clean up PyCrypto references in the readme and get a PR sent over.

[1] https://github.com/mattsb42-aws/pysnmp/tree/pyca-cryptography [2] https://travis-ci.org/mattsb42-aws/pysnmp/builds/336730799

etingof commented 6 years ago

Thanks for raising this issue!

I think it makes sense to migrate to cryptography given the reasons you've mentioned.

However I do not think it would be fair to drop legacy Python support because of the telcos -- those guys are (reportedly) still stuck at RHEL 4/5. So even if Python 2.4/2.5 support is going to obsolete in the nearest future, I hate to say that Python 2.6 might need some more years to die off reliably.

Having said that: would it make sense/be possible to abstract away the crypto primitives that pysnmp relies upon and make it using either of the crypto libraries available on the machine it runs? This may probably involve some conditional dependencies in requirements.txt and/or command-line option to setup.py to let the user chose the crypto lib to install...

WDYT?

hugovk commented 6 years ago

EOL Python versions no longer receive security (or any other) updates from the core team.

That's a long time if security is important. And it is possible to upgrade to a supported Python version on RHEL 5/6, using for example SCL. Probably with RHEL 4 too, but anyway that was shipped with Python 2.3 (EOL 2008-03-11) which PySNMP doesn't support.

All the EOL Python versions are little used.

Here's the pip installs for PySNMP from PyPI for last month:

python_version percent download_count
2.7 95.8% 141,417
3.6 1.7% 2,523
3.5 1.3% 1,906
3.4 1.0% 1,536
2.6 0.1% 99
3.7 0.0% 35
3.3 0.0% 14
3.2 0.0% 11

Source: pypinfo --start-date -34 --end-date -4 --percent --pip --markdown pysnmp pyversion

etingof commented 6 years ago

I totally agree that running non-EOL'ed Python is the best thing since sliced bread! The sad reality is that people can't always upgrade in time. On top of that RHEL has its own EOL timing with its own security team back porting critical vulnerabilities of the programs that EOL'ed upstream.

If we drop support for Python 2.4-2.6 now, the users that are stuck at those versions will have to either:

  1. pin to the last pysnmp version that is not based on cryptography
  2. revamp their old systems by bumping their Python version one way or the other

Realistically, they may prefer option 1 as the easiest and less intrusive. That effectively cuts them off from further pysnmp upgrades which does not improve the security situation to them.

I mistakenly referred to RHEL 4, you are right - pysnmp won't run on it.

The download stats looks fishy - is the world still at Py 2.7? Where are all the Py3 users? %-/

Anyway, do you think that having pysnmp supporting both PyCryptodomex and cryptography (whatever is available) would be a bad design?

hugovk commented 6 years ago

The download stats looks fishy - is the world still at Py 2.7? Where are all the Py3 users? %-/

Well, it varies a lot by project. Here's a few others.

rq-dashboard

python_version percent download_count
3.6 52.4% 6,751
2.7 40.7% 5,237
3.5 5.9% 758
3.4 1.0% 130
2.6 0.0% 1

Flask-SQLAlchemy

python_version percent download_count
2.7 85.8% 645,538
3.6 7.7% 57,602
3.5 3.8% 28,411
3.4 2.5% 18,775
2.6 0.1% 868
3.7 0.1% 532
3.3 0.0% 346
None 0.0% 4
3.2 0.0% 1

unicodecsv

python_version percent download_count
2.7 91.2% 464,979
3.6 4.4% 22,307
3.5 2.4% 12,449
3.4 1.7% 8,855
2.6 0.3% 1,306
3.7 0.0% 133
3.3 0.0% 86
None 0.0% 1

tqdm

python_version percent download_count
3.5 50.6% 228,303
2.7 28.6% 129,062
3.6 16.0% 72,107
3.4 4.1% 18,633
3.7 0.3% 1,136
3.3 0.2% 1,020
2.6 0.2% 773
3.2 0.1% 490
None 0.0% 3

Pillow

python_version percent download_count
2.7 61.6% 840,801
3.6 21.0% 287,160
3.5 13.0% 177,938
3.4 4.0% 54,313
3.7 0.2% 3,014
2.6 0.1% 1,447
3.3 0.1% 1,293
3.2 0.0% 30
None 0.0% 19

PySNMP

We can get figures by distro versions too.

system_name distro_name distro_version percent download_count
Linux CentOS Linux 7 79.1% 112,400
Linux Ubuntu 16.04 5.7% 8,089
Linux CentOS 6.9 4.5% 6,430
Linux Ubuntu 14.04 3.7% 5,221
Linux None None 2.6% 3,633
Windows None None 1.6% 2,246
Linux CentOS Linux 7.3.1611 1.1% 1,615
Linux Debian GNU/Linux 8 0.8% 1,156
Linux Debian GNU/Linux 9 0.6% 802
Linux Ubuntu 16.10 0.4% 584

pypinfo --start-date -35 --end-date -5 --percent --pip --markdown pysnmp system distro distro-version

Across all PyPI

python_version percent download_count
2.7 76.5% 411,798,144
3.6 10.0% 54,006,295
3.5 8.3% 44,528,155
3.4 4.0% 21,657,569
2.6 0.9% 5,051,339
3.3 0.1% 526,609
3.7 0.1% 412,965
3.2 0.0% 28,598
None 0.0% 17,265
3.1 0.0% 108

pypinfo --start-date -35 --end-date -5 --percent --pip --markdown "" pyversion

system_name distro_name distro_version percent download_count
Linux None None 29.7% 137,388,244
Linux Ubuntu 16.04 19.4% 90,047,548
Linux Ubuntu 14.04 14.9% 69,088,760
Linux Debian GNU/Linux 8 14.7% 67,983,565
Linux Amazon Linux AMI 2017.09 6.1% 28,466,275
Windows None None 5.9% 27,511,742
Linux CentOS Linux 7 4.3% 19,817,279
Linux Amazon Linux AMI 2017.03 1.8% 8,555,639
Linux Debian GNU/Linux 9 1.6% 7,625,630
Darwin macOS 10.12.6 1.4% 6,608,145

pypinfo --start-date -35 --end-date -5 --percent --pip --markdown "" system distro distro-version

mattsb42-aws commented 6 years ago

It would definitely be possible to isolate the crypto code and use whichever backend is available (preferring pyca/cryptography), then add logic in setup.py that selects pyca/cryptography if you're on 2.7/3.4+ and pycryptodomex otherwise.

I believe that legacy version support needs to be dropped at some point, but this would enable a more incremental transition if you are not comfortable dropping support entirely yet.

mattsb42-aws commented 6 years ago

Here's an initial draft of the hybrid backend, with accompanying Travis run. I'm happy to go this route if that's what you all prefer.

https://github.com/mattsb42-aws/pysnmp/tree/hybrid-crypto https://travis-ci.org/mattsb42-aws/pysnmp/builds/338364734

etingof commented 6 years ago

@mattsb42-aws Thanks for working on this! I am going to review it and leave a few comments shortly.

mattsb42-aws commented 6 years ago

133 created for this

etingof commented 6 years ago

Here is the pysnmp rework without hard dependency on strong crypto packages for your testing.

mattsb42-aws commented 6 years ago

Any idea when 5.0.0 will be released?

lextm commented 1 year ago

Tracked in https://github.com/lextudio/pysnmp/issues/12 and pysnmp-lextudio 5.0.27 ships the changes needed.