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