crypto.go nitpicks

crypto/crypto.go

@@ -14,13 +14,13 @@ import (
 )
 
 const (
-	aesKeySize  = 32                        // for AES256
+	aesKeySize  = 32                        // for AES-256
 	macKeySizeK = 16                        // for AES-128
 	macKeySizeR = 16                        // for Poly1305
 	macKeySize  = macKeySizeK + macKeySizeR // for Poly1305-AES128
 	ivSize      = aes.BlockSize
 
-	macSize   = poly1305.TagSize // Poly1305 size is 16 byte
+	macSize   = poly1305.TagSize
 	Extension = ivSize + macSize
 )
 
@@ -45,8 +45,8 @@ type Key struct {
 
 type EncryptionKey [32]byte
 type SigningKey struct {
-	K [16]byte `json:"k"` // for AES128
-	R [16]byte `json:"r"` // for Poly1305
+	K [16]byte // for AES-128
+	R [16]byte // for Poly1305
 
 	masked bool // remember if the signing key has already been masked
 }
@@ -71,27 +71,9 @@ var poly1305KeyMask = [16]byte{
 	0x0f, // 15: top four bits zero
 }
 
-// key is a [32]byte, in the form k||r
 func poly1305Sign(msg []byte, nonce []byte, key *SigningKey) []byte {
-	// prepare key for low-level poly1305.Sum(): r||n
-	var k [32]byte
+	k := poly1305PrepareKey(nonce, key)
 
-	// make sure key is masked
-	if !key.masked {
-		maskKey(key)
-	}
-
-	// fill in nonce, encrypted with AES and key[:16]
-	cipher, err := aes.NewCipher(key.K[:])
-	if err != nil {
-		panic(err)
-	}
-	cipher.Encrypt(k[16:], nonce[:])
-
-	// copy r
-	copy(k[:16], key.R[:])
-
-	// save mac in out
 	var out [16]byte
 	poly1305.Sum(&out, msg, &k)
 
@@ -100,9 +82,10 @@ func poly1305Sign(msg []byte, nonce []byte, key *SigningKey) []byte {
 
 // mask poly1305 key
 func maskKey(k *SigningKey) {
-	if k == nil {
+	if k == nil || k.masked {
 		return
 	}
+
 	for i := 0; i < poly1305.TagSize; i++ {
 		k.R[i] = k.R[i] & poly1305KeyMask[i]
 	}
@@ -117,36 +100,36 @@ func macKeyFromSlice(mk *SigningKey, data []byte) {
 	maskKey(mk)
 }
 
-// key: k||r
-func poly1305Verify(msg []byte, nonce []byte, key *SigningKey, mac []byte) bool {
-	// prepare key for low-level poly1305.Sum(): r||n
+// prepare key for low-level poly1305.Sum(): r||n
+func poly1305PrepareKey(nonce []byte, key *SigningKey) [32]byte {
 	var k [32]byte
 
-	// make sure key is masked
-	if !key.masked {
-		maskKey(key)
-	}
+	maskKey(key)
 
-	// fill in nonce, encrypted with AES and key[:16]
 	cipher, err := aes.NewCipher(key.K[:])
 	if err != nil {
 		panic(err)
 	}
 	cipher.Encrypt(k[16:], nonce[:])
 
-	// copy r
 	copy(k[:16], key.R[:])
 
-	// copy mac to array
+	return k
+}
+
+func poly1305Verify(msg []byte, nonce []byte, key *SigningKey, mac []byte) bool {
+	k := poly1305PrepareKey(nonce, key)
+
 	var m [16]byte
 	copy(m[:], mac)
 
 	return poly1305.Verify(&m, msg, &k)
 }
 
-// NewKey returns new encryption and signing keys.
-func NewKey() (k *Key) {
-	k = &Key{}
+// NewRandomKey returns new encryption and signing keys.
+func NewRandomKey() *Key {
+	k := &Key{}
+
 	n, err := rand.Read(k.Encrypt[:])
 	if n != aesKeySize || err != nil {
 		panic("unable to read enough random bytes for encryption key")
@@ -161,9 +144,8 @@ func NewKey() (k *Key) {
 	if n != macKeySizeR || err != nil {
 		panic("unable to read enough random bytes for mac signing key")
 	}
-	// mask r
-	maskKey(&k.Sign)
 
+	maskKey(&k.Sign)
 	return k
 }
 
@@ -220,7 +202,7 @@ var ErrInvalidCiphertext = errors.New("invalid ciphertext, same slice used for p
 // MAC. Encrypt returns the new ciphertext slice, which is extended when
 // necessary. ciphertext and plaintext may not point to (exactly) the same
 // slice or non-intersecting slices.
-func Encrypt(ks *Key, ciphertext, plaintext []byte) ([]byte, error) {
+func Encrypt(ks *Key, ciphertext []byte, plaintext []byte) ([]byte, error) {
 	ciphertext = ciphertext[:cap(ciphertext)]
 
 	// test for same slice, if possible
@@ -245,11 +227,11 @@ func Encrypt(ks *Key, ciphertext, plaintext []byte) ([]byte, error) {
 
 	e.XORKeyStream(ciphertext[ivSize:], plaintext)
 	copy(ciphertext, iv[:])
-	// truncate to only conver iv and actual ciphertext
+
+	// truncate to only cover iv and actual ciphertext
 	ciphertext = ciphertext[:ivSize+len(plaintext)]
 
 	mac := poly1305Sign(ciphertext[ivSize:], ciphertext[:ivSize], &ks.Sign)
-	// append the mac tag
 	ciphertext = append(ciphertext, mac...)
 
 	return ciphertext, nil
@@ -258,28 +240,28 @@ func Encrypt(ks *Key, ciphertext, plaintext []byte) ([]byte, error) {
 // Decrypt verifies and decrypts the ciphertext. Ciphertext must be in the form
 // IV || Ciphertext || MAC. plaintext and ciphertext may point to (exactly) the
 // same slice.
-func Decrypt(ks *Key, plaintext, ciphertext []byte) ([]byte, error) {
+func Decrypt(ks *Key, plaintext []byte, ciphertextWithMac []byte) ([]byte, error) {
 	// check for plausible length
-	if len(ciphertext) < ivSize+macSize {
+	if len(ciphertextWithMac) < ivSize+macSize {
 		panic("trying to decrypt invalid data: ciphertext too small")
 	}
 
-	if cap(plaintext) < len(ciphertext) {
+	if cap(plaintext) < len(ciphertextWithMac) {
 		// extend plaintext
-		plaintext = append(plaintext, make([]byte, len(ciphertext)-cap(plaintext))...)
+		plaintext = append(plaintext, make([]byte, len(ciphertextWithMac)-cap(plaintext))...)
 	}
 
 	// extract mac
-	l := len(ciphertext) - macSize
-	ciphertext, mac := ciphertext[:l], ciphertext[l:]
+	l := len(ciphertextWithMac) - macSize
+	ciphertextWithIV, mac := ciphertextWithMac[:l], ciphertextWithMac[l:]
 
 	// verify mac
-	if !poly1305Verify(ciphertext[ivSize:], ciphertext[:ivSize], &ks.Sign, mac) {
+	if !poly1305Verify(ciphertextWithIV[ivSize:], ciphertextWithIV[:ivSize], &ks.Sign, mac) {
 		return nil, ErrUnauthenticated
 	}
 
 	// extract iv
-	iv, ciphertext := ciphertext[:ivSize], ciphertext[ivSize:]
+	iv, ciphertext := ciphertextWithIV[:ivSize], ciphertextWithIV[ivSize:]
 
 	// decrypt data
 	c, err := aes.NewCipher(ks.Encrypt[:])

crypto/crypto_int_test.go

@@ -61,15 +61,16 @@ func TestPoly1305(t *testing.T) {
 }
 
 var testValues = []struct {
-	ekey        EncryptionKey
-	skey        SigningKey
-	ciphertext  []byte
-	plaintext   []byte
-	shouldPanic bool
+	ekey       EncryptionKey
+	skey       SigningKey
+	ciphertext []byte
+	plaintext  []byte
 }{
 	{
-		ekey: EncryptionKey([...]byte{0x30, 0x3e, 0x86, 0x87, 0xb1, 0xd7, 0xdb, 0x18, 0x42, 0x1b, 0xdc, 0x6b, 0xb8, 0x58, 0x8c, 0xca,
-			0xda, 0xc4, 0xd5, 0x9e, 0xe8, 0x7b, 0x8f, 0xf7, 0x0c, 0x44, 0xe6, 0x35, 0x79, 0x0c, 0xaf, 0xef}),
+		ekey: EncryptionKey([...]byte{
+			0x30, 0x3e, 0x86, 0x87, 0xb1, 0xd7, 0xdb, 0x18, 0x42, 0x1b, 0xdc, 0x6b, 0xb8, 0x58, 0x8c, 0xca,
+			0xda, 0xc4, 0xd5, 0x9e, 0xe8, 0x7b, 0x8f, 0xf7, 0x0c, 0x44, 0xe6, 0x35, 0x79, 0x0c, 0xaf, 0xef,
+		}),
 		skey: SigningKey{
 			K: [...]byte{0xef, 0x4d, 0x88, 0x24, 0xcb, 0x80, 0xb2, 0xbc, 0xc5, 0xfb, 0xff, 0x8a, 0x9b, 0x12, 0xa4, 0x2c},
 			R: [...]byte{0xcc, 0x8d, 0x4b, 0x94, 0x8e, 0xe0, 0xeb, 0xfe, 0x1d, 0x41, 0x5d, 0xe9, 0x21, 0xd1, 0x03, 0x53},
@@ -84,19 +85,6 @@ func decodeHex(s string) []byte {
 	return d
 }
 
-// returns true if function called panic
-func shouldPanic(f func()) (didPanic bool) {
-	defer func() {
-		if r := recover(); r != nil {
-			didPanic = true
-		}
-	}()
-
-	f()
-
-	return false
-}
-
 func TestCrypto(t *testing.T) {
 	msg := make([]byte, 0, 8*1024*1024) // use 8MiB for now
 	for _, tv := range testValues {

crypto/crypto_test.go

@@ -17,7 +17,7 @@ import (
 var testLargeCrypto = flag.Bool("test.largecrypto", false, "also test crypto functions with large payloads")
 
 func TestEncryptDecrypt(t *testing.T) {
-	k := crypto.NewKey()
+	k := crypto.NewRandomKey()
 
 	tests := []int{5, 23, 2<<18 + 23, 1 << 20}
 	if *testLargeCrypto {
@@ -48,7 +48,7 @@ func TestEncryptDecrypt(t *testing.T) {
 }
 
 func TestSmallBuffer(t *testing.T) {
-	k := crypto.NewKey()
+	k := crypto.NewRandomKey()
 
 	size := 600
 	data := make([]byte, size)
@@ -73,7 +73,7 @@ func TestSmallBuffer(t *testing.T) {
 }
 
 func TestSameBuffer(t *testing.T) {
-	k := crypto.NewKey()
+	k := crypto.NewRandomKey()
 
 	size := 600
 	data := make([]byte, size)
@@ -96,7 +96,7 @@ func TestSameBuffer(t *testing.T) {
 }
 
 func TestCornerCases(t *testing.T) {
-	k := crypto.NewKey()
+	k := crypto.NewRandomKey()
 
 	// nil plaintext should encrypt to the empty string
 	// nil ciphertext should allocate a new slice for the ciphertext
@@ -122,7 +122,7 @@ func TestLargeEncrypt(t *testing.T) {
 		t.SkipNow()
 	}
 
-	k := crypto.NewKey()
+	k := crypto.NewRandomKey()
 
 	for _, size := range []int{chunker.MaxSize, chunker.MaxSize + 1, chunker.MaxSize + 1<<20} {
 		data := make([]byte, size)
@@ -146,7 +146,7 @@ func BenchmarkEncryptWriter(b *testing.B) {
 	size := 8 << 20 // 8MiB
 	rd := RandomReader(23, size)
 
-	k := crypto.NewKey()
+	k := crypto.NewRandomKey()
 
 	b.ResetTimer()
 	b.SetBytes(int64(size))
@@ -166,7 +166,7 @@ func BenchmarkEncrypt(b *testing.B) {
 	size := 8 << 20 // 8MiB
 	data := make([]byte, size)
 
-	k := crypto.NewKey()
+	k := crypto.NewRandomKey()
 	buf := make([]byte, len(data)+crypto.Extension)
 
 	b.ResetTimer()
@@ -181,7 +181,7 @@ func BenchmarkEncrypt(b *testing.B) {
 func BenchmarkDecryptReader(b *testing.B) {
 	size := 8 << 20 // 8MiB
 	buf := Random(23, size)
-	k := crypto.NewKey()
+	k := crypto.NewRandomKey()
 
 	ciphertext := make([]byte, len(buf)+crypto.Extension)
 	_, err := crypto.Encrypt(k, ciphertext, buf)
@@ -203,7 +203,7 @@ func BenchmarkDecryptReader(b *testing.B) {
 }
 
 func BenchmarkEncryptDecryptReader(b *testing.B) {
-	k := crypto.NewKey()
+	k := crypto.NewRandomKey()
 
 	size := 8 << 20 // 8MiB
 	rd := RandomReader(23, size)
@@ -234,7 +234,7 @@ func BenchmarkDecrypt(b *testing.B) {
 	size := 8 << 20 // 8MiB
 	data := make([]byte, size)
 
-	k := crypto.NewKey()
+	k := crypto.NewRandomKey()
 
 	ciphertext := restic.GetChunkBuf("BenchmarkDecrypt")
 	defer restic.FreeChunkBuf("BenchmarkDecrypt", ciphertext)
@@ -254,7 +254,7 @@ func BenchmarkDecrypt(b *testing.B) {
 }
 
 func TestEncryptStreamWriter(t *testing.T) {
-	k := crypto.NewKey()
+	k := crypto.NewRandomKey()
 
 	tests := []int{5, 23, 2<<18 + 23, 1 << 20}
 	if *testLargeCrypto {
@@ -288,7 +288,7 @@ func TestEncryptStreamWriter(t *testing.T) {
 }
 
 func TestDecryptStreamReader(t *testing.T) {
-	k := crypto.NewKey()
+	k := crypto.NewRandomKey()
 
 	tests := []int{5, 23, 2<<18 + 23, 1 << 20}
 	if *testLargeCrypto {
@@ -322,7 +322,7 @@ func TestDecryptStreamReader(t *testing.T) {
 }
 
 func TestEncryptWriter(t *testing.T) {
-	k := crypto.NewKey()
+	k := crypto.NewRandomKey()
 
 	tests := []int{5, 23, 2<<18 + 23, 1 << 20}
 	if *testLargeCrypto {

crypto/reader.go

@@ -77,7 +77,6 @@ func DecryptFrom(ks *Key, rd io.Reader) (io.ReadCloser, error) {
 
 	ciphertext := buf.Bytes()
 
-	// decrypt
 	ciphertext, err = Decrypt(ks, ciphertext, ciphertext)
 	if err != nil {
 		return nil, err

pack/pack_test.go

@@ -37,7 +37,7 @@ func TestCreatePack(t *testing.T) {
 	file := bytes.NewBuffer(nil)
 
 	// create random keys
-	k := crypto.NewKey()
+	k := crypto.NewRandomKey()
 
 	// pack blobs
 	p := pack.NewPacker(k, file)

server/key.go

@@ -176,7 +176,7 @@ func AddKey(s *Server, password string, template *Key) (*Key, error) {
 
 	if template == nil {
 		// generate new random master keys
-		newkey.master = crypto.NewKey()
+		newkey.master = crypto.NewRandomKey()
 		// generate random polynomial for cdc
 		p, err := chunker.RandomPolynomial()
 		if err != nil {