Skip to content

Commit 7c4591c

Browse files
committed
[refactor] clean security helpers to avoid reflection
there's one ugly case left around Cipher, shouldn't be necessary but will require a deeper review, follow up on GH-197
1 parent 4d9fd5e commit 7c4591c

File tree

1 file changed

+15
-109
lines changed

1 file changed

+15
-109
lines changed

src/main/java/org/jruby/ext/openssl/SecurityHelper.java

Lines changed: 15 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -284,8 +284,6 @@ public static KeyFactory getKeyFactory(final String algorithm)
284284

285285
static KeyFactory getKeyFactory(final String algorithm, final Provider provider)
286286
throws NoSuchAlgorithmException {
287-
KeyFactorySpi spi = (KeyFactorySpi) getImplEngine("KeyFactory", algorithm);
288-
if ( spi == null ) throw new NoSuchAlgorithmException(algorithm + " not found");
289287
return KeyFactory.getInstance(algorithm, provider);
290288
}
291289

@@ -305,28 +303,7 @@ public static KeyPairGenerator getKeyPairGenerator(final String algorithm)
305303
@SuppressWarnings("unchecked")
306304
static KeyPairGenerator getKeyPairGenerator(final String algorithm, final Provider provider)
307305
throws NoSuchAlgorithmException {
308-
final Object spi = getImplEngine("KeyPairGenerator", algorithm);
309-
if ( spi == null ) {
310-
throw new NoSuchAlgorithmException(algorithm + " KeyPairGenerator not available");
311-
}
312-
313-
final KeyPairGenerator keyPairGenerator;
314-
if ( spi instanceof KeyPairGenerator ) {
315-
keyPairGenerator = (KeyPairGenerator) spi;
316-
}
317-
else {
318-
final Class<? extends KeyPairGenerator> delegate;
319-
try {
320-
delegate = (Class<? extends KeyPairGenerator>)
321-
Class.forName(KeyPairGenerator.class.getName() + "$Delegate");
322-
} catch (ClassNotFoundException e) { throw new RuntimeException(e); }
323-
324-
keyPairGenerator = newInstance(delegate,
325-
new Class[] { KeyPairGeneratorSpi.class, String.class }, spi, algorithm
326-
);
327-
}
328-
setField(keyPairGenerator, KeyPairGenerator.class, "provider", provider);
329-
return keyPairGenerator;
306+
return KeyPairGenerator.getInstance(algorithm, provider);
330307
}
331308

332309
/**
@@ -365,26 +342,7 @@ public static MessageDigest getMessageDigest(final String algorithm) throws NoSu
365342
@SuppressWarnings("unchecked")
366343
static MessageDigest getMessageDigest(final String algorithm, final Provider provider)
367344
throws NoSuchAlgorithmException {
368-
final Object spi = getImplEngine("MessageDigest", algorithm);
369-
if ( spi == null ) throw new NoSuchAlgorithmException(algorithm + " not found");
370-
371-
final MessageDigest messageDigest;
372-
if ( spi instanceof MessageDigest ) {
373-
messageDigest = (MessageDigest) spi;
374-
}
375-
else {
376-
final Class<? extends MessageDigest> delegate;
377-
try {
378-
delegate = (Class<? extends MessageDigest>)
379-
Class.forName(MessageDigest.class.getName() + "$Delegate");
380-
} catch (ClassNotFoundException e) { throw new RuntimeException(e); }
381-
382-
messageDigest = newInstance(delegate,
383-
new Class[] { MessageDigestSpi.class, String.class }, spi, algorithm
384-
);
385-
}
386-
setField(messageDigest, MessageDigest.class, "provider", provider);
387-
return messageDigest;
345+
return MessageDigest.getInstance(algorithm, provider);
388346
}
389347

390348
public static SecureRandom getSecureRandom() {
@@ -403,13 +361,7 @@ public static SecureRandom getSecureRandom() {
403361

404362
private static SecureRandom getSecureRandom(final String algorithm, final Provider provider)
405363
throws NoSuchAlgorithmException {
406-
final SecureRandomSpi spi = (SecureRandomSpi) getImplEngine("SecureRandom", algorithm);
407-
if ( spi == null ) throw new NoSuchAlgorithmException(algorithm + " not found");
408-
409-
return newInstance(SecureRandom.class,
410-
new Class[] { SecureRandomSpi.class, Provider.class, String.class },
411-
new Object[] { spi, provider, algorithm }
412-
);
364+
return SecureRandom.getInstance(algorithm, provider);
413365
}
414366

415367
// NOTE: none (at least for BC 1.47)
@@ -481,7 +433,6 @@ private static Cipher getCipherInternal(String transformation, final Provider pr
481433

482434
spi = (CipherSpi) getImplEngine("Cipher", algorithm);
483435
if ( spi == null ) {
484-
// if ( silent ) return null;
485436
throw new NoSuchAlgorithmException(transformation + " not found");
486437
}
487438

@@ -500,14 +451,9 @@ private static Cipher getCipherInternal(String transformation, final Provider pr
500451
}
501452
try {
502453
// this constructor does not verify the provider
503-
Cipher cipher = newInstance(Cipher.class,
504-
new Class[] { CipherSpi.class, String.class },
505-
new Object[] { spi, transformation }
506-
);
507-
setField(cipher, Cipher.class, "provider", provider);
508-
return cipher;
454+
return Cipher.getInstance(transformation, provider);
509455
}
510-
catch( Exception e ) {
456+
catch (Exception e) { // TODO now seems like a redundant left over
511457
// this constructor does verify the provider which might fail
512458
return newInstance(Cipher.class,
513459
new Class[] { CipherSpi.class, Provider.class, String.class },
@@ -531,25 +477,7 @@ public static Signature getSignature(final String algorithm) throws NoSuchAlgori
531477
@SuppressWarnings("unchecked")
532478
static Signature getSignature(final String algorithm, final Provider provider)
533479
throws NoSuchAlgorithmException {
534-
final Object spi = getImplEngine("Signature", algorithm);
535-
if ( spi == null ) throw new NoSuchAlgorithmException(algorithm + " Signature not available");
536-
537-
final Signature signature;
538-
if ( spi instanceof Signature ) {
539-
signature = (Signature) spi;
540-
} else {
541-
final Class<? extends Signature> delegate;
542-
try {
543-
delegate = (Class<? extends Signature>)
544-
Class.forName(Signature.class.getName() + "$Delegate");
545-
} catch (ClassNotFoundException e) { throw new RuntimeException(e); }
546-
547-
signature = newInstance(delegate,
548-
new Class[] { SignatureSpi.class, String.class }, spi, algorithm
549-
);
550-
}
551-
setField(signature, Signature.class, "provider", provider);
552-
return signature;
480+
return Signature.getInstance(algorithm, provider);
553481
}
554482

555483
/**
@@ -572,15 +500,13 @@ static Mac getMac(final String algorithm, final Provider provider)
572500

573501
private static Mac getMac(final String algorithm, final Provider provider, boolean silent)
574502
throws NoSuchAlgorithmException {
575-
MacSpi spi = (MacSpi) getImplEngine("Mac", algorithm);
576-
if ( spi == null ) {
503+
try {
504+
return Mac.getInstance(algorithm, provider);
505+
}
506+
catch (NoSuchAlgorithmException e) {
577507
if ( silent ) return null;
578-
throw new NoSuchAlgorithmException(algorithm + " not found");
508+
throw e;
579509
}
580-
return newInstance(Mac.class,
581-
new Class[] { MacSpi.class, Provider.class, String.class },
582-
new Object[] { spi, provider, algorithm }
583-
);
584510
}
585511

586512
/**
@@ -598,13 +524,7 @@ public static KeyGenerator getKeyGenerator(final String algorithm) throws NoSuch
598524

599525
static KeyGenerator getKeyGenerator(final String algorithm, final Provider provider)
600526
throws NoSuchAlgorithmException {
601-
final KeyGeneratorSpi spi = (KeyGeneratorSpi) getImplEngine("KeyGenerator", algorithm);
602-
if ( spi == null ) throw new NoSuchAlgorithmException(algorithm + " not found");
603-
604-
return newInstance(KeyGenerator.class,
605-
new Class[] { KeyGeneratorSpi.class, Provider.class, String.class },
606-
new Object[] { spi, provider, algorithm }
607-
);
527+
return KeyGenerator.getInstance(algorithm, provider);
608528
}
609529

610530
/**
@@ -622,13 +542,7 @@ public static KeyAgreement getKeyAgreement(final String algorithm) throws NoSuch
622542

623543
static KeyAgreement getKeyAgreement(final String algorithm, final Provider provider)
624544
throws NoSuchAlgorithmException {
625-
final KeyAgreementSpi spi = (KeyAgreementSpi) getImplEngine("KeyAgreement", algorithm);
626-
if ( spi == null ) throw new NoSuchAlgorithmException(algorithm + " not found");
627-
628-
return newInstance(KeyAgreement.class,
629-
new Class[] { KeyAgreementSpi.class, Provider.class, String.class },
630-
new Object[] { spi, provider, algorithm }
631-
);
545+
return KeyAgreement.getInstance(algorithm, provider);
632546
}
633547

634548
/**
@@ -646,13 +560,7 @@ public static SecretKeyFactory getSecretKeyFactory(final String algorithm) throw
646560

647561
static SecretKeyFactory getSecretKeyFactory(final String algorithm, final Provider provider)
648562
throws NoSuchAlgorithmException {
649-
final SecretKeyFactorySpi spi = (SecretKeyFactorySpi) getImplEngine("SecretKeyFactory", algorithm);
650-
if ( spi == null ) throw new NoSuchAlgorithmException(algorithm + " not found");
651-
652-
return newInstance(SecretKeyFactory.class,
653-
new Class[] { SecretKeyFactorySpi.class, Provider.class, String.class },
654-
new Object[] { spi, provider, algorithm }
655-
);
563+
return SecretKeyFactory.getInstance(algorithm, provider);
656564
}
657565

658566
private static final String providerSSLContext; // NOTE: experimental support for using BCJSSE
@@ -738,7 +646,7 @@ static boolean verify(final X509CRL crl, final PublicKey publicKey, final boolea
738646
catch (CertException e) {
739647
throw new SignatureException(e);
740648
}
741-
// can happen if the input is DER but does not match expected strucure
649+
// can happen if the input is DER but does not match expected structure
742650
catch (ClassCastException e) {
743651
throw new SignatureException(e);
744652
}
@@ -812,8 +720,6 @@ private static Object findImplEngine(final String baseName, String algorithm) {
812720
}
813721
}
814722

815-
// the obligratory "reflection crap" :
816-
817723
private static <T> T newInstance(Class<T> klass, Class<?>[] paramTypes, Object... params) {
818724
final Constructor<T> constructor;
819725
try {

0 commit comments

Comments
 (0)