Alex Gaynor | c72e63f | 2013-09-09 21:44:26 -0700 | [diff] [blame] | 1 | Contributing |
| 2 | ============ |
| 3 | |
| 4 | Process |
| 5 | ------- |
| 6 | |
| 7 | As an open source project, ``cryptography`` welcomes contributions of all |
| 8 | forms. These can include: |
| 9 | |
| 10 | * Bug reports and feature requests |
| 11 | * Pull requests for both code and documentation |
| 12 | * Patch reviews |
| 13 | |
Alex Gaynor | 2c67c82 | 2013-09-09 23:44:13 -0700 | [diff] [blame] | 14 | You can file bugs and submit pull requests on `GitHub`_. To discuss larger |
Alex Gaynor | c72e63f | 2013-09-09 21:44:26 -0700 | [diff] [blame] | 15 | changes you can start a conversation on `our mailing list`_. |
| 16 | |
| 17 | Because cryptography is so complex, and the implications of getting it wrong so |
| 18 | devastating, ``cryptography`` has a strict code review policy: |
| 19 | |
| 20 | * Patches must *never* be pushed directly to ``master``, all changes (even the |
| 21 | most trivial typo fixes!) must be submitted as a pull request. |
| 22 | * A committer may *never* merge their own pull request, a second party must |
Alex Gaynor | e6466a5 | 2013-10-18 14:53:04 -0700 | [diff] [blame] | 23 | merge their changes. If multiple people work on a pull request, it must be |
| 24 | merged by someone who did not work on it. |
Alex Gaynor | c72e63f | 2013-09-09 21:44:26 -0700 | [diff] [blame] | 25 | * A patch which breaks tests, or introduces regressions by changing or removing |
| 26 | existing tests should not be merged. Tests must always be passing on |
| 27 | ``master``. |
| 28 | * If somehow the tests get into a failing state on ``master`` (such as by a |
| 29 | backwards incompatible release of a dependency) no pull requests may be |
| 30 | merged until this is rectified. |
Alex Gaynor | f3f0018 | 2013-12-13 19:22:33 -0800 | [diff] [blame] | 31 | * All merged patches must have 100% test coverage. |
Alex Gaynor | c72e63f | 2013-09-09 21:44:26 -0700 | [diff] [blame] | 32 | |
| 33 | The purpose of these policies is to minimize the chances we merge a change |
| 34 | which jeopardizes our users' security. |
| 35 | |
Alex Gaynor | 99b69d9 | 2013-10-19 17:52:58 -0700 | [diff] [blame] | 36 | If you believe you've identified a security issue in ``cryptography``, please |
| 37 | follow the directions on the :doc:`security page </security>`. |
Alex Gaynor | c72e63f | 2013-09-09 21:44:26 -0700 | [diff] [blame] | 38 | |
| 39 | Code |
| 40 | ---- |
| 41 | |
| 42 | When in doubt, refer to `PEP 8`_ for Python code. |
| 43 | |
| 44 | Every code file must start with the boilerplate notice of the Apache License. |
| 45 | Additionally, every Python code file must contain |
| 46 | |
| 47 | .. code-block:: python |
| 48 | |
| 49 | from __future__ import absolute_import, division, print_function |
| 50 | |
Alex Gaynor | e21b0b2 | 2013-12-12 12:39:05 -0800 | [diff] [blame] | 51 | API Considerations |
| 52 | ~~~~~~~~~~~~~~~~~~ |
| 53 | |
| 54 | Most projects' APIs are designed with a philosophy of "make easy things easy, |
| 55 | and make hard things possible". One of the perils of writing cryptographic code |
Alex Gaynor | 95243f5 | 2013-12-21 19:37:24 -0800 | [diff] [blame] | 56 | is that secure code looks just like insecure code, and its results are almost |
Alex Gaynor | 15cf6b9 | 2013-12-21 19:22:39 -0800 | [diff] [blame] | 57 | always indistinguishable. As a result ``cryptography`` has, as a design |
| 58 | philosophy: "make it hard to do insecure things". Here are a few strategies for |
| 59 | API design which should be both followed, and should inspire other API choices: |
Alex Gaynor | e21b0b2 | 2013-12-12 12:39:05 -0800 | [diff] [blame] | 60 | |
| 61 | If it is incorrect to ignore the result of a method, it should raise an |
| 62 | exception, and not return a boolean ``True``/``False`` flag. For example, a |
| 63 | method to verify a signature should raise ``InvalidSignature``, and not return |
| 64 | whether the signature was valid. |
| 65 | |
| 66 | .. code-block:: python |
| 67 | |
| 68 | # This is bad. |
| 69 | def verify(sig): |
| 70 | # ... |
| 71 | return is_valid |
| 72 | |
| 73 | # Good! |
| 74 | def verify(sig): |
| 75 | # ... |
| 76 | if not is_valid: |
| 77 | raise InvalidSignature |
| 78 | |
Alex Gaynor | 6955ea3 | 2013-12-21 19:26:19 -0800 | [diff] [blame] | 79 | Every recipe should include a version or algorithmic marker of some sort in its |
| 80 | output in order to allow transparent upgrading of the algorithms in use, as |
| 81 | the algorithms or parameters needed to achieve a given security margin evolve. |
| 82 | |
Alex Gaynor | e21b0b2 | 2013-12-12 12:39:05 -0800 | [diff] [blame] | 83 | APIs at the :doc:`/hazmat/primitives/index` layer should always take an |
| 84 | explicit backend, APIs at the recipes layer should automatically use the |
Alex Gaynor | f8796b1 | 2013-12-13 20:28:55 -0800 | [diff] [blame] | 85 | :func:`~cryptography.hazmat.backends.default_backend`, but optionally allow |
Alex Gaynor | 2a5b4a8 | 2013-12-12 17:53:08 -0800 | [diff] [blame] | 86 | specifying a different backend. |
Alex Gaynor | e21b0b2 | 2013-12-12 12:39:05 -0800 | [diff] [blame] | 87 | |
Alex Gaynor | e6466a5 | 2013-10-18 14:53:04 -0700 | [diff] [blame] | 88 | C bindings |
Alex Gaynor | 5246e2d | 2013-12-12 12:23:18 -0800 | [diff] [blame] | 89 | ~~~~~~~~~~ |
Alex Gaynor | e6466a5 | 2013-10-18 14:53:04 -0700 | [diff] [blame] | 90 | |
| 91 | When binding C code with ``cffi`` we have our own style guide, it's pretty |
| 92 | simple. |
| 93 | |
| 94 | Don't name parameters: |
| 95 | |
| 96 | .. code-block:: c |
| 97 | |
| 98 | // Good |
| 99 | long f(long); |
| 100 | // Bad |
| 101 | long f(long x); |
| 102 | |
Paul Kehrer | 3ed80ba | 2013-10-19 20:00:26 -0500 | [diff] [blame] | 103 | ...unless they're inside a struct: |
| 104 | |
| 105 | .. code-block:: c |
| 106 | |
| 107 | struct my_struct { |
| 108 | char *name; |
| 109 | int number; |
| 110 | ...; |
| 111 | }; |
| 112 | |
Paul Kehrer | 047dab8 | 2013-12-27 16:45:52 -0600 | [diff] [blame^] | 113 | Include ``void`` if the function takes no arguments: |
Alex Gaynor | e6466a5 | 2013-10-18 14:53:04 -0700 | [diff] [blame] | 114 | |
| 115 | .. code-block:: c |
| 116 | |
| 117 | // Good |
Alex Gaynor | e6466a5 | 2013-10-18 14:53:04 -0700 | [diff] [blame] | 118 | long f(void); |
Paul Kehrer | 047dab8 | 2013-12-27 16:45:52 -0600 | [diff] [blame^] | 119 | // Bad |
| 120 | long f(); |
Alex Gaynor | e6466a5 | 2013-10-18 14:53:04 -0700 | [diff] [blame] | 121 | |
| 122 | Wrap lines at 80 characters like so: |
| 123 | |
| 124 | .. code-block:: c |
| 125 | |
| 126 | // Pretend this went to 80 characters |
| 127 | long f(long, long, |
| 128 | int *) |
| 129 | |
Alex Gaynor | 1e8744a | 2013-10-18 14:57:18 -0700 | [diff] [blame] | 130 | Include a space after commas between parameters: |
| 131 | |
| 132 | .. code-block:: c |
| 133 | |
| 134 | // Good |
| 135 | long f(int, char *) |
| 136 | // Bad |
| 137 | long f(int,char *) |
| 138 | |
Alex Gaynor | c72e63f | 2013-09-09 21:44:26 -0700 | [diff] [blame] | 139 | Documentation |
| 140 | ------------- |
| 141 | |
| 142 | All features should be documented with prose. |
| 143 | |
| 144 | Docstrings should be written like this: |
| 145 | |
| 146 | .. code-block:: python |
| 147 | |
| 148 | def some_function(some_arg): |
| 149 | """ |
| 150 | Does some things. |
| 151 | |
| 152 | :param some_arg: Some argument. |
| 153 | """ |
| 154 | |
| 155 | So, specifically: |
| 156 | |
Alex Gaynor | 05a190e | 2013-10-29 17:11:25 -0700 | [diff] [blame] | 157 | * Always use three double quotes. |
| 158 | * Put the three double quotes on their own line. |
| 159 | * No blank line at the end. |
| 160 | * Use Sphinx parameter/attribute documentation `syntax`_. |
| 161 | |
Alex Gaynor | a659688 | 2013-11-13 12:54:03 -0800 | [diff] [blame] | 162 | Because of the inherent challenges in implementing correct cryptographic |
Alex Gaynor | e9d64d7 | 2013-11-13 10:28:01 -0800 | [diff] [blame] | 163 | systems, we want to make our documentation point people in the right directions |
| 164 | as much as possible. To that end: |
| 165 | |
| 166 | * When documenting a generic interface, use a strong algorithm in examples. |
| 167 | (e.g. when showing a hashing example, don't use |
Alex Gaynor | 15cf6b9 | 2013-12-21 19:22:39 -0800 | [diff] [blame] | 168 | :class:`~cryptography.hazmat.primitives.hashes.MD5`) |
Alex Gaynor | 5cbab0c | 2013-11-13 11:55:57 -0800 | [diff] [blame] | 169 | * When giving prescriptive advice, always provide references and supporting |
Alex Gaynor | e9d64d7 | 2013-11-13 10:28:01 -0800 | [diff] [blame] | 170 | material. |
Alex Gaynor | d118c91 | 2013-11-13 11:56:49 -0800 | [diff] [blame] | 171 | * When there is real disagreement between cryptographic experts, represent both |
Alex Gaynor | 54e0400 | 2013-11-15 16:44:41 -0800 | [diff] [blame] | 172 | sides of the argument and describe the trade-offs clearly. |
Alex Gaynor | e9d64d7 | 2013-11-13 10:28:01 -0800 | [diff] [blame] | 173 | |
Alex Gaynor | 05a190e | 2013-10-29 17:11:25 -0700 | [diff] [blame] | 174 | When documenting a new module in the ``hazmat`` package, its documentation |
| 175 | should begin with the "Hazardous Materials" warning: |
| 176 | |
| 177 | .. code-block:: rest |
| 178 | |
| 179 | .. hazmat:: |
Alex Gaynor | c72e63f | 2013-09-09 21:44:26 -0700 | [diff] [blame] | 180 | |
Alex Gaynor | 953ebf8 | 2013-12-08 10:28:30 -0800 | [diff] [blame] | 181 | When referring to a hypothetical individual (such as "a person receiving an |
| 182 | encrypted message") use gender neutral pronouns (they/them/their). |
| 183 | |
Richard Wall | 40cde82 | 2013-10-01 20:20:15 +0100 | [diff] [blame] | 184 | Development Environment |
| 185 | ----------------------- |
Richard Wall | 0d9bb14 | 2013-10-01 16:17:24 +0100 | [diff] [blame] | 186 | |
| 187 | Working on ``cryptography`` requires the installation of a small number of |
Alex Gaynor | 166cbd3 | 2013-10-01 13:30:29 -0700 | [diff] [blame] | 188 | development dependencies. These are listed in ``dev-requirements.txt`` and they |
| 189 | can be installed in a `virtualenv`_ using `pip`_. Once you've installed the |
| 190 | dependencies, install ``cryptography`` in ``editable`` mode. For example: |
Richard Wall | 0d9bb14 | 2013-10-01 16:17:24 +0100 | [diff] [blame] | 191 | |
Alex Gaynor | ae5c907 | 2013-10-06 11:04:08 -0700 | [diff] [blame] | 192 | .. code-block:: console |
Richard Wall | 0d9bb14 | 2013-10-01 16:17:24 +0100 | [diff] [blame] | 193 | |
Alex Gaynor | 7587ded | 2013-10-06 12:14:05 -0700 | [diff] [blame] | 194 | $ # Create a virtualenv and activate it |
Richard Wall | 7d4ca1e | 2013-10-01 21:10:45 +0100 | [diff] [blame] | 195 | $ pip install --requirement dev-requirements.txt |
| 196 | $ pip install --editable . |
Richard Wall | 0d9bb14 | 2013-10-01 16:17:24 +0100 | [diff] [blame] | 197 | |
| 198 | You are now ready to run the tests and build the documentation. |
Richard Wall | 0d9bb14 | 2013-10-01 16:17:24 +0100 | [diff] [blame] | 199 | |
Richard Wall | 40cde82 | 2013-10-01 20:20:15 +0100 | [diff] [blame] | 200 | Running Tests |
Alex Gaynor | 5246e2d | 2013-12-12 12:23:18 -0800 | [diff] [blame] | 201 | ~~~~~~~~~~~~~ |
Richard Wall | 0d9bb14 | 2013-10-01 16:17:24 +0100 | [diff] [blame] | 202 | |
Alex Gaynor | 166cbd3 | 2013-10-01 13:30:29 -0700 | [diff] [blame] | 203 | ``cryptography`` unit tests are found in the ``tests/`` directory and are |
| 204 | designed to be run using `pytest`_. `pytest`_ will discover the tests |
| 205 | automatically, so all you have to do is: |
Richard Wall | 0d9bb14 | 2013-10-01 16:17:24 +0100 | [diff] [blame] | 206 | |
Alex Gaynor | ae5c907 | 2013-10-06 11:04:08 -0700 | [diff] [blame] | 207 | .. code-block:: console |
Richard Wall | 0d9bb14 | 2013-10-01 16:17:24 +0100 | [diff] [blame] | 208 | |
Richard Wall | 7d4ca1e | 2013-10-01 21:10:45 +0100 | [diff] [blame] | 209 | $ py.test |
Richard Wall | 0d9bb14 | 2013-10-01 16:17:24 +0100 | [diff] [blame] | 210 | ... |
Alex Gaynor | 15cf6b9 | 2013-12-21 19:22:39 -0800 | [diff] [blame] | 211 | 62746 passed in 220.43 seconds |
Richard Wall | 0d9bb14 | 2013-10-01 16:17:24 +0100 | [diff] [blame] | 212 | |
| 213 | This runs the tests with the default Python interpreter. |
| 214 | |
| 215 | You can also verify that the tests pass on other supported Python interpreters. |
Richard Wall | c3d1eb5 | 2013-10-01 16:29:07 +0100 | [diff] [blame] | 216 | For this we use `tox`_, which will automatically create a `virtualenv`_ for |
Richard Wall | 40cde82 | 2013-10-01 20:20:15 +0100 | [diff] [blame] | 217 | each supported Python version and run the tests. For example: |
Richard Wall | 0d9bb14 | 2013-10-01 16:17:24 +0100 | [diff] [blame] | 218 | |
Alex Gaynor | ae5c907 | 2013-10-06 11:04:08 -0700 | [diff] [blame] | 219 | .. code-block:: console |
Richard Wall | 0d9bb14 | 2013-10-01 16:17:24 +0100 | [diff] [blame] | 220 | |
Richard Wall | 7d4ca1e | 2013-10-01 21:10:45 +0100 | [diff] [blame] | 221 | $ tox |
Richard Wall | 0d9bb14 | 2013-10-01 16:17:24 +0100 | [diff] [blame] | 222 | ... |
Richard Wall | 40cde82 | 2013-10-01 20:20:15 +0100 | [diff] [blame] | 223 | ERROR: py26: InterpreterNotFound: python2.6 |
| 224 | py27: commands succeeded |
| 225 | ERROR: pypy: InterpreterNotFound: pypy |
| 226 | ERROR: py32: InterpreterNotFound: python3.2 |
| 227 | py33: commands succeeded |
| 228 | docs: commands succeeded |
| 229 | pep8: commands succeeded |
Richard Wall | 0d9bb14 | 2013-10-01 16:17:24 +0100 | [diff] [blame] | 230 | |
Alex Gaynor | 166cbd3 | 2013-10-01 13:30:29 -0700 | [diff] [blame] | 231 | You may not have all the required Python versions installed, in which case you |
| 232 | will see one or more ``InterpreterNotFound`` errors. |
Richard Wall | 0d9bb14 | 2013-10-01 16:17:24 +0100 | [diff] [blame] | 233 | |
| 234 | Building Documentation |
Alex Gaynor | 5246e2d | 2013-12-12 12:23:18 -0800 | [diff] [blame] | 235 | ~~~~~~~~~~~~~~~~~~~~~~ |
Richard Wall | 0d9bb14 | 2013-10-01 16:17:24 +0100 | [diff] [blame] | 236 | |
Alex Gaynor | 166cbd3 | 2013-10-01 13:30:29 -0700 | [diff] [blame] | 237 | ``cryptography`` documentation is stored in the ``docs/`` directory. It is |
| 238 | written in `reStructured Text`_ and rendered using `Sphinx`_. |
Richard Wall | 0d9bb14 | 2013-10-01 16:17:24 +0100 | [diff] [blame] | 239 | |
Richard Wall | 7d4ca1e | 2013-10-01 21:10:45 +0100 | [diff] [blame] | 240 | Use `tox`_ to build the documentation. For example: |
Richard Wall | 0d9bb14 | 2013-10-01 16:17:24 +0100 | [diff] [blame] | 241 | |
Alex Gaynor | ae5c907 | 2013-10-06 11:04:08 -0700 | [diff] [blame] | 242 | .. code-block:: console |
Richard Wall | 0d9bb14 | 2013-10-01 16:17:24 +0100 | [diff] [blame] | 243 | |
Richard Wall | 7d4ca1e | 2013-10-01 21:10:45 +0100 | [diff] [blame] | 244 | $ tox -e docs |
Richard Wall | 0d9bb14 | 2013-10-01 16:17:24 +0100 | [diff] [blame] | 245 | ... |
Richard Wall | c3d1eb5 | 2013-10-01 16:29:07 +0100 | [diff] [blame] | 246 | docs: commands succeeded |
Richard Wall | 0d9bb14 | 2013-10-01 16:17:24 +0100 | [diff] [blame] | 247 | congratulations :) |
| 248 | |
Alex Gaynor | 15cf6b9 | 2013-12-21 19:22:39 -0800 | [diff] [blame] | 249 | The HTML documentation index can now be found at |
| 250 | ``docs/_build/html/index.html``. |
Richard Wall | 40cde82 | 2013-10-01 20:20:15 +0100 | [diff] [blame] | 251 | |
Alex Gaynor | c72e63f | 2013-09-09 21:44:26 -0700 | [diff] [blame] | 252 | |
Donald Stufft | 8570794 | 2013-10-04 23:55:27 -0400 | [diff] [blame] | 253 | .. _`GitHub`: https://github.com/pyca/cryptography |
Alex Gaynor | c72e63f | 2013-09-09 21:44:26 -0700 | [diff] [blame] | 254 | .. _`our mailing list`: https://mail.python.org/mailman/listinfo/cryptography-dev |
| 255 | .. _`PEP 8`: http://www.peps.io/8/ |
| 256 | .. _`syntax`: http://sphinx-doc.org/domains.html#info-field-lists |
Richard Wall | c3d1eb5 | 2013-10-01 16:29:07 +0100 | [diff] [blame] | 257 | .. _`pytest`: https://pypi.python.org/pypi/pytest |
| 258 | .. _`tox`: https://pypi.python.org/pypi/tox |
| 259 | .. _`virtualenv`: https://pypi.python.org/pypi/virtualenv |
| 260 | .. _`pip`: https://pypi.python.org/pypi/pip |
| 261 | .. _`sphinx`: https://pypi.python.org/pypi/sphinx |
Alex Gaynor | a05358d | 2013-11-06 11:01:22 -0800 | [diff] [blame] | 262 | .. _`reStructured Text`: http://sphinx-doc.org/rest.html |