KeyGenerator uses unseeded random number generator; generates identical key pairs (Bug #135)


Added by Murray Grant almost 4 years ago. Updated almost 4 years ago.


Status:Resolved Start date:04/05/2013
Priority:Immediate Due date:
Assignee:Daniel Nauck % Done:

100%

Category:-
Target version:1.1.0

Description

The SecureRandom instance you are using in the KeyGenerator constructor does not self-initialise its seed. Thus every key pair generated is identical. And there is also no way to pass a seed into KeyGenerator.

In effect, anyone using Portable.Licensing to date will have identical key pairs. This basicly negates any security or integrity it provides. That is, you really need to fix this and get any developers using it to re-generate their key pairs!

To duplicate, run the following console app twice. You will get identical public and private keys.

    var pass = "a";
    var keygenerator = new KeyGenerator(521);
    var pair = keygenerator.GenerateKeyPair();
    var privateKey = pair.ToEncryptedPrivateKeyString(pass);
    var publicKey = pair.ToPublicKeyString();

    Console.WriteLine(privateKey);
    Console.WriteLine(publicKey);

I've worked around this by duplicating your code and calling Bouncy Castle directly. This now generates different key pairs.

            var pass = "a";   // This is read from the console.

            // From KeyGenerator constructor / GenerateKeyPair().
            var keySize = 521;
            var secureRandom = SecureRandom.GetInstance("SHA256PRNG");
            // These 4 lines are the key to generating different keys!
            var rng = new RNGCryptoServiceProvider();
            var seed = new byte[32];
            rng.GetBytes(seed);
            secureRandom.SetSeed(seed);
            var keyParams = new KeyGenerationParameters(secureRandom, keySize);
            var keyGenerator = new ECKeyPairGenerator() as IAsymmetricCipherKeyPairGenerator;
            keyGenerator.Init(keyParams);
            var pair = keyGenerator.GenerateKeyPair();

            // From KeyFactory.ToEncryptedPrivateKeyString()
            var salt = new byte[16];
            secureRandom.NextBytes(salt);
            string keyEncryptionAlgorithm = PkcsObjectIdentifiers.PbeWithShaAnd3KeyTripleDesCbc.Id;
            var privateKey = Convert.ToBase64String(
                        PrivateKeyFactory.EncryptKey(keyEncryptionAlgorithm, pass.ToCharArray(), salt, 10, pair.Private)
                        );

            // From KeyFactory.ToPublicKeyString()
            var publicKey = Convert.ToBase64String(
                        SubjectPublicKeyInfoFactory.CreateSubjectPublicKeyInfo(pair.Public)
                                               .ToAsn1Object()
                                               .GetDerEncoded()
                                               );

I'd recommend you initialise any instance of Bouncy Castle SecureRandom using RNGCryptoServiceProvider (I also found an instance of it KeyFactory.ToEncryptedPrivateKeyString()). And its probably worth allowing consumers to pass in their own SecureRandom instance (so they can control the random numbers entirely), or possibly their own seed. But I'll let you decide how you want to change your API, if at all.

Note: I have not tried duplicating this issue on other computers, so it may be environmental.

Regards
Murray


Associated revisions

Revision b3a81028
Added by Daniel Nauck almost 4 years ago

Add test to verify that generated keys are unique, refs #135

Revision c4c4c2b3
Added by Daniel Nauck almost 4 years ago

Initialize the SecureRandom generator with a seed

Also provide the ability to provide a custom seed.
Fixes #135

Revision 751f3a19
Added by Daniel Nauck almost 4 years ago

Initialize the SecureRandom generator with a seed, refs #135

Revision 4d9bbca7
Added by Daniel Nauck almost 4 years ago

Update Portable.Licensing to version 1.1.0, refs #135

See http://dev.nauck-it.de/news/5

History

Updated by Daniel Nauck almost 4 years ago

Thank you for your complete bug report. I'll fix this as soon as possible.

As long as the BouncyCastle library is not available in version 1.8 with Portable Class Library support i can't open it to the public api. Currently its merged as internal. So maybe offer a method where you can pass your seed is also enough?

  • Status changed from New to Assigned
  • Assignee set to Daniel Nauck
  • Target version set to 1.1.0

Updated by Daniel Nauck almost 4 years ago

I found the issue, its a TODO item in the code of BouncyCastle 1.7 -> SecureRandom Line 41

I'll provide an overload to pass your own seed and by default a new random seed will be generated.

Updated by Murray Grant almost 4 years ago

Thanks for the quick response, Daniel.

Not seeding the RNG in Bouncy Castle seems like a pretty big oversight to me. Oh well.

I presume you don't have access to RNGCryptoServiceProvider in the portable library world. If you can pass in a byte[] as a seed, that should cover most scenarios (and would allow callers to use whatever random generator they want as a seed). And the ThreadedSeedGenerator in Bouncy should be good enough if the developer supplies no seed.

Regards
Murray

Updated by Daniel Nauck almost 4 years ago

  • Status changed from Assigned to Resolved
  • % Done changed from 0 to 100

Also available in: Atom PDF