code review request: 7171982 Cipher getParameters() throws RuntimeException: Cannot find SunJCE provider

Brad Wetmore bradford.wetmore at oracle.com
Thu Mar 28 21:34:30 UTC 2013


(Vinnie, what do you think about the SunJCE item below?)

On 3/22/2013 11:57 AM, Anthony Scarpino wrote:
> Hi all,
>
> I need a code review for below webrev.  The changes are to have SunJCE
> call itself, using it's current instance, for checking such things as
> parameters, instead of searching through the provider list or creating a
> one time instance.
>
> http://cr.openjdk.java.net/~mullan/webrevs/ascarpin/webrev.00/

PBES1Core.java
==============
173:  indention problem.  Should be at the same level as (algo...)

PBES2Core.java:173
PKCS12PBECipherCore.java:147
SealedObjectForKeyProtector:50/57
========================
Indention problem. Normally 4 spaces unless you're trying to line it up 
with something.

SealedObjectForKeyProtector.java
================================
54/57:  In general, you should initCause() everywhere you possibly can. 
  This will help people (us) debug the real underlying root cause, 
instead of just the top-level error message.

SunJCE.java
===========
781:  Your code could race during initialization and potentially have 
many SunJCE instances active at once.

Either make instance a volatile (will reduce some of the race 
opportunity), or instead, add locking around assignment/use.  You may 
still be creating multiple SunJCEs, but only one instance will ever be 
obtained from getInstance:

     synchronized (SunJCE.class) {
         if (instance == null) {
             instance = this;
         }
     }

and

     static SunJCE getInstance() {
         if (instance == null) {
             new SunJCE();
         }
         synchronized (SunJCE.class) {
             return instance;
         }
     }

Also, when you get ready to push, be sure to address also the closed 
side: that is, please remember to build/integrate the signed 
sunjce_provider.jar file in the closed repo.

HTH,

Brad





More information about the security-dev mailing list