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