Sorry, I might be not clear enough about the usage of MoreDrbgParameters and securerandom.drbg.config. You are still using MoreDrbgParameters in ApiTest.java. For example,
131 SecureRandomParameters mParam = new MoreDrbgParameters( 132 null, mech, alg, nonce, df, (Instantiation) param); 133 Security.setProperty(DRBG_CONFIG, mech); 134 SecureRandom sr = SecureRandom.getInstance("DRBG", mParam); What I wish to see is something like Security.setProperty(DRBG_CONFIG, mech + "," + alg + "," + (df ? "use_df" : "no_df")); SecureRandom sr = SecureRandom.getInstance("DRBG", param); So you are still able to iterate through different combinations of mech + alg + df using a publicly supported API. (Nonce will not be tested, it's not a part of the API.) Thanks Max > On May 16, 2016, at 6:40 PM, Sibabrata Sahoo <sibabrata.sa...@oracle.com> > wrote: > > Hi Max, > > Please find the updated webrev: > http://cr.openjdk.java.net/~ssahoo/8141039/webrev.02/ > > The changes includes, > - ApiTest.java : Removed printing unnecessary message inside checkException() > method. > - GetInstanceTest.java > - Moved the test back into " java/security/SecureRandom/ ". > - Removed all reference to MoreDrbgParameters and EntropySource. > > > Thanks, > Siba > > -----Original Message----- > From: Wang Weijun > Sent: Thursday, May 12, 2016 9:14 AM > To: Sibabrata Sahoo > Cc: security-dev@openjdk.java.net > Subject: Re: [9] RFR: 8141039: Test Task: Develop new tests for JEP 273: > DRBG-Based SecureRandom Implementations > > >> On May 12, 2016, at 1:46 AM, Sibabrata Sahoo <sibabrata.sa...@oracle.com> >> wrote: >> >> Hi Max, >> >> Please find the updated webrev: >> http://cr.openjdk.java.net/~ssahoo/8141039/webrev.01/ >> >> Changes includes: >> - Removed otherVM option and default "securerandom.drbg.config" will get >> reset after each DRBG test run. This change affected to all test files. >> - Addressed all the following comments for GetAlgorithm.java, >> EnoughSeedTest.java, MultiThreadTest.java. >> - Moved ApiTest.java, into "sun/security/provider/SecureRandom" because it >> uses MoreDrbgParameters. >> - remove SHA1 and TDEA, 3KEYTDEA" ("3 KEY TDEA", "DESEDE") >> - corrected the logic for checkException() reported bellow. > > I don't see a difference. You print out messages in those "if (strength > n)" > blocks but there is no other action. What are the messages for? Information > or warning? In my opinion, a test should print out as little as possible > unless an error occurs. (Sometimes "Testing XYZ..." is OK). > >> - Moved GetInstanceTest.java into "sun/security/provider/SecureRandom" >> because it uses MoreDrbgParameters. >> - I am using default as well as custom entropy source just to make sure the >> extension works for getInstance() method. >> - Again, I use MoreDrbgParameter, just to make sure the instance of it can >> be acceptable by corresponding getInstance() method. > > Well, I have a different opinion. > > I think most of your tests should be categorized as functional (or > conformance) tests, so they should be focused on public APIs. In this case, > the test should set "securerandom.drbg.config" to Hash_DRBG/SHA-512 instead > of using MoreDrbgParameters to set algorithm to SHA-512. > > In fact, there is no requirement that MoreDrbgParameters must be accepted by > getInstance(). > > MoreDrbgParameters and EntropySource are implementation details. Sometimes > they are necessary (For example, CAVP tests), but you'd better avoid touching > them. I did wrote some tests on them (DRBGAlg, AbstractDrbgSpec) in an > implementor's perspective but you can ignore them. > >> - About SerializedSeedTest.java, I think the SHA1PRNG bug fix will get >> pushed to repo very soon. So the test is expected to work as the fix pushed >> into repo. But if you wish, I can change it accordingly. > > Both the SHA1PRNG setSeed bug and the DRBG synchronization bug are already > fixed and pushed. > > Thanks > Max > >