commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Marcelo Vanzin (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CRYPTO-133) OpenSslCryptoRandomNative.nextRandBytes not thread safe
Date Tue, 14 Feb 2017 17:30:41 GMT

    [ https://issues.apache.org/jira/browse/CRYPTO-133?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15866192#comment-15866192
] 

Marcelo Vanzin commented on CRYPTO-133:
---------------------------------------

There are hooks to make OpenSSL calls thread safe, and commons-crypto seems to install those
hooks (and the code looked fine when I looked at it). But maybe there are issues in some specific
environment, which is why I asked for that info; test works fine in my version even with the
thread safety hooks removed.

Anyway, yes, it's hard to completely guarantee thread safety when you're depending on code
you do not fully control for that. If there's some bug in some version of OpenSSL, there's
not much that can be done (aside from making the Java method synchronized).

> OpenSslCryptoRandomNative.nextRandBytes not thread safe
> -------------------------------------------------------
>
>                 Key: CRYPTO-133
>                 URL: https://issues.apache.org/jira/browse/CRYPTO-133
>             Project: Commons Crypto
>          Issue Type: Bug
>            Reporter: Hendrik Saly
>
> Seems that AbstractRandomTest.testRandomBytesMultiThreaded is failing for OpenSslCryptoRandomNative.nextRandBytes.
> Testcase throws exceptions like
> {code}
> java.lang.IllegalArgumentException: The nextRandBytes method failed
> 	at org.apache.commons.crypto.random.OpenSslCryptoRandom.nextBytes(OpenSslCryptoRandom.java:108)
> 	at org.apache.commons.crypto.random.AbstractRandomTest.checkRandomBytes(AbstractRandomTest.java:94)
> 	at org.apache.commons.crypto.random.AbstractRandomTest.access$000(AbstractRandomTest.java:30)
> 	at org.apache.commons.crypto.random.AbstractRandomTest$1.run(AbstractRandomTest.java:63)
> {code}
> When adding a 'synchronized' modifier to OpenSslCryptoRandomNative.nextRandBytes it works.
> So IMHO there are two bugs that need to be resolved:
> 1) fix testcase AbstractRandomTest.testRandomBytesMultiThreaded in that way that it fails
when exception are thrown
> 2) fix OpenSslCryptoRandomNative.nextRandBytes no be thread safe (of course not by adding
'synchronized', seems like locks_setup() is broken somehow in https://github.com/apache/commons-crypto/blob/master/src/main/native/org/apache/commons/crypto/random/OpenSslCryptoRandomNative.c#L299

> The testcase can be fixed with something like this
> {code}
>     @Test(timeout = 120000)
>     public void testRandomBytesMultiThreaded() throws Exception {
>         final int threadCount = 100;
>         final AtomicBoolean hasErrors = new AtomicBoolean();
>         try (final CryptoRandom random = getCryptoRandom()) {
>             final List<Thread> threads = new ArrayList<>(threadCount);
>             for (int i = 0; i < threadCount; i++) {
>                 Thread t = new Thread(new Runnable() {
>                     @Override
>                     public void run() {
>                         try {
> 							checkRandomBytes(random, 10);
> 							checkRandomBytes(random, 1000);
> 							checkRandomBytes(random, 100000);
> 						} catch (Exception e) {
> 							hasErrors.set(true);
> 							e.printStackTrace();
> 						}
>                     }
>                 });
>                 t.start();
>                 threads.add(t);
>             }
>             for (Thread t : threads) {
>                 if (!t.getState().equals(State.NEW)) {
>                     t.join();
>                 }
>             }
>             
>             if(hasErrors.get()) {
>             	Assert.fail();
>             }
>         }
>     }
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Mime
View raw message