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

[java] New performance rule: Reuse / share SecureRandom instance #3222

Open
milossesic opened this issue Apr 16, 2021 · 4 comments
Open

[java] New performance rule: Reuse / share SecureRandom instance #3222

milossesic opened this issue Apr 16, 2021 · 4 comments
Labels
a:new-rule Proposal to add a new built-in rule

Comments

@milossesic
Copy link

milossesic commented Apr 16, 2021

Proposed Rule Name: ReuseSecureRandom

Proposed Category: Performance

Description: A rule that would make sure that you should only be using the primed static SecureRandom instance (this is mainly because the application, if the object is not being reused, is getting slow and I mean like 90 seconds slower to do the seeding and all other stuff that does in the background, so the solution is to create SecureRandom and reuse it if required, creating new SecureRandom impacts performance of the application dramatically).

Code Sample: This should include code, that should be flagged by the rule. If possible, the "correct" code
according to this new rule should also be demonstrated.

package com.example.demo.random;

import java.security.Provider;
import java.security.SecureRandom;
import java.security.Security;
import org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;


public class FooCorrect {
	
	 private static SecureRandom randomGen = null;

	  private static Provider fipsProvider = null;

	  static {

	    fipsProvider = Security.getProvider(BouncyCastleFipsProvider.PROVIDER_NAME);
	    if (fipsProvider == null) {
	      fipsProvider = new BouncyCastleFipsProvider();
	      Security.addProvider(fipsProvider);
	    }

	    try {
	      randomGen = SecureRandom.getInstance("NONCEANDIV", FooCorrect.getFipsProvider());
	      randomGen.setSeed(randomGen.generateSeed(256));
	    } catch (final Exception e) {
	      throw new ExceptionInInitializerError(e);
	    }
	  }

	  public static SecureRandom getRandomGen() {
	    return randomGen;
	  }

	  public static Provider getFipsProvider() {
	    return fipsProvider;
	  }
	}

Negative example:

package com.example.demo.random;

import java.security.Provider;
import java.security.SecureRandom;
import java.security.Security;
import org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;


public class FooIncorrect {

	   Provider fipsProvider = new BouncyCastleFipsProvider();
           {
	   fipsProvider = Security.getProvider(BouncyCastleFipsProvider.PROVIDER_NAME);
	   Security.addProvider(fipsProvider);
	   try {
	    SecureRandom randomGen = new SecureRandom();
	    randomGen = SecureRandom.getInstance("NONCEANDIV", fipsProvider);
	    randomGen.setSeed(randomGen.generateSeed(256));
	      
	   }  catch (final Exception e) {
		      throw new ExceptionInInitializerError(e);
		}
	}
}

Possible Properties:

  • Should this rule be customizable via properties?
    Not sure about the properties, what do you think?
@milossesic milossesic added the a:new-rule Proposal to add a new built-in rule label Apr 16, 2021
@oowekyala oowekyala changed the title New security rule : Primed static SecureRandom instance [java] New security rule: Primed static SecureRandom instance Apr 17, 2021
@linusjf

This comment has been minimized.

@milossesic

This comment has been minimized.

@adangel
Copy link
Member

adangel commented Apr 23, 2021

If I understand this correctly, the goal would be to reuse a instance of SecureRandom rather than always creating a fresh one when needed. This helps in performance, because then the reseeding (which may block on calls to nextBytes()) occurs less often.
With that, I'd suggest the category "performance" and a rule name like "ReuseSecureRandom" or "ShareSecureRandom". Making it static is just one implementation solution to share the secure random.

@milossesic
Copy link
Author

Done @adangel

@adangel adangel changed the title [java] New security rule: Primed static SecureRandom instance [java] New performance rule: Reuse / share SecureRandom instance Apr 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:new-rule Proposal to add a new built-in rule
Projects
None yet
Development

No branches or pull requests

3 participants