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

{Packaging} Bump cryptography from 2.8 to 3.3.2 #15687

Merged
merged 9 commits into from
Feb 19, 2021

Conversation

dependabot[bot]
Copy link
Contributor

@dependabot dependabot bot commented on behalf of github Oct 27, 2020

Bumps cryptography from 2.8 to 3.2.

Changelog

Sourced from cryptography's changelog.

3.2 - 2020-10-25


* **SECURITY ISSUE:** Attempted to make RSA PKCS#1v1.5 decryption more constant
  time, to protect against Bleichenbacher vulnerabilities. Due to limitations
  imposed by our API, we cannot completely mitigate this vulnerability and a
  future release will contain a new API which is designed to be resilient to
  these for contexts where it is required. Credit to **Hubert Kario** for
  reporting the issue. *CVE-2020-25659*
* Support for OpenSSL 1.0.2 has been removed. Users on older version of OpenSSL
  will need to upgrade.
* Added basic support for PKCS7 signing (including SMIME) via
  :class:`~cryptography.hazmat.primitives.serialization.pkcs7.PKCS7SignatureBuilder`.

.. _v3-1-1:

3.1.1 - 2020-09-22

  • Updated Windows, macOS, and manylinux wheels to be compiled with OpenSSL 1.1.1h.

.. _v3-1:

3.1 - 2020-08-26


* **BACKWARDS INCOMPATIBLE:** Removed support for ``idna`` based
  :term:`U-label` parsing in various X.509 classes. This support was originally
  deprecated in version 2.1 and moved to an extra in 2.5.
* Deprecated OpenSSL 1.0.2 support. OpenSSL 1.0.2 is no longer supported by
  the OpenSSL project. The next version of ``cryptography`` will drop support
  for it.
* Deprecated support for Python 3.5. This version sees very little use and will
  be removed in the next release.
* ``backend`` arguments to functions are no longer required and the
  default backend will automatically be selected if no ``backend`` is provided.
* Added initial support for parsing certificates from PKCS7 files with
  :func:`~cryptography.hazmat.primitives.serialization.pkcs7.load_pem_pkcs7_certificates`
  and
  :func:`~cryptography.hazmat.primitives.serialization.pkcs7.load_der_pkcs7_certificates`
  .
* Calling ``update`` or ``update_into`` on
  :class:`~cryptography.hazmat.primitives.ciphers.CipherContext` with ``data``
  longer than 2\ :sup:`31` bytes no longer raises an ``OverflowError``. This
  also resolves the same issue in :doc:`/fernet`.

.. _v3-0:

3.0 - 2020-07-20 </tr></table>

... (truncated)

Commits

Dependabot compatibility score

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
  • @dependabot use these labels will set the current labels as the default for future PRs for this repo and language
  • @dependabot use these reviewers will set the current reviewers as the default for future PRs for this repo and language
  • @dependabot use these assignees will set the current assignees as the default for future PRs for this repo and language
  • @dependabot use this milestone will set the current milestone as the default for future PRs for this repo and language

You can disable automated security fix PRs for this repo from the Security Alerts page.

@ghost ghost added the customer-reported Issues that are reported by GitHub users external to the Azure organization. label Oct 27, 2020
@ghost
Copy link

ghost commented Oct 27, 2020

Thank you for your contribution dependabot[bot]! We will review the pull request and get back to you soon.

@yonzhan
Copy link
Collaborator

yonzhan commented Oct 28, 2020

cryptography

@yonzhan
Copy link
Collaborator

yonzhan commented Oct 28, 2020

triggered by a bot.

@fengzhou-msft
Copy link
Member

The new version deprecated OpenSSL 1.0.2 support. The bump could be risky.

@bim-msft
Copy link
Contributor

Holding this PR open until we do a full regression test and related investigations.

@yungezz yungezz assigned fengzhou-msft and unassigned bim-msft Dec 22, 2020
@jiasli jiasli changed the title Bump cryptography from 2.8 to 3.2 in /src/azure-cli {Packaging} Bump cryptography from 2.8 to 3.2 Dec 31, 2020
@jiasli jiasli requested review from jiasli and removed request for bim-msft and haroldrandom December 31, 2020 08:33
@jiasli
Copy link
Member

jiasli commented Jan 20, 2021

The latest PyJWT 2.0.1 requires

https://github.com/jpadilla/pyjwt/blob/3993ce1d3503b58cf74699a89ba9e5c18ef9b556/setup.cfg#L55

cryptography>=3.3.1,<4.0.0

We have to bump to at least 3.3.1, otherwise PyJWT will fail (#16416).

@jiasli jiasli requested a review from houk-ms January 20, 2021 09:57
Copy link
Member

@fengzhou-msft fengzhou-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No regression in live tests for keyvault.

src/azure-cli/requirements.py3.Darwin.txt Outdated Show resolved Hide resolved
src/azure-cli/requirements.py3.Linux.txt Outdated Show resolved Hide resolved
src/azure-cli/requirements.py3.windows.txt Outdated Show resolved Hide resolved
Copy link
Contributor

@houk-ms houk-ms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Locally verified live tests for managed HSM could pass for 3.3.2

@@ -47,6 +47,7 @@
'argcomplete~=1.8',
'azure-cli-telemetry==1.0.6.*',
'colorama~=0.4.1',
'cryptography>=3.2,<3.4',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adal relies on cryptography, so moved it to azure-cli-core.

3.4+ versions rely on rust and cause install issues on some platforms. Let's set the upper bound for now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3.4+ versions rely on rust and cause install issues on some platforms. Let's set the upper bound for now.

The original discussion: pyca/cryptography#5771

Copy link
Member

@jiasli jiasli Feb 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

azure-devops (#16858) and azure-cli-ml (#16880) shouldn't rely on adal or cryptography directly. They should use those provided by azure-cli-core.

Copy link
Member

@fengzhou-msft fengzhou-msft Feb 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I will submit issues to azure-devops (submitted Azure/azure-devops-cli-extension#1108). Machine learning team is working on a new extension to replace the old one, I will request a cleaning of their dependnecies.

@@ -131,7 +131,6 @@
'azure-synapse-accesscontrol~=0.2.0',
'azure-synapse-artifacts~=0.3.0',
'azure-synapse-spark~=0.2.0',
'cryptography>=2.3.1,<3.0.0',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will remove the dependency here impacting extensions etc? since we don't want extension directly depend on azure-cli-core?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

azure-cli will always install azure-cli-core, so moving the dependency version specifier to azure-cli-core will have no impact on azure-cli and extensions. Previously, pip install azure-cli-core individually will fail as azure-cli-core indirectly depends on cryptography through adal, but it's not pinned there.

@fengzhou-msft fengzhou-msft merged commit 1f87589 into dev Feb 19, 2021
@dependabot dependabot bot deleted the dependabot/pip/src/azure-cli/cryptography-3.2 branch February 19, 2021 09:42
@fengzhou-msft fengzhou-msft changed the title {Packaging} Bump cryptography from 2.8 to 3.2 {Packaging} Bump cryptography from 2.8 to 3.3.2 Feb 19, 2021
@jiasli jiasli mentioned this pull request May 7, 2021
9 tasks
@glaubitz
Copy link

Is there any particular reason why the minimum version needs to be 3.3.2 now? Bumping major components like cryptography to much newer versions causes a lot of headaches for enterprise distributions package maintainers.

@jiasli
Copy link
Member

jiasli commented Jul 13, 2021

Is there any particular reason why the minimum version needs to be 3.3.2 now?

This is not true. The current dev branch specifies

'cryptography>=3.2,<3.4',

so the minimum version is 3.2.

requirements.py3.*.txt files are only used by our build pipeline to build msi, deb, rpm packages, and it should not be used directly by the end user. Instead, the user should only use setup.py.

Per the change log of cryptography, 3.3.2 includes a security fix for CVE-2020-36242, that's why we specify 3.3.2
in requirements.py3.*.txt files to make sure our released packages have the security fix to meet our internal security scan requirement.

Bumping major components like cryptography to much newer versions causes a lot of headaches for enterprise distributions package maintainers.

In the future, we will eventually bump to cryptography 3.4+ which will break more Linux distributions due to the rust dependency. However, as pointed out by pyca/cryptography#5771, it should be the Linux distribution maintainer's responsibility to make the distribution compatible with new cryptography to meet the security requirement.

Thanks for understanding.

@glaubitz
Copy link

Is there any particular reason why the minimum version needs to be 3.3.2 now?

This is not true. The current dev branch specifies

'cryptography>=3.2,<3.4',

so the minimum version is 3.2.

That was not my point. My point is, it's not a good idea to raise the minimum version numbers in the dependencies when you are actually not making use of any newer API.

Linux distributions are packaging the Azure SDK/CLI to ship it to their customers and customers want a newer version of the SDK/CLI in the distribution from time to time.

So, if upstream is regularly bumping the dependency versions without actually requiring the new upstream version of - let's say - cryptography, you are forcing distributions to update the cryptography dependency as well which is not easy since a lot of other packages depend on cryptography and we risk potential breakage of other software that we ship to customers if we just upgrade cryptography.

That's why upstream projects should not bump version dependencies unless it's actually necessary. Or, at least, add a comment that it will work with older versions XYZ as well as long as these receive security support.

requirements.py3.*.txt files are only used by our build pipeline to build msi, deb, rpm packages, and it should not be used directly by the end user. Instead, the user should only use setup.py.

The dependencies in setup.py tell me as a packager which version requirements I need to put into the spec file. If there is no particular information given why a certain dependency version is being used, I have to assume the minimum version is required because a certain API is being used by Azure SDK/CLI.

Per the change log of cryptography, 3.3.2 includes a security fix for CVE-2020-36242, that's why we specify 3.3.2
in requirements.py3.*.txt files to make sure our released packages have the security fix to meet our internal security scan requirement.

(Enterprise) Linux distributions provide maintain security support even for older packages. So, if a package like cryptography suffers from a vulnerability, we actually backport the patch to fix the vulnerability as we can't just update the package without having to fear breakage.

Bumping major components like cryptography to much newer versions causes a lot of headaches for enterprise distributions package maintainers.

In the future, we will eventually bump to cryptography 3.4+ which will break more Linux distributions due to the rust dependency.

There are projects like gccrs and rustc_codegen_gcc that hugely expand the portability of Rust, so no, cryptography won't break Linux distributions in the future. But since these Rust projects are not finished yet, you shouldn't depend on cryptography 3.4 yet unless you already require any of the new API.

However, as pointed out by pyca/cryptography#5771, it should be the Linux distribution maintainer's responsibility to make the distribution compatible with new cryptography to meet the security requirement.

My point is: Please don't bump dependency versions for packages like cryptography unless you actually need any functionality of the new version. Linux distributions maintain security updates for older versions, so we don't randomly update packages like cryptography unless we actually need a new feature.

Please be more considerate of enterprise users, they are your main paying customers in the public cloud.

@jiasli
Copy link
Member

jiasli commented Sep 22, 2021

@glaubitz, thanks for the detailed explanation. I am now loosening the dependency on cryptograph in #19639. Does 'cryptography~=3.0' suit your need?

@glaubitz
Copy link

We currently ship python-cryptography 2.8 in both SUSE Linux Enterprise 12 and 15. So, if you could go down to 2.8 - if possible - that would be great.

If it's not possible, we will have to upgrade python-cryptography when we update azure-cli to newer versions which will then take a little longer.

@jiasli
Copy link
Member

jiasli commented Sep 23, 2021

@glaubitz, let's discuss in #19639.

@glaubitz
Copy link

OK.

@jiasli
Copy link
Member

jiasli commented Sep 24, 2021

Hi @glaubitz, we are pending on your reply for #19639 (comment) to get #19639 merged. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customer-reported Issues that are reported by GitHub users external to the Azure organization. External Dependency
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants