Merge pull request #11 from capitalone/trailing-zeros

Trailing zeros fix, README updates

README.md

@@ -92,15 +92,17 @@ It's important to note that, as with any cryptographic package, managing and pro
 
 Overall, this was originally written with the following goals:
 
-  * Be idiomatic as possible from a Go language, package, and interface perspective
+  * Be as idiomatic as possible from a Go language, package, and interface perspective
   * Follow the algorithm as outlined in the NIST recommendation as closely as possible
   * Attempt to be a reference implementation since one does not exist yet
 
-As such, it was not necessarily written from a performance perspective.
+As such, it was not necessarily written from a performance perspective. While some performance optimizations have been added in v1.1, the nature of format preserving encryption is to operate on strings, which are inherently slow as compared to traditional encryption algorithms which operate on bytes.
 
-As of Go 1.8.1, the standard library's [math/big](https://golang.org/pkg/math/big/) package did not support radices/bases higher than 36. As such, this initial release only supports base 36 strings, which can contain numeric digits 0-9 or lowercase alphabetic characters a-z.
+Further, while the test vectors all pass, the lack of a reference implementation makes it difficult to test ALL input combinations for correctness.
 
-Base 62 support involves simple changes to the `math/big` package; hopefully that can be contributed soon to `math/big` soon. Creating a modified `math/big` sub-package just for 4 lines of changed code seemed like overkill, hence `math/big` being updated is a better solution long term. Ideally, it can be developed further into arbitrary alphabet and base support, which may alleviate the need to use a new Go version where `math/big` has the base 62 support.
+As of Go 1.9, the standard library's [math/big](https://golang.org/pkg/math/big/) package did not support radices/bases higher than 36. As such, the initial release only supports base 36 strings, which can contain numeric digits 0-9 or lowercase alphabetic characters a-z.
+
+Base 62 support should be available come Go 1.10. See [this commit](https://github.com/golang/go/commit/51cfe6849a2b945c9a2bb9d271bf142f3bb99eca) and [this tracking issue](https://github.com/capitalone/fpe/issues/1).
 
 The only cryptographic primitive used for FF1 and FF3 is AES. This package uses Go's standard library's `crypto/aes` package for this. Note that while it technically uses AES-CBC mode, in practice it almost always is meant to act on a single-block with an IV of 0, which is effectively ECB mode. AES is also the only block cipher function that works at the moment, and the only allowed block cipher to be used for FF1/FF3, as per the spec.
 

ff1/ff1.go

@@ -41,7 +41,7 @@ const (
 
 var (
 	// For all AES-CBC calls, IV is always 0
-	ivZero = make([]byte, aes.BlockSize)
+	ivZero = make([]byte, blockSize)
 
 	// ErrStringNotInRadix is returned if input or intermediate strings cannot be parsed in the given radix
 	ErrStringNotInRadix = errors.New("string is not within base/radix")
@@ -162,7 +162,7 @@ func (c Cipher) Encrypt(X string) (string, error) {
 	// Calculate P, doesn't change in each loop iteration
 	// P's length is always 16, so it can stay on the stack, separate from buf
 	const lenP = blockSize
-	P := make([]byte, aes.BlockSize)
+	P := make([]byte, blockSize)
 
 	P[0] = 0x01
 	P[1] = 0x02
@@ -262,9 +262,12 @@ func (c Cipher) Encrypt(X string) (string, error) {
 
 		numBBytes = numB.Bytes()
 
-		// These middle bytes need to be reset to 0
-		for j := 0; j < (lenQ - t - numPad - len(numBBytes)); j++ {
-			Q[t+numPad+j+1] = 0x00
+		// Zero out the rest of Q
+		// When the second half of X is all 0s, numB is 0, so numBytes is an empty slice
+		// So, zero out the rest of Q instead of just the middle bytes, which covers the numB=0 case
+		// See https://github.com/capitalone/fpe/issues/10
+		for j := t + numPad + 1; j < lenQ; j++ {
+			Q[j] = 0x00
 		}
 
 		// B must only take up the last b bytes
@@ -296,7 +299,7 @@ func (c Cipher) Encrypt(X string) (string, error) {
 
 			// XOR R and j in place
 			// R, xored are always 16 bytes
-			for x := 0; x < aes.BlockSize; x++ {
+			for x := 0; x < blockSize; x++ {
 				xored[offset+x] = R[x] ^ xored[offset+x]
 			}
 
@@ -384,7 +387,7 @@ func (c Cipher) Decrypt(X string) (string, error) {
 	// Calculate P, doesn't change in each loop iteration
 	// P's length is always 16, so it can stay on the stack, separate from buf
 	const lenP = blockSize
-	P := make([]byte, aes.BlockSize)
+	P := make([]byte, blockSize)
 
 	P[0] = 0x01
 	P[1] = 0x02
@@ -484,9 +487,12 @@ func (c Cipher) Decrypt(X string) (string, error) {
 
 		numABytes = numA.Bytes()
 
-		// These middle bytes need to be reset to 0
-		for j := 0; j < (lenQ - t - numPad - len(numABytes)); j++ {
-			Q[t+numPad+j+1] = 0x00
+		// Zero out the rest of Q
+		// When the second half of X is all 0s, numB is 0, so numBytes is an empty slice
+		// So, zero out the rest of Q instead of just the middle bytes, which covers the numB=0 case
+		// See https://github.com/capitalone/fpe/issues/10
+		for j := t + numPad + 1; j < lenQ; j++ {
+			Q[j] = 0x00
 		}
 
 		// B must only take up the last b bytes
@@ -518,7 +524,7 @@ func (c Cipher) Decrypt(X string) (string, error) {
 
 			// XOR R and j in place
 			// R, xored are always 16 bytes
-			for x := 0; x < aes.BlockSize; x++ {
+			for x := 0; x < blockSize; x++ {
 				xored[offset+x] = R[x] ^ xored[offset+x]
 			}
 
@@ -565,7 +571,7 @@ func (c Cipher) Decrypt(X string) (string, error) {
 func (c Cipher) ciph(input []byte) ([]byte, error) {
 	// These are checked here manually because the CryptBlocks function panics rather than returning an error
 	// So, catch the potential error earlier
-	if len(input)%aes.BlockSize != 0 {
+	if len(input)%blockSize != 0 {
 		return nil, errors.New("length of ciph input must be multiple of 16")
 	}
 
@@ -586,5 +592,5 @@ func (c Cipher) prf(input []byte) ([]byte, error) {
 	}
 
 	// Only return the last block (CBC-MAC)
-	return cipher[len(cipher)-aes.BlockSize:], nil
+	return cipher[len(cipher)-blockSize:], nil
 }

ff3/ff3.go

@@ -41,7 +41,7 @@ const (
 
 var (
 	// For all AES-CBC calls, IV is always 0
-	ivZero = make([]byte, aes.BlockSize)
+	ivZero = make([]byte, blockSize)
 
 	// ErrStringNotInRadix is returned if input or intermediate strings cannot be parsed in the given radix
 	ErrStringNotInRadix = errors.New("string is not within base/radix")