[PATCH] 8170876: NPE in Cipher with java.security.debug=provider
Sean Mullan
sean.mullan at oracle.com
Tue Dec 20 21:50:21 UTC 2016
On 12/20/16 4:31 PM, Adam Petcher wrote:
> 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.
Nope, you're right - I see more of the context now and it looks fine.
--Sean
>
>
> 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