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

Add option to randomize RandomData #155

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ph4r05
Copy link
Contributor

@ph4r05 ph4r05 commented May 19, 2020

Add option to randomize RandomData seed:

  • if the System property com.licel.jcardsim.randomdata.seed is set, the hex-decoded value of the property is added as a seed material to the RandomData on initialization
  • else if the System property com.licel.jcardsim.randomdata.secure is set to 1, the SecureRandom is used to generate 32 random bytes that are added as a seed material to the RandomData
  • else the original behavior is preserved to be consistent with previous versions (some tests might rely on the fixed randomness)
  • Addresses also the Random numbers appear not to be random #139, I've chosen a bit different approach to Make SecureRandom securely random #141 mainly to preserve backward compatibility as some users may rely on the fixed seed

- if the System property `com.licel.jcardsim.randomdata.seed` is set, the hex-decoded value of the property is added as a seed material to the RandomData on initialization
- else if the System property `com.licel.jcardsim.randomdata.secure` is set to `1`, the SecureRandom is used to generate 32 random bytes that are added as a seed material to the RandomData
- else the original behavior is preserved to be consistent with previous versions (some tests might rely on the fixed randomness)
@frankmorgner
Copy link

Having a secure random number generator breaks your use case 😱? I wonder what you are doing, actually 🤔

I suggest you create a PR adding an insecure number generator...

@ph4r05
Copy link
Contributor Author

ph4r05 commented May 19, 2020

Having a secure random number generator breaks your use case 😱? I wonder what you are doing, actually 🤔

I suggest you create a PR adding an insecure number generator...

I have absolutely no idea what are you talking about @frankmorgner.

Currently (master), the RandomData is statically seeded so it generates the same random data stream after each applet load.

And yes, there may be users of the JCardSim who actually rely on this original behavior as the tests may be fixed, i.e., JCardSim usage in test cases produce deterministically always the same results, which is a normal practice.

I preserve this behavior in my PR and on top of that I enable to a) specify your own seed to have a different and still deterministic random streams and b) to seed the stream securely from the SecureRandom

@ph4r05
Copy link
Contributor Author

ph4r05 commented May 19, 2020

Maintainers are free to pick which PR suits them the best, either #141 breaking the backward compatibility with users using existing test cases (which is unmerged since Jan 2019) or this one with backward compatibility.

I incorporated my PR in my own fork https://github.com/ph4r05/jcardsim which I am using in my projects so basically I don't need to push my solution to the problem. It's not my fight to pick the best solution ;)

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.

None yet

2 participants