WalletProtobufSerializer to handle delimited reading/writing

[GoogleCodeExporter]

While working on the new backup format I've hit the next limitation. Due to 0's 
appended to the serialized form during encryption/base64 encoding (padding), 
the wallet cannot be loaded.

It tries to load the 0 as a tag which is invalid.

Either we find a way to end loading when a 0 tag is read (preferred but maybe 
not possible).

Or I propose to add a pair of readDelimitedWallet/writeDelimitedWallet to the 
serializer that internally uses protobuf's writeDelimitedTo/readDelimitedFrom.

Original issue reported on code.google.com by andreas....@gmail.com on 5 Apr 2014 at 11:20

[GoogleCodeExporter]

Strange, I don't think that either encrypting or base64 encoding should be 
padding with zeros.

Original comment by c1.devra...@niftybox.net on 6 Apr 2014 at 3:08

[GoogleCodeExporter]

I printed all the steps of encryption and decryption. The plaintext in this 
example is "plain".getBytes().

=== encrypt
plain:       [112, 108, 97, 105, 110]
cipher:      [40, 109, -2, 89, -19, -101, 97, 5, -128, -78, 105, 45, -60, -70, 
-73, -23, 104, -92, -52, -25, 31, 118, -42, 97]
cipher+salt: [83, 97, 108, 116, 101, 100, 95, 95, 40, 109, -2, 89, -19, -101, 
97, 5, -128, -78, 105, 45, -60, -70, -73, -23, 104, -92, -52, -25, 31, 118, 
-42, 97]
base64:      U2FsdGVkX18obf5Z7ZthBYCyaS3EurfpaKTM5x921mE
=== decrypt
base64:      U2FsdGVkX18obf5Z7ZthBYCyaS3EurfpaKTM5x921mE
cipher+salt: [83, 97, 108, 116, 101, 100, 95, 95, 40, 109, -2, 89, -19, -101, 
97, 5, -128, -78, 105, 45, -60, -70, -73, -23, 104, -92, -52, -25, 31, 118, 
-42, 97]
cipher:      [40, 109, -2, 89, -19, -101, 97, 5, -128, -78, 105, 45, -60, -70, 
-73, -23, 104, -92, -52, -25, 31, 118, -42, 97]
plain:       [112, 108, 97, 105, 110, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]

As you can see, it's the encryption/decryption steps that pad the plaintext to 
128 bits. Isn't that a property of block ciphers? Do you still think it 
shouldn't be padded?

My code should be functionally equivalent to what MultiBit uses. I remember I 
changed it a little, like using another Base64 implementation and changing 
methods to static.

Original comment by andreas....@gmail.com on 6 Apr 2014 at 8:11

[GoogleCodeExporter]

In the output above, the "cipher" outputs actually contain 8 bytes of random 
salt.

I looked at the encryption code and found this:

// The following code uses an AES cipher to encrypt the message.
final BufferedBlockCipher cipher = new PaddedBufferedBlockCipher(new 
CBCBlockCipher(new AESFastEngine()));
cipher.init(true, key);
final byte[] encryptedBytes = new 
byte[cipher.getOutputSize(plainTextAsBytes.length)];
final int length = cipher.processBytes(plainTextAsBytes, 0, 
plainTextAsBytes.length, encryptedBytes, 0);

cipher.doFinal(encryptedBytes, length);

That really looks to me like it is being padded. )-:

Original comment by andreas....@gmail.com on 6 Apr 2014 at 8:19

[GoogleCodeExporter]

I investigated a bit into if its possible to just end serialization if a 0 tag 
is read.

https://developers.google.com/protocol-buffers/docs/reference/java/com/google/pr
otobuf/CodedInputStream#readTag%28%29

Coincidentally, this method returns 0 on EOF (so I assume all code calling it 
will already do the "right thing"). However, unfortunately if a real 0 is read 
as a tag, the method throws an exception. Grrrr...

Original comment by andreas....@gmail.com on 6 Apr 2014 at 10:32

[GoogleCodeExporter]

PaddedBufferedBlockCipher uses PKCS7 by default.  See:

https://en.wikipedia.org/wiki/Padding_%28cryptography%29#PKCS7

The padding basically consists of "n" bytes, each of which is "n".  E.g. if 
there are 4 padding bytes, the padding will be [4,4,4,4]. The padding should 
have been removed by the decryption process in bouncycastle.

Original comment by c1.devra...@niftybox.net on 6 Apr 2014 at 2:24

[GoogleCodeExporter]

Well, from my output it's pretty obvious that it doesn't work like you say. 
Instead, it pads with zeroes and the block size is 128 bits (16 bytes).

I have pushed my code to a temporary branch -- the Crypto class can be reviewed 
here:
https://github.com/schildbach/bitcoin-wallet/blob/debug-crypto-padding/wallet/sr
c/de/schildbach/wallet/util/Crypto.java

Original comment by andreas....@gmail.com on 6 Apr 2014 at 2:53

[GoogleCodeExporter]

On decryption, cipher.doFinal returns the output size minus the padding.  You 
have to truncate the output byte array to this length.

Original comment by c1.devra...@niftybox.net on 6 Apr 2014 at 5:18

[GoogleCodeExporter]

Thanks for the hint. I just found and fixed the issue independently.

Original comment by andreas....@gmail.com on 6 Apr 2014 at 6:22