Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Blockchain.info wallet import #25

Merged
merged 2 commits into from Apr 20, 2012
Merged

Blockchain.info wallet import #25

merged 2 commits into from Apr 20, 2012

Conversation

ghost
Copy link

@ghost ghost commented Apr 20, 2012

Added the ability to import .aes.json and .json backups from blockchain.info. It requires json-simple (http://code.google.com/p/json-simple/) as a dependancy.

Hopefully should be useful for both multi bit and blcockhain.info users.

The patch might have made unnecessary changes to the eclipse project, i'm not sure how to revert to the original file.

Thanks,
Ben

jim618 added a commit that referenced this pull request Apr 20, 2012
Blockchain.info wallet import
@jim618 jim618 merged commit 67886ab into Multibit-Legacy:v0.3 Apr 20, 2012
@jim618
Copy link
Contributor

jim618 commented Apr 23, 2012

Hello Ben,

Before I do my MultiBit releases I have a little check list that I go
through to check that I have not broken the basic functionality (yeah I
know unit tests are SUPPOSED to find these but it is amazing what you
find).

I test them on my Mac and an Ubuntu + Win XP virtual machines.

Anyhow, the blockchain.info wallet imports worked fine on MacOS and
Linux but not on Win XP.
After a bit of digging, it came down to this problem:

http://stackoverflow.com/questions/6481627/java-security-illegal-key-size-or-default-parameters

It is these JCE imports in MyWallet that trigger it:
import javax.crypto.Cipher;
import javax.crypto.SecretKey;
import javax.crypto.SecretKeyFactory;
import javax.crypto.spec.IvParameterSpec;
import javax.crypto.spec.PBEKeySpec;
import javax.crypto.spec.SecretKeySpec;

Bloody stupid those JCE policy files.
For the encryption I do for MultiBit (which is just the encrypted
exported private keys at the moment) I exclusively use the BouncyCastle
API code and never pull in any javax.crypto classes. This handily
avoids the problem.

I wondered if you would be interested in doing the same in your MyWallet
class ? If you look at the EncrypterDecrypter class you can see that it
is very similar classes but just typed org.bouncycastle rather than
javax.crypto

I don't really want to start copying around those JCE policy files as it
is error prone - the user just has to switch to a JRE that does not have
them and it stops working. The files are different between Java 6 and 7
too.

Anyhow, let me know what you think.
I will hold off releasing what I have at the moment - this is no problem
as the blockchain.info wallet import is the main new feature that I
wanted to get out and I would like to get it unbreakable.

Regards,

Jim

http://multibit.org Money, reinvented

@ghost
Copy link
Author

ghost commented Apr 24, 2012

Jim,

Damn sorry I missed this, I forgot I had already modified the policy files on my local machine.

I had a look at EncrypterDecrypter but can't quite get it working correctly with bouncycastle:

private static CipherParameters getAESPasswordKey(char[] password, byte[] salt) throws EncrypterDecrypterException {
    try {
        PBEParametersGenerator generator = new OpenSSLPBEParametersGenerator();
        generator.init(PBEParametersGenerator.PKCS5PasswordToBytes(password), salt, PBKDF2Iterations);

        ParametersWithIV key = (ParametersWithIV) generator.generateDerivedParameters(256, 128);

        return key;
    } catch (Exception e) {
        throw new EncrypterDecrypterException("Could not generate key from password of length " + password.length
                + " and salt '" + Utils.bytesToHexString(salt), e);
    }
}

public static String decrypt(String ciphertext, String password) {
    try {

        byte[] cipherdata = Base64.decode(ciphertext);

        //Sperate the IV and cipher data
        byte[] iv = Arrays.copyOfRange(cipherdata, 0, AESBlockSize * 4);

        byte[] input = Arrays.copyOfRange(cipherdata, AESBlockSize * 4, cipherdata.length);

        ParametersWithIV key = (ParametersWithIV) getAESPasswordKey(password.toCharArray(), iv);

        // decrypt the message
        BufferedBlockCipher cipher = new PaddedBufferedBlockCipher(new CBCBlockCipher(new AESFastEngine()), new ISO10126d2Padding());
        cipher.init(false, key);

        byte[] decryptedBytes = new byte[cipher.getOutputSize(input.length)];
        int length = cipher.processBytes(input, 0, input.length, decryptedBytes, 0); 

        cipher.doFinal(decryptedBytes, length);

        // reconstruct the original string, trimming off any whitespace
        // added by block padding
        String decryptedText = new String(decryptedBytes, "UTF-8").trim();

        return decryptedText;
    } catch (Exception e) {

        System.out.println(e.getLocalizedMessage());

        throw new EncrypterDecrypterException("Could not decrypt input string", e);
    }
}

throws the Exception "pad block corrupted". I'm not sure whether "ISO10126d2" padding differs from "ISO10126". Any ideas?

Also i'm not sure if the following might be a suitable workaround.

    Cipher cipher = Cipher.getInstance("AES/CBC/ISO10126Padding", new BouncyCastleProvider());

Sorry for delaying the release.

Cheers,
Ben

On 23 Apr 2012, at 16:45, Jim Burton wrote:

Hello Ben,

Before I do my MultiBit releases I have a little check list that I go
through to check that I have not broken the basic functionality (yeah I
know unit tests are SUPPOSED to find these but it is amazing what you
find).

I test them on my Mac and an Ubuntu + Win XP virtual machines.

Anyhow, the blockchain.info wallet imports worked fine on MacOS and
Linux but not on Win XP.
After a bit of digging, it came down to this problem:

http://stackoverflow.com/questions/6481627/java-security-illegal-key-size-or-default-parameters

It is these JCE imports in MyWallet that trigger it:
import javax.crypto.Cipher;
import javax.crypto.SecretKey;
import javax.crypto.SecretKeyFactory;
import javax.crypto.spec.IvParameterSpec;
import javax.crypto.spec.PBEKeySpec;
import javax.crypto.spec.SecretKeySpec;

Bloody stupid those JCE policy files.
For the encryption I do for MultiBit (which is just the encrypted
exported private keys at the moment) I exclusively use the BouncyCastle
API code and never pull in any javax.crypto classes. This handily
avoids the problem.

I wondered if you would be interested in doing the same in your MyWallet
class ? If you look at the EncrypterDecrypter class you can see that it
is very similar classes but just typed org.bouncycastle rather than
javax.crypto

I don't really want to start copying around those JCE policy files as it
is error prone - the user just has to switch to a JRE that does not have
them and it stops working. The files are different between Java 6 and 7
too.

Anyhow, let me know what you think.
I will hold off releasing what I have at the moment - this is no problem
as the blockchain.info wallet import is the main new feature that I
wanted to get out and I would like to get it unbreakable.

Regards,

Jim

http://multibit.org Money, reinvented


Reply to this email directly or view it on GitHub:
#25 (comment)

@jim618
Copy link
Contributor

jim618 commented Apr 24, 2012

Hi Ben,

No problem about the delay - I appreciate you will want to make sure it
is right.
I don't know the difference between that padding sorry. I think it is
one of those things that you can only be confident about once you have
roundtripped the encrypt and decrypt of several test wallets and seen it
work for real.

Yes, those policy files are something you copy and then forget all
about. It was more by luck than anything I picked it up.

It will definitely be worth removing the javax.crypto as now with your
import code (in multibit) any bitcoinj based solution can import
blockchain.info wallets with minimal effort.

On a different tangent - I have blocked the next month or two for
upgrading the wallets in multibit. Encryption etc. I hope to get
Pieter's hierarchical deterministic wallets in too. Were you planning to
add those to blockchain.info ?

I am hoping to get them into bitcoinj directly, though it is hard work
getting it into upstream projects ! :-)

I know Alan has pretty much done HD wallets in Armory so when they
appear in Satoshi 0.7 I think they will catch on pretty quickly.

To have fully interoperable deterministic wallets across pretty much
everything would be brilliant. I do not know if ThomasV is planning to
add them to Electrum - would be good if he did.

Jim
On Tue, Apr 24, 2012, at 03:30 AM, Ben Reeves wrote:

Jim,

Damn sorry I missed this, I forgot I had already modified the policy
files on my local machine.

I had a look at EncrypterDecrypter but can't quite get it working
correctly with bouncycastle:

private static CipherParameters getAESPasswordKey(char[] password,
byte[] salt) throws EncrypterDecrypterException {
    try {
        PBEParametersGenerator generator = new
        OpenSSLPBEParametersGenerator();
        generator.init(PBEParametersGenerator.PKCS5PasswordToBytes(password),
        salt, PBKDF2Iterations);

        ParametersWithIV key = (ParametersWithIV)
        generator.generateDerivedParameters(256, 128);

        return key;
    } catch (Exception e) {
        throw new EncrypterDecrypterException("Could not generate key
        from password of length " + password.length
                + " and salt '" + Utils.bytesToHexString(salt), e);
    }
}

public static String decrypt(String ciphertext, String password) {
    try {

      byte[] cipherdata = Base64.decode(ciphertext);

      //Sperate the IV and cipher data
      byte[] iv = Arrays.copyOfRange(cipherdata, 0, AESBlockSize * 4);

      byte[] input = Arrays.copyOfRange(cipherdata, AESBlockSize * 4, cipherdata.length);

        ParametersWithIV key = (ParametersWithIV)
        getAESPasswordKey(password.toCharArray(), iv);

        // decrypt the message
        BufferedBlockCipher cipher = new
        PaddedBufferedBlockCipher(new CBCBlockCipher(new
        AESFastEngine()), new ISO10126d2Padding());
        cipher.init(false, key);

        byte[] decryptedBytes = new
        byte[cipher.getOutputSize(input.length)];
        int length = cipher.processBytes(input, 0, input.length,
        decryptedBytes, 0); 

        cipher.doFinal(decryptedBytes, length);

        // reconstruct the original string, trimming off any
        whitespace
        // added by block padding
        String decryptedText = new String(decryptedBytes,
        "UTF-8").trim();

        return decryptedText;
    } catch (Exception e) {

      System.out.println(e.getLocalizedMessage());

        throw new EncrypterDecrypterException("Could not decrypt
        input string", e);
    }
}

throws the Exception "pad block corrupted". I'm not sure whether
"ISO10126d2" padding differs from "ISO10126". Any ideas?

Also i'm not sure if the following might be a suitable workaround.

  Cipher cipher = Cipher.getInstance("AES/CBC/ISO10126Padding", new BouncyCastleProvider());

Sorry for delaying the release.

Cheers,
Ben

On 23 Apr 2012, at 16:45, Jim Burton wrote:

Hello Ben,

Before I do my MultiBit releases I have a little check list that I go
through to check that I have not broken the basic functionality (yeah I
know unit tests are SUPPOSED to find these but it is amazing what you
find).

I test them on my Mac and an Ubuntu + Win XP virtual machines.

Anyhow, the blockchain.info wallet imports worked fine on MacOS and
Linux but not on Win XP.
After a bit of digging, it came down to this problem:

http://stackoverflow.com/questions/6481627/java-security-illegal-key-size-or-default-parameters

It is these JCE imports in MyWallet that trigger it:
import javax.crypto.Cipher;
import javax.crypto.SecretKey;
import javax.crypto.SecretKeyFactory;
import javax.crypto.spec.IvParameterSpec;
import javax.crypto.spec.PBEKeySpec;
import javax.crypto.spec.SecretKeySpec;

Bloody stupid those JCE policy files.
For the encryption I do for MultiBit (which is just the encrypted
exported private keys at the moment) I exclusively use the BouncyCastle
API code and never pull in any javax.crypto classes. This handily
avoids the problem.

I wondered if you would be interested in doing the same in your MyWallet
class ? If you look at the EncrypterDecrypter class you can see that it
is very similar classes but just typed org.bouncycastle rather than
javax.crypto

I don't really want to start copying around those JCE policy files as it
is error prone - the user just has to switch to a JRE that does not have
them and it stops working. The files are different between Java 6 and 7
too.

Anyhow, let me know what you think.
I will hold off releasing what I have at the moment - this is no problem
as the blockchain.info wallet import is the main new feature that I
wanted to get out and I would like to get it unbreakable.

Regards,

Jim

http://multibit.org Money, reinvented


Reply to this email directly or view it on GitHub:
#25 (comment)


Reply to this email directly or view it on GitHub:

#25 (comment)

http://multibit.org Money, reinvented

@ghost
Copy link
Author

ghost commented Apr 24, 2012

Jim,

Could you give the following patch a quick test:

https://github.com/zootreeves/multibit/commit/614b9c91bf7c8b74fd5c59e47b701b6aabfe3832

I tried it with the test wallets and my own wallet and the padding appears to be fine. But it still uses javax.crypto for PBKDF2 but i'm don't that that is subjected to the key size limitations.

I'm not sure about deterministic wallets, I need to look at the spec in more detail.

Thanks,
Ben

On 24 Apr 2012, at 11:52, Jim Burton wrote:

Hi Ben,

No problem about the delay - I appreciate you will want to make sure it
is right.
I don't know the difference between that padding sorry. I think it is
one of those things that you can only be confident about once you have
roundtripped the encrypt and decrypt of several test wallets and seen it
work for real.

Yes, those policy files are something you copy and then forget all
about. It was more by luck than anything I picked it up.

It will definitely be worth removing the javax.crypto as now with your
import code (in multibit) any bitcoinj based solution can import
blockchain.info wallets with minimal effort.

On a different tangent - I have blocked the next month or two for
upgrading the wallets in multibit. Encryption etc. I hope to get
Pieter's hierarchical deterministic wallets in too. Were you planning to
add those to blockchain.info ?

I am hoping to get them into bitcoinj directly, though it is hard work
getting it into upstream projects ! :-)

I know Alan has pretty much done HD wallets in Armory so when they
appear in Satoshi 0.7 I think they will catch on pretty quickly.

To have fully interoperable deterministic wallets across pretty much
everything would be brilliant. I do not know if ThomasV is planning to
add them to Electrum - would be good if he did.

Jim
On Tue, Apr 24, 2012, at 03:30 AM, Ben Reeves wrote:

Jim,

Damn sorry I missed this, I forgot I had already modified the policy
files on my local machine.

I had a look at EncrypterDecrypter but can't quite get it working
correctly with bouncycastle:

private static CipherParameters getAESPasswordKey(char[] password,
byte[] salt) throws EncrypterDecrypterException {
try {
PBEParametersGenerator generator = new
OpenSSLPBEParametersGenerator();
generator.init(PBEParametersGenerator.PKCS5PasswordToBytes(password),
salt, PBKDF2Iterations);

       ParametersWithIV key = (ParametersWithIV)
       generator.generateDerivedParameters(256, 128);

       return key;
   } catch (Exception e) {
       throw new EncrypterDecrypterException("Could not generate key
       from password of length " + password.length
               + " and salt '" + Utils.bytesToHexString(salt), e);
   }

}

public static String decrypt(String ciphertext, String password) {
try {

     byte[] cipherdata = Base64.decode(ciphertext);

     //Sperate the IV and cipher data
     byte[] iv = Arrays.copyOfRange(cipherdata, 0, AESBlockSize * 4);

     byte[] input = Arrays.copyOfRange(cipherdata, AESBlockSize * 4, cipherdata.length);

       ParametersWithIV key = (ParametersWithIV)
       getAESPasswordKey(password.toCharArray(), iv);

       // decrypt the message
       BufferedBlockCipher cipher = new
       PaddedBufferedBlockCipher(new CBCBlockCipher(new
       AESFastEngine()), new ISO10126d2Padding());
       cipher.init(false, key);

       byte[] decryptedBytes = new
       byte[cipher.getOutputSize(input.length)];
       int length = cipher.processBytes(input, 0, input.length,
       decryptedBytes, 0); 

       cipher.doFinal(decryptedBytes, length);

       // reconstruct the original string, trimming off any
       whitespace
       // added by block padding
       String decryptedText = new String(decryptedBytes,
       "UTF-8").trim();

       return decryptedText;
   } catch (Exception e) {

     System.out.println(e.getLocalizedMessage());

       throw new EncrypterDecrypterException("Could not decrypt
       input string", e);
   }

}

throws the Exception "pad block corrupted". I'm not sure whether
"ISO10126d2" padding differs from "ISO10126". Any ideas?

Also i'm not sure if the following might be a suitable workaround.

 Cipher cipher = Cipher.getInstance("AES/CBC/ISO10126Padding", new BouncyCastleProvider());

Sorry for delaying the release.

Cheers,
Ben

On 23 Apr 2012, at 16:45, Jim Burton wrote:

Hello Ben,

Before I do my MultiBit releases I have a little check list that I go
through to check that I have not broken the basic functionality (yeah I
know unit tests are SUPPOSED to find these but it is amazing what you
find).

I test them on my Mac and an Ubuntu + Win XP virtual machines.

Anyhow, the blockchain.info wallet imports worked fine on MacOS and
Linux but not on Win XP.
After a bit of digging, it came down to this problem:

http://stackoverflow.com/questions/6481627/java-security-illegal-key-size-or-default-parameters

It is these JCE imports in MyWallet that trigger it:
import javax.crypto.Cipher;
import javax.crypto.SecretKey;
import javax.crypto.SecretKeyFactory;
import javax.crypto.spec.IvParameterSpec;
import javax.crypto.spec.PBEKeySpec;
import javax.crypto.spec.SecretKeySpec;

Bloody stupid those JCE policy files.
For the encryption I do for MultiBit (which is just the encrypted
exported private keys at the moment) I exclusively use the BouncyCastle
API code and never pull in any javax.crypto classes. This handily
avoids the problem.

I wondered if you would be interested in doing the same in your MyWallet
class ? If you look at the EncrypterDecrypter class you can see that it
is very similar classes but just typed org.bouncycastle rather than
javax.crypto

I don't really want to start copying around those JCE policy files as it
is error prone - the user just has to switch to a JRE that does not have
them and it stops working. The files are different between Java 6 and 7
too.

Anyhow, let me know what you think.
I will hold off releasing what I have at the moment - this is no problem
as the blockchain.info wallet import is the main new feature that I
wanted to get out and I would like to get it unbreakable.

Regards,

Jim

http://multibit.org Money, reinvented


Reply to this email directly or view it on GitHub:
#25 (comment)


Reply to this email directly or view it on GitHub:

#25 (comment)

http://multibit.org Money, reinvented


Reply to this email directly or view it on GitHub:
#25 (comment)

@jim618
Copy link
Contributor

jim618 commented Apr 24, 2012

Thanks Ben,

I will have a look at that this evening and try it out.

Regards,

Jim

jim618@fastmail.co.uk

On 24 Apr 2012, at 13:19, Ben Reeves reply@reply.github.com wrote:

Jim,

Could you give the following patch a quick test:

https://github.com/zootreeves/multibit/commit/614b9c91bf7c8b74fd5c59e47b701b6aabfe3832

I tried it with the test wallets and my own wallet and the padding appears to be fine. But it still uses javax.crypto for PBKDF2 but i'm don't that that is subjected to the key size limitations.

I'm not sure about deterministic wallets, I need to look at the spec in more detail.

Thanks,
Ben

On 24 Apr 2012, at 11:52, Jim Burton wrote:

Hi Ben,

No problem about the delay - I appreciate you will want to make sure it
is right.
I don't know the difference between that padding sorry. I think it is
one of those things that you can only be confident about once you have
roundtripped the encrypt and decrypt of several test wallets and seen it
work for real.

Yes, those policy files are something you copy and then forget all
about. It was more by luck than anything I picked it up.

It will definitely be worth removing the javax.crypto as now with your
import code (in multibit) any bitcoinj based solution can import
blockchain.info wallets with minimal effort.

On a different tangent - I have blocked the next month or two for
upgrading the wallets in multibit. Encryption etc. I hope to get
Pieter's hierarchical deterministic wallets in too. Were you planning to
add those to blockchain.info ?

I am hoping to get them into bitcoinj directly, though it is hard work
getting it into upstream projects ! :-)

I know Alan has pretty much done HD wallets in Armory so when they
appear in Satoshi 0.7 I think they will catch on pretty quickly.

To have fully interoperable deterministic wallets across pretty much
everything would be brilliant. I do not know if ThomasV is planning to
add them to Electrum - would be good if he did.

Jim
On Tue, Apr 24, 2012, at 03:30 AM, Ben Reeves wrote:

Jim,

Damn sorry I missed this, I forgot I had already modified the policy
files on my local machine.

I had a look at EncrypterDecrypter but can't quite get it working
correctly with bouncycastle:

private static CipherParameters getAESPasswordKey(char[] password,
byte[] salt) throws EncrypterDecrypterException {
try {
PBEParametersGenerator generator = new
OpenSSLPBEParametersGenerator();
generator.init(PBEParametersGenerator.PKCS5PasswordToBytes(password),
salt, PBKDF2Iterations);

      ParametersWithIV key = (ParametersWithIV)
      generator.generateDerivedParameters(256, 128);

      return key;
  } catch (Exception e) {
      throw new EncrypterDecrypterException("Could not generate key
      from password of length " + password.length
              + " and salt '" + Utils.bytesToHexString(salt), e);
  }

}

public static String decrypt(String ciphertext, String password) {
try {

      byte[] cipherdata = Base64.decode(ciphertext);

      //Sperate the IV and cipher data
      byte[] iv = Arrays.copyOfRange(cipherdata, 0, AESBlockSize * 4);

      byte[] input = Arrays.copyOfRange(cipherdata, AESBlockSize * 4, cipherdata.length);

      ParametersWithIV key = (ParametersWithIV)
      getAESPasswordKey(password.toCharArray(), iv);

      // decrypt the message
      BufferedBlockCipher cipher = new
      PaddedBufferedBlockCipher(new CBCBlockCipher(new
      AESFastEngine()), new ISO10126d2Padding());
      cipher.init(false, key);

      byte[] decryptedBytes = new
      byte[cipher.getOutputSize(input.length)];
      int length = cipher.processBytes(input, 0, input.length,
      decryptedBytes, 0); 

      cipher.doFinal(decryptedBytes, length);

      // reconstruct the original string, trimming off any
      whitespace
      // added by block padding
      String decryptedText = new String(decryptedBytes,
      "UTF-8").trim();

      return decryptedText;
  } catch (Exception e) {

      System.out.println(e.getLocalizedMessage());

      throw new EncrypterDecrypterException("Could not decrypt
      input string", e);
  }

}

throws the Exception "pad block corrupted". I'm not sure whether
"ISO10126d2" padding differs from "ISO10126". Any ideas?

Also i'm not sure if the following might be a suitable workaround.

   Cipher cipher = Cipher.getInstance("AES/CBC/ISO10126Padding", new BouncyCastleProvider());

Sorry for delaying the release.

Cheers,
Ben

On 23 Apr 2012, at 16:45, Jim Burton wrote:

Hello Ben,

Before I do my MultiBit releases I have a little check list that I go
through to check that I have not broken the basic functionality (yeah I
know unit tests are SUPPOSED to find these but it is amazing what you
find).

I test them on my Mac and an Ubuntu + Win XP virtual machines.

Anyhow, the blockchain.info wallet imports worked fine on MacOS and
Linux but not on Win XP.
After a bit of digging, it came down to this problem:

http://stackoverflow.com/questions/6481627/java-security-illegal-key-size-or-default-parameters

It is these JCE imports in MyWallet that trigger it:
import javax.crypto.Cipher;
import javax.crypto.SecretKey;
import javax.crypto.SecretKeyFactory;
import javax.crypto.spec.IvParameterSpec;
import javax.crypto.spec.PBEKeySpec;
import javax.crypto.spec.SecretKeySpec;

Bloody stupid those JCE policy files.
For the encryption I do for MultiBit (which is just the encrypted
exported private keys at the moment) I exclusively use the BouncyCastle
API code and never pull in any javax.crypto classes. This handily
avoids the problem.

I wondered if you would be interested in doing the same in your MyWallet
class ? If you look at the EncrypterDecrypter class you can see that it
is very similar classes but just typed org.bouncycastle rather than
javax.crypto

I don't really want to start copying around those JCE policy files as it
is error prone - the user just has to switch to a JRE that does not have
them and it stops working. The files are different between Java 6 and 7
too.

Anyhow, let me know what you think.
I will hold off releasing what I have at the moment - this is no problem
as the blockchain.info wallet import is the main new feature that I
wanted to get out and I would like to get it unbreakable.

Regards,

Jim

http://multibit.org Money, reinvented


Reply to this email directly or view it on GitHub:
#25 (comment)


Reply to this email directly or view it on GitHub:

#25 (comment)

http://multibit.org Money, reinvented


Reply to this email directly or view it on GitHub:
#25 (comment)


Reply to this email directly or view it on GitHub:
#25 (comment)

@jim618
Copy link
Contributor

jim618 commented Apr 24, 2012

Hi Ben,

I just copied that patch into multibit and built it.
It works fine on the Win XP virtual machine that had the problem before.
Looks like it is fixed.

I am busy with something else tomorrow but will aim to get it released
on
Thursday.

Cheers,

Jim

On Tue, Apr 24, 2012, at 05:19 AM, Ben Reeves wrote:

Jim,

Could you give the following patch a quick test:

https://github.com/zootreeves/multibit/commit/614b9c91bf7c8b74fd5c59e47b701b6aabfe3832

I tried it with the test wallets and my own wallet and the padding
appears to be fine. But it still uses javax.crypto for PBKDF2 but i'm
don't that that is subjected to the key size limitations.

I'm not sure about deterministic wallets, I need to look at the spec in
more detail.

Thanks,
Ben

On 24 Apr 2012, at 11:52, Jim Burton wrote:

Hi Ben,

No problem about the delay - I appreciate you will want to make sure it
is right.
I don't know the difference between that padding sorry. I think it is
one of those things that you can only be confident about once you have
roundtripped the encrypt and decrypt of several test wallets and seen it
work for real.

Yes, those policy files are something you copy and then forget all
about. It was more by luck than anything I picked it up.

It will definitely be worth removing the javax.crypto as now with your
import code (in multibit) any bitcoinj based solution can import
blockchain.info wallets with minimal effort.

On a different tangent - I have blocked the next month or two for
upgrading the wallets in multibit. Encryption etc. I hope to get
Pieter's hierarchical deterministic wallets in too. Were you planning to
add those to blockchain.info ?

I am hoping to get them into bitcoinj directly, though it is hard work
getting it into upstream projects ! :-)

I know Alan has pretty much done HD wallets in Armory so when they
appear in Satoshi 0.7 I think they will catch on pretty quickly.

To have fully interoperable deterministic wallets across pretty much
everything would be brilliant. I do not know if ThomasV is planning to
add them to Electrum - would be good if he did.

Jim
On Tue, Apr 24, 2012, at 03:30 AM, Ben Reeves wrote:

Jim,

Damn sorry I missed this, I forgot I had already modified the policy
files on my local machine.

I had a look at EncrypterDecrypter but can't quite get it working
correctly with bouncycastle:

private static CipherParameters getAESPasswordKey(char[] password,
byte[] salt) throws EncrypterDecrypterException {
try {
PBEParametersGenerator generator = new
OpenSSLPBEParametersGenerator();
generator.init(PBEParametersGenerator.PKCS5PasswordToBytes(password),
salt, PBKDF2Iterations);

       ParametersWithIV key = (ParametersWithIV)
       generator.generateDerivedParameters(256, 128);

       return key;
   } catch (Exception e) {
       throw new EncrypterDecrypterException("Could not generate key
       from password of length " + password.length
               + " and salt '" + Utils.bytesToHexString(salt), e);
   }

}

public static String decrypt(String ciphertext, String password) {
try {

       byte[] cipherdata = Base64.decode(ciphertext);

       //Sperate the IV and cipher data
       byte[] iv = Arrays.copyOfRange(cipherdata, 0, AESBlockSize * 4);

       byte[] input = Arrays.copyOfRange(cipherdata, AESBlockSize * 4, cipherdata.length);

       ParametersWithIV key = (ParametersWithIV)
       getAESPasswordKey(password.toCharArray(), iv);

       // decrypt the message
       BufferedBlockCipher cipher = new
       PaddedBufferedBlockCipher(new CBCBlockCipher(new
       AESFastEngine()), new ISO10126d2Padding());
       cipher.init(false, key);

       byte[] decryptedBytes = new
       byte[cipher.getOutputSize(input.length)];
       int length = cipher.processBytes(input, 0, input.length,
       decryptedBytes, 0); 

       cipher.doFinal(decryptedBytes, length);

       // reconstruct the original string, trimming off any
       whitespace
       // added by block padding
       String decryptedText = new String(decryptedBytes,
       "UTF-8").trim();

       return decryptedText;
   } catch (Exception e) {

       System.out.println(e.getLocalizedMessage());

       throw new EncrypterDecrypterException("Could not decrypt
       input string", e);
   }

}

throws the Exception "pad block corrupted". I'm not sure whether
"ISO10126d2" padding differs from "ISO10126". Any ideas?

Also i'm not sure if the following might be a suitable workaround.

   Cipher cipher = Cipher.getInstance("AES/CBC/ISO10126Padding", new BouncyCastleProvider());

Sorry for delaying the release.

Cheers,
Ben

On 23 Apr 2012, at 16:45, Jim Burton wrote:

Hello Ben,

Before I do my MultiBit releases I have a little check list that I go
through to check that I have not broken the basic functionality (yeah I
know unit tests are SUPPOSED to find these but it is amazing what you
find).

I test them on my Mac and an Ubuntu + Win XP virtual machines.

Anyhow, the blockchain.info wallet imports worked fine on MacOS and
Linux but not on Win XP.
After a bit of digging, it came down to this problem:

http://stackoverflow.com/questions/6481627/java-security-illegal-key-size-or-default-parameters

It is these JCE imports in MyWallet that trigger it:
import javax.crypto.Cipher;
import javax.crypto.SecretKey;
import javax.crypto.SecretKeyFactory;
import javax.crypto.spec.IvParameterSpec;
import javax.crypto.spec.PBEKeySpec;
import javax.crypto.spec.SecretKeySpec;

Bloody stupid those JCE policy files.
For the encryption I do for MultiBit (which is just the encrypted
exported private keys at the moment) I exclusively use the BouncyCastle
API code and never pull in any javax.crypto classes. This handily
avoids the problem.

I wondered if you would be interested in doing the same in your MyWallet
class ? If you look at the EncrypterDecrypter class you can see that it
is very similar classes but just typed org.bouncycastle rather than
javax.crypto

I don't really want to start copying around those JCE policy files as it
is error prone - the user just has to switch to a JRE that does not have
them and it stops working. The files are different between Java 6 and 7
too.

Anyhow, let me know what you think.
I will hold off releasing what I have at the moment - this is no problem
as the blockchain.info wallet import is the main new feature that I
wanted to get out and I would like to get it unbreakable.

Regards,

Jim

http://multibit.org Money, reinvented


Reply to this email directly or view it on GitHub:
#25 (comment)


Reply to this email directly or view it on GitHub:

#25 (comment)

http://multibit.org Money, reinvented


Reply to this email directly or view it on GitHub:
#25 (comment)


Reply to this email directly or view it on GitHub:

#25 (comment)

http://multibit.org Money, reinvented

@ghost
Copy link
Author

ghost commented Apr 25, 2012

Awesome, thanks Jim.

On 24 Apr 2012, at 18:58, Jim Burton wrote:

Hi Ben,

I just copied that patch into multibit and built it.
It works fine on the Win XP virtual machine that had the problem before.
Looks like it is fixed.

I am busy with something else tomorrow but will aim to get it released
on
Thursday.

Cheers,

Jim

On Tue, Apr 24, 2012, at 05:19 AM, Ben Reeves wrote:

Jim,

Could you give the following patch a quick test:

https://github.com/zootreeves/multibit/commit/614b9c91bf7c8b74fd5c59e47b701b6aabfe3832

I tried it with the test wallets and my own wallet and the padding
appears to be fine. But it still uses javax.crypto for PBKDF2 but i'm
don't that that is subjected to the key size limitations.

I'm not sure about deterministic wallets, I need to look at the spec in
more detail.

Thanks,
Ben

On 24 Apr 2012, at 11:52, Jim Burton wrote:

Hi Ben,

No problem about the delay - I appreciate you will want to make sure it
is right.
I don't know the difference between that padding sorry. I think it is
one of those things that you can only be confident about once you have
roundtripped the encrypt and decrypt of several test wallets and seen it
work for real.

Yes, those policy files are something you copy and then forget all
about. It was more by luck than anything I picked it up.

It will definitely be worth removing the javax.crypto as now with your
import code (in multibit) any bitcoinj based solution can import
blockchain.info wallets with minimal effort.

On a different tangent - I have blocked the next month or two for
upgrading the wallets in multibit. Encryption etc. I hope to get
Pieter's hierarchical deterministic wallets in too. Were you planning to
add those to blockchain.info ?

I am hoping to get them into bitcoinj directly, though it is hard work
getting it into upstream projects ! :-)

I know Alan has pretty much done HD wallets in Armory so when they
appear in Satoshi 0.7 I think they will catch on pretty quickly.

To have fully interoperable deterministic wallets across pretty much
everything would be brilliant. I do not know if ThomasV is planning to
add them to Electrum - would be good if he did.

Jim
On Tue, Apr 24, 2012, at 03:30 AM, Ben Reeves wrote:

Jim,

Damn sorry I missed this, I forgot I had already modified the policy
files on my local machine.

I had a look at EncrypterDecrypter but can't quite get it working
correctly with bouncycastle:

private static CipherParameters getAESPasswordKey(char[] password,
byte[] salt) throws EncrypterDecrypterException {
try {
PBEParametersGenerator generator = new
OpenSSLPBEParametersGenerator();
generator.init(PBEParametersGenerator.PKCS5PasswordToBytes(password),
salt, PBKDF2Iterations);

      ParametersWithIV key = (ParametersWithIV)
      generator.generateDerivedParameters(256, 128);

      return key;
  } catch (Exception e) {
      throw new EncrypterDecrypterException("Could not generate key
      from password of length " + password.length
              + " and salt '" + Utils.bytesToHexString(salt), e);
  }

}

public static String decrypt(String ciphertext, String password) {
try {

   byte[] cipherdata = Base64.decode(ciphertext);

   //Sperate the IV and cipher data
   byte[] iv = Arrays.copyOfRange(cipherdata, 0, AESBlockSize * 4);

   byte[] input = Arrays.copyOfRange(cipherdata, AESBlockSize * 4, cipherdata.length);

      ParametersWithIV key = (ParametersWithIV)
      getAESPasswordKey(password.toCharArray(), iv);

      // decrypt the message
      BufferedBlockCipher cipher = new
      PaddedBufferedBlockCipher(new CBCBlockCipher(new
      AESFastEngine()), new ISO10126d2Padding());
      cipher.init(false, key);

      byte[] decryptedBytes = new
      byte[cipher.getOutputSize(input.length)];
      int length = cipher.processBytes(input, 0, input.length,
      decryptedBytes, 0); 

      cipher.doFinal(decryptedBytes, length);

      // reconstruct the original string, trimming off any
      whitespace
      // added by block padding
      String decryptedText = new String(decryptedBytes,
      "UTF-8").trim();

      return decryptedText;
  } catch (Exception e) {

   System.out.println(e.getLocalizedMessage());

      throw new EncrypterDecrypterException("Could not decrypt
      input string", e);
  }

}

throws the Exception "pad block corrupted". I'm not sure whether
"ISO10126d2" padding differs from "ISO10126". Any ideas?

Also i'm not sure if the following might be a suitable workaround.

   Cipher cipher = Cipher.getInstance("AES/CBC/ISO10126Padding", new BouncyCastleProvider());

Sorry for delaying the release.

Cheers,
Ben

On 23 Apr 2012, at 16:45, Jim Burton wrote:

Hello Ben,

Before I do my MultiBit releases I have a little check list that I go
through to check that I have not broken the basic functionality (yeah I
know unit tests are SUPPOSED to find these but it is amazing what you
find).

I test them on my Mac and an Ubuntu + Win XP virtual machines.

Anyhow, the blockchain.info wallet imports worked fine on MacOS and
Linux but not on Win XP.
After a bit of digging, it came down to this problem:

http://stackoverflow.com/questions/6481627/java-security-illegal-key-size-or-default-parameters

It is these JCE imports in MyWallet that trigger it:
import javax.crypto.Cipher;
import javax.crypto.SecretKey;
import javax.crypto.SecretKeyFactory;
import javax.crypto.spec.IvParameterSpec;
import javax.crypto.spec.PBEKeySpec;
import javax.crypto.spec.SecretKeySpec;

Bloody stupid those JCE policy files.
For the encryption I do for MultiBit (which is just the encrypted
exported private keys at the moment) I exclusively use the BouncyCastle
API code and never pull in any javax.crypto classes. This handily
avoids the problem.

I wondered if you would be interested in doing the same in your MyWallet
class ? If you look at the EncrypterDecrypter class you can see that it
is very similar classes but just typed org.bouncycastle rather than
javax.crypto

I don't really want to start copying around those JCE policy files as it
is error prone - the user just has to switch to a JRE that does not have
them and it stops working. The files are different between Java 6 and 7
too.

Anyhow, let me know what you think.
I will hold off releasing what I have at the moment - this is no problem
as the blockchain.info wallet import is the main new feature that I
wanted to get out and I would like to get it unbreakable.

Regards,

Jim

http://multibit.org Money, reinvented


Reply to this email directly or view it on GitHub:
#25 (comment)


Reply to this email directly or view it on GitHub:

#25 (comment)

http://multibit.org Money, reinvented


Reply to this email directly or view it on GitHub:
#25 (comment)


Reply to this email directly or view it on GitHub:

#25 (comment)

http://multibit.org Money, reinvented


Reply to this email directly or view it on GitHub:
#25 (comment)

@jim618
Copy link
Contributor

jim618 commented Apr 26, 2012

Hello Ben,

I have just been trying out yout iPhone blockchain.info wallet.

It is excellent!

With the option to scan a QR code I tried it out with the zoomed QR codes MultiBit produces. It scanned it fine but I noticed that you did not reuse the 'label' field, only the address and amount. When it asked if I wanted to save it to the address book I had to type in the label again - you could prepopulate the field with the bitcoin URI value to save the user having to retype.

(note: I URI encode my labels to take account of spaces and non - ASCII characters etc - the spec is a bit woolly on this - but it seemed the sensible thing to do).

I expect you are pretty busy what with the extra Forbes publicity but I wanted to mention it.

Cheers,

Jim

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant