From c7bf86aabc125e964c4d8aecf3b6018e29792417 Mon Sep 17 00:00:00 2001 From: Joseph Richey Date: Sun, 11 Feb 2018 23:11:11 -0800 Subject: docs: Update CONTRIBUTING.md and README.md PR #85 failed to update the documentation. This is now fixed with some additional cleanup. --- CONTRIBUTING.md | 143 +++++++++++++++++++++++++++++--------------------------- README.md | 49 ++++++++++--------- 2 files changed, 102 insertions(+), 90 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 20fb884..ff76d59 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -15,7 +15,7 @@ You generally only need to submit a CLA once, so if you've already submitted one (even if it was for a different project), you probably don't need to do it again. -## Reporting an Issue or Discussing Design +## Reporting an Issue, Discussing Design, or Asking a Question __IMPORTANT__: Any significant security issues should __NOT__ be reported in the public issue tracker. Practice responsible disclosure by emailing @@ -25,7 +25,7 @@ Any bugs, problems, or design discussion relating to fscrypt should be raised in the [Github Issue Tracker](https://github.com/google/fscrypt/issues/new). When reporting an issue or problem, be sure to give as much information as -possible. Also, make sure you are running the `fscrypt` and `pam_fscrypt` +possible. Also, make sure you are running the `fscrypt` and `pam_fscrypt.so` built from the current `master` branch. If reporting an issue around the fscrypt command-line tool, post the @@ -48,92 +48,97 @@ Be sure to correctly tag your issue. The usage for the tags is as follows: * This usally turns into a `documentation` issue. * `testing` - Strange test failures or missing tests -## Code reviews +## Submitting a Change to fscrypt All submissions, including submissions by project members, require review. We use GitHub pull requests for this purpose. Consult [GitHub Help](https://help.github.com/articles/about-pull-requests/) for more information on using pull requests. -## Working on fscrypt - On every pull request, [Travis CI](https://travis-ci.org/google/fscrypt) runs -unit tests, integration tests, code formatters, and linters. You can also run -these commands when writing your code. - -### Building and Testing - -As mentioned in `README.md`, running `make` will build the fscrypt executable -and the PAM module `pam_fscrypt.so`. Running `make go` will build each package -and run the tests, but just running `make go` with nothing else will skip the -integration tests. - -To run the integration tests, you will need a filesystem that supports -encryption. If you already have some empty filesystem at `/foo/bar`, just run: -```bash -make go MOUNT=/foo/bar +unit tests, integration tests, code formatters, and linters. To pass these +checks you should make sure that in your submission: +- `make` properly builds `fscrypt` and `pam_fscrypt.so`. +- All tests, including integration tests (see below), should pass. +- `make format` has been run. +- If you made any changes to files ending in `.proto`, the corresponding + `.pb.go` files should be regenerated with `make gen`. +- Any issues found by `make lint` have been addressed. +- If any dependancies have changed, `Gopkg.toml` should be updated and + [`dep ensure`](https://github.com/golang/dep) should be run. +- `make coverage.out` can be used to generate a coverage report for all of the + tests, but isn't required for submission + (ideally most code would be tested, we are far from that ideal). + +Essentially, if you run: ``` - -Otherwise, you can use the `make test-setup` and `make test-teardown` commands -to create a fake filesystem for testing. Note that the commands require `sudo`, -and the `make test-setup` command requires `e2fsprogs` version 1.43 or later. -For example: -```bash make test-setup -make go +make all make test-teardown +dep ensure +``` +and everything succeeds, and no files are changed, you're good to submit. + +The `Makefile` should automatically download and build whatever it needs. +The only exceptions to this rule are: + - `make format` requires + [`clang-format`](https://clang.llvm.org/docs/ClangFormat.html). + - `make test-setup` requires + [`e2fsprogs`](https://en.wikipedia.org/wiki/E2fsprogs) version 1.43 + or later (or any patched version that supports `-O encrypt`). + +### Running Integration Tests + +Running `make test` will build each package and run the unit tests, but will +skip the integration tests. To run the integration tests, you will need a +filesystem that supports encryption. If you already have some empty filesystem +at `/foo/bar`, just run: +```bash +make test MOUNT=/foo/bar ``` -### Formatting and Linting - -The `make format` command formats all the code in fscrypt with either -[`goimports`](https://godoc.org/golang.org/x/tools/cmd/goimports) (for Go code) -or [`clang-format`](https://clang.llvm.org/docs/ClangFormat.html) (for C code). -`goimports` can be installed with `go get`; `clang-format` can be installed -with your package manager. - -The `make lint` command runs a series of static analysis checks on your code. -This requires the -[megacheck](https://github.com/dominikh/go-tools/tree/master/cmd/megacheck) and -[golint](https://github.com/golang/lint) tools. - -### Changing proto files - -If you make any changes to files ending in `.proto`, the corresponding `.pb.go` -files have to be regenerated with `make gen`. This requires version 3.0.0 or -later of `protoc` the -[protobuf compiler](https://github.com/google/protobuf) and -[protoc-gen-go](https://github.com/golang/protobuf). +Otherwise, you can use the `make test-setup`/`make test-teardown` commands to +create/destory a test filesystem for running integration tests. Note that these +commands require `sudo` to mount/unmount the test filesystem. The fake +filesystem generated by these commands will automatically be detected by +`make test`, so running `make test-setup` then `make test` will run all the +integration tests. ### Changing dependencies -fscrypt vendors all of it's Go dependencies. If you add or remove a dependency -on an external Go package, be sure to run `make update` to resync the `vendor/` -directory. This requires [govendor](https://github.com/kardianos/govendor). +fscrypt vendors all of it's dependancies using `dep`. If you add or remove a +dependency, be sure to update `Gopkg.toml` and run `dep ensure`. -Also, if adding in an external Go package, be sure that he license of the -package is compatible with the -[Apache 2.0 License](https://www.apache.org/licenses/LICENSE-2.0). See the +Also, when adding a dependancy, the license of the package must be compatible +with [Apache 2.0](https://www.apache.org/licenses/LICENSE-2.0). See the [FSF's article](https://www.gnu.org/licenses/license-list.html) for more information. This (unfortunately) means we cannot use external packages under the [GPL](https://choosealicense.com/licenses/gpl-3.0) or [LGPL](https://choosealicense.com/licenses/lgpl-3.0/). We also cannot use -packages with missing or joke licenses (see [Unlicense](http://unlicense.org/), -[WTFPL](http://www.wtfpl.net/), or +packages with missing, misleading, or joke licenses (e.g. +[Unlicense](http://unlicense.org/), [WTFPL](http://www.wtfpl.net/), [CC0](https://creativecommons.org/publicdomain/zero/1.0/)). -### Putting it all together - -Run `make go-tools` to install all the Go tools mentioned above (make sure that -`$GOPATH/bin` is in you `$PATH`). Install `protoc` and `clang-format` with your -system's package manager. In the case of `protoc`, your system's version might -be older than v3.0.0. In that case, just get the build -[directly from GitHub](https://github.com/google/protobuf/releases/latest). - -After installing everything, running `make all` will run all the commands -mentioned above (except for `make gen` as different versions of protoc can -rearrange things differently). As with `make test`, you can run the integration -tests by either using `make all MOUNT=/path/to/my/filesystem` or using the -`make test-setup` and `make test-teardown` commands. - -`make all` should always be run before submitting a pull request. +### Build System Details ### + +Under the hood, the Makefile uses many go tools to generate, format, and lint +your code. + +`make gen`: + - Downloads [`protoc`](https://github.com/google/protobuf) to compile the + `.proto` files. + - Turns each `.proto` file into a matching `.pb.go` file using + [`protoc-gen-go`](https://github.com/golang/protobuf/tree/master/protoc-gen-go) + (built from source in `vendor/`). + +`make format` runs: + - [`goimports`](https://godoc.org/golang.org/x/tools/cmd/goimports) + (built from source in `vendor/`) on the `.go` files. + - [`clang-format`](https://clang.llvm.org/docs/ClangFormat.html) + on the `.c` an `.h` files. + +`make lint` runs: + - [`go vet`](https://golang.org/cmd/vet/) + - [`golint`](https://github.com/golang/lint) (built from source in `vendor/`) + - [`megacheck`](https://github.com/dominikh/go-tools/tree/master/cmd/megacheck) + (built from source in `vendor/`) diff --git a/README.md b/README.md index 4f76000..7d647d8 100644 --- a/README.md +++ b/README.md @@ -105,29 +105,36 @@ information about each of the commands. ## Building and Installing -fscrypt has the following build dependencies: +fscrypt has a minimal set of build dependencies: * [Go](https://golang.org/doc/install) * A C compiler (`gcc` or `clang`) * `make` -* Headers for `libpam`. Install them with the appropriate package manager. - - `sudo apt-get install libpam0g-dev` - - `sudo yum install pam-devel` - - `pam` package for Arch (part of the `base` group) +* Headers for [`libpam`](http://www.linux-pam.org/). + Install them with the appropriate package manager: + - Debain/Ubuntu: `sudo apt install libpam0g-dev` + - Red Hat: `sudo yum install pam-devel` + - Arch: [`pam`](https://www.archlinux.org/packages/core/x86_64/pam/) + package (usually installed by default) Once all the dependencies are installed, you can get the repository by running: ```shell go get -d github.com/google/fscrypt/... ``` -and then you can run `make` in `$GOPATH/src/github.com/google/fscrypt` to build the -executable and PAM module in that directory. Running `sudo make install` installs the binary to -`/usr/local/bin`. +Running `make` in `$GOPATH/src/github.com/google/fscrypt` builds the +executable (`fscrypt`) and PAM module (`pam-fscrypt.so`) in the `bin/` +directory. Use `make bin/fscrypt` or `make bin/pam_fscrypt.so` +to build only one. -See the `Makefile` for instructions on how to customize the build. This includes -building a static binary (C libraries used by fscrypt will be dynamically linked -by default). +Running `sudo make install` installs `fscrypt` to `/usr/local/bin` and +`pam-fscrypt` to `/lib/security`. Use `make install-bin` or `make install-pam` +to install only one. + +See the `Makefile` for instructions on how to customize the build (e.g. installing +to a custom location, using different build flags, building a static binary, +etc ...) Alternatively, if you only want to install the fscrypt binary to `$GOPATH/bin`, -it is enough to just run: +simply run: ```shell go get github.com/google/fscrypt/cmd/fscrypt ``` @@ -567,15 +574,15 @@ encryption has been explictly disabled in the kernel config. __IMPORTANT:__ Before enabling encryption on an ext4 filesystem __ALL__ of the following should be true: - * Your filesystem is formatted as ext4. Other filesystems will have - different ways of enabling encryption. - * Your kernel page size (run `getconf PAGE_SIZE`) and your filesystem - block size (run `tune2fs -l /dev/device | grep 'Block size'`) are the - same. - * You are ok with not being able to mount this filesystem with a v4.0 - kernel or older. - * You are __NOT__ using GRUB to boot directly off this filesystem. If - you have a sperate `/boot` partition, you are fine. + - Your filesystem is formatted as ext4. Other filesystems will have + different ways of enabling encryption. + - Your kernel page size (run `getconf PAGE_SIZE`) and your filesystem + block size (run `tune2fs -l /dev/device | grep 'Block size'`) are the same. + - You are ok with not being able to mount this filesystem with a v4.0 + kernel or older. + - You are __NOT__ using GRUB to boot directly off this filesystem. If + you have a sperate `/boot` partition, you are fine. + If any of the above is not true, __DO NOT ENABLE FILESYSTEM ENCRYPTION__. To turn on encryption for your filesystem, run -- cgit v1.2.3