Merge pull request #49 from google/misc

Fix a wide variety of small issues and update documentation

9 files changed, 65 insertions, 14 deletions

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index c74f0be..e7a2a75 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -21,7 +21,7 @@ __IMPORTANT__: Any significant security issues should __NOT__ be reported in
 the public issue tracker. Practice responsible disclosure by emailing
 <joerichey@google.com> and <tyhicks@canonical.com> directly.
 
-Any bugs, problems, or design discussion relating to fscrypt should be rasied
+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
diff --git a/Makefile b/Makefile
index 328f19b..1ec009e 100644
--- a/Makefile
+++ b/Makefile
@@ -142,7 +142,7 @@ format-check:
 # Run lint rules (skipping generated files)
 .PHONY: lint
 lint:
-	@go vet $(GO_PKGS)
+	@go tool vet -buildtags=false .
 	@golint $(GO_PKGS) | grep -v "pb.go" | ./input_fail.py
 	@megacheck -unused.exported $(GO_PKGS)
 
diff --git a/README.md b/README.md
index c1d1734..59eac5d 100644
--- a/README.md
+++ b/README.md
@@ -187,7 +187,7 @@ to add the line:
 ```
 auth        optional    pam_fscrypt.so
 ```
-after `pam_unix.so` in `/etc/pam.d/common-password` or similar, and to add the
+after `pam_unix.so` in `/etc/pam.d/common-auth` or similar, and to add the
 line:
 ```
 session     optional    pam_fscrypt.so drop_caches lock_policies
@@ -577,19 +577,40 @@ To trigger a password authentication event, run `su $(whoami) -c exit`.
 #### Getting "encryption not enabled" on an ext4 filesystem.
 
 Getting this error on an ext4 system usually means the filesystem has not been
-setup for encryption. To setup a filesystem to support encryption, first check
-that your block size is equal to your page size by comparing the outputs of
-`getconf PAGE_SIZE` and `tune2fs -l /dev/device | grep 'Block size'`. If these
-are not the same, DO NOT ENABLE ENCRYPTION.
+setup for encryption. The only other way to get this error is if filesystem
+encryption has been explictly disabled in the kernel config.
 
-To turn on the encryption feature flag for your filesystem, run
+__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.
+If any of the above is not true, __DO NOT ENABLE FILESYSTEM ENCRYPTION__.
+
+To turn on encryption for your filesystem, run
 ```
 tune2fs -O encrypt /dev/device
 ```
-This command requires root privileges and `e2fsprogs` v1.43 or later. Once the
-filesystem flag is enabled, older kernels may not be able to mount this
-filesystem. Note that there was a bug in older kernel versions that allowed
-encryption policies to be set on ext4 filesystems without this flag.
+To turn off encryption for your filesystem, run
+```
+fsck -fn /dev/device
+debugfs -w -R "feature -encrypt" /dev/device
+fsck -fn /dev/device
+``` 
+
+Note: It is actually possible to get GRUB to boot an encrypted ext4 filesystem.
+However, it requires GRUB 2.02 (__NOT__ the 2.02 beta) to be installed as the
+bootloader. As this version was released in April 2017, most systems __WILL
+FAIL TO BOOT__ with an ext4 encrypted boot directory. Note that this is only
+relevant to systems without a seperate boot partition. Sytems with `/boot` on
+a different partition than the one being encrypted (including all UEFI systems)
+are not effected by this.
 
 ## Legal
 
diff --git a/cmd/fscrypt/errors.go b/cmd/fscrypt/errors.go
index 10dbf1e..b2aa57e 100644
--- a/cmd/fscrypt/errors.go
+++ b/cmd/fscrypt/errors.go
@@ -32,6 +32,7 @@ import (
 	"github.com/urfave/cli"
 
 	"github.com/google/fscrypt/actions"
+	"github.com/google/fscrypt/crypto"
 	"github.com/google/fscrypt/filesystem"
 	"github.com/google/fscrypt/metadata"
 	"github.com/google/fscrypt/util"
@@ -77,6 +78,12 @@ func getErrorSuggestions(err error) string {
 	switch errors.Cause(err) {
 	case filesystem.ErrNotSetup:
 		return fmt.Sprintf(`Run "fscrypt setup %s" to use fscrypt on this filesystem.`, mountpointArg)
+	case crypto.ErrKeyLock:
+		return `Too much memory was requested to be locked in RAM. The
+			current limit for this user can be checked with "ulimit
+			-l". The limit can be modified by either changing the
+			"memlock" item in /etc/security/limits.conf or by
+			changing the "LimitMEMLOCK" value in systemd.`
 	case metadata.ErrEncryptionNotSupported:
 		return `Encryption for this type of filesystem is not supported
 			on this kernel version.`
diff --git a/cmd/fscrypt/fscrypt.go b/cmd/fscrypt/fscrypt.go
index fe1e0c9..93df341 100644
--- a/cmd/fscrypt/fscrypt.go
+++ b/cmd/fscrypt/fscrypt.go
@@ -19,8 +19,11 @@
  */
 
 /*
-fscrypt is a comprehensive command line tool for managing filesystem encryption.
+fscrypt is a command line tool for managing linux filesystem encryption.
 */
+
+// +build linux,cgo
+
 package main
 
 import (
diff --git a/crypto/crypto.go b/crypto/crypto.go
index 62226b9..a85d345 100644
--- a/crypto/crypto.go
+++ b/crypto/crypto.go
@@ -62,6 +62,7 @@ var (
 	ErrGetrandomFail  = util.SystemError("getrandom() failed")
 	ErrKeyAlloc       = util.SystemError("could not allocate memory for key")
 	ErrKeyFree        = util.SystemError("could not free memory of key")
+	ErrKeyLock        = errors.New("could not lock key in memory")
 )
 
 // panicInputLength panics if "name" has invalid length (expected != actual)
diff --git a/crypto/crypto_test.go b/crypto/crypto_test.go
index a069b1b..719db00 100644
--- a/crypto/crypto_test.go
+++ b/crypto/crypto_test.go
@@ -294,6 +294,20 @@ func TestRandomKeyGen(t *testing.T) {
 	}
 }
 
+func TestBigKeyGen(t *testing.T) {
+	key, err := NewRandomKey(4096 * 4096)
+	switch err {
+	case nil:
+		key.Wipe()
+		return
+	case ErrKeyLock:
+		// Don't fail just because "ulimit -l" is too low.
+		return
+	default:
+		t.Fatal(err)
+	}
+}
+
 // didCompress checks if the given data can be compressed. Specifically, it
 // returns true if running zlib on the provided input produces a shorter output.
 func didCompress(input []byte) bool {
diff --git a/crypto/key.go b/crypto/key.go
index 497a0ef..ec37330 100644
--- a/crypto/key.go
+++ b/crypto/key.go
@@ -109,6 +109,9 @@ func newBlankKey(length int) (*Key, error) {
 
 	// See MAP_ANONYMOUS in http://man7.org/linux/man-pages/man2/mmap.2.html
 	data, err := unix.Mmap(-1, 0, length, keyProtection, flags)
+	if err == unix.EAGAIN {
+		return nil, ErrKeyLock
+	}
 	if err != nil {
 		log.Printf("unix.Mmap() with length=%d failed: %v", length, err)
 		return nil, ErrKeyAlloc
@@ -329,7 +332,7 @@ func ReadRecoveryCode(reader io.Reader) (*Key, error) {
 	for blockStart := blockSize; blockStart < encodedLength; blockStart += blockSize {
 		r.Read(inputSeparator)
 		if r.Err() == nil && !bytes.Equal(separator, inputSeparator) {
-			err := errors.Wrapf(ErrRecoveryCode, "invalid seperator %q", inputSeparator)
+			err := errors.Wrapf(ErrRecoveryCode, "invalid separator %q", inputSeparator)
 			return nil, err
 		}
 
diff --git a/pam_fscrypt/pam_fscrypt.go b/pam_fscrypt/pam_fscrypt.go
index 21bc779..5beb311 100644
--- a/pam_fscrypt/pam_fscrypt.go
+++ b/pam_fscrypt/pam_fscrypt.go
@@ -17,6 +17,8 @@
  * the License.
  */
 
+// +build linux,cgo
+
 package main
 
 /*