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