84b5e7ec48
This introduces a number of secure development practices that were written by the OSSG during the mid-cycle meetup of 2015. Credit goes to the original authors and editors of this content: Dave Belcher (HP) Eric Brown (VMWare) Doug Chivers (HP) Rob Clarke (HP) Nathaniel Dillon (HP) Lucas Fisher (Was Nebula) Jamie Finnigan (HP) Rob Fletcher (Uber) Tim Kelsey (HP) Brant Knudson (IBM) Paul McMillan (Was Nebula) Travis McPeak (HP) Grant Murphy (HP) The following is an attempt to track the origins on this content as it was developed. It is not a complete version history but indicates the initial import of the content into respective repositories prior to ending up here. Hopefully this will be enough to track the changes and contributions made by members of the OpenStack community. { "origins": [ { "author": "Travis McPeak (HP)", "file": "doc/source/guidelines/dg_apply-restrictive-file-permissions.rst", "history": [ { "commit": "d5f6ad970ec24564cfaffc13dba26b0006841d5b", "id": "1", "repo": "git://github.com/hyakuhei/OSSG-Security-Practices" }, { "commit": "c7d0e14e9a71fa9bbe301aa76a8adcc55ade52de", "id": "2", "repo": "github.com/openstack-security/Developer-Guidance" } ] }, { "author": "Lucas Fisher (Nebula)", "file": "doc/source/guidelines/dg_avoid-dangerous-input-parsing-libraries.rst", "history": [ { "commit": "9f12b0bacc7bf207df5615dce39940aaede66e17", "id": "1", "repo": "git://github.com/hyakuhei/OSSG-Security-Practices" }, { "commit": "9b4181fe3c2e5451ebf65b99b05fe169b6eda63c", "id": "2", "repo": "github.com/openstack-security/Developer-Guidance" } ] }, { "author": "Paul McMillan (Nebula)", "comment": "The file was not renamed correctly!", "file": "doc/source/guidelines/dg_avoid-shell-true.rst", "history": [ { "commit": "ef6338caa2a6f5cfb67dd2d77a38574d45ae5d2c", "id": "1", "repo": "git://github.com/hyakuhei/OSSG-Security-Practices" }, { "commit": "c7ade476015c1b0fbb1b100b990a10e8373027c2", "id": "2", "repo": "github.com/openstack-security/Developer-Guidance" } ] }, { "author": "Grant Murphy (HP)", "file": "doc/source/guidelines/dg_avoid-unvalidated-redirects.rst", "history": [ { "commit": "2f0c954bd4f25a06b8a1439739d494c3a9d0c2de", "id": "1", "repo": "git://github.com/hyakuhei/OSSG-Security-Practices" }, { "commit": "8fb7804da189c95e9bcd2b19b516077b8fb6a44a", "id": "2", "repo": "github.com/openstack-security/Developer-Guidance" } ] }, { "author": "Dave Belcher (HP)", "file": "doc/source/guidelines/dg_cross-site-request-forgery-csrf.rst", "history": [ { "commit": "34146ded7f5c8ca3b0be29c17b30fb887a47b955", "id": "1", "repo": "git://github.com/hyakuhei/OSSG-Security-Practices" }, { "commit": "688a73070e52a693f1fc7b827cfbfe1d03bd7f9c", "id": "2", "repo": "github.com/openstack-security/Developer-Guidance" } ] }, { "author": "Dave Belcher (HP)", "file": "doc/source/guidelines/dg_cross-site-scripting-xss.rst", "history": [ { "commit": "f4261162a2b07df282fbfab68245b23c39f46fb9", "id": "1", "repo": "git://github.com/hyakuhei/OSSG-Security-Practices" }, { "commit": "e8a3cf91778081e0c822277951c0f5cbc6293c21", "id": "2", "repo": "github.com/openstack-security/Developer-Guidance" } ] }, { "author": "Rob Clarke (HP)", "file": "doc/source/guidelines/dg_move-data-securely.rst", "history": [ { "commit": "5d57e38c71ec7fd9113bb5574c143b4b146b95d5", "id": "1", "repo": "git://github.com/hyakuhei/OSSG-Security-Practices" }, { "commit": "6d266e56a1b4b746deab7b26d6b8bf96774dca72", "id": "2", "repo": "github.com/openstack-security/Developer-Guidance" } ] }, { "author": "Tim Kelsey", "file": "doc/source/guidelines/dg_parameterize-database-queries.rst", "history": [ { "commit": "70969ec41f9ce5de47090ac7a5cb4e303671b756", "id": "1", "repo": "git://github.com/hyakuhei/OSSG-Security-Practices" }, { "commit": "", "id": "2", "repo": "github.com/openstack-security/Developer-Guidance" } ] }, { "author": "Eric Brown (VMware)", "file": "doc/source/guidelines/dg_protect-sensitive-data-in-files.rst", "history": [ { "commit": "038d65e6fd8c17dffa28f55c99e218d09802f5ec", "id": "1", "repo": "git://github.com/hyakuhei/OSSG-Security-Practices" }, { "commit": "8094050151347ac83b6a05c5895dc9f670b70c29", "id": "2", "repo": "github.com/openstack-security/Developer-Guidance" } ] }, { "author": "Eric Brown (VMware)", "file": "doc/source/guidelines/dg_validate-certificates.rst", "history": [ { "commit": "f74fecc0360a9f9c6984d821b128c4830bd71b0f", "id": "1", "repo": "git://github.com/hyakuhei/OSSG-Security-Practices" }, { "commit": "2ffd29660d9f48079ab1c9a0de170eb660ba22fe", "id": "2", "repo": "github.com/openstack-security/Developer-Guidance" } ] }, { "author": "Lucas Fisher (Nebula)", "file": "doc/source/guidelines/dg_rootwrap-recommendations-and-plans.rst", "history": [ { "commit": "bd5a51411420fd70ee410ff72cb396287ce0c60e", "id": "1", "repo": "git://github.com/hyakuhei/OSSG-Security-Practices" }, { "commit": "6a13d1ecd8ec73f7038c8e3238e24ac32ac60e58", "id": "2", "repo": "github.com/openstack-security/Developer-Guidance" } ] }, { "author": "Jamie Finnigan (HP)", "file": "doc/source/guidelines/dg_strong-crypto.rst", "history": [ { "commit": "cefac3ea2ab56e4e7c6238f69f63b7f88d1b58ec", "id": "1", "repo": "git://github.com/hyakuhei/OSSG-Security-Practices" }, { "commit": "4d430b4cf18687b322c931f1bddf8147da853bb5", "id": "2", "repo": "github.com/openstack-security/Developer-Guidance" } ] }, { "author": "Eric Brown (VMware)", "file": "doc/source/guidelines/dg_use-oslo-rootwrap-securely.rst", "history": [ { "commit": "921f9fc2d949a6a48ba8c936a81b7f15024e50f8", "id": "1", "repo": "git://github.com/hyakuhei/OSSG-Security-Practices" }, { "commit": "3fc0ca5df47cd118d111e6020e1f1101b3c4fbc3", "id": "2", "repo": "github.com/openstack-security/Developer-Guidance" } ] }, { "author": "Paul McMillan (Nebula)", "file": "doc/source/guidelines/dg_use-subprocess-securely.rst", "history": [ { "commit": "7a305304d600b1e0d868e1fa174d31bdb460c041", "id": "1", "repo": "git://github.com/hyakuhei/OSSG-Security-Practices" }, { "commit": "f4ccd51a10ca0380737d6be8bc5ae6c677a60b39", "id": "2", "repo": "github.com/openstack-security/Developer-Guidance" } ] }, { "author": "Grant Murphy (HP)", "file": "doc/source/guidelines/dg_using-file-paths.rst", "history": [ { "commit": "f2d93147e7b8e30750b2f6ad548a1478f74e4489", "id": "1", "repo": "git://github.com/hyakuhei/OSSG-Security-Practices" }, { "commit": "d8593eaea31e9d4756005555dfb85f41013ee6d4", "id": "2", "repo": "github.com/openstack-security/Developer-Guidance" } ] }, { "author": "b005cc2968ecbcdbf7cc455ecac1e8ede2b2fa21", "file": "doc/source/guidelines/dg_using-temporary-files-securely.rst", "history": [ { "commit": "cf1981af0bba72558b8cfa349d841ceee4722b7f", "id": "1", "repo": "git://github.com/hyakuhei/OSSG-Security-Practices" }, { "commit": "", "id": "2", "repo": "github.com/openstack-security/Developer-Guidance" } ] } ] } Change-Id: I8ae30f526523860c0fc069cf1e683f1b8a69eb4d
111 lines
4.0 KiB
ReStructuredText
111 lines
4.0 KiB
ReStructuredText
.. :Copyright: 2015, OpenStack Foundation
|
|
.. :License: This work is licensed under a Creative Commons
|
|
Attribution 3.0 Unported License.
|
|
http://creativecommons.org/licenses/by/3.0/legalcode
|
|
|
|
|
|
Python Pipes to Avoid Shells
|
|
============================
|
|
|
|
You should take a look at the :doc:`shell injection document <dg_use-subprocess-securely>` before this one.
|
|
|
|
A lot of the time, our codebase uses ``shell=True`` because it's
|
|
convenient. The shell provides the ability to pipe things around
|
|
without buffering them in memory, and allows a malicious user to chain
|
|
additional commands after a legitimate command is run.
|
|
|
|
Incorrect
|
|
~~~~~~~~~
|
|
|
|
Here is a simple function that uses curl to grab a page from a website, and
|
|
pipe it directly to the ``wordcount`` program to tell us how many
|
|
lines there are in the HTML source code.
|
|
|
|
.. code:: python
|
|
|
|
def count_lines(website):
|
|
return subprocess.check_output('curl %s | wc -l' % website, shell=True)
|
|
|
|
#>>> count_lines('www.google.com')
|
|
#'7\n'
|
|
|
|
(That output is correct, by the way - the google html source does have
|
|
7 lines.)
|
|
|
|
The function is insecure because it uses ``shell=True``, which allows
|
|
:doc:`shell injection <dg_use-subprocess-securely>`. A user to who instructs your
|
|
code to fetch the website ``; rm -rf /`` can do terrible things to what
|
|
used to be your machine.
|
|
|
|
If we convert the function to use ``shell=False``, it doesn't work.
|
|
|
|
.. code:: python
|
|
|
|
def count_lines(website):
|
|
args = ['curl', website, '|', 'wc', '-l']
|
|
return subprocess.check_output(args, shell=False)
|
|
|
|
# >>> count_lines('www.google.com')
|
|
# curl: (6) Could not resolve host: |
|
|
# curl: (6) Could not resolve host: wc
|
|
# Traceback (most recent call last):
|
|
# File "<stdin>", line 3, in count_lines
|
|
# File "/usr/lib/python2.7/subprocess.py", line 573, in check_output
|
|
# raise CalledProcessError(retcode, cmd, output=output)
|
|
# subprocess.CalledProcessError: Command
|
|
# '['curl', 'www.google.com', '|', 'wc', '-l']' returned non-zero exit status 6
|
|
|
|
The pipe doesn't mean anything special when shell=False, and so curl
|
|
tries to download the website called '\|'. This does not fix the
|
|
issue, rather it causes it to be more broken than before.
|
|
|
|
If we can't rely on pipes if we have shell=False, how should we do this?
|
|
|
|
Correct
|
|
~~~~~~~
|
|
|
|
.. code:: python
|
|
|
|
def count_lines(website):
|
|
args = ['curl', website]
|
|
args2 = ['wc', '-l']
|
|
process_curl = subprocess.Popen(args, stdout=subprocess.PIPE,
|
|
shell=False)
|
|
process_wc = subprocess.Popen(args2, stdin=process_curl.stdout,
|
|
stdout=subprocess.PIPE, shell=False)
|
|
# Allow process_curl to receive a SIGPIPE if process_wc exits.
|
|
process_curl.stdout.close()
|
|
return process_wc.communicate()[0]
|
|
|
|
# >>> count_lines('www.google.com')
|
|
# '7\n'
|
|
|
|
Rather than calling a single shell process that runs each of our
|
|
programs, we run them separately and connect stdout from curl to stdin
|
|
for wc. We specify ``stdout=subprocess.PIPE``, which tells subprocess
|
|
to send that output to the respective file handler.
|
|
|
|
Treat pipes like file descriptors (you can actually use FDs if you want)
|
|
they may block on reading and writing if nothing is connected to the
|
|
other end. That's why we use ``communicate()``, which reads until EOF
|
|
on the output and then waits for the process to terminate. You should
|
|
generally avoid reading and writing to pipes directly unless you
|
|
really know what you're doing - it's easy to work yourself into a situation
|
|
that can deadlock.
|
|
|
|
Note that ``communicate()`` buffers the result in memory - if that's not
|
|
what you want, use a file descriptor for ``stdout`` to pipe that output
|
|
into a file.
|
|
|
|
Consequences
|
|
------------
|
|
|
|
- Using pipes helps you avoid shell injection in more complex
|
|
situations
|
|
- It's possible to deadlock things with pipes (in Python or in shell)
|
|
|
|
References
|
|
----------
|
|
|
|
- https://docs.python.org/2/library/subprocess.html#subprocess.Popen.stdin
|