diff options
Diffstat (limited to '')
| -rw-r--r-- | docs/develop.adoc | 435 | ||||
| -rw-r--r-- | docs/develop/cpp-usage.adoc | 49 | ||||
| -rw-r--r-- | docs/develop/packaging.adoc | 78 | ||||
| -rw-r--r-- | docs/develop/release-workflow.adoc | 122 |
4 files changed, 684 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 diff --git a/docs/develop/cpp-usage.adoc b/docs/develop/cpp-usage.adoc new file mode 100644 index 0000000..e0c1842 --- /dev/null +++ b/docs/develop/cpp-usage.adoc @@ -0,0 +1,49 @@ += Usage of {cpp} within RNP + +This is a provisional document reflecting the recent conversion from C +to {cpp}. It should be revisited as experience with using {cpp} within RNP +codebase increases. + +== Encouraged Features + +These are features which seem broadly useful, their downsides are minimal +and well understood. + + - STL types std::vector, std::string, std::unique_ptr, std::map + + - RAII techniques (destructors, smart pointers) to minimize use of + goto to handle cleanup. + + - Value types, that is to say types which simply encapsulate some + data. + + - std::function or virtual functions to replace function pointers. + + - Prefer virtual functions only on "interface" classes (with no data), + and derive only one level of classes from this interface class. + + - Anonymous namespaces are an alternative to `static` functions. + +== Questionable Features + +These are features that may be useful in certain situations, but should +be used carefully. + + - Exceptions. While convenient, they do have a non-zero cost in runtime + and binary size. + +== Forbidden Features + +These are {cpp} features that simply should be avoided, at least until a +very clear use case for them has been identified and no other approach +suffices. + + - RTTI. This has a significant runtime cost and usually there are + better alternatives. + + - Multiple inheritance. This leads to many confusing and problematic + scenarios. + + - Template metaprogramming. If you have a problem, and you think + template metaprogramming will solve it, now you have two problems, + and one of them is incomprehensible. diff --git a/docs/develop/packaging.adoc b/docs/develop/packaging.adoc new file mode 100644 index 0000000..917c9aa --- /dev/null +++ b/docs/develop/packaging.adoc @@ -0,0 +1,78 @@ += Packaging + +== CentOS 7 + +=== 1. Retrieve the source + +==== Tarball + +[source,console] +-- +curl -LO https://github.com/rnpgp/rnp/archive/v0.9.0.tar.gz +tar xzf v0.9.0.tar.gz +cd rnp-0.9.0 +-- + +==== Git + +[source,console] +-- +git clone https://github.com/rnpgp/rnp +cd rnp +git checkout v0.9.0 +-- + +=== 2. Launch a container + +[source,console] +-- +docker run -ti --rm -v $PWD:/usr/local/rnp centos:7 bash +-- + +From this point, all commands are executed in the container. + +==== 3. Install pre-requisites + +[source,console] +-- +# for newer cmake and other things +yum -y install epel-release + +# rnp +yum -y install git cmake3 make gcc-c++ +yum -y install bzip2-devel zlib-devel json-c12-devel + +# botan +rpm --import https://github.com/riboseinc/yum/raw/master/ribose-packages.pub +rpm --import https://github.com/riboseinc/yum/raw/master/ribose-packages-next.pub +curl -L https://github.com/riboseinc/yum/raw/master/ribose.repo > /etc/yum.repos.d/ribose.repo +yum -y install botan2-devel +-- + +=== 4. Build the RPM + +[source,console] +-- +yum -y install rpm-build +mkdir ~/build +cd ~/build +cmake3 -DBUILD_SHARED_LIBS=on -DBUILD_TESTING=off -DCPACK_GENERATOR=RPM /usr/local/rnp +make package +-- + +=== 5. Check and Install the RPM + +It may be helpful to run `rpmlint` on the RPM and note new warnings or errors. + +[source,console] +-- +yum -y install rpmlint +rpmlint rnp-0.9.0-1.el7.centos.x86_64.rpm +-- + +At this point, you can test that the RPM installs successfully: + +[source,console] +-- +yum localinstall rnp-0.9.0-1.el7.centos.x86_64.rpm +-- diff --git a/docs/develop/release-workflow.adoc b/docs/develop/release-workflow.adoc new file mode 100644 index 0000000..40f4203 --- /dev/null +++ b/docs/develop/release-workflow.adoc @@ -0,0 +1,122 @@ += Releases + +== General notes + +* Avoid tagging commits in the `main` branch. +* Release branches should have annotated tags and a CHANGELOG.md. +* The steps below detail creation of a brand new 1.0.0 release. + Some steps would be omitted for minor releases. + +== Creating an initial release + +=== Update documentation + +Update references to version numbers in relevant documentation to the new +version you intend to release. + +[source,console] +---- +git checkout main +vim docs/installation.adoc +git add docs/installation.adoc +git commit +git push +---- + +=== Create branch + +Release branches have names of the form `release/N.x`, where N is the major +version (and `x` is a literal -- not a placeholder). + +[source,console] +---- +git checkout -b release/1.x main +---- + +[[update-changelog-and-version]] +=== Update CHANGELOG and version + +[source,console] +---- +vim CHANGELOG.md +# Add/update CHANGELOG entry for the new version +git add CHANGELOG.md + +echo 1.0.0 > version.txt +git add -f version.txt + +git commit +---- + +=== Create tag + +An initial release would be tagged as follows: + +[source,console] +---- +git tag -a v1.0.0 -m '' +---- + +=== Push branch and tag + +[source,console] +---- +# push the branch +git push origin release/1.x + +# push the tag +git push origin v1.0.0 +---- + +=== Edit tagged release description on GitHub + +. Navigate to the link:#https://github.com/rnpgp/rnp/releases[Releases] page; + +. Edit the tag that was just pushed; + +. Fill the tag's description with data from the corresponding `CHANGELOG` + entries of the same tag version; + +. Publish the release. + + +== Creating a new release + +Maintaining a release branch involves cherry-picking hotfixes and +similar commits from the `main` branch, while following the rules for +Semantic Versioning. + +The steps below will show the release of version 1.0.1. + +=== Add desired changes + +Cherry-pick the appropriate commits into the appropriate `release/N.x` branch. + +To see what commits are in `main` that are not in the release branch, you +can observe the lines starting with `+` in: + +[source,console] +---- +git cherry -v release/1.x main +---- + +It is often useful to pick a range of commits. For example: + +[source,console] +---- +git checkout release/0.x +git cherry-pick a57b36f^..e23352c +---- + +If there are merge commits in this range, this will not work. +Instead, try: + +[source,console] +---- +git checkout release/0.x +git cherry release/0.x main | grep '^+ ' | cut -c 3-9 | \ + while read commit; do git cherry-pick $commit; done +---- + +From here, you can follow the steps for an initial release, +starting with <<update-changelog-and-version>>. |