Re: [Ejbca-develop] Dangerous pattern of seeding SecureRandom instances

[Tomas G. <to...@pr...>]

Hi,

Tests are executed buy JUnit, which forks, can run individual tests in a
single JVM, runs different batches etc. So a global SecureRandom for
that I think may not be logical as tests are in different modules. Hard
to find a common base.

In production code, SecureRandom is already initialized on startup, and
self-seeded by getting the first random number (which leeds to very long
startup times if there is not enough entropy available).

(SecureRandom only self-seeds once, all instances retrieved in the same
JVM use the same base.)

Cheers,
Tomas

On 2019-03-14 14:31, Jaime Hablutzel wrote:
> 
> 
> On Wed, Mar 13, 2019 at 3:18 AM Tomas Gustavsson <to...@pr...
> <mailto:to...@pr...>> wrote:
> 
> 
>     Hi,
> 
>     For test code it can prevent things getting stuck, as self-seeding of
>     SecureRandom can take a long time, especially in VM environments,
>     depending on configuration.
> 
>     I agree it's bad template code, as someone might copy-paste it
>     somewhere. Imho it might be better to use a normal "Random" where
>     security does not matter. Then it's not mistaken for security, and
>     nobody will copy-paste the usage of SecureRandom.
> 
>     What do you think about that?
> 
> 
> What about maintaining a single global instance of a self-seeded, well
> documented (with an explanation of the warning of seeding it manually)
> SecureRandom to be used by tests and even production code?, maybe a
> thread-local to avoid the performance penalty of locking?.
> 
> That way, there wouldn't exist any misleading code that could be copied
> by mistake and the costly self-seeding wouldn't happen every time.
>  
> 
> 
>     Cheers,
>     Tomas
> 
>     On 2019-03-12 21:02, Jaime Hablutzel wrote:
>     > I'm looking the following code pattern in several places of the source
>     > code (mostly in tests):
>     >
>     > SecureRandom random = SecureRandom.getInstance("SHA1PRNG");
>     > random.setSeed(new Date().getTime());
>     > random.nextBytes(serno);
>     >
>     > Where the setSeed call just before the call to nextBytes prevents the
>     > SHA1PRNG default implementation from feeding itself from system
>     entropy,
>     > so it relies on the provided timestamp as its only source of entropy,
>     > which looks like a bad idea.
>     >
>     > Anyway, the only one production code that I see being affected by this
>     > is one overload of the method
>     > org.cesecore.util.CertTools#genSelfCertForPurpose, and even when I
>     can't
>     > find any real security impact (after a quick trace of the usages
>     of this
>     > method), wouldn't it be better to remove all the calls
>     > java.security.SecureRandom#setSeed anywhere (even in tests code)  to
>     > prevent any future problem or mistake?.
>     >
>     > Regards.
>     >
>     > --
>     > Jaime Hablutzel -  RPC 994690880
>     >
>     >
>     > _______________________________________________
>     > Ejbca-develop mailing list
>     > Ejb...@li...
>     <mailto:Ejb...@li...>
>     > https://lists.sourceforge.net/lists/listinfo/ejbca-develop
>     >
> 
> 
>     _______________________________________________
>     Ejbca-develop mailing list
>     Ejb...@li...
>     <mailto:Ejb...@li...>
>     https://lists.sourceforge.net/lists/listinfo/ejbca-develop
> 
> 
> 
> -- 
> Jaime Hablutzel -  RPC 994690880
> 
> 
> _______________________________________________
> Ejbca-develop mailing list
> Ejb...@li...
> https://lists.sourceforge.net/lists/listinfo/ejbca-develop
>