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

replace pycryptodome with pyca/cryptography 2.0.3 #3458

Merged
merged 4 commits into from
May 24, 2018
Merged

Conversation

redshiftzero
Copy link
Contributor

@redshiftzero redshiftzero commented May 18, 2018

Status

Ready for review

Description of Changes

Fixes #3399, remix of #3400

See #3441 for why cryptography 2.0.3 is what we'll need to install in 0.8.0

Testing

The integration tests and the secure tempfile unit tests here do test this functionality.

In addition, one can manually test by submitting a file that is greater than 512KB and ensuring that you can successfully decrypt it.

Deployment

It will install without issue on existing and new installs

Checklist

If you made changes to the server application code:

  • Linting (make ci-lint) and tests (make -C securedrop test) pass in the development container

reaperhulk and others added 2 commits May 17, 2018 17:59
Latest pyca/cryptography requires setuptools>18.5, which we do not
have on prod installs. In order to make this transition in the next
minor release however, we can temporarily pin to a slightly older
version of pyca/cryptography, such that we can install it using
the version of setuptools that we have.

For the Docker dev env, we need libffi-dev installed to pip install
cryptography.
@redshiftzero
Copy link
Contributor Author

redshiftzero commented May 19, 2018

On the build on 117acb664588c8664f23ee0101279e35bf601279, there is a test failure on tests/test_secure_tempfile.py::test_buffered_read, only the first 1024 bytes are being decrypted properly

@codecov-io
Copy link

codecov-io commented May 19, 2018

Codecov Report

Merging #3458 into develop will increase coverage by 0.05%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #3458      +/-   ##
===========================================
+ Coverage    85.87%   85.93%   +0.05%     
===========================================
  Files           34       34              
  Lines         2167     2175       +8     
  Branches       241      241              
===========================================
+ Hits          1861     1869       +8     
  Misses         250      250              
  Partials        56       56
Impacted Files Coverage Δ
secure_tempfile.py 100% <0%> (ø) ⬆️
crypto_util.py 96% <0%> (+0.04%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aee07c8...7a35752. Read the comment docs.

@redshiftzero redshiftzero changed the title [wip] replace pycryptodome with pyca/cryptography 2.0.3 replace pycryptodome with pyca/cryptography 2.0.3 May 22, 2018
@redshiftzero redshiftzero requested a review from emkll May 22, 2018 16:43
@redshiftzero
Copy link
Contributor Author

Hey @emkll, take a look at this one when you get a chance 😇

Copy link
Contributor

@emkll emkll left a comment

Choose a reason for hiding this comment

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

Changes look good to me, thanks @reaperhulk and @redshiftzero !

@emkll emkll merged commit 2dd7bbe into develop May 24, 2018
@heartsucker heartsucker deleted the cryptography-2.0.3 branch February 27, 2019 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants