Set Default SecureRandom Algorithm - #1580
Conversation
a87d847 to
bd1ba68
Compare
bd1ba68 to
2c028ca
Compare
Add code to be able to set the Default SecureRandom algorithm from provider. fixes: IBM#1553 Signed-off-by: johnpeck-us-ibm <johnpeck@us.ibm.com>
8a4f265 to
632c68b
Compare
| + " # SecureRandom\n" | ||
| + " # =======================================================================\n" | ||
| + " #\n" | ||
| + "securerandomdefault = SHA256DRBG\n" |
There was a problem hiding this comment.
Should we follow a pattern more similar to the existing ones? Like SecureRandom.default = SHA256DRBG.
There was a problem hiding this comment.
I did this way since this was a general parameter like name, and description.
| * @param services the list of services to order | ||
| * @return a new list with the designated SecureRandom service placed first | ||
| */ | ||
| public List<ServiceDefinition> orderServices(List<ServiceDefinition> services) { |
There was a problem hiding this comment.
Is there a chance that someone will expect that the algorithm to be used by default is the one they actually specified first in the config file, instead of the one indicated with the specific property?
Is that something we'd want to allow if that property hasn't be set? And if not, we should probably document it.
There was a problem hiding this comment.
The first in the list behavior is not something that is documented anywhere that I could find. Unfortunately, with out this the default Secure Random Alg is random. In my testing I have seen it switch back and forth between the two we define.
There was a problem hiding this comment.
No, I didn't say remove this. It was more of a question for potential additional functionality if we want to or can even do it. Whether we could honour someone's order if the default wasn't set. Of course that might not even be possible.
There was a problem hiding this comment.
We can not honor the first item they set as the default if the default is not called out. I have seen this switch back ans forth randomly. We could but we would have to move away from using the Properties class.
Note: There is nothing in any documentation that talks about the first one defined is the default in any documentation. I see this only in the comments in the Oracle code.
| assertTrue (acceptableValues.contains(secureRandom.getAlgorithm()), "SecureRandom not SunJCE - " + secureRandom.getAlgorithm()); | ||
|
|
||
| //Now Put OpenJCEPlus first in the list. | ||
| Provider provider = (Provider) Class.forName("com.ibm.crypto.plus.provider.OpenJCEPlus").getDeclaredConstructor().newInstance(); |
There was a problem hiding this comment.
Why are we using reflection here?
There was a problem hiding this comment.
We don't have too, It is just one of a few ways this can be done.
There was a problem hiding this comment.
Wouldn't it be simpler to do Provider provider = new com.ibm.crypto.plus.provider.OpenJCEPlus();
There was a problem hiding this comment.
Ether way works. This does follow how we do it in our test code though.
| assertTrue (acceptableValues.contains(secureRandom.getAlgorithm()), "SecureRandom not SunJCE"); | ||
|
|
||
| //Now Put OpenJCEPlus first in the list. | ||
| Provider provider = (Provider) Class.forName("com.ibm.crypto.plus.provider.OpenJCEPlusFIPS").getDeclaredConstructor().newInstance(); |
There was a problem hiding this comment.
We don't have too, It is just one of a few ways this can be done.
| default = true | ||
| ``` | ||
|
|
||
| # Set Default Secure Random algorithm |
There was a problem hiding this comment.
Do we also need a ```properties here as well?
There was a problem hiding this comment.
Only if the configuration is not a propertyconfig file like in ProviderDefAttrs.config but is stored in a variable like in DefaultProviderAttrs.java
There was a problem hiding this comment.
No, I'm talking about the way the md file is shown. I think we need a ```properties here. Otherwise it doesn't render properly.
| - **name**: Unique identifier for the provider (required) | ||
| - **description**: Human-readable description of the provider (required) | ||
| - **default**: Whether to load default provider attributes (optional, values: `true`, `false`, `1`, `0`) | ||
| - **securerandomdefault**: Determines which Secure Random Algorithm is the default one if this provider is first in the provider list |
There was a problem hiding this comment.
Should we also document that if it's not set, it defaults to a value? That is if we decide that we won't honour user-specifies order if it's not.
There was a problem hiding this comment.
We can not determine the one that will be selected. It is random. So, not sure what to say here.
There was a problem hiding this comment.
If I'm getting this right, when someone doesn't specify this in the configuration, this defaults to SHA256DRBG. Should we mention this in this md file?
There was a problem hiding this comment.
As indicated in the comment with out this setting the default could be either SHA256DRBG or SHA512DRBG what is selected is random. It could be either one.
4 participants
Add code to be able to set the Default SecureRandom algorithm from provider.
fixes: #1553
Signed-off-by: johnpeck-us-ibm johnpeck@us.ibm.com