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

Ensure python tooling requires hashes, bump builders trusty->xenial #3477

Closed
wants to merge 8 commits into from

Conversation

msheiny
Copy link
Contributor

@msheiny msheiny commented May 24, 2018

This change helps maintain that each developer is pulling the exact same
resources, help mitigate against nefarious actors in the pip pipeline,
and solve male baldness.

Status

Work in Progress

Description of Changes

Kinda, sorta addresses #3270 .
Doesn't do anything to squash the multiple hashes for some packages tho.

Changes proposed in this pull request:

  • Breaks out pip update make target into a script
  • Adds hashes and removes the banner on those generated requirements files.
  • Replaces trusty with xenial for the builder containers. Why did I do this? Well I encountered a fun chicken before the egg problem with pip. Specifically I wanted to utilize hashes but the pip version installed in trusty does not support hashes (its really old, like 1.x series). It would have been super silly to peg all hashes and then just wildly run pip install -U pip before that. Sooooo I made the call to jump the builders to xenial which has a much newer version of pip pre-installed.
  • Fixes some misc ansible errors

On a side note, our quay Clare scanner reported dramatically less vulnerabilities when using xenial vs. trusty. Take that report with a grain of salt but hey i consider this a win.

Testing

How should the reviewer test this PR?

  • Make sure CI passes 👍
  • Make sure you can build debs and that vagrant up /staging/ comes up.

Deployment

Any special considerations for deployment?

This doesn't dramatically change deployment story but it does update a lot of the tooling behind the builder.

@msheiny msheiny requested a review from conorsch as a code owner May 24, 2018 19:34
@msheiny msheiny requested a review from a user May 24, 2018 19:34
msheiny added 6 commits May 24, 2018 15:45
This change helps maintain that each developer is pulling the exact same
resources, help mitigate against nefarious actors in the pip pipeline,
and solve male baldness.
The biggest win was this includes a much more recent version of pip so
we dont have the problem of trying to update pip using unsafe commands
(without hash verification).
Ansible doesnt like jinja in when statements and we can utilize AND by
converting the previous multi-line statement to a list.
* Makes sure we dont wreck the short lived container python tooling
* Utilize the require-hashes for installs
* Make sure we know what version of pip/wheel we are using
This test is now covered by the security test checker. Which confirms
that specifically no security updates are needed.
@msheiny msheiny force-pushed the FlagAllPipThings branch from 2a8fe33 to c53a38d Compare May 24, 2018 19:45
msheiny added 2 commits May 24, 2018 16:02
With latest python pip hash changes, the developer trusty container's
native pip version has no idea how to process those hashes.
Copy link
Contributor

@heartsucker heartsucker left a comment

Choose a reason for hiding this comment

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

Testing the build locally now. Some notes so far.

@@ -0,0 +1,28 @@
#!/bin/bash
#
# Needs to be run from repo root directory
Copy link
Contributor

Choose a reason for hiding this comment

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

You can add this to ensure it's always run at the repo root. Portable across Linux/Mac.

get_script_dir() {
  declare src="${BASH_SOURCE[0]}"
  declare dir=

  while [ -h "$src" ]; do
    dir="$(cd -P "$( dirname "$src")" && pwd)"
    src="$(readlink "$src")"
    [[ $src != /* ]] && src="$dir/$src"
  done
  cd -P "$(dirname "$src")" && pwd 
}
cd "$(get_script_dir)/../../"

ossec_download.stat.md5 == "{{ ossec_md5_checksum }}" and
ossec_download.stat.checksum == "{{ ossec_sha1_checksum }}")
when:
- not ossec_download.stat.exists
Copy link
Contributor

Choose a reason for hiding this comment

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

This logic isn't the same. You need to add a != for the other two to get the correct negations.

@@ -1,9 +1,9 @@
# April 28th, 2018 ubuntu:trusty
FROM ubuntu@sha256:b8855dc848e2622653ab557d1ce2f4c34218a9380cceaa51ced85c5f3c8eb201
# ubuntu:xenial-20180417
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@heartsucker
Copy link
Contributor

Doesn't do anything to squash the multiple hashes for some packages tho.

This is intentional since there's like eggs and wheels and all sorts of other ways to actually install the software. Look at the bottom of the page for cryptography on PyPI.

@heartsucker
Copy link
Contributor

heartsucker commented May 25, 2018

On a fresh checkout

vagrant destroy -f /staging/
rm -rf venv
virtualenv -p `which python2` venv
source venv/bin/activate
pip install -r securedrop/requirements/develop-requirements.txt
make build-debs
vagrant up /staging/

gives me this on .deb install:

failed: [app-staging] (item=securedrop-app-code-0.8.0~rc1-amd64.deb) => {"changed": true, "cmd": ["dpkg", "-i", "/root/securedrop-app-code-0.8.0~rc1-amd64.deb"], "delta": "0:00:01.176646", "end": "2018-05-25 09:08:38.402801", "item": "securedrop-app-code-0.8.0~rc1-amd64.deb", "msg": "non-zero return code", "rc": 1, "start": "2018-05-25 09:08:37.226155", "stderr": "debconf: unable to initialize frontend: Dialog\ndebconf: (Dialog frontend will not work on a dumb terminal, an emacs shell buffer, or without a controlling terminal.)\ndebconf: falling back to frontend: Readline\ndebconf: unable to initialize frontend: Readline\ndebconf: (This frontend requires a controlling tty.)\ndebconf: falling back to frontend: Teletype\n+ pip install --no-index --find-links=/var/securedrop/wheelhouse --upgrade -r /var/www/securedrop/requirements/securedrop-app-code-requirements.txt\ndpkg: error processing package securedrop-app-code (--install):\n subprocess installed post-installation script returned error exit status 2\nErrors were encountered while processing:\n securedrop-app-code", "stderr_lines": ["debconf: unable to initialize frontend: Dialog", "debconf: (Dialog frontend will not work on a dumb terminal, an emacs shell buffer, or without a controlling terminal.)", "debconf: falling back to frontend: Readline", "debconf: unable to initialize frontend: Readline", "debconf: (This frontend requires a controlling tty.)", "debconf: falling back to frontend: Teletype", "+ pip install --no-index --find-links=/var/securedrop/wheelhouse --upgrade -r /var/www/securedrop/requirements/securedrop-app-code-requirements.txt", "dpkg: error processing package securedrop-app-code (--install):", " subprocess installed post-installation script returned error exit status 2", "Errors were encountered while processing:", " securedrop-app-code"], "stdout": "(Reading database ... 44568 files and directories currently installed.)\nPreparing to unpack .../securedrop-app-code-0.8.0~rc1-amd64.deb ...\nUnpacking securedrop-app-code (0.8.0~rc1) over (0.8.0~rc1) ...\nSetting up securedrop-app-code (0.8.0~rc1) ...\nIgnoring indexes: https://pypi.python.org/simple/\nException:\nTraceback (most recent call last):\n  File \"/usr/lib/python2.7/dist-packages/pip/basecommand.py\", line 122, in main\n    status = self.run(options, args)\n  File \"/usr/lib/python2.7/dist-packages/pip/commands/install.py\", line 262, in run\n    for req in parse_requirements(filename, finder=finder, options=options, session=session):\n  File \"/usr/lib/python2.7/dist-packages/pip/req.py\", line 1632, in parse_requirements\n    req = InstallRequirement.from_line(line, comes_from, prereleases=getattr(options, \"pre\", None))\n  File \"/usr/lib/python2.7/dist-packages/pip/req.py\", line 173, in from_line\n    return cls(req, comes_from, url=url, prereleases=prereleases)\n  File \"/usr/lib/python2.7/dist-packages/pip/req.py\", line 71, in __init__\n    req = pkg_resources.Requirement.parse(req)\n  File \"/usr/lib/python2.7/dist-packages/pkg_resources.py\", line 2667, in parse\n    reqs = list(parse_requirements(s))\n  File \"/usr/lib/python2.7/dist-packages/pkg_resources.py\", line 2605, in parse_requirements\n    line, p, specs = scan_list(VERSION,LINE_END,line,p,(1,2),\"version spec\")\n  File \"/usr/lib/python2.7/dist-packages/pkg_resources.py\", line 2583, in scan_list\n    \"Expected ',' or end-of-list in\",line,\"at\",line[p:]\nValueError: (\"Expected ',' or end-of-list in\", 'asn1crypto==0.24.0 \\\\', 'at', ' \\\\')\n\nStoring debug log for failure in /root/.pip/pip.log", "stdout_lines": ["(Reading database ... 44568 files and directories currently installed.)", "Preparing to unpack .../securedrop-app-code-0.8.0~rc1-amd64.deb ...", "Unpacking securedrop-app-code (0.8.0~rc1) over (0.8.0~rc1) ...", "Setting up securedrop-app-code (0.8.0~rc1) ...", "Ignoring indexes: https://pypi.python.org/simple/", "Exception:", "Traceback (most recent call last):", "  File \"/usr/lib/python2.7/dist-packages/pip/basecommand.py\", line 122, in main", "    status = self.run(options, args)", "  File \"/usr/lib/python2.7/dist-packages/pip/commands/install.py\", line 262, in run", "    for req in parse_requirements(filename, finder=finder, options=options, session=session):", "  File \"/usr/lib/python2.7/dist-packages/pip/req.py\", line 1632, in parse_requirements", "    req = InstallRequirement.from_line(line, comes_from, prereleases=getattr(options, \"pre\", None))", "  File \"/usr/lib/python2.7/dist-packages/pip/req.py\", line 173, in from_line", "    return cls(req, comes_from, url=url, prereleases=prereleases)", "  File \"/usr/lib/python2.7/dist-packages/pip/req.py\", line 71, in __init__", "    req = pkg_resources.Requirement.parse(req)", "  File \"/usr/lib/python2.7/dist-packages/pkg_resources.py\", line 2667, in parse", "    reqs = list(parse_requirements(s))", "  File \"/usr/lib/python2.7/dist-packages/pkg_resources.py\", line 2605, in parse_requirements", "    line, p, specs = scan_list(VERSION,LINE_END,line,p,(1,2),\"version spec\")", "  File \"/usr/lib/python2.7/dist-packages/pkg_resources.py\", line 2583, in scan_list", "    \"Expected ',' or end-of-list in\",line,\"at\",line[p:]", "ValueError: (\"Expected ',' or end-of-list in\", 'asn1crypto==0.24.0 \\\\', 'at', ' \\\\')", "", "Storing debug log for failure in /root/.pip/pip.log"]}
root@app-staging:~# cat .pip/pip.log 
------------------------------------------------------------
/usr/bin/pip run on Fri May 25 09:08:38 2018
Ignoring indexes: https://pypi.python.org/simple/
Exception:
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/pip/basecommand.py", line 122, in main
    status = self.run(options, args)
  File "/usr/lib/python2.7/dist-packages/pip/commands/install.py", line 262, in run
    for req in parse_requirements(filename, finder=finder, options=options, session=session):
  File "/usr/lib/python2.7/dist-packages/pip/req.py", line 1632, in parse_requirements
    req = InstallRequirement.from_line(line, comes_from, prereleases=getattr(options, "pre", None))
  File "/usr/lib/python2.7/dist-packages/pip/req.py", line 173, in from_line
    return cls(req, comes_from, url=url, prereleases=prereleases)
  File "/usr/lib/python2.7/dist-packages/pip/req.py", line 71, in __init__
    req = pkg_resources.Requirement.parse(req)
  File "/usr/lib/python2.7/dist-packages/pkg_resources.py", line 2667, in parse
    reqs = list(parse_requirements(s))
  File "/usr/lib/python2.7/dist-packages/pkg_resources.py", line 2605, in parse_requirements
    line, p, specs = scan_list(VERSION,LINE_END,line,p,(1,2),"version spec")
  File "/usr/lib/python2.7/dist-packages/pkg_resources.py", line 2583, in scan_list
    "Expected ',' or end-of-list in",line,"at",line[p:]
ValueError: ("Expected ',' or end-of-list in", 'asn1crypto==0.24.0 \\', 'at', ' \\')

@msheiny
Copy link
Contributor Author

msheiny commented May 25, 2018

yeahh thanks for the early review @heartsucker ... its obviously not ready for merge right now ... i also encountered breaking tests during local development that i wasnt able to fix. if someone else wants to step in and help out here ❤️ ❤️ otherwise im going to jump to some other tasks for a few days. I kind of did this ticket on a whim and I thought it would be easy-peasy...that older pip issue ended up causing a lot of headache and will require more work.

@redshiftzero
Copy link
Contributor

now that we're on xenial only for the 0.13.x release series, closing in favor of #4435 ❤️

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.

3 participants