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

securedrop-app-code python packages not installed when package is already provided by other OS package #1367

Closed
psivesely opened this issue Aug 10, 2016 · 2 comments
Assignees
Milestone

Comments

@psivesely
Copy link
Contributor

Although in the Ansible output it was not specified which OS packages provided the conflicting Python packages, it appears that all SecureDrop instances are running out-of-date versions of colorama and six. I should have realized this was the case when I made #1302 as a fix to #856--the same phenomenon we'd use to our advantage in that PR is here a potentially serious problem.

colorama has no git history back to version 0.2.5 and the old Google Code page just redirects to the Github repo now. The changelog does not mention any security issues. Anyway, the only function that is called is colorama.init() by qrcode. No public Internet facing code uses the out-of-date colorama code. Also, as noted in #1352 (comment) a PR of mine was merged into qrcode such that colorama will no longer be a dependency in the next version of SD.

six again is only used by qrcode and does not seem to have had any security issues between 1.5.2 and the present.

I think it is safe to conclude that, very fortunately, this problem has not left us vulnerable. Had some Ubuntu base distro package or some other apt dependency of SD provided or pulled in and installed some out-of-date Python package that did include vulnerable code (or should this happen in the future) that would be a serious security issue. I think it's important we take care of this relatively quickly (again, not urgent because there's not actually a security issue here, just the potential for one) and ship out a 0.3.9 that fixes this and #856.

"Setting up securedrop-app-code (0.3.8) ...", "Ignoring indexes:
https://pypi.python.org/simple/", "Downloading/unpacking click==6.6 (from -r
/var/www/securedrop/requirements/securedrop-requirements.txt (line 8))",
"Downloading/unpacking colorama==0.3.7 (from -r
/var/www/securedrop/requirements/securedrop-requirements.txt (line 9))",
"Downloading/unpacking Flask-WTF==0.12 (from -r
/var/www/securedrop/requirements/securedrop-requirements.txt (line 10))",
"Downloading/unpacking Flask==0.10.1 (from -r
/var/www/securedrop/requirements/securedrop-requirements.txt (line 11))",
"Downloading/unpacking future==0.15.2 (from -r
/var/www/securedrop/requirements/securedrop-requirements.txt (line 12))",
"Downloading/unpacking gnupg==2.0.2 (from -r
/var/www/securedrop/requirements/securedrop-requirements.txt (line 13))",
"Downloading/unpacking itsdangerous==0.24 (from -r
/var/www/securedrop/requirements/securedrop-requirements.txt (line 14))",
"Downloading/unpacking Jinja2==2.8 (from -r
/var/www/securedrop/requirements/securedrop-requirements.txt (line 15))",
"Downloading/unpacking MarkupSafe==0.23 (from -r
/var/www/securedrop/requirements/securedrop-requirements.txt (line 16))",
"Downloading/unpacking psutil==4.1.0 (from -r
/var/www/securedrop/requirements/securedrop-requirements.txt (line 17))",
"Downloading/unpacking pycrypto==2.6.1 (from -r
/var/www/securedrop/requirements/securedrop-requirements.txt (line 18))",
"Downloading/unpacking pyotp==2.1.1 (from -r
/var/www/securedrop/requirements/securedrop-requirements.txt (line 19))",
"Downloading/unpacking qrcode==5.2.2 (from -r
/var/www/securedrop/requirements/securedrop-requirements.txt (line 20))",
"Downloading/unpacking redis==2.10.5 (from -r
/var/www/securedrop/requirements/securedrop-requirements.txt (line 21))",
"Downloading/unpacking rq==0.6.0 (from -r
/var/www/securedrop/requirements/securedrop-requirements.txt (line 22))",
"Downloading/unpacking scrypt==0.7.1 (from -r
/var/www/securedrop/requirements/securedrop-requirements.txt (line 23))",
"Downloading/unpacking six==1.10.0 (from -r
/var/www/securedrop/requirements/securedrop-requirements.txt (line 24))",
"Downloading/unpacking SQLAlchemy==1.0.13 (from -r
/var/www/securedrop/requirements/securedrop-requirements.txt (line 25))",
"Downloading/unpacking Werkzeug==0.11.9 (from -r
/var/www/securedrop/requirements/securedrop-requirements.txt (line 26))",
"Downloading/unpacking WTForms==2.1 (from -r
/var/www/securedrop/requirements/securedrop-requirements.txt (line 27))",
"Installing collected packages: click, colorama, Flask-WTF, Flask, future,
gnupg, itsdangerous, Jinja2, MarkupSafe, psutil, pycrypto, pyotp, qrcode, redis,
rq, scrypt, six, SQLAlchemy, Werkzeug, WTForms", "  Found existing installation:
colorama 0.2.5", "    Not uninstalling colorama at
/usr/lib/python2.7/dist-packages, owned by OS", "  Found existing installation:
six 1.5.2", "    Not uninstalling six at /usr/lib/python2.7/dist-packages, owned
by OS", "Successfully installed click colorama Flask-WTF Flask future gnupg
itsdangerous Jinja2 MarkupSafe psutil pycrypto pyotp qrcode redis rq scrypt six
SQLAlchemy Werkzeug WTForms"
@psivesely psivesely added this to the 0.3.9 milestone Aug 10, 2016
@psivesely psivesely self-assigned this Aug 10, 2016
@conorsch
Copy link
Contributor

Great research here, @fowlslegs. Looks like the "owned by OS" functionality is coming from a Debian patch, as described here: pypa/pip#2964 (comment)

Definitely something we'll want to track more closely. At the very least, we can force our tests to confirm specific version numbers for the SD-related pip packages, so we can catch occurrences that affect packages more critical than colorama and six.

In terms of actually fixing the problem, two ideas come to mind:

  • leverage the Replaces control field in the Debian package for securedrop-app-code to ensure our version wins
  • write the wheel packages to a Python path separate from the OS-wide directory, and update the webserver config to read from that dedicated location

Both are worth investigating.

@psivesely
Copy link
Contributor Author

Okay, so our postinst scipt runs the command pip install --no-index --find-links=/var/securedrop/wheelhouse -r /var/www/securedrop/requirements/securedrop-requirements.txt, which installs packages to /usr/local/lib/python2.7/dist-packages. The colorama and six that aren't getting uninstalled because they're owned by OS packages are located under /usr/lib/python2.7/dist-packages. The sys.path variable determines which one gets loaded first:

>>> sys.path
['', '/usr/lib/python2.7', '/usr/lib/python2.7/plat-x86_64-linux-gnu',
'/usr/lib/python2.7/lib-tk', '/usr/lib/python2.7/lib-old',
'/usr/lib/python2.7/lib-dynload', '/usr/local/lib/python2.7/dist-packages',
'/usr/lib/python2.7/dist-packages']

So, it turns out that (fortunately) this was never actually a problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants