[PATCH] 8170876: NPE in Cipher with java.security.debug=provider
Adam Petcher
adam.petcher at oracle.com
Tue Dec 20 21:31:29 UTC 2016
KeyPairGenerator didn't require any changes because that class does not
reference its provider member. KeyPairGenerator.Delegate has similar
code that prints the name of its provider, but this is not an instance
of the pattern that I was addressing, and I don't have any reason to
believe there is anything wrong with it.
Though maybe I missed something here. If you still think this class
needs a modification, please point me to a line number that has this issue.
On 12/20/2016 4:02 PM, Sean Mullan wrote:
> I think you missed java.security.KeyPairGenerator which has the same
> issue. Otherwise looks good.
>
> --Sean
>
> On 12/15/16 3:08 PM, Adam Petcher wrote:
>> I'm continuing my quest to rid engine classes of NullPointerException
>> due to calling getName() on a null provider. This patch fixes Cipher
>> (which fails in a test using NullCipher) and all other engine classes
>> that have this pattern. I found them by searching the codebase for
>> references to Provider::getName. This fix does not address NPEs caused
>> by calls to other methods of Provider, or calls that occur outside the
>> engine classes (e.g. someEngine.getProvider().getName()).
>>
>> I augmented an existing test so it will check for the NPE in Cipher. The
>> diff also containsa fix for a small typo in the NullProvider test for
>> Signature.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8170876
>>
>> Diff:
>>
>> diff --git a/src/java.base/share/classes/java/security/KeyStore.java
>> b/src/java.base/share/classes/java/security/KeyStore.java
>> --- a/src/java.base/share/classes/java/security/KeyStore.java
>> +++ b/src/java.base/share/classes/java/security/KeyStore.java
>> @@ -824,10 +824,14 @@
>>
>> if (!skipDebug && pdebug != null) {
>> pdebug.println("KeyStore." + type.toUpperCase() + " type
>> from: " +
>> - this.provider.getName());
>> + getProviderName());
>> }
>> }
>>
>> + private String getProviderName() {
>> + return (provider == null) ? "(no provider)" :
>> provider.getName();
>> + }
>> +
>> /**
>> * Returns a keystore object of the specified type.
>> *
>> diff --git
>> a/src/java.base/share/classes/java/security/MessageDigest.java
>> b/src/java.base/share/classes/java/security/MessageDigest.java
>> --- a/src/java.base/share/classes/java/security/MessageDigest.java
>> +++ b/src/java.base/share/classes/java/security/MessageDigest.java
>> @@ -430,13 +430,17 @@
>> return digest();
>> }
>>
>> + private String getProviderName() {
>> + return (provider == null) ? "(no provider)" :
>> provider.getName();
>> + }
>> +
>> /**
>> * Returns a string representation of this message digest object.
>> */
>> public String toString() {
>> ByteArrayOutputStream baos = new ByteArrayOutputStream();
>> PrintStream p = new PrintStream(baos);
>> - p.print(algorithm+" Message Digest from "+provider.getName()+",
>> ");
>> + p.print(algorithm+" Message Digest from
>> "+getProviderName()+", ");
>> switch (state) {
>> case INITIAL:
>> p.print("<initialized>");
>> diff --git a/src/java.base/share/classes/java/security/SecureRandom.java
>> b/src/java.base/share/classes/java/security/SecureRandom.java
>> --- a/src/java.base/share/classes/java/security/SecureRandom.java
>> +++ b/src/java.base/share/classes/java/security/SecureRandom.java
>> @@ -310,10 +310,14 @@
>>
>> if (!skipDebug && pdebug != null) {
>> pdebug.println("SecureRandom." + algorithm +
>> - " algorithm from: " + this.provider.getName());
>> + " algorithm from: " + getProviderName());
>> }
>> }
>>
>> + private String getProviderName() {
>> + return (provider == null) ? "(no provider)" :
>> provider.getName();
>> + }
>> +
>> /**
>> * Returns a {@code SecureRandom} object that implements the
>> specified
>> * Random Number Generator (RNG) algorithm.
>> diff --git a/src/java.base/share/classes/javax/crypto/Cipher.java
>> b/src/java.base/share/classes/javax/crypto/Cipher.java
>> --- a/src/java.base/share/classes/javax/crypto/Cipher.java
>> +++ b/src/java.base/share/classes/javax/crypto/Cipher.java
>> @@ -611,6 +611,10 @@
>> return getInstance(transformation, p);
>> }
>>
>> + private String getProviderName() {
>> + return (provider == null) ? "(no provider)" :
>> provider.getName();
>> + }
>> +
>> /**
>> * Returns a {@code Cipher} object that implements the specified
>> * transformation.
>> @@ -1278,7 +1282,7 @@
>> if (!skipDebug && pdebug != null) {
>> pdebug.println("Cipher." + transformation + " " +
>> getOpmodeString(opmode) + " algorithm from: " +
>> - this.provider.getName());
>> + getProviderName());
>> }
>> }
>>
>> @@ -1421,7 +1425,7 @@
>> if (!skipDebug && pdebug != null) {
>> pdebug.println("Cipher." + transformation + " " +
>> getOpmodeString(opmode) + " algorithm from: " +
>> - this.provider.getName());
>> + getProviderName());
>> }
>> }
>>
>> @@ -1564,7 +1568,7 @@
>> if (!skipDebug && pdebug != null) {
>> pdebug.println("Cipher." + transformation + " " +
>> getOpmodeString(opmode) + " algorithm from: " +
>> - this.provider.getName());
>> + getProviderName());
>> }
>> }
>>
>> @@ -1754,7 +1758,7 @@
>> if (!skipDebug && pdebug != null) {
>> pdebug.println("Cipher." + transformation + " " +
>> getOpmodeString(opmode) + " algorithm from: " +
>> - this.provider.getName());
>> + getProviderName());
>> }
>> }
>>
>> diff --git a/src/java.base/share/classes/javax/crypto/KeyAgreement.java
>> b/src/java.base/share/classes/javax/crypto/KeyAgreement.java
>> --- a/src/java.base/share/classes/javax/crypto/KeyAgreement.java
>> +++ b/src/java.base/share/classes/javax/crypto/KeyAgreement.java
>> @@ -484,7 +484,7 @@
>>
>> if (!skipDebug && pdebug != null) {
>> pdebug.println("KeyAgreement." + algorithm + " algorithm
>> from: " +
>> - this.provider.getName());
>> + getProviderName());
>> }
>> }
>>
>> @@ -517,6 +517,10 @@
>> init(key, params, JceSecurity.RANDOM);
>> }
>>
>> + private String getProviderName() {
>> + return (provider == null) ? "(no provider)" :
>> provider.getName();
>> + }
>> +
>> /**
>> * Initializes this key agreement with the given key, set of
>> * algorithm parameters, and source of randomness.
>> @@ -545,7 +549,7 @@
>>
>> if (!skipDebug && pdebug != null) {
>> pdebug.println("KeyAgreement." + algorithm + " algorithm
>> from: " +
>> - this.provider.getName());
>> + getProviderName());
>> }
>> }
>>
>> diff --git a/src/java.base/share/classes/javax/crypto/KeyGenerator.java
>> b/src/java.base/share/classes/javax/crypto/KeyGenerator.java
>> --- a/src/java.base/share/classes/javax/crypto/KeyGenerator.java
>> +++ b/src/java.base/share/classes/javax/crypto/KeyGenerator.java
>> @@ -154,7 +154,7 @@
>>
>> if (!skipDebug && pdebug != null) {
>> pdebug.println("KeyGenerator." + algorithm + " algorithm
>> from: " +
>> - this.provider.getName());
>> + getProviderName());
>> }
>> }
>>
>> @@ -172,10 +172,14 @@
>>
>> if (!skipDebug && pdebug != null) {
>> pdebug.println("KeyGenerator." + algorithm + " algorithm
>> from: " +
>> - this.provider.getName());
>> + getProviderName());
>> }
>> }
>>
>> + private String getProviderName() {
>> + return (provider == null) ? "(no provider)" :
>> provider.getName();
>> + }
>> +
>> /**
>> * Returns the algorithm name of this {@code KeyGenerator} object.
>> *
>> diff --git a/src/java.base/share/classes/javax/crypto/Mac.java
>> b/src/java.base/share/classes/javax/crypto/Mac.java
>> --- a/src/java.base/share/classes/javax/crypto/Mac.java
>> +++ b/src/java.base/share/classes/javax/crypto/Mac.java
>> @@ -415,6 +415,10 @@
>> return spi.engineGetMacLength();
>> }
>>
>> + private String getProviderName() {
>> + return (provider == null) ? "(no provider)" :
>> provider.getName();
>> + }
>> +
>> /**
>> * Initializes this {@code Mac} object with the given key.
>> *
>> @@ -437,7 +441,7 @@
>>
>> if (!skipDebug && pdebug != null) {
>> pdebug.println("Mac." + algorithm + " algorithm from: " +
>> - this.provider.getName());
>> + getProviderName());
>> }
>> }
>>
>> @@ -464,7 +468,7 @@
>>
>> if (!skipDebug && pdebug != null) {
>> pdebug.println("Mac." + algorithm + " algorithm from: " +
>> - this.provider.getName());
>> + getProviderName());
>> }
>> }
>>
>> diff --git a/test/java/security/Signature/NoProvider.java
>> b/test/java/security/Signature/NoProvider.java
>> --- a/test/java/security/Signature/NoProvider.java
>> +++ b/test/java/security/Signature/NoProvider.java
>> @@ -25,7 +25,7 @@
>> * @test
>> * @bug 8165751
>> * @summary Verify that that a subclass of Signature that does not
>> contain a
>> - * provider can be used verify.
>> + * provider can be used to verify.
>> * @run main/othervm -Djava.security.debug=provider NoProvider
>> */
>>
>> diff --git a/test/javax/crypto/NullCipher/TestNPE.java
>> b/test/javax/crypto/NullCipher/TestNPE.java
>> --- a/test/javax/crypto/NullCipher/TestNPE.java
>> +++ b/test/javax/crypto/NullCipher/TestNPE.java
>> @@ -1,5 +1,5 @@
>> /*
>> - * Copyright (c) 2003, 2007, Oracle and/or its affiliates. All rights
>> reserved.
>> + * Copyright (c) 2003, 2016, Oracle and/or its affiliates. All rights
>> reserved.
>> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>> *
>> * This code is free software; you can redistribute it and/or modify it
>> @@ -23,10 +23,12 @@
>>
>> /*
>> * @test
>> - * @bug 4937853
>> + * @bug 4937853 8170876
>> * @summary Make sure normal calls of NullCipher does not throw NPE.
>> * @author Valerie Peng
>> * @key randomness
>> + * @run main TestNPE
>> + * @run main/othervm -Djava.security.debug=provider TestNPE
>> */
>> import java.util.Arrays;
>> import java.security.AlgorithmParameters;
>>
More information about the security-dev
mailing list