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

Requirements pinning for app code build #4435

Merged
merged 5 commits into from
May 16, 2019
Merged

Requirements pinning for app code build #4435

merged 5 commits into from
May 16, 2019

Conversation

emkll
Copy link
Contributor

@emkll emkll commented May 13, 2019

Status

Ready for review

Description of Changes

Towards #3270, #1617 : Pin hashes for securedrop-app-code python requirements. Until now, we were blocked by Trusty tooling (see #3477)

This should be a stopgap until we implement a more complete solution that will also offer .deb reproducibility, described in freedomofpress/securedrop-builder#34.

Testing

Test plan:

  • Changes to the app code build logic make sense
  • All tests pass when running make build-debs
  • .deb build successful and application/infra tests pass (CI should pass)
  • Local upgrade testing is successful
  • Changing hashes for the source tarballs in securedrop-app-code-requirements.txt should make the build fail.
  • Modify securedrop-app-code-requirements.txt file for a source tarball should result in build failure.

Since the build process is automated, proper verification requires adding a wait_for: timout=3600s at the end of install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/tasks/build_securedrop_app_code_deb.yml and using diffoscope to look at build wheels in the wheelhouse vs wheels in the deb package

  • Inside the container, /tmp/w.out contains build information of the wheels (confirm that we actually built the wheels)
  • Wheels in /tmp/securedrop-app-code_0.13.0~rc1+xenial_amd64/var/securedrop/wheelhouse/ are nearly identical to those in the deb package while using diffoscope, the difference is in the zip files and metadata within.

You can unzip the wheels and diff the folder contents, eg:

$diffoscope cryptography-2.6.1.dist-info/ ../../cryptography-2.6.1.dist-info/
 |#################################################################|  100%                             Time: 0:00:00 
--- cryptography-2.6.1.dist-info/
+++ ../../cryptography-2.6.1.dist-info/
├── stat {}
│ @@ -1,8 +1,8 @@
│  
│    Size: 4096      	Blocks: 8          IO Block: 4096   directory
│  Links: 2
│  Access: (0755/drwxr-xr-x)  Uid: ( 1000/       m)   Gid: ( 1000/       m)
│  
│ -Modify: 2019-05-15 18:59:21.382879799 +0000
│ +Modify: 2019-05-15 18:57:38.587575536 +0000
│  
[...]

and

$ diffoscope cryptography/ ../../cryptography/
 |#################################################################|  100%                             Time: 0:00:03 
--- cryptography/
+++ ../../cryptography/
├── stat {}
│ @@ -1,8 +1,8 @@
│  
│    Size: 4096      	Blocks: 8          IO Block: 4096   directory
│  Links: 4
│  Access: (0755/drwxr-xr-x)  Uid: ( 1000/       m)   Gid: ( 1000/       m)
│  
│ -Modify: 2019-05-15 18:59:21.438879419 +0000
│ +Modify: 2019-05-15 18:57:38.583575562 +0000
│  
│   Birth: -
│   --- cryptography/__about__.py
├── +++ ../../cryptography/__about__.py
├── stat {}
│ │ @@ -1,8 +1,8 @@
│ │  
│ │    Size: 816       	Blocks: 8          IO Block: 4096   regular file
│ │  Links: 1
│ │  Access: (0644/-rw-r--r--)  Uid: ( 1000/       m)   Gid: ( 1000/       m)
│ │  
│ │ -Modify: 2019-02-27 16:02:06.000000000 +0000
│ │ +Modify: 2019-02-28 04:27:52.000000000 +0000
│ │  
│ │   Birth: -
│   --- cryptography/__init__.py
├── +++ ../../cryptography/__init__.py
├── stat {}
│ │ @@ -1,8 +1,8 @@
│ │  
│ │    Size: 527       	Blocks: 8          IO Block: 4096   regular file
│ │  Links: 1
│ │  Access: (0644/-rw-r--r--)  Uid: ( 1000/       m)   Gid: ( 1000/       m)
│ │  
│ │ -Modify: 2019-02-27 16:02:06.000000000 +0000
│ │ +Modify: 2019-02-28 04:27:52.000000000 +0000
│ │  

Deployment

Mostly a dev/build environment change, but all changes to prod instances will be delivered via the securedrop-app-code package.

Checklist

If you made changes to the server application code:

If you made non-trivial code changes:

  • I have written a test plan and validated it for this PR

@emkll
Copy link
Contributor Author

emkll commented May 15, 2019

This should be ready for re-review, I've updated the test plan and all tests pass locally.

@redshiftzero
Copy link
Contributor

redshiftzero commented May 15, 2019

my testing thus far:

  • Changes to the app code build logic make sense
  • All tests pass when running make build-debs - yes modulo known issue test_deb_package_contains_expected_conffiles fails on macOS #4191
  • .deb build successful and application/infra tests pass (CI should pass) - waiting on CI
  • Local upgrade testing is successful - not doing, @conorsch kicking this off
  • Changing hashes for the source tarballs in securedrop-app-code-requirements.txt should make the build fail:
    TASK [build-securedrop-app-code-deb-pkg : Ensure source hash sums matched at wheel build-time] ***
    fatal: [xenial-sd-app]: FAILED! => {"changed": false, "failed_when_result": true, "msg": "Source hash sum mismatch, build cannot continue."}
  • Inside the container, /tmp/w.out contains build information of the wheels (confirm that we actually built the wheels)
  • Wheels in /tmp/securedrop-app-code_0.13.0~rc1+xenial_amd64/var/securedrop/wheelhouse/ are nearly identical to those in the deb package while using diffoscope, the difference is in the zip files and metadata within.

@redshiftzero
Copy link
Contributor

staging build failure due to too long with no output, gonna restart once..

@redshiftzero
Copy link
Contributor

this time another timeout but later in the provisioning process:

    RUNNING HANDLER [common : Wait for server to come back.] ***********************
    ok: [mon-staging -> localhost]
    fatal: [app-staging -> localhost]: FAILED! => {"changed": false, "elapsed": 300, "msg": "Timeout when waiting for search string OpenSSH in 192.168.121.127:22"}

@conorsch
Copy link
Contributor

  • Local upgrade testing is successful

Performed interactive testing post-upgrade, including:

  • submitting documents and messages
  • replying to submissions
  • viewing replies via source account

LGTM. We're blocked here by recurring CI failures, opened #4440 to track.

emkll added 2 commits May 16, 2019 10:04
Use no-binary and require hashes option at wheel build time, and generate new requirements file (without hashes) for securedrop-app-code deb package. This is because dpkg-buildpackage modifies the zip data of the built wheel files, making it difficult to require hashes at install-time server-side. The deb package is signed and as such would be difficult to tamper with wheel contents in transit
@emkll
Copy link
Contributor Author

emkll commented May 16, 2019

rebased on latest develop

redshiftzero
redshiftzero previously approved these changes May 16, 2019
Copy link
Contributor

@redshiftzero redshiftzero left a comment

Choose a reason for hiding this comment

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

based on testing performed yesterday, approving for merge provided the staging CI job passes

@emkll
Copy link
Contributor Author

emkll commented May 16, 2019

@redshiftzero apologies, this will need another stamp: I had to force push another change to resolve lint failure: the develop-requirements were using bandit 1.6.0, i reverted to 1.4.0 (see #4424) .

This will make the build process marginally faster and provide better integrity
emkll added 2 commits May 16, 2019 12:32
Adds python-wheel and latest security patches
Before, build would fail due to absence of wheels in the /var/securedrop/wheelhouse, at a later step.
@emkll
Copy link
Contributor Author

emkll commented May 16, 2019

Due to some issues with linting in ci [1,2], I've dropped the commits that would contain the develop-requirements hashes. Let's open a follow-up task to track this to unblock merge of the app code requirements hashes.

[1] : https://circleci.com/gh/freedomofpress/securedrop/27948
[2] : https://circleci.com/gh/freedomofpress/securedrop/27927

Copy link
Contributor

@redshiftzero redshiftzero left a comment

Choose a reason for hiding this comment

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

re-approving after removing the dev requirements

@eloquence eloquence changed the title Requirements pinning for app code build and install Requirements pinning for app code build May 16, 2019
@emkll emkll mentioned this pull request May 16, 2019
2 tasks
@zenmonkeykstop zenmonkeykstop merged commit 7394fd5 into develop May 16, 2019
@redshiftzero redshiftzero deleted the pin-hashes branch June 25, 2019 23:08
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

Successfully merging this pull request may close these issues.

4 participants