RFR 10 (XS): 8184673: Fix compatibility issue in AlgorithmChecker for 3rd party JCE providers

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RFR 10 (XS): 8184673: Fix compatibility issue in AlgorithmChecker for 3rd party JCE providers

Langer, Christoph

Hi,

 

after the discussion in thread http://mail.openjdk.java.net/pipermail/security-dev/2017-July/016068.html, please review my proposed change:

 

Bug: https://bugs.openjdk.java.net/browse/JDK-8184673

Change:

 

diff -r 76fca9438ee9 -r 9c2438e0a823 src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java

--- a/src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java  Thu Jul 13 13:42:39 2017 +0200

+++ b/src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java  Fri Jul 14 17:35:36 2017 +0200

@@ -270,7 +270,7 @@

 

         AlgorithmParameters currSigAlgParams = algorithmId.getParameters();

         PublicKey currPubKey = cert.getPublicKey();

-        String currSigAlg = ((X509Certificate)cert).getSigAlgName();

+        currSigAlg = x509Cert.getSigAlgName();

 

         // Check the signature algorithm and parameters against constraints.

         if (!constraints.permits(SIGNATURE_PRIMITIVE_SET, currSigAlg,

 

 

Thanks and best regards

Christoph

 

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR 10 (XS): 8184673: Fix compatibility issue in AlgorithmChecker for 3rd party JCE providers

Anthony Scarpino
On 07/14/2017 08:37 AM, Langer, Christoph wrote:

> Hi,
>
> after the discussion in thread
> http://mail.openjdk.java.net/pipermail/security-dev/2017-July/016068.html,
> please review my proposed change:
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8184673
>
> Change:
>
> *diff -r 76fca9438ee9 -r 9c2438e0a823
> src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java*
>
> ---  a/src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java  
> Thu Jul 13 13:42:39 2017 +0200
> +++  b/src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java  
> Fri Jul 14 17:35:36 2017 +0200
>
> @@ -270,7 +270,7 @@
>
>           AlgorithmParameters currSigAlgParams =
> algorithmId.getParameters();
>
>          PublicKey currPubKey = cert.getPublicKey();
> -        String currSigAlg = ((X509Certificate)cert).getSigAlgName();
> +        currSigAlg = x509Cert.getSigAlgName();

I think you need to prepend with "String " to your change.

>
>           // Check the signature algorithm and parameters against constraints.
>
>           if (!constraints.permits(SIGNATURE_PRIMITIVE_SET, currSigAlg,

Otherwise it looks fine.

Tony

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR 10 (XS): 8184673: Fix compatibility issue in AlgorithmChecker for 3rd party JCE providers

Sean Mullan
It would be nice to write a regression test for this, but I suspect it
is quite a bit of work or not practical. Please consider it, or add an
appropriate noreg label to the bug.

--Sean

On 7/14/17 12:56 PM, Anthony Scarpino wrote:

> On 07/14/2017 08:37 AM, Langer, Christoph wrote:
>> Hi,
>>
>> after the discussion in thread
>> http://mail.openjdk.java.net/pipermail/security-dev/2017-July/016068.html,
>> please review my proposed change:
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8184673
>>
>> Change:
>>
>> *diff -r 76fca9438ee9 -r 9c2438e0a823
>> src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java*
>>
>>
>> ---  
>> a/src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java
>> Thu Jul 13 13:42:39 2017 +0200
>> +++  
>> b/src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java
>> Fri Jul 14 17:35:36 2017 +0200
>>
>> @@ -270,7 +270,7 @@
>>
>>           AlgorithmParameters currSigAlgParams =
>> algorithmId.getParameters();
>>
>>          PublicKey currPubKey = cert.getPublicKey();
>> -        String currSigAlg = ((X509Certificate)cert).getSigAlgName();
>> +        currSigAlg = x509Cert.getSigAlgName();
>
> I think you need to prepend with "String " to your change.
>
>>
>>           // Check the signature algorithm and parameters against
>> constraints.
>>
>>           if (!constraints.permits(SIGNATURE_PRIMITIVE_SET, currSigAlg,
>
> Otherwise it looks fine.
>
> Tony
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR 10 (XS): 8184673: Fix compatibility issue in AlgorithmChecker for 3rd party JCE providers

Anthony Scarpino
I'm working on a test so we avoid this in the future.

Tony

On 07/14/2017 11:05 AM, Sean Mullan wrote:

> It would be nice to write a regression test for this, but I suspect it
> is quite a bit of work or not practical. Please consider it, or add an
> appropriate noreg label to the bug.
>
> --Sean
>
> On 7/14/17 12:56 PM, Anthony Scarpino wrote:
>> On 07/14/2017 08:37 AM, Langer, Christoph wrote:
>>> Hi,
>>>
>>> after the discussion in thread
>>> http://mail.openjdk.java.net/pipermail/security-dev/2017-July/016068.html,
>>> please review my proposed change:
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8184673
>>>
>>> Change:
>>>
>>> *diff -r 76fca9438ee9 -r 9c2438e0a823
>>> src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java*
>>>
>>>
>>> ---
>>> a/src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java
>>> Thu Jul 13 13:42:39 2017 +0200
>>> +++
>>> b/src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java
>>> Fri Jul 14 17:35:36 2017 +0200
>>>
>>> @@ -270,7 +270,7 @@
>>>
>>>           AlgorithmParameters currSigAlgParams =
>>> algorithmId.getParameters();
>>>
>>>          PublicKey currPubKey = cert.getPublicKey();
>>> -        String currSigAlg = ((X509Certificate)cert).getSigAlgName();
>>> +        currSigAlg = x509Cert.getSigAlgName();
>>
>> I think you need to prepend with "String " to your change.
>>
>>>
>>>           // Check the signature algorithm and parameters against
>>> constraints.
>>>
>>>           if (!constraints.permits(SIGNATURE_PRIMITIVE_SET, currSigAlg,
>>
>> Otherwise it looks fine.
>>
>> Tony
>>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RE: RFR 10 (XS): 8184673: Fix compatibility issue in AlgorithmChecker for 3rd party JCE providers

Langer, Christoph
In reply to this post by Anthony Scarpino
Hi Tony

> > ---
> a/src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChe
> cker.java
> > Thu Jul 13 13:42:39 2017 +0200
> > +++
> b/src/java.base/share/classes/sun/security/provider/certpath/AlgorithmCh
> ecker.java
> > Fri Jul 14 17:35:36 2017 +0200
> >
> > @@ -270,7 +270,7 @@
> >
> >           AlgorithmParameters currSigAlgParams =
> > algorithmId.getParameters();
> >
> >          PublicKey currPubKey = cert.getPublicKey();
> > -        String currSigAlg = ((X509Certificate)cert).getSigAlgName();
> > +        currSigAlg = x509Cert.getSigAlgName();
>
> I think you need to prepend with "String " to your change.

Yes, you are right. I had some debug code before which I just removed. I did not build the new version - which I would do before submit.


> Otherwise it looks fine.

Thanks
Christoph
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RE: RFR 10 (XS): 8184673: Fix compatibility issue in AlgorithmChecker for 3rd party JCE providers

Langer, Christoph
In reply to this post by Anthony Scarpino
Hi,

> From: Anthony Scarpino [mailto:[hidden email]]

> I'm working on a test so we avoid this in the future.

OK, so, shall I submit the fix and you do the test in a separate issue? Or shall I wait and let you do it altogether?

With my limited expertise in the security area, I would not have a better idea for a test than to ask iaik if we could use their binary. Or can you write a dummy provider or modify the sun provider in a certain way?

Thanks
Christoph

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR 10 (XS): 8184673: Fix compatibility issue in AlgorithmChecker for 3rd party JCE providers

Anthony Scarpino

> On Jul 14, 2017, at 12:01 PM, Langer, Christoph <[hidden email]> wrote:
>
> Hi,
>
>> From: Anthony Scarpino [mailto:[hidden email]]
>
>> I'm working on a test so we avoid this in the future.
>
> OK, so, shall I submit the fix and you do the test in a separate issue? Or shall I wait and let you do it altogether?
>
> With my limited expertise in the security area, I would not have a better idea for a test than to ask iaik if we could use their binary. Or can you write a dummy provider or modify the sun provider in a certain way?
>
> Thanks
> Christoph
>

The test will have to go back at a later time as a separate issue.

Tony

Loading...