eestream: make secretbox example harder to fall victim to nonce reuse

pkg/eestream/bits.go

@@ -0,0 +1,40 @@
+// Copyright (C) 2018 Storj Labs, Inc.
+// See LICENSE for copying information.
+
+package eestream
+
+import "math/big"
+
+// incrementBytes takes a byte slice buf and treats it like a big-endian
+// encoded unsigned integer. it adds amount to it (which must be nonnegative)
+// in place. if rollover happens (the most significant bytes don't fit
+// anymore), truncated is true.
+func incrementBytes(buf []byte, amount int64) (truncated bool,
+	err error) {
+	if amount < 0 {
+		return false, Error.New("amount was negative")
+	}
+	// use math/big for the actual incrementing
+	var val big.Int
+	val.SetBytes(buf)
+	val.Add(&val, big.NewInt(amount))
+	data := val.Bytes()
+
+	// we went past the available memory. truncate the most significant bytes
+	// off
+	if len(data) > len(buf) {
+		data = data[len(data)-len(buf):]
+		truncated = true
+	}
+
+	// math/big doesn't return leading 0 bytes so add them back if they're
+	// missing
+	for i := len(buf) - len(data) - 1; i >= 0; i-- {
+		buf[i] = 0
+	}
+
+	// write the data out inplace
+	copy(buf[len(buf)-len(data):], data[:])
+
+	return truncated, nil
+}

pkg/eestream/bits_test.go

@@ -0,0 +1,63 @@
+// Copyright (C) 2018 Storj Labs, Inc.
+// See LICENSE for copying information.
+
+package eestream
+
+import (
+	"bytes"
+	"testing"
+)
+
+func TestIncrementBytes(t *testing.T) {
+	for _, test := range []struct {
+		inbuf     []byte
+		amount    int64
+		err       bool
+		outbuf    []byte
+		truncated bool
+	}{
+		{nil, 10, false, nil, true},
+		{nil, 0, false, nil, false},
+		{nil, -1, true, nil, false},
+		{nil, -1, true, nil, false},
+		{nil, -457, true, nil, false},
+		{[]byte{0}, 0, false, []byte{0}, false},
+		{[]byte{0}, 1, false, []byte{1}, false},
+		{[]byte{0}, 254, false, []byte{0xfe}, false},
+		{[]byte{1}, 254, false, []byte{0xff}, false},
+		{[]byte{0}, 255, false, []byte{0xff}, false},
+		{[]byte{0, 0, 1}, 3, false, []byte{0, 0, 4}, false},
+		{[]byte{0}, 256, false, []byte{0}, true},
+		{[]byte{0}, 257, false, []byte{1}, true},
+		{[]byte{0xfe, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, 1,
+			false, []byte{0xff, 0, 0, 0, 0, 0, 0, 0, 0, 0}, false},
+		{[]byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, 1,
+			false, []byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, true},
+		{[]byte{0xfe, 0xff, 0xff, 0xff, 0xfe, 0xff, 0xff, 0xff, 0xff, 0xff}, 1,
+			false, []byte{0xfe, 0xff, 0xff, 0xff, 0xff, 0, 0, 0, 0, 0}, false},
+		{[]byte{0xfe, 0xff, 0xff, 0xff, 0xfe, 0xff, 0xff, 0, 0xff, 0xfe}, 0xff0001,
+			false, []byte{0xfe, 0xff, 0xff, 0xff, 0xfe, 0xff, 0xff, 0xff, 0xff, 0xff},
+			false},
+		{[]byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0, 0xff, 0xfe}, 0xff0002,
+			false, []byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, true},
+		{[]byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0, 0xff, 0xfe}, 0xff0003,
+			false, []byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 1}, true},
+	} {
+		trunc, err := incrementBytes(test.inbuf, test.amount)
+		if err != nil {
+			if !test.err {
+				t.Fatalf("unexpected err: %v", err)
+			}
+			continue
+		}
+		if test.err {
+			t.Fatalf("err expected but no err happened")
+		}
+		if trunc != test.truncated {
+			t.Fatalf("truncated rv mismatch")
+		}
+		if !bytes.Equal(test.outbuf, test.inbuf) {
+			t.Fatalf("result mismatch")
+		}
+	}
+}

pkg/eestream/secretbox.go

@@ -4,33 +4,38 @@
 package eestream
 
 import (
-	"encoding/binary"
-
 	"golang.org/x/crypto/nacl/secretbox"
 )
 
 type secretboxEncrypter struct {
-	blockSize int
-	key       [32]byte
-}
-
-func setKey(dst *[32]byte, key []byte) error {
-	if len((*dst)[:]) != len(key) {
-		return Error.New("invalid key length, expected %d", len((*dst)[:]))
-	}
-	copy((*dst)[:], key)
-	return nil
+	blockSize     int
+	key           [32]byte
+	startingNonce [24]byte
 }
 
 // NewSecretboxEncrypter returns a Transformer that encrypts the data passing
 // through with key.
-func NewSecretboxEncrypter(key []byte, encryptedBlockSize int) (
-	Transformer, error) {
+//
+// startingNonce is treated as a big-endian encoded unsigned
+// integer, and as blocks pass through, their block number and the starting
+// nonce is added together to come up with that block's nonce. Encrypting
+// different data with the same key and the same nonce is a huge security
+// issue. It's safe to always encode new data with a random key and random
+// startingNonce. The monotonically-increasing nonce (that rolls over) is to
+// protect against data reordering.
+//
+// When in doubt, generate a new key from crypto/rand and a startingNonce
+// from crypto/rand as often as possible.
+func NewSecretboxEncrypter(key *[32]byte, startingNonce *[24]byte,
+	encryptedBlockSize int) (Transformer, error) {
 	if encryptedBlockSize <= secretbox.Overhead {
 		return nil, Error.New("block size too small")
 	}
-	rv := &secretboxEncrypter{blockSize: encryptedBlockSize - secretbox.Overhead}
-	return rv, setKey(&rv.key, key)
+	return &secretboxEncrypter{
+		blockSize:     encryptedBlockSize - secretbox.Overhead,
+		key:           *key,
+		startingNonce: *startingNonce,
+	}, nil
 }
 
 func (s *secretboxEncrypter) InBlockSize() int {
@@ -41,33 +46,43 @@ func (s *secretboxEncrypter) OutBlockSize() int {
 	return s.blockSize + secretbox.Overhead
 }
 
-func calcNonce(blockNum int64) *[24]byte {
-	var buf [uint32Size]byte
-	binary.BigEndian.PutUint32(buf[:], uint32(blockNum))
-	var nonce [24]byte
-	copy(nonce[:], buf[1:])
-	return &nonce
+func calcNonce(startingNonce *[24]byte, blockNum int64) (rv [24]byte,
+	err error) {
+	if copy(rv[:], (*startingNonce)[:]) != len(rv) {
+		return rv, Error.New("didn't copy memory?!")
+	}
+	_, err = incrementBytes(rv[:], blockNum)
+	return rv, err
 }
 
 func (s *secretboxEncrypter) Transform(out, in []byte, blockNum int64) (
 	[]byte, error) {
-	return secretbox.Seal(out, in, calcNonce(blockNum), &s.key), nil
+	n, err := calcNonce(&s.startingNonce, blockNum)
+	if err != nil {
+		return nil, err
+	}
+	return secretbox.Seal(out, in, &n, &s.key), nil
 }
 
 type secretboxDecrypter struct {
-	blockSize int
-	key       [32]byte
+	blockSize     int
+	key           [32]byte
+	startingNonce [24]byte
 }
 
 // NewSecretboxDecrypter returns a Transformer that decrypts the data passing
-// through with key.
-func NewSecretboxDecrypter(key []byte, encryptedBlockSize int) (
-	Transformer, error) {
+// through with key. See the comments for NewSecretboxEncrypter about
+// startingNonce.
+func NewSecretboxDecrypter(key *[32]byte, startingNonce *[24]byte,
+	encryptedBlockSize int) (Transformer, error) {
 	if encryptedBlockSize <= secretbox.Overhead {
 		return nil, Error.New("block size too small")
 	}
-	rv := &secretboxDecrypter{blockSize: encryptedBlockSize - secretbox.Overhead}
-	return rv, setKey(&rv.key, key)
+	return &secretboxDecrypter{
+		blockSize:     encryptedBlockSize - secretbox.Overhead,
+		key:           *key,
+		startingNonce: *startingNonce,
+	}, nil
 }
 
 func (s *secretboxDecrypter) InBlockSize() int {
@@ -80,7 +95,11 @@ func (s *secretboxDecrypter) OutBlockSize() int {
 
 func (s *secretboxDecrypter) Transform(out, in []byte, blockNum int64) (
 	[]byte, error) {
-	rv, success := secretbox.Open(out, in, calcNonce(blockNum), &s.key)
+	n, err := calcNonce(&s.startingNonce, blockNum)
+	if err != nil {
+		return nil, err
+	}
+	rv, success := secretbox.Open(out, in, &n, &s.key)
 	if !success {
 		return nil, Error.New("failed decrypting")
 	}

pkg/eestream/secretbox_test.go

@@ -20,15 +20,18 @@ func randData(amount int) []byte {
 }
 
 func TestSecretbox(t *testing.T) {
-	key := randData(32)
-	encrypter, err := NewSecretboxEncrypter(key, 4*1024)
+	var key [32]byte
+	copy(key[:], randData(32))
+	var firstNonce [24]byte
+	copy(firstNonce[:], randData(24))
+	encrypter, err := NewSecretboxEncrypter(&key, &firstNonce, 4*1024)
 	if err != nil {
 		t.Fatal(err)
 	}
 	data := randData(encrypter.InBlockSize() * 10)
 	encrypted := TransformReader(bytes.NewReader(data),
 		encrypter, 0)
-	decrypter, err := NewSecretboxDecrypter(key, 4*1024)
+	decrypter, err := NewSecretboxDecrypter(&key, &firstNonce, 4*1024)
 	if err != nil {
 		t.Fatal(err)
 	}