summaryrefslogtreecommitdiffstats
path: root/docs/develop.adoc
diff options
context:
space:
mode:
authorDaniel Baumann <daniel.baumann@progress-linux.org>2024-04-19 03:32:49 +0000
committerDaniel Baumann <daniel.baumann@progress-linux.org>2024-04-19 03:32:49 +0000
commit8053187731ae8e3eb368d8360989cf5fd6eed9f7 (patch)
tree32bada84ff5d7460cdf3934fcbdbe770d6afe4cd /docs/develop.adoc
parentInitial commit. (diff)
downloadrnp-8053187731ae8e3eb368d8360989cf5fd6eed9f7.tar.xz
rnp-8053187731ae8e3eb368d8360989cf5fd6eed9f7.zip
Adding upstream version 0.17.0.upstream/0.17.0
Signed-off-by: Daniel Baumann <daniel.baumann@progress-linux.org>
Diffstat (limited to 'docs/develop.adoc')
-rw-r--r--docs/develop.adoc435
1 files changed, 435 insertions, 0 deletions
diff --git a/docs/develop.adoc b/docs/develop.adoc
new file mode 100644
index 0000000..e0c0e41
--- /dev/null
+++ b/docs/develop.adoc
@@ -0,0 +1,435 @@
+= RNP development guide
+
+The following are a set of conventions and items that are relevant to
+contributors.
+
+== Contributing
+
+=== Pull Requests
+
+See also: https://github.com/thoughtbot/guides/tree/master/code-review[Thoughtbot’s Code Review guide]
+
+Pull Requests should be used for any non-trivial changes. This presents
+an opportunity for feedback and allows the CI tests to complete prior to
+merging.
+
+The `master` branch should generally always be in a buildable and
+functional state.
+
+Pull Requests should be:
+
+* Focused. Do not include changes that are unrelated to the main purpose
+ of the PR.
+* As small as possible. Sometimes large pull requests may be necessary
+ for adding complex features, but generally they should be kept as small
+ as possible to ensure a quick and thorough review process.
+* Related to a GH issue to which you are assigned. If there is none,
+ file one (but search first!). This ensures there is no duplication of
+ effort and allows for a discussion prior to beginning work.
+ (This may not be necessary for PRs that are purely documentation updates)
+* Approved by **2** reviewers before merging.
+ (Updates related to policies, like this section, should be approved by
+ the project owner)
+* Merged by a reviewer via the most appropriate method
+ (see https://github.com/rnpgp/guides/tree/master/protocol/git[here]).
+
+=== Branches
+
+See also: https://github.com/rnpgp/guides/tree/master/protocol/git[Guides - Protocol / Git]
+
+Git branches should be used generously. Most branches should be topic branches,
+created for adding a specific feature or fixing a specific bug.
+
+Keep branches short-lived (treat them as disposable/transient) and try to
+avoid long-running branches.
+
+A good example of using a branch would be:
+
+* User `@joe` notices a bug where a NULL pointer is dereferenced during
+ key export. He creates GH issue `#500`.
+* He creates a new branch to fix this bug named
+ `joe-500-fix-null-deref-in-pgp_export_key`.
+* Joe commits a fix for the issue to this new branch.
+* Joe creates a Pull Request to merge this branch in to main.
+* Once merged, Joe deletes the branch since it is no longer useful.
+
+Branch names may vary but should be somewhat descriptive, with words
+separated by hyphens. It is also helpful to start the branch name with
+your GitHub username, to make it clear who created the branch and
+prevent naming conflicts.
+
+Remember that branch names may be preserved permanently in the commit
+history of `main`, depending on how they are merged.
+
+=== Commits
+
+* Try to keep commits as small as possible. This may be difficult or
+ impractical at times, so use your best judgement.
+* Each commit should be buildable and should pass all tests. This helps
+ to ensure that git bisect remains a useful method of pinpointing issues.
+* Commit messages should follow 50/72 rule.
+* When integrating pull requests, merge function should be preferred over
+ squashing. From the other hand, developers should squash commits and
+ create meaningful commit stack before PR is merged into mainstream branch.
+ Merging commits like "Fix build" or "Implement comments from code review"
+ should be avoided.
+
+== Continuous Integration (Github Actions)
+
+Github actions are used for continuously testing new commits and pull requests.
+Those include testing for different operating systems, linting via clang-format and shellcheck,
+and code coverage and quality checks via `Codecov` and `LGTM.io`.
+
+For Github workflows sources see `.github/workflows/` folder and scripts from the `ci/` folder.
+Also there is a Cirrus CI runner, configuration for which is stored in `.cirrus.yml`.
+
+=== Reproducing Locally
+
+If tests fail in CI, you may attempt to reproduce those locally via `ctest` command:
+
+[source,console]
+--
+ctest -j4 -V -R rnp_tests
+--
+
+Or, more specific:
+
+[source,console]
+--
+ctest -V -R cli_tests-Misc
+--
+
+If test fails under the specific OS, you should construct corresponding Docker container and run tests inside, taking Github workflows as a guide.
+
+== Code Coverage
+
+CodeCov is used for assessing our test coverage.
+The current coverage can always be viewed here: https://codecov.io/github/rnpgp/rnp/
+
+== Security / Bug Hunting
+
+=== Static Analysis
+
+==== Coverity Scan
+
+Coverity Scan is used for static analysis of the code base.
+It is run daily on the main branch via the Github actions.
+See `.github/workflows/coverity.yml` for the details.
+
+The results can be accessed on https://scan.coverity.com/projects/rnpgp-rnp.
+You will need to create an account and request access to the rnpgp/rnp project.
+
+Since the scan results are not updated live, line numbers may no longer
+be accurate against the `main` branch, issues may already be resolved,
+etc.
+
+==== Clang Static Analyzer
+
+Clang includes a useful static analyzer that can also be used to locate
+potential bugs.
+
+Note: It is normal for the build time to increase significantly when using this static analyzer.
+
+[source,console]
+--
+# it's important to start fresh for this!
+rm -rf build && mkdir build && cd build
+scan-build cmake .. && scan-build make -j8
+[...]
+scan-build: 61 bugs found.
+scan-build: Run 'scan-view /tmp/scan-build-2018-09-17-085354-22998-1' to examine bug reports.
+--
+
+Then use `scan-view`, as indicated above, to start a web server and use
+your web browser to view the results.
+
+=== Dynamic Analysis
+
+==== Fuzzing
+
+It is often useful to utilize a fuzzer like
+http://lcamtuf.coredump.cx/afl/["american fuzzy lop" ("AFL")] or
+https://llvm.org/docs/LibFuzzer.html["libfuzzer"] to find
+ways to improve the robustness of the code base.
+
+Presently, rnp builds in
+https://github.com/google/oss-fuzz/tree/master/projects/rnp["OSS-Fuzz"]
+and certain fuzzers are enabled there.
+
+In the `src/fuzzing` directory, we have the fuzzers that run in OSS-Fuzz.
+Setting `-DENABLE_SANITIZERS=1 -DENABLE_FUZZERS=1` will build these fuzzers
+with the libfuzzer engine; and running the resulting executables will perform
+the fuzzing.
+
+To build and run fuzzers locally, or reproduce an issue, see https://google.github.io/oss-fuzz/advanced-topics/reproducing/
+
+===== Further Reading
+
+* AFL's `README`, `parallel_fuzzing.txt`, and other bundled documentation.
+* See https://fuzzing-project.org/tutorial3.html[Tutorial: Instrumented fuzzing with american fuzzy lop]
+
+==== Clang Sanitizer
+
+Clang and GCC both support a number of sanitizers that can help locate
+issues in the code base during runtime.
+
+To use them, you should rebuild with the sanitizers enabled, and then
+run the tests (or any executable):
+
+[source,console]
+--
+env CXX=clang++ CXXFLAGS="-fsanitize=address,undefined" LDFLAGS="-fsanitize=address,undefined" ./configure
+make -j4
+src/tests/rnp_tests
+--
+
+Here we are using the
+https://clang.llvm.org/docs/AddressSanitizer.html[AddressSanitizer]
+and
+https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html[UndefinedBehaviorSanitizer].
+
+This will produce output showing any memory leaks, heap overflows, or
+other issues.
+
+== Code Conventions
+
+C is a very flexible and powerful language. Because of this, it is
+important to establish a set of conventions to avoid common problems and
+to maintain a consistent code base.
+
+=== Code Formatting
+
+`clang-format` (v9.0.0) can be used to format the code base, utilizing
+the `.clang-format` file included in the repository.
+
+==== clang-format git hook
+
+A git pre-commit hook exists to perform this task automatically, and can
+be enabled like so:
+
+[source,console]
+--
+cd rnp
+git-hooks/enable.sh
+--
+
+If you do not have clang-format v9.0.0 available, you can use a docker
+container for this purpose by setting `USE_DOCKER="yes"` in
+`git-hooks/pre-commit.sh`.
+
+This should generally work if you commit from the command line.
+
+Note that if you have unstaged changes on some of the files you are
+attempting to commit, which have formatting issues detected, you will
+have to resolve this yourself (the script will inform you of this).
+
+If your commit does not touch any `.c`/`.h` files, you can skip the
+pre-commit hook with git's `--no-verify`/`-n` option.
+
+==== clang-format (manually)
+
+If you are not able to use the git hook, you can run `clang-format`
+manually in a docker container.
+
+Create a suitable container image with:
+
+[source,console]
+--
+docker run --name=clang-format alpine:latest apk --no-cache add clang
+docker commit clang-format clang-format
+docker rm clang-format
+--
+
+You can then reformat a file (say, `src/lib/crypto/bn.cpp`) like so:
+
+[source,console]
+--
+cd rnp
+docker run --rm -v $PWD:/rnp -w /rnp clang-format clang-format -style=file -i src/lib/crypto/bn.cpp
+--
+
+Also you may wish to reformat all modified uncommitted files:
+
+[source,console]
+--
+docker run --rm -v $PWD:/rnp -w /rnp clang-format clang-format -style=file -i `git ls-files -m |grep "\.\(c\|h\|cpp\)\$"`
+--
+
+...or files, modified since referenced commit, say `54c5476`:
+
+[source,console]
+--
+docker run --rm -v $PWD:/rnp -w /rnp clang-format clang-format -style=file -i `git diff --name-only 54c5476..HEAD |grep "\.\(c\|h\|cpp\)\$"`
+--
+
+=== Style Guide
+
+In order to keep the code base consistent, we should define and adhere
+to a single style.
+
+When in doubt, consult the existing code base.
+
+==== Naming
+
+The following are samples that demonstrate the style for naming
+different things.
+
+* Functions: `some_function`
+* Variables: `some_variable`
+* Filenames: `packet-parse.c` `packet-parse.h`
+* Struct: `pgp_key_t`
+* Typedefed Enums: `pgp_pubkey_alg_t`
+* Enum Values: `PGP_PKA_RSA = 1`
+* Constants (macro): `RNP_BUFSIZ`
+
+==== General Guidelines
+
+Do:
+
+* Do use header guards (`#ifndef SOME_HEADER_H [...]`) in headers.
+* Do use `sizeof(variable)`, rather than `sizeof(type)`. Or
+ `sizeof(*variable)` as appropriate.
+* Do use commit messages that close GitHub issues automatically, when
+ applicable. `Fix XYZ. Closes #78.` See
+ https://help.github.com/articles/closing-issues-via-commit-messages/[here].
+* Do declare functions `static` when they do not need to be referenced
+ outside the current source file.
+* Do always use braces for conditionals, even if the block only contains a
+ single statement.
++
+[source,c]
+--
+if (something) {
+ return val;
+}
+--
+
+* Do use a default failure (not success) value for `ret` variables. Example:
++
+[source,c]
+--
+rnp_result_t ret = RNP_ERROR_GENERIC;
+// ...
+
+return ret;
+--
+
+Do not:
+
+* Do not use the static storage class for local variables, *unless* they
+ are constant.
++
+**Not OK**
++
+[source,c]
+--
+int somefunc() {
+ static char buffer[256];
+ //...
+}
+--
++
+**OK**
++
+[source,c]
+--
+int somefunc() {
+ static const uint16_t some_data[] = {
+ 0x00, 0x01, 0x02, //...
+ };
+}
+--
+
+* Do not use `pragma`, and try to avoid `__attribute__` as well.
+
+* Do not use uninitialized memory. Try to ensure your code will not cause any errors in valgrind and other memory checkers.
+
+==== Documentation
+
+Documentation is done in Doxygen comments format, which must be put in header files.
+
+Exception are static or having only definition functions - it is not required to document them,
+however if they are documented then this should be done in the source file and using the @private tag.
+
+Comments should use doxygen markdown style, like the following example:
+
+[source,c]
+--
+/** Some comments regarding the file purpose, like 'PGP packet parsing utilities'
+ * @file
+ */
+
+/** brief description of the sample function which does something, keyword 'brief' is omitted
+ * Which may be continued here
+ *
+ * After an empty line you may add detailed description in case it is needed. You may put
+ * details about the memory allocation, what happens if function fails and so on.
+ *
+ * @param param1 first parameter, null-terminated string which should not be NULL
+ * @param param2 integer, some number representing something
+ * @param size number of bytes available to store in buffer
+ * @param buffer buffer to store results, may be NULL. In this case size can be used to
+ * obtain the required buffer length
+ * @return 0 if operation succeeds, or error code otherwise. If operation succeeds then buffer
+ * is populated with the resulting data, and size contains the length of this data.
+ * if error code is E_BUF_TOOSMALL then size will contain the required size to store
+ * the result
+ **/
+rnp_result_t
+rnp_do_operation(const char *param1, const int param2, int *size, char *buffer);
+--
+
+== OpenPGP protocol specification
+
+During development you'll need to reference OpenPGP protocol and related documents.
+Here is the list of RFCs and Internet Drafts available at the moment:
+
+* https://www.ietf.org/rfc/rfc1991.txt[RFC 1991]: PGP Message Exchange Formats. Now obsolete, but may have some historical interest.
+* https://www.ietf.org/rfc/rfc2440.txt[RFC 2440]: OpenPGP Message Format. Superseded by RFC 4880.
+* https://www.ietf.org/rfc/rfc4880.txt[RFC 4880]: OpenPGP Message Format. Latest RFC available at the moment, however has a lot of suggested changes via RFC 4880bis
+* https://tools.ietf.org/rfc/rfc5581.txt[RFC 5581]: The Camellia cipher in OpenPGP.
+* https://www.ietf.org/id/draft-ietf-openpgp-rfc4880bis-09.txt[RFC 4880bis-09]: OpenPGP Message Format. Latest suggested update to the RFC 4880.
+
+More information sources:
+
+* https://mailarchive.ietf.org/arch/browse/openpgp/[OpenPGP Working Group mailing list]. Here you can pick up all the latest discussions and suggestions regarding the update of RFC 4880
+* https://gitlab.com/openpgp-wg/rfc4880bis[OpenPGP Working Group gitlab]. Latest work on RFC update is available here.
+
+== Reviewers and Responsibility areas
+
+The individuals are responsible for the following areas of `rnp`.
+When submitting a Pull Request please seek reviews by whoever is
+responsible according to this list.
+
+General:
+
+* Code style: @dewyatt
+* Algorithms: @randombit, @dewyatt, @flowher, @catap, @ni4
+* Performance: @catap, @ni4
+* CLI: @ni4
+* GnuPG compatibility: @MohitKumarAgniotri, @frank-trampe, @ni4
+* Security Testing/Analysis: @MohitKumarAgniotri, @flowher
+* Autotools: @randombit, @zgyarmati, @catap
+
+Data formats:
+
+* OpenPGP Packet: @randombit, @catap, @ni4
+* Keystore: @catap
+* JSON: @zgyarmati
+* SSH: @ni4
+
+Bindings:
+
+* FFI: @dewyatt
+* Ruby: @dewyatt
+* Java/JNI: @catap
+* Obj-C/Swift: @ni4
+* Python: @dewyatt, @ni4
+
+Platforms:
+
+* RHEL/CentOS: @dewyatt
+* BSD:
+* Windows: @rrrooommmaaa
+* macOS / iOS / Homebrew: @ni4
+* Debian: @zgyarmati