From ae6f06155a1bc93246209030d1ac46c75e5b59d8 Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Fri, 26 Oct 2018 18:01:07 -0700 Subject: [PATCH 1/3] watchtower/blob/justice_kit_test: use test.Run for sub tests --- watchtower/blob/justice_kit_test.go | 158 ++++++++++++++-------------- 1 file changed, 79 insertions(+), 79 deletions(-) diff --git a/watchtower/blob/justice_kit_test.go b/watchtower/blob/justice_kit_test.go index 4d9c7537..ae81f3cb 100644 --- a/watchtower/blob/justice_kit_test.go +++ b/watchtower/blob/justice_kit_test.go @@ -27,7 +27,7 @@ func makeSig(i int) lnwire.Sig { return sig } -var descriptorTests = []struct { +type descriptorTest struct { name string encVersion uint16 decVersion uint16 @@ -40,7 +40,9 @@ var descriptorTests = []struct { commitToRemoteSig lnwire.Sig encErr error decErr error -}{ +} + +var descriptorTests = []descriptorTest{ { name: "to-local only", encVersion: 0, @@ -89,82 +91,80 @@ var descriptorTests = []struct { // when passed invalid combinations, and that all successfully encrypted blobs // are of constant size. func TestBlobJusticeKitEncryptDecrypt(t *testing.T) { - for i, test := range descriptorTests { - boj := &blob.JusticeKit{ - RevocationPubKey: test.revPubKey, - LocalDelayPubKey: test.delayPubKey, - CSVDelay: test.csvDelay, - CommitToLocalSig: test.commitToLocalSig, - CommitToRemotePubKey: test.commitToRemotePubKey, - CommitToRemoteSig: test.commitToRemoteSig, - } - - // Generate a random encryption key for the blob. The key is - // sized at 32 byte, as in practice we will be using the remote - // party's commitment txid as the key. - key := make([]byte, blob.KeySize) - _, err := io.ReadFull(rand.Reader, key) - if err != nil { - t.Fatalf("test #%d %s -- unable to generate blob "+ - "encryption key: %v", i, test.name, err) - } - - nonce := make([]byte, blob.NonceSize) - _, err = io.ReadFull(rand.Reader, nonce) - if err != nil { - t.Fatalf("test #%d %s -- unable to generate nonce "+ - "nonce: %v", i, test.name, err) - } - - // Encrypt the blob plaintext using the generated key and - // target version for this test. - ctxt, err := boj.Encrypt(nonce, key, test.encVersion) - if err != test.encErr { - t.Fatalf("test #%d %s -- unable to encrypt blob: %v", - i, test.name, err) - } else if test.encErr != nil { - // If the test expected an encryption failure, we can - // continue to the next test. - continue - } - - // Ensure that all encrypted blobs are padded out to the same - // size: 282 bytes for version 0. - if len(ctxt) != blob.Size(test.encVersion) { - t.Fatalf("test #%d %s -- expected blob to have "+ - "size %d, got %d instead", i, test.name, - blob.Size(test.encVersion), len(ctxt)) - - } - - // Decrypt the encrypted blob, reconstructing the original - // blob plaintext from the decrypted contents. We use the target - // decryption version specified by this test case. - boj2, err := blob.Decrypt(nonce, key, ctxt, test.decVersion) - if err != test.decErr { - t.Fatalf("test #%d %s -- unable to decrypt blob: %v", - i, test.name, err) - } else if test.decErr != nil { - // If the test expected an decryption failure, we can - // continue to the next test. - continue - } - - // Check that the decrypted blob properly reports whether it has - // a to-remote output or not. - if boj2.HasCommitToRemoteOutput() != test.hasCommitToRemote { - t.Fatalf("test #%d %s -- expected blob has_to_remote "+ - "to be %v, got %v", i, test.name, - test.hasCommitToRemote, - boj2.HasCommitToRemoteOutput()) - } - - // Check that the original blob plaintext matches the - // one reconstructed from the encrypted blob. - if !reflect.DeepEqual(boj, boj2) { - t.Fatalf("test #%d %s -- decrypted plaintext does not "+ - "match original, want: %v, got %v", - i, test.name, boj, boj2) - } + for _, test := range descriptorTests { + t.Run(test.name, func(t *testing.T) { + testBlobJusticeKitEncryptDecrypt(t, test) + }) + } +} + +func testBlobJusticeKitEncryptDecrypt(t *testing.T, test descriptorTest) { + boj := &blob.JusticeKit{ + RevocationPubKey: test.revPubKey, + LocalDelayPubKey: test.delayPubKey, + CSVDelay: test.csvDelay, + CommitToLocalSig: test.commitToLocalSig, + CommitToRemotePubKey: test.commitToRemotePubKey, + CommitToRemoteSig: test.commitToRemoteSig, + } + + // Generate a random encryption key for the blob. The key is + // sized at 32 byte, as in practice we will be using the remote + // party's commitment txid as the key. + key := make([]byte, blob.KeySize) + _, err := io.ReadFull(rand.Reader, key) + if err != nil { + t.Fatalf("unable to generate blob encryption key: %v", err) + } + + nonce := make([]byte, blob.NonceSize) + _, err = io.ReadFull(rand.Reader, nonce) + if err != nil { + t.Fatalf("unable to generate nonce nonce: %v", err) + } + + // Encrypt the blob plaintext using the generated key and + // target version for this test. + ctxt, err := boj.Encrypt(nonce, key, test.encVersion) + if err != test.encErr { + t.Fatalf("unable to encrypt blob: %v", err) + } else if test.encErr != nil { + // If the test expected an encryption failure, we can + // continue to the next test. + return + } + + // Ensure that all encrypted blobs are padded out to the same + // size: 282 bytes for version 0. + if len(ctxt) != blob.Size(test.encVersion) { + t.Fatalf("expected blob to have size %d, got %d instead", + blob.Size(test.encVersion), len(ctxt)) + + } + + // Decrypt the encrypted blob, reconstructing the original + // blob plaintext from the decrypted contents. We use the target + // decryption version specified by this test case. + boj2, err := blob.Decrypt(nonce, key, ctxt, test.decVersion) + if err != test.decErr { + t.Fatalf("unable to decrypt blob: %v", err) + } else if test.decErr != nil { + // If the test expected an decryption failure, we can + // continue to the next test. + return + } + + // Check that the decrypted blob properly reports whether it has + // a to-remote output or not. + if boj2.HasCommitToRemoteOutput() != test.hasCommitToRemote { + t.Fatalf("expected blob has_to_remote to be %v, got %v", + test.hasCommitToRemote, boj2.HasCommitToRemoteOutput()) + } + + // Check that the original blob plaintext matches the + // one reconstructed from the encrypted blob. + if !reflect.DeepEqual(boj, boj2) { + t.Fatalf("decrypted plaintext does not match original, "+ + "want: %v, got %v", boj, boj2) } } From b7d811b3dd1fb50e0fb0d22560fdd30daf1eb89d Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Mon, 29 Oct 2018 14:20:21 -0700 Subject: [PATCH 2/3] watchtower/blob/justice_kit: add variable length sweep addr This commit fixes an oversight in the previous design of the watchtower blob, by introducing a length byte for sweep addresses. The previous format supposed that addresses would be padded to 42 bytes, but had no indication of the address's actual length. To rememdy this, we introduce a single byte indicating the actual size of the address, such that the padding can be removed upon decoding. --- watchtower/blob/justice_kit.go | 69 ++++++++++++++++++++++++++++------ 1 file changed, 58 insertions(+), 11 deletions(-) diff --git a/watchtower/blob/justice_kit.go b/watchtower/blob/justice_kit.go index c61cf50e..e436fdbb 100644 --- a/watchtower/blob/justice_kit.go +++ b/watchtower/blob/justice_kit.go @@ -33,14 +33,19 @@ const ( CiphertextExpansion = 16 // V0PlaintextSize is the plaintext size of a version 0 encoded blob. - // sweep address: 42 bytes + // sweep address length: 1 byte + // padded sweep address: 42 bytes // revocation pubkey: 33 bytes // local delay pubkey: 33 bytes // csv delay: 4 bytes // commit to-local revocation sig: 64 bytes // commit to-remote pubkey: 33 bytes, maybe blank // commit to-remote sig: 64 bytes, maybe blank - V0PlaintextSize = 273 + V0PlaintextSize = 274 + + // MaxSweepAddrSize defines the maximum sweep address size that can be + // encoded in a blob. + MaxSweepAddrSize = 42 ) // Size returns the size of the encoded-and-encrypted blob in bytes. @@ -89,6 +94,14 @@ var ( ErrNoCommitToRemoteOutput = errors.New( "cannot obtain commit to-remote p2wkh output script from blob", ) + + // ErrSweepAddressToLong is returned when trying to encode or decode a + // sweep address with length greater than the maximum length of 42 + // bytes, which supports p2wkh and p2sh addresses. + ErrSweepAddressToLong = fmt.Errorf( + "sweep address must be less than or equal to %d bytes long", + MaxSweepAddrSize, + ) ) // PubKey is a 33-byte, serialized compressed public key. @@ -108,7 +121,7 @@ type JusticeKit struct { // // NOTE: This is chosen to be the length of a maximally sized witness // program. - SweepAddress [42]byte + SweepAddress []byte // RevocationPubKey is the compressed pubkey that guards the revocation // clause of the remote party's to-local output. @@ -318,10 +331,11 @@ func (b *JusticeKit) decode(r io.Reader, ver uint16) error { // encodeV0 encodes the JusticeKit using the version 0 encoding scheme to the // provided io.Writer. The encoding supports sweeping of the commit to-local // output, and optionally the commit to-remote output. The encoding produces a -// constant-size plaintext size of 273 bytes. +// constant-size plaintext size of 274 bytes. // // blob version 0 plaintext encoding: -// sweep address: 42 bytes +// sweep address length: 1 byte +// padded sweep address: 42 bytes // revocation pubkey: 33 bytes // local delay pubkey: 33 bytes // csv delay: 4 bytes @@ -329,8 +343,23 @@ func (b *JusticeKit) decode(r io.Reader, ver uint16) error { // commit to-remote pubkey: 33 bytes, maybe blank // commit to-remote sig: 64 bytes, maybe blank func (b *JusticeKit) encodeV0(w io.Writer) error { - // Write 42-byte sweep address where client funds will be deposited. - _, err := w.Write(b.SweepAddress[:]) + // Assert the sweep address length is sane. + if len(b.SweepAddress) > MaxSweepAddrSize { + return ErrSweepAddressToLong + } + + // Write the actual length of the sweep address as a single byte. + err := binary.Write(w, byteOrder, uint8(len(b.SweepAddress))) + if err != nil { + return err + } + + // Pad the sweep address to our maximum length of 42 bytes. + var sweepAddressBuf [MaxSweepAddrSize]byte + copy(sweepAddressBuf[:], b.SweepAddress) + + // Write padded 42-byte sweep address. + _, err = w.Write(sweepAddressBuf[:]) if err != nil { return err } @@ -371,12 +400,13 @@ func (b *JusticeKit) encodeV0(w io.Writer) error { } // decodeV0 reconstructs a JusticeKit from the io.Reader, using version 0 -// encoding scheme. This will parse a constant size input stream of 273 bytes to +// encoding scheme. This will parse a constant size input stream of 274 bytes to // recover information for the commit to-local output, and possibly the commit // to-remote output. // // blob version 0 plaintext encoding: -// sweep address: 42 bytes +// sweep address length: 1 byte +// padded sweep address: 42 bytes // revocation pubkey: 33 bytes // local delay pubkey: 33 bytes // csv delay: 4 bytes @@ -384,12 +414,29 @@ func (b *JusticeKit) encodeV0(w io.Writer) error { // commit to-remote pubkey: 33 bytes, maybe blank // commit to-remote sig: 64 bytes, maybe blank func (b *JusticeKit) decodeV0(r io.Reader) error { - // Read 42-byte sweep address where client funds will be deposited. - _, err := io.ReadFull(r, b.SweepAddress[:]) + // Read the sweep address length as a single byte. + var sweepAddrLen uint8 + err := binary.Read(r, byteOrder, &sweepAddrLen) if err != nil { return err } + // Assert the sweep address length is sane. + if sweepAddrLen > MaxSweepAddrSize { + return ErrSweepAddressToLong + } + + // Read padded 42-byte sweep address. + var sweepAddressBuf [MaxSweepAddrSize]byte + _, err = io.ReadFull(r, sweepAddressBuf[:]) + if err != nil { + return err + } + + // Parse sweep address from padded buffer. + b.SweepAddress = make([]byte, sweepAddrLen) + copy(b.SweepAddress, sweepAddressBuf[:]) + // Read 33-byte revocation public key. _, err = io.ReadFull(r, b.RevocationPubKey[:]) if err != nil { From 2255ce17db9d9e062affe524c148370078319d04 Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Mon, 29 Oct 2018 14:23:31 -0700 Subject: [PATCH 3/3] watchtower/blob/justice_kit_test: add sweep addr tests Adds vectors to the justice kit tests to ensure variable length sweep addresses are properly encoded/decoded. --- watchtower/blob/justice_kit_test.go | 46 +++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/watchtower/blob/justice_kit_test.go b/watchtower/blob/justice_kit_test.go index ae81f3cb..606e43e5 100644 --- a/watchtower/blob/justice_kit_test.go +++ b/watchtower/blob/justice_kit_test.go @@ -27,10 +27,20 @@ func makeSig(i int) lnwire.Sig { return sig } +func makeAddr(size int) []byte { + addr := make([]byte, size) + if _, err := io.ReadFull(rand.Reader, addr); err != nil { + panic("unable to create addr") + } + + return addr +} + type descriptorTest struct { name string encVersion uint16 decVersion uint16 + sweepAddr []byte revPubKey blob.PubKey delayPubKey blob.PubKey csvDelay uint32 @@ -47,6 +57,7 @@ var descriptorTests = []descriptorTest{ name: "to-local only", encVersion: 0, decVersion: 0, + sweepAddr: makeAddr(22), revPubKey: makePubKey(0), delayPubKey: makePubKey(1), csvDelay: 144, @@ -56,6 +67,7 @@ var descriptorTests = []descriptorTest{ name: "to-local and p2wkh", encVersion: 0, decVersion: 0, + sweepAddr: makeAddr(22), revPubKey: makePubKey(0), delayPubKey: makePubKey(1), csvDelay: 144, @@ -68,6 +80,7 @@ var descriptorTests = []descriptorTest{ name: "unknown encrypt version", encVersion: 1, decVersion: 0, + sweepAddr: makeAddr(34), revPubKey: makePubKey(0), delayPubKey: makePubKey(1), csvDelay: 144, @@ -78,12 +91,44 @@ var descriptorTests = []descriptorTest{ name: "unknown decrypt version", encVersion: 0, decVersion: 1, + sweepAddr: makeAddr(34), revPubKey: makePubKey(0), delayPubKey: makePubKey(1), csvDelay: 144, commitToLocalSig: makeSig(1), decErr: blob.ErrUnknownBlobVersion, }, + { + name: "sweep addr length zero", + encVersion: 0, + decVersion: 0, + sweepAddr: makeAddr(0), + revPubKey: makePubKey(0), + delayPubKey: makePubKey(1), + csvDelay: 144, + commitToLocalSig: makeSig(1), + }, + { + name: "sweep addr max size", + encVersion: 0, + decVersion: 0, + sweepAddr: makeAddr(blob.MaxSweepAddrSize), + revPubKey: makePubKey(0), + delayPubKey: makePubKey(1), + csvDelay: 144, + commitToLocalSig: makeSig(1), + }, + { + name: "sweep addr too long", + encVersion: 0, + decVersion: 0, + sweepAddr: makeAddr(blob.MaxSweepAddrSize + 1), + revPubKey: makePubKey(0), + delayPubKey: makePubKey(1), + csvDelay: 144, + commitToLocalSig: makeSig(1), + encErr: blob.ErrSweepAddressToLong, + }, } // TestBlobJusticeKitEncryptDecrypt asserts that encrypting and decrypting a @@ -100,6 +145,7 @@ func TestBlobJusticeKitEncryptDecrypt(t *testing.T) { func testBlobJusticeKitEncryptDecrypt(t *testing.T, test descriptorTest) { boj := &blob.JusticeKit{ + SweepAddress: test.sweepAddr, RevocationPubKey: test.revPubKey, LocalDelayPubKey: test.delayPubKey, CSVDelay: test.csvDelay,