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