[EXTERNAL]Re: SecureRandom regression with certain security providers

[Valerie Peng]

BTW, I have tentatively filed 
https://bugs.openjdk.java.net/browse/JDK-8248885 for Entrust NSAE 
problem. Just FYI.

Valerie

On 7/6/2020 12:07 PM, Valerie Peng wrote:
>
> Hi John,
>
> Thanks for looking into this on your end. It's interesting how Entrust 
> has to do this deletion/re-insertion of providers and it's interesting 
> that adding a new instance of Entrust provider inside the 
> Security.insertProviderAt() call makes this problem go away.
>
> Please find my questions and comments in line below.
>
> On 7/3/2020 1:13 PM, John Gray wrote:
>>
>> Thanks for your comments!  They sparked off a lot more investigation 
>> on my end.   I created a test provider and could not reproduce the 
>> issue.   That led me to investigate how our provider was being 
>> installed.   We use our own internal Initializer() class to install 
>> providers in various orders (we have had to work around bugs in 
>> different JVM's in the past).   That work-around required we remove 
>> the provider from the Security provider list (basically to clean it 
>> out), then we run a simple crypto test with a new instantiation, and 
>> then install that provider in 1st position.
>>
> Does this Initializer() class does all this before the new 
> SecureRandom() call? Does the Entrust provider remove or changes its 
> registrations ever, i.e. is the provider mutable? One possible 
> scenario for legacy provider which add/remove registrations is that 
> every update to the legacy map will leads to new re-parsing and new 
> service being created as a result which may leads to failing the check 
> inside Service.newInstance() call and thus the NSAE.
>
>> If I change the highlighted line above (the last line) to the 
>> following, it works.
>>
>> Security.insertProviderAt(new Entrust(), 1);
>>
>> Having to make such a change seems strange.    It seems that creating 
>> a new provider, using it to get an instance of an algorithm, and then 
>> adding that same provider into first position doesn’t work.   I'm 
>> guessing because of the recent changes you made the provider can’t be 
>> used before it is inserted into the provider order because it may 
>> hold onto some data from the previous usage?   So this led me to 
>> investigate some more…..
>>
> Yes, it's indeed strange. Is the "entrustCsp" instance being modified 
> in anyway after its creation?
>
>> When it fails, the type and algorithm are “SecureRandom” and 
>> “DRBGUsingSHA512”
>>
> Is “DRBGUsingSHA512" the expected default algorithm for Entrust 
> provider? Is it being picked up as expected if basing on registration 
> ordering?
>
>> The Provider.getService() code fails to match the “previousKey” 
>> ServiceKey type and algorithms. In my test code I was testing an AES 
>> algorithm, so the previous key type and Algorithm is “Cipher” and 
>> “AES/CBC/PKCS5PADDING” in the getService() call which doesn’t match 
>> the type “SecureRandom” and “DRBGUsingSHA512”.   So it looks like 
>> there is a bug caused by holding on to existing data.
>>
> The previousKey is just an optimization to avoid repetitive allocation 
> on the same type and algorithm. If either of these two does not match, 
> it will be discarded and new key object created for subsequent calls. 
> So, this should not be the root cause.
>>
>> So I think when I create a brand new Entrust() instance it works 
>> because the previous ServiceKey() contains the correct data and it 
>> matches. Debugging showed it to work that way.   So I think using a 
>> provider before installing it in the provider order is what is 
>> causing the issue because of internal data in the Provider class.
>>
> There is something deeper for the Entrust NSAE problem instead of the 
> previousKey usage per my comment above. Could you please double check 
> the Initializer class and whether the Entrust provider entries are 
> modified after it's constructed and when new SecureRandom() is called?
>
> Thanks for looking into this~
>
> Valerie
>
>> It looks like I **could** put in this weird work-around (just create 
>> a fresh instance of Entrust()) when installing the provider to work 
>> around the issue, but I wonder if there will be other consequences 
>> because of the way this previousKey is used?    I can make the simple 
>> change to our toolkit without breaking FIPS (the initialization class 
>> is not in the FIPS boundary).   In fact, I assume I don’t need to 
>> keep that old work-around for the old IBM JVM anymore either..
>>
>> Thanks for your help!
>>
>> Happy July 4^th   (I live in Ottawa Canada, so we had our muted 
>> Canada day celebrations a couple days ago on July 1^st ).
>>
>> John Gray
>>
>> -----Original Message-----
>> From: Valerie Peng [mailto:valerie.peng at oracle.com]
>> Sent: Thursday, July 2, 2020 8:34 PM
>> To: John Gray <John.Gray at entrustdatacard.com>; 
>> security-dev at openjdk.java.net
>> Cc: John Mahoney <John.Mahoney at entrustdatacard.com>; Muthu Kannappan 
>> <muthu at entrustdatacard.com>
>> Subject: Re: [EXTERNAL]Re: SecureRandom regression with certain 
>> security providers
>>
>> Hi John,
>>
>> Unfortunately this cannot wait til July 13th if this issue needs to 
>> be fixed for jdk 15.
>>
>> Maybe you can try the webrev out or share more details on how Entrust 
>> provider does its registration and what Provider APIs it overrides. I 
>> need more info to help identifying the trigger for the NSAE in 
>> Entrust's case. I have verified that the current webrev works with 
>> BCFIPS provider.
>>
>> Regards and an early happy July 4th,
>>
>> Valerie
>>
>> On 7/2/2020 3:17 PM, Valerie Peng wrote:
>>
>> > I can certainly help you verify the fix.   Let me know how I can help
>>
>> > verify it for you. 😊
>>
>> >
>>
>> > Note:   I will be on vacation next week, so I'll be back July 13th...
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20200706/ee8953db/attachment.htm>