go.crypto/blowfish: support salts larger than 16 bytes.

Supporting larger salts makes it possible to implement bcrypt-pbkdf
(http://www.tedunangst.com/flak/post/bcrypt-pbkdf), which is used in
the latest OpenSSH versions to encrypt key files.

This change makes expandKeyWithSalt (and thus NewSaltedCipher) about
20% slower, but since it isn't used in the "heavy" phase of computation
in bcrypt (it uses ExpandKey, which didn't change), the effect on
bcrypt performance is negligible.

The 16-byte limit was an artifact of optimization and due to the fact
that it was enough for bcrypt; Eksblowfish spec or other
implementations don't limit the salt size.

Additionally, there was a bug in the previous implementation: it only
generated correct results for salts consisting of 4, 8, or 16 bytes, and panicked if salt had zero length.

LGTM=agl
R=golang-codereviews, gobot, agl
CC=golang-codereviews
https://golang.org/cl/102720045

blowfish/block.go

@@ -4,6 +4,22 @@
 
 package blowfish
 
+// getNextWord returns the next big-endian uint32 value from the byte slice
+// at the given position in a circular manner, updating the position.
+func getNextWord(b []byte, pos *int) uint32 {
+	var w uint32
+	j := *pos
+	for i := 0; i < 4; i++ {
+		w = w<<8 | uint32(b[j])
+		j++
+		if j >= len(b) {
+			j = 0
+		}
+	}
+	*pos = j
+	return w
+}
+
 // ExpandKey performs a key expansion on the given *Cipher. Specifically, it
 // performs the Blowfish algorithm's key schedule which sets up the *Cipher's
 // pi and substitution tables for calls to Encrypt. This is used, primarily,
@@ -12,6 +28,7 @@ package blowfish
 func ExpandKey(key []byte, c *Cipher) {
 	j := 0
 	for i := 0; i < 18; i++ {
+		// Using inlined getNextWord for performance.
 		var d uint32
 		for k := 0; k < 4; k++ {
 			d = d<<8 | uint32(key[j])
@@ -54,86 +71,44 @@ func ExpandKey(key []byte, c *Cipher) {
 func expandKeyWithSalt(key []byte, salt []byte, c *Cipher) {
 	j := 0
 	for i := 0; i < 18; i++ {
-		var d uint32
-		for k := 0; k < 4; k++ {
-			d = d<<8 | uint32(key[j])
-			j++
-			if j >= len(key) {
-				j = 0
-			}
-		}
-		c.p[i] ^= d
+		c.p[i] ^= getNextWord(key, &j)
 	}
 
 	j = 0
-	var expandedSalt [4]uint32
-	for i := range expandedSalt {
-		var d uint32
-		for k := 0; k < 4; k++ {
-			d = d<<8 | uint32(salt[j])
-			j++
-			if j >= len(salt) {
-				j = 0
-			}
-		}
-		expandedSalt[i] = d
-	}
-
 	var l, r uint32
 	for i := 0; i < 18; i += 2 {
-		l ^= expandedSalt[i&2]
-		r ^= expandedSalt[(i&2)+1]
+		l ^= getNextWord(salt, &j)
+		r ^= getNextWord(salt, &j)
 		l, r = encryptBlock(l, r, c)
 		c.p[i], c.p[i+1] = l, r
 	}
 
-	for i := 0; i < 256; i += 4 {
-		l ^= expandedSalt[2]
-		r ^= expandedSalt[3]
+	for i := 0; i < 256; i += 2 {
+		l ^= getNextWord(salt, &j)
+		r ^= getNextWord(salt, &j)
 		l, r = encryptBlock(l, r, c)
 		c.s0[i], c.s0[i+1] = l, r
-
-		l ^= expandedSalt[0]
-		r ^= expandedSalt[1]
-		l, r = encryptBlock(l, r, c)
-		c.s0[i+2], c.s0[i+3] = l, r
-
 	}
 
-	for i := 0; i < 256; i += 4 {
-		l ^= expandedSalt[2]
-		r ^= expandedSalt[3]
+	for i := 0; i < 256; i += 2 {
+		l ^= getNextWord(salt, &j)
+		r ^= getNextWord(salt, &j)
 		l, r = encryptBlock(l, r, c)
 		c.s1[i], c.s1[i+1] = l, r
-
-		l ^= expandedSalt[0]
-		r ^= expandedSalt[1]
-		l, r = encryptBlock(l, r, c)
-		c.s1[i+2], c.s1[i+3] = l, r
 	}
 
-	for i := 0; i < 256; i += 4 {
-		l ^= expandedSalt[2]
-		r ^= expandedSalt[3]
+	for i := 0; i < 256; i += 2 {
+		l ^= getNextWord(salt, &j)
+		r ^= getNextWord(salt, &j)
 		l, r = encryptBlock(l, r, c)
 		c.s2[i], c.s2[i+1] = l, r
-
-		l ^= expandedSalt[0]
-		r ^= expandedSalt[1]
-		l, r = encryptBlock(l, r, c)
-		c.s2[i+2], c.s2[i+3] = l, r
 	}
 
-	for i := 0; i < 256; i += 4 {
-		l ^= expandedSalt[2]
-		r ^= expandedSalt[3]
+	for i := 0; i < 256; i += 2 {
+		l ^= getNextWord(salt, &j)
+		r ^= getNextWord(salt, &j)
 		l, r = encryptBlock(l, r, c)
 		c.s3[i], c.s3[i+1] = l, r
-
-		l ^= expandedSalt[0]
-		r ^= expandedSalt[1]
-		l, r = encryptBlock(l, r, c)
-		c.s3[i+2], c.s3[i+3] = l, r
 	}
 }
 
@@ -182,9 +157,3 @@ func decryptBlock(l, r uint32, c *Cipher) (uint32, uint32) {
 	xr ^= c.p[0]
 	return xr, xl
 }
-
-func zero(x []uint32) {
-	for i := range x {
-		x[i] = 0
-	}
-}

blowfish/blowfish_test.go

@@ -4,9 +4,7 @@
 
 package blowfish
 
-import (
-	"testing"
-)
+import "testing"
 
 type CryptTest struct {
 	key []byte
@@ -202,3 +200,75 @@ func TestSaltedCipherKeyLength(t *testing.T) {
 		t.Errorf("NewSaltedCipher with long key, gave error %#v", err)
 	}
 }
+
+// Test vectors generated with Blowfish from OpenSSH.
+var saltedVectors = [][8]byte{
+	{0x0c, 0x82, 0x3b, 0x7b, 0x8d, 0x01, 0x4b, 0x7e},
+	{0xd1, 0xe1, 0x93, 0xf0, 0x70, 0xa6, 0xdb, 0x12},
+	{0xfc, 0x5e, 0xba, 0xde, 0xcb, 0xf8, 0x59, 0xad},
+	{0x8a, 0x0c, 0x76, 0xe7, 0xdd, 0x2c, 0xd3, 0xa8},
+	{0x2c, 0xcb, 0x7b, 0xee, 0xac, 0x7b, 0x7f, 0xf8},
+	{0xbb, 0xf6, 0x30, 0x6f, 0xe1, 0x5d, 0x62, 0xbf},
+	{0x97, 0x1e, 0xc1, 0x3d, 0x3d, 0xe0, 0x11, 0xe9},
+	{0x06, 0xd7, 0x4d, 0xb1, 0x80, 0xa3, 0xb1, 0x38},
+	{0x67, 0xa1, 0xa9, 0x75, 0x0e, 0x5b, 0xc6, 0xb4},
+	{0x51, 0x0f, 0x33, 0x0e, 0x4f, 0x67, 0xd2, 0x0c},
+	{0xf1, 0x73, 0x7e, 0xd8, 0x44, 0xea, 0xdb, 0xe5},
+	{0x14, 0x0e, 0x16, 0xce, 0x7f, 0x4a, 0x9c, 0x7b},
+	{0x4b, 0xfe, 0x43, 0xfd, 0xbf, 0x36, 0x04, 0x47},
+	{0xb1, 0xeb, 0x3e, 0x15, 0x36, 0xa7, 0xbb, 0xe2},
+	{0x6d, 0x0b, 0x41, 0xdd, 0x00, 0x98, 0x0b, 0x19},
+	{0xd3, 0xce, 0x45, 0xce, 0x1d, 0x56, 0xb7, 0xfc},
+	{0xd9, 0xf0, 0xfd, 0xda, 0xc0, 0x23, 0xb7, 0x93},
+	{0x4c, 0x6f, 0xa1, 0xe4, 0x0c, 0xa8, 0xca, 0x57},
+	{0xe6, 0x2f, 0x28, 0xa7, 0x0c, 0x94, 0x0d, 0x08},
+	{0x8f, 0xe3, 0xf0, 0xb6, 0x29, 0xe3, 0x44, 0x03},
+	{0xff, 0x98, 0xdd, 0x04, 0x45, 0xb4, 0x6d, 0x1f},
+	{0x9e, 0x45, 0x4d, 0x18, 0x40, 0x53, 0xdb, 0xef},
+	{0xb7, 0x3b, 0xef, 0x29, 0xbe, 0xa8, 0x13, 0x71},
+	{0x02, 0x54, 0x55, 0x41, 0x8e, 0x04, 0xfc, 0xad},
+	{0x6a, 0x0a, 0xee, 0x7c, 0x10, 0xd9, 0x19, 0xfe},
+	{0x0a, 0x22, 0xd9, 0x41, 0xcc, 0x23, 0x87, 0x13},
+	{0x6e, 0xff, 0x1f, 0xff, 0x36, 0x17, 0x9c, 0xbe},
+	{0x79, 0xad, 0xb7, 0x40, 0xf4, 0x9f, 0x51, 0xa6},
+	{0x97, 0x81, 0x99, 0xa4, 0xde, 0x9e, 0x9f, 0xb6},
+	{0x12, 0x19, 0x7a, 0x28, 0xd0, 0xdc, 0xcc, 0x92},
+	{0x81, 0xda, 0x60, 0x1e, 0x0e, 0xdd, 0x65, 0x56},
+	{0x7d, 0x76, 0x20, 0xb2, 0x73, 0xc9, 0x9e, 0xee},
+}
+
+func TestSaltedCipher(t *testing.T) {
+	var key, salt [32]byte
+	for i := range key {
+		key[i] = byte(i)
+		salt[i] = byte(i + 32)
+	}
+	for i, v := range saltedVectors {
+		c, err := NewSaltedCipher(key[:], salt[:i])
+		if err != nil {
+			t.Fatal(err)
+		}
+		var buf [8]byte
+		c.Encrypt(buf[:], buf[:])
+		if v != buf {
+			t.Errorf("%d: expected %x, got %x", i, v, buf)
+		}
+	}
+}
+
+func BenchmarkExpandKeyWithSalt(b *testing.B) {
+	key := make([]byte, 32)
+	salt := make([]byte, 16)
+	c, _ := NewCipher(key)
+	for i := 0; i < b.N; i++ {
+		expandKeyWithSalt(key, salt, c)
+	}
+}
+
+func BenchmarkExpandKey(b *testing.B) {
+	key := make([]byte, 32)
+	c, _ := NewCipher(key)
+	for i := 0; i < b.N; i++ {
+		ExpandKey(key, c)
+	}
+}

blowfish/cipher.go

@@ -40,8 +40,11 @@ func NewCipher(key []byte) (*Cipher, error) {
 // NewSaltedCipher creates a returns a Cipher that folds a salt into its key
 // schedule. For most purposes, NewCipher, instead of NewSaltedCipher, is
 // sufficient and desirable. For bcrypt compatiblity, the key can be over 56
-// bytes. Only the first 16 bytes of salt are used.
+// bytes.
 func NewSaltedCipher(key, salt []byte) (*Cipher, error) {
+	if len(salt) == 0 {
+		return NewCipher(key)
+	}
 	var result Cipher
 	if k := len(key); k < 1 {
 		return nil, KeySizeError(k)