Consolidate and optimize zero functions.

This introduce a new internal package to deal with the explicit
clearing of data (such as private keys) in byte slices, byte arrays
(32 and 64-bytes long), and multi-precision "big" integers.

Benchmarks from a xeon e3 (Xor is the zeroing funcion which Bytes
replaces):

BenchmarkXor32  30000000                52.1 ns/op
BenchmarkXor64  20000000                91.5 ns/op
BenchmarkRange32        50000000                31.8 ns/op
BenchmarkRange64        30000000                49.5 ns/op
BenchmarkBytes32        200000000               10.1 ns/op
BenchmarkBytes64        100000000               15.4 ns/op
BenchmarkBytea32        1000000000               2.24 ns/op
BenchmarkBytea64        300000000                4.46 ns/op

Removes an XXX from the votingpool package.

internal/zero/benchmark_test.go

@@ -0,0 +1,83 @@
+package zero_test
+
+import (
+	"testing"
+
+	. "github.com/btcsuite/btcwallet/internal/zero"
+)
+
+var (
+	bytes32 = make([]byte, 32) // typical key size
+	bytes64 = make([]byte, 64) // passphrase hash size
+	bytea32 = new([32]byte)
+	bytea64 = new([64]byte)
+)
+
+// xor is the "slow" byte zeroing implementation which this package
+// originally replaced.  If this function benchmarks faster than the
+// functions exported by this package in a future Go version (perhaps
+// by calling runtime.memclr), replace the "optimized" versions with
+// this.
+func xor(b []byte) {
+	for i := range b {
+		b[i] ^= b[i]
+	}
+}
+
+// zrange is an alternative zero implementation that, while currently
+// slower than the functions provided by this package, may be faster
+// in a future Go release.  Switch to this or the xor implementation
+// if they ever become faster.
+func zrange(b []byte) {
+	for i := range b {
+		b[i] = 0
+	}
+}
+
+func BenchmarkXor32(b *testing.B) {
+	for i := 0; i < b.N; i++ {
+		xor(bytes32)
+	}
+}
+
+func BenchmarkXor64(b *testing.B) {
+	for i := 0; i < b.N; i++ {
+		xor(bytes64)
+	}
+}
+
+func BenchmarkRange32(b *testing.B) {
+	for i := 0; i < b.N; i++ {
+		zrange(bytes32)
+	}
+}
+
+func BenchmarkRange64(b *testing.B) {
+	for i := 0; i < b.N; i++ {
+		zrange(bytes64)
+	}
+}
+
+func BenchmarkBytes32(b *testing.B) {
+	for i := 0; i < b.N; i++ {
+		Bytes(bytes32)
+	}
+}
+
+func BenchmarkBytes64(b *testing.B) {
+	for i := 0; i < b.N; i++ {
+		Bytes(bytes64)
+	}
+}
+
+func BenchmarkBytea32(b *testing.B) {
+	for i := 0; i < b.N; i++ {
+		Bytea32(bytea32)
+	}
+}
+
+func BenchmarkBytea64(b *testing.B) {
+	for i := 0; i < b.N; i++ {
+		Bytea64(bytea64)
+	}
+}

internal/zero/zero.go

@@ -0,0 +1,49 @@
+// Package zero contains functions to clear data from byte slices and
+// multi-precision integers.
+package zero
+
+import (
+	"math/big"
+)
+
+// Bytes sets all bytes in the passed slice to zero.  This is used to
+// explicitly clear private key material from memory.
+//
+// In general, prefer to use the fixed-sized zeroing functions (Bytea*)
+// when zeroing bytes as they are much more efficient than the variable
+// sized zeroing func Bytes.
+func Bytes(b []byte) {
+	z := [32]byte{}
+	n := uint(copy(b, z[:]))
+	for n < uint(len(b)) {
+		copy(b[n:], b[:n])
+		n <<= 1
+	}
+}
+
+// Bytea32 clears the 32-byte array by filling it with the zero value.
+// This is used to explicitly clear private key material from memory.
+func Bytea32(b *[32]byte) {
+	*b = [32]byte{}
+}
+
+// Bytea64 clears the 64-byte array by filling it with the zero value.
+// This is used to explicitly clear sensitive material from memory.
+func Bytea64(b *[64]byte) {
+	*b = [64]byte{}
+}
+
+// BigInt sets all bytes in the passed big int to zero and then sets the
+// value to 0.  This differs from simply setting the value in that it
+// specifically clears the underlying bytes whereas simply setting the value
+// does not.  This is mostly useful to forcefully clear private keys.
+func BigInt(x *big.Int) {
+	b := x.Bits()
+	z := [16]big.Word{}
+	n := uint(copy(b, z[:]))
+	for n < uint(len(b)) {
+		copy(b[n:], b[:n])
+		n <<= 1
+	}
+	x.SetInt64(0)
+}

internal/zero/zero_test.go

@@ -0,0 +1,143 @@
+package zero_test
+
+import (
+	"fmt"
+	"math/big"
+	"strings"
+	"testing"
+
+	. "github.com/btcsuite/btcwallet/internal/zero"
+)
+
+func makeOneBytes(n int) []byte {
+	b := make([]byte, n)
+	for i := range b {
+		b[i] = 1
+	}
+	return b
+}
+
+func checkZeroBytes(b []byte) error {
+	for i, v := range b {
+		if v != 0 {
+			return fmt.Errorf("b[%d] = %d", i, v)
+		}
+	}
+	return nil
+}
+
+func TestBytes(t *testing.T) {
+	tests := []int{
+		0,
+		31,
+		32,
+		33,
+		127,
+		128,
+		129,
+		255,
+		256,
+		256,
+		257,
+		383,
+		384,
+		385,
+		511,
+		512,
+		513,
+	}
+
+	for i, n := range tests {
+		b := makeOneBytes(n)
+		Bytes(b)
+		err := checkZeroBytes(b)
+		if err != nil {
+			t.Errorf("Test %d (n=%d) failed: %v", i, n, err)
+			continue
+		}
+	}
+}
+
+func checkZeroWords(b []big.Word) error {
+	for i, v := range b {
+		if v != 0 {
+			return fmt.Errorf("b[%d] = %d", i, v)
+		}
+	}
+	return nil
+}
+
+var bigZero = new(big.Int)
+
+func TestBigInt(t *testing.T) {
+	tests := []string{
+		// 16 0xFFFFFFFF 32-bit uintptrs
+		strings.Repeat("FFFFFFFF", 16),
+
+		// 17 32-bit uintptrs, minimum value which enters loop on 32-bit
+		"01" + strings.Repeat("00000000", 16),
+
+		// 32 0xFFFFFFFF 32-bit uintptrs, maximum value which enters loop exactly once on 32-bit
+		strings.Repeat("FFFFFFFF", 32),
+
+		// 33 32-bit uintptrs, minimum value which enters loop twice on 32-bit
+		"01" + strings.Repeat("00000000", 32),
+
+		// 16 0xFFFFFFFFFFFFFFFF 64-bit uintptrs
+		strings.Repeat("FFFFFFFFFFFFFFFF", 16),
+
+		// 17 64-bit uintptrs, minimum value which enters loop on 64-bit
+		"01" + strings.Repeat("0000000000000000", 16),
+
+		// 32 0xFFFFFFFFFFFFFFFF 64-bit uintptrs, maximum value which enters loop exactly once on 64-bit
+		strings.Repeat("FFFFFFFFFFFFFFFF", 32),
+
+		// 33 64-bit uintptrs, minimum value which enters loop twice on 64-bit
+		"01" + strings.Repeat("0000000000000000", 32),
+	}
+
+	for i, s := range tests {
+		v, ok := new(big.Int).SetString(s, 16)
+		if !ok {
+			t.Errorf("Test %d includes invalid hex number %s", i, s)
+			continue
+		}
+
+		BigInt(v)
+		err := checkZeroWords(v.Bits())
+		if err != nil {
+			t.Errorf("Test %d (s=%s) failed: %v", i, s, err)
+			continue
+		}
+		if v.Cmp(bigZero) != 0 {
+			t.Errorf("Test %d (s=%s) zeroed big.Int represents non-zero number %v", i, s, v)
+			continue
+		}
+	}
+}
+
+func TestBytea32(t *testing.T) {
+	const sz = 32
+	var b [sz]byte
+	copy(b[:], makeOneBytes(sz))
+
+	Bytea32(&b)
+
+	err := checkZeroBytes(b[:])
+	if err != nil {
+		t.Error(err)
+	}
+}
+
+func TestBytea64(t *testing.T) {
+	const sz = 64
+	var b [sz]byte
+	copy(b[:], makeOneBytes(sz))
+
+	Bytea64(&b)
+
+	err := checkZeroBytes(b[:])
+	if err != nil {
+		t.Error(err)
+	}
+}

snacl/snacl.go

@@ -8,6 +8,7 @@ import (
 	"io"
 	"runtime/debug"
 
+	"github.com/btcsuite/btcwallet/internal/zero"
 	"github.com/btcsuite/fastsha256"
 	"github.com/btcsuite/golangcrypto/nacl/secretbox"
 	"github.com/btcsuite/golangcrypto/scrypt"
@@ -23,13 +24,6 @@ var (
 	ErrDecryptFailed   = errors.New("unable to decrypt")
 )
 
-// Zero out a byte slice.
-func zero(b []byte) {
-	for i := range b {
-		b[i] ^= b[i]
-	}
-}
-
 const (
 	// Expose secretbox's Overhead const here for convenience.
 	Overhead  = secretbox.Overhead
@@ -79,7 +73,7 @@ func (ck *CryptoKey) Decrypt(in []byte) ([]byte, error) {
 // rather than waiting until it's reclaimed by the garbage collector.  The
 // key is no longer usable after this call.
 func (ck *CryptoKey) Zero() {
-	zero(ck[:])
+	zero.Bytea32((*[KeySize]byte)(ck))
 }
 
 // GenerateCryptoKey generates a new crypotgraphically random key.
@@ -120,7 +114,7 @@ func (sk *SecretKey) deriveKey(password *[]byte) error {
 		return err
 	}
 	copy(sk.Key[:], key)
-	zero(key)
+	zero.Bytes(key)
 
 	// I'm not a fan of forced garbage collections, but scrypt allocates a
 	// ton of memory and calling it back to back without a GC cycle in

votingpool/pool.go

@@ -23,6 +23,7 @@ import (
 	"github.com/btcsuite/btcd/txscript"
 	"github.com/btcsuite/btcutil"
 	"github.com/btcsuite/btcutil/hdkeychain"
+	"github.com/btcsuite/btcwallet/internal/zero"
 	"github.com/btcsuite/btcwallet/waddrmgr"
 	"github.com/btcsuite/btcwallet/walletdb"
 )
@@ -326,7 +327,7 @@ func (vp *Pool) decryptExtendedKey(keyType waddrmgr.CryptoKeyType, encrypted []b
 		return nil, managerError(waddrmgr.ErrCrypto, str, err)
 	}
 	result, err := hdkeychain.NewKeyFromString(string(decrypted))
-	zero(decrypted)
+	zero.Bytes(decrypted)
 	if err != nil {
 		str := fmt.Sprintf("cannot get key from string %v", decrypted)
 		return nil, managerError(waddrmgr.ErrKeyChain, str, err)
@@ -608,15 +609,3 @@ func (s *SeriesData) IsEmpowered() bool {
 func managerError(c waddrmgr.ErrorCode, desc string, err error) waddrmgr.ManagerError {
 	return waddrmgr.ManagerError{ErrorCode: c, Description: desc, Err: err}
 }
-
-// zero sets all bytes in the passed slice to zero.  This is used to
-// explicitly clear private key material from memory.
-//
-// XXX(lars) there exists currently around 4-5 other zero functions
-// with at least 3 different implementations. We should try to
-// consolidate these.
-func zero(b []byte) {
-	for i := range b {
-		b[i] ^= b[i]
-	}
-}

waddrmgr/address.go

@@ -19,39 +19,14 @@ package waddrmgr
 import (
 	"encoding/hex"
 	"fmt"
-	"math/big"
 	"sync"
 
 	"github.com/btcsuite/btcd/btcec"
 	"github.com/btcsuite/btcutil"
 	"github.com/btcsuite/btcutil/hdkeychain"
+	"github.com/btcsuite/btcwallet/internal/zero"
 )
 
-// zero sets all bytes in the passed slice to zero.  This is used to
-// explicitly clear private key material from memory.
-func zero(b []byte) {
-	for i := range b {
-		b[i] ^= b[i]
-	}
-}
-
-// zeroBigInt sets all bytes in the passed big int to zero and then sets the
-// value to 0.  This differs from simply setting the value in that it
-// specifically clears the underlying bytes whereas simply setting the value
-// does not.  This is mostly useful to forcefully clear private keys.
-func zeroBigInt(x *big.Int) {
-	// NOTE: This could make use of .Xor, however this is safer since the
-	// specific implementation of Xor could technically change in such a way
-	// as the original bits aren't cleared.  This function would silenty
-	// fail in that case and it's best to avoid that possibility.
-	bits := x.Bits()
-	numBits := len(bits)
-	for i := 0; i < numBits; i++ {
-		bits[i] ^= bits[i]
-	}
-	x.SetInt64(0)
-}
-
 // ManagedAddress is an interface that provides acces to information regarding
 // an address managed by an address manager. Concrete implementations of this
 // type may provide further fields to provide information specific to that type
@@ -159,7 +134,7 @@ func (a *managedAddress) lock() {
 	// Zero and nil the clear text private key associated with this
 	// address.
 	a.privKeyMutex.Lock()
-	zero(a.privKeyCT)
+	zero.Bytes(a.privKeyCT)
 	a.privKeyCT = nil
 	a.privKeyMutex.Unlock()
 }
@@ -260,7 +235,7 @@ func (a *managedAddress) PrivKey() (*btcec.PrivateKey, error) {
 	}
 
 	privKey, _ := btcec.PrivKeyFromBytes(btcec.S256(), privKeyCopy)
-	zero(privKeyCopy)
+	zero.Bytes(privKeyCopy)
 	return privKey, nil
 }
 
@@ -351,7 +326,7 @@ func newManagedAddressFromExtKey(m *Manager, account uint32, key *hdkeychain.Ext
 
 		// Ensure the temp private key big integer is cleared after use.
 		managedAddr, err = newManagedAddress(m, account, privKey, true)
-		zeroBigInt(privKey.D)
+		zero.BigInt(privKey.D)
 		if err != nil {
 			return nil, err
 		}
@@ -413,7 +388,7 @@ func (a *scriptAddress) unlock(key EncryptorDecryptor) ([]byte, error) {
 func (a *scriptAddress) lock() {
 	// Zero and nil the clear text script associated with this address.
 	a.scriptMutex.Lock()
-	zero(a.scriptCT)
+	zero.Bytes(a.scriptCT)
 	a.scriptCT = nil
 	a.scriptMutex.Unlock()
 }

waddrmgr/manager.go

@@ -27,6 +27,7 @@ import (
 	"github.com/btcsuite/btcd/wire"
 	"github.com/btcsuite/btcutil"
 	"github.com/btcsuite/btcutil/hdkeychain"
+	"github.com/btcsuite/btcwallet/internal/zero"
 	"github.com/btcsuite/btcwallet/snacl"
 	"github.com/btcsuite/btcwallet/walletdb"
 )
@@ -298,7 +299,7 @@ func (m *Manager) lock() {
 	m.masterKeyPriv.Zero()
 
 	// Zero the hashed passphrase.
-	zero(m.hashedPrivPassphrase[:])
+	zero.Bytea64(&m.hashedPrivPassphrase)
 
 	// NOTE: m.cryptoKeyPub is intentionally not cleared here as the address
 	// manager needs to be able to continue to read and decrypt public data
@@ -723,7 +724,7 @@ func (m *Manager) ChangePassphrase(oldPassphrase, newPassphrase []byte, private
 			return managerError(ErrCrypto, str, err)
 		}
 		encPriv, err := newMasterKey.Encrypt(decPriv)
-		zero(decPriv)
+		zero.Bytes(decPriv)
 		if err != nil {
 			str := "failed to encrypt crypto private key"
 			return managerError(ErrCrypto, str, err)
@@ -737,7 +738,7 @@ func (m *Manager) ChangePassphrase(oldPassphrase, newPassphrase []byte, private
 			return managerError(ErrCrypto, str, err)
 		}
 		encScript, err := newMasterKey.Encrypt(decScript)
-		zero(decScript)
+		zero.Bytes(decScript)
 		if err != nil {
 			str := "failed to encrypt crypto script key"
 			return managerError(ErrCrypto, str, err)
@@ -754,7 +755,7 @@ func (m *Manager) ChangePassphrase(oldPassphrase, newPassphrase []byte, private
 			saltedPassphrase := append(passphraseSalt[:],
 				newPassphrase...)
 			hashedPassphrase = sha512.Sum512(saltedPassphrase)
-			zero(saltedPassphrase)
+			zero.Bytes(saltedPassphrase)
 		}
 
 		// Save the new keys and params to the the db in a single
@@ -856,7 +857,7 @@ func (m *Manager) ConvertToWatchingOnly() error {
 
 	// Clear and remove all of the encrypted acount private keys.
 	for _, acctInfo := range m.acctInfo {
-		zero(acctInfo.acctKeyEncrypted)
+		zero.Bytes(acctInfo.acctKeyEncrypted)
 		acctInfo.acctKeyEncrypted = nil
 	}
 
@@ -865,19 +866,19 @@ func (m *Manager) ConvertToWatchingOnly() error {
 	for _, ma := range m.addrs {
 		switch addr := ma.(type) {
 		case *managedAddress:
-			zero(addr.privKeyEncrypted)
+			zero.Bytes(addr.privKeyEncrypted)
 			addr.privKeyEncrypted = nil
 		case *scriptAddress:
-			zero(addr.scriptEncrypted)
+			zero.Bytes(addr.scriptEncrypted)
 			addr.scriptEncrypted = nil
 		}
 	}
 
 	// Clear and remove encrypted private and script crypto keys.
-	zero(m.cryptoKeyScriptEncrypted)
+	zero.Bytes(m.cryptoKeyScriptEncrypted)
 	m.cryptoKeyScriptEncrypted = nil
 	m.cryptoKeyScript = nil
-	zero(m.cryptoKeyPrivEncrypted)
+	zero.Bytes(m.cryptoKeyPrivEncrypted)
 	m.cryptoKeyPrivEncrypted = nil
 	m.cryptoKeyPriv = nil
 
@@ -976,7 +977,7 @@ func (m *Manager) ImportPrivateKey(wif *btcutil.WIF, bs *BlockStamp) (ManagedPub
 	if !m.watchingOnly {
 		privKeyBytes := wif.PrivKey.Serialize()
 		encryptedPrivKey, err = m.cryptoKeyPriv.Encrypt(privKeyBytes)
-		zero(privKeyBytes)
+		zero.Bytes(privKeyBytes)
 		if err != nil {
 			str := fmt.Sprintf("failed to encrypt private key for %x",
 				serializedPubKey)
@@ -1196,7 +1197,7 @@ func (m *Manager) Unlock(passphrase []byte) error {
 		saltedPassphrase := append(m.privPassphraseSalt[:],
 			passphrase...)
 		hashedPassphrase := sha512.Sum512(saltedPassphrase)
-		zero(saltedPassphrase)
+		zero.Bytes(saltedPassphrase)
 		if hashedPassphrase != m.hashedPrivPassphrase {
 			m.lock()
 			str := "invalid passphrase for master private key"
@@ -1225,7 +1226,7 @@ func (m *Manager) Unlock(passphrase []byte) error {
 		return managerError(ErrCrypto, str, err)
 	}
 	m.cryptoKeyPriv.CopyBytes(decryptedKey)
-	zero(decryptedKey)
+	zero.Bytes(decryptedKey)
 
 	// Use the crypto private key to decrypt all of the account private
 	// extended keys.
@@ -1239,7 +1240,7 @@ func (m *Manager) Unlock(passphrase []byte) error {
 		}
 
 		acctKeyPriv, err := hdkeychain.NewKeyFromString(string(decrypted))
-		zero(decrypted)
+		zero.Bytes(decrypted)
 		if err != nil {
 			m.lock()
 			str := fmt.Sprintf("failed to regenerate account %d "+
@@ -1267,7 +1268,7 @@ func (m *Manager) Unlock(passphrase []byte) error {
 
 		privKeyBytes := privKey.Serialize()
 		privKeyEncrypted, err := m.cryptoKeyPriv.Encrypt(privKeyBytes)
-		zeroBigInt(privKey.D)
+		zero.BigInt(privKey.D)
 		if err != nil {
 			m.lock()
 			str := fmt.Sprintf("failed to encrypt private key for "+
@@ -1285,7 +1286,7 @@ func (m *Manager) Unlock(passphrase []byte) error {
 	m.locked = false
 	saltedPassphrase := append(m.privPassphraseSalt[:], passphrase...)
 	m.hashedPrivPassphrase = sha512.Sum512(saltedPassphrase)
-	zero(saltedPassphrase)
+	zero.Bytes(saltedPassphrase)
 	return nil
 }
 
@@ -1796,7 +1797,7 @@ func loadManager(namespace walletdb.Namespace, pubPassphrase []byte, chainParams
 		return nil, managerError(ErrCrypto, str, err)
 	}
 	cryptoKeyPub.CopyBytes(cryptoKeyPubCT)
-	zero(cryptoKeyPubCT)
+	zero.Bytes(cryptoKeyPubCT)
 
 	// Create the sync state struct.
 	syncInfo := newSyncState(startBlock, syncedTo, recentHeight, recentHashes)