Code Review Request 8049321 Support SHA256WithDSA in JSSE

[Sean Mullan]

This explanation makes sense to me and the fix looks fine.

--Sean

On 12/17/2015 01:51 AM, Xuelei Fan wrote:
> On 12/17/2015 7:52 AM, Bradford Wetmore wrote:
>>
>>
>> On 12/16/2015 3:22 PM, Xuelei Fan wrote:
>>> On 12/17/2015 3:14 AM, Bradford Wetmore wrote:
>>>> The change itself looks ok, but a question on the previous code.
>>>>
>>>> 420:  Why is SHA224 disabled when SunMSCAPI is present?  Or
>>>> alternatively, why is SHA224 enabled when SunMSCAPI not present?
>>> SunMSCAPI does not support SHA-224 yet.
>>
>> That was my guess, however...
>>
>>>> Shouldn't this be based on whether there is a SHA224 implementation
>>>> available?
>>> The SHA-224 is always available with JDK providers.  But SHA224withRSA
>>> and SHA224withDSA does not work for SunMSCAPI.  So we need to filter out
>>> SHA-224 for SunMSCAPI.
>>
>>          if (Security.getProvider("SunMSCAPI") == null) {
>>              supports(HashAlgorithm.SHA224,      SignatureAlgorithm.DSA,
>>                   "SHA224withDSA",        --p);
>>
>> Those two algs are always being filtered out regardless of whether other
>> providers might support it.  For all platforms, the Sun provider is
>> available for SHA224withDSA/SHA256withDSA, right?
>>
>>      map.put("Signature.SHA224withDSA",
>>          "sun.security.provider.DSA$SHA224withDSA");
>>      map.put("Signature.SHA256withDSA",
>>          "sun.security.provider.DSA$SHA256withDSA");
>>
>> So if SunMSCAPI is registered as an active provider (on a Windows
>> platform), SHA224/SHA256 will be disabled.  This doesn't seem right.
>>
> Right.  It is a compromise solution to remove SHA224 completely, even
> other providers support it.   If SHA224withRSA is enabled, and SunMSCAPI
> is preferred at the same time, there is a problem per current
> implementation.  During TLS handshaking process, RSA private will be
> used in SunMSCAPI provider at first (for example, for server
> authentication cert), and then SHA224withRSA in other providers (Say
> SunJCE) will be used for signature.  However, as the RSA private key is
> in MSCAPI, no way to use SHA224withRSA in providers other than SunMSCAP.
>   Problem happens, see JDK-8064330.
>
> We might want to take SHA224withRSA back later if required.
>
>>>> And if so, why are we not verifying that an implementation
>>>> exists (getInstance("SHA256") doesn't throw exception) for the other
>>>> algorithms also (SHA1/RSA/etc)?
>>>>
>>> Need to check the full signature name.  But no checking at present as
>>> these algorithms are supported by JDK providers except the SHA-224 based
>>> ones.  May make improvement later.
>>
>> This does require that at least the Sun provider always be available then.
>>
> May be, if SunJCE is used.
>
> Thanks,
> Xuelei
>
>> Brad
>>
>>
>>
>>>
>>> Thanks,
>>> Xuelei
>>>
>>>> The synopsis should probably be:
>>>>
>>>>       Support SHA224withDSA/SHA256withDSA in TLSv1.2 \
>>>>       signature_algorithms extension
>>>>
>>>> Also, note the case of the "W" in "SHA256WithDSA".
>>>>
>>>> Brad
>>>>
>>>>
>>>> On 12/14/2015 9:47 PM, Xuelei Fan wrote:
>>>>> On 12/15/2015 1:40 PM, Xuelei Fan wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Please this enhancement to the JSSE implementation:
>>>>>>
>>>>> Please review this enhancement to the JSSE implementation:
>>>>>
>>>>>>       http://cr.openjdk.java.net/~xuelei/8049321/webrev.00/
>>>>>>
>>>>>> This change will add support for the SHA224withDSA and SHA256withDSA
>>>>>> algorithms in the TLS "signature_algorithms" extension in the SunJSSE
>>>>>> provider.  Note that this extension does not apply to TLS 1.1 and
>>>>>> previous versions.
>>>>>>
>>>>>> Thanks,
>>>>>> Xuelei
>>>>>>
>>>>>
>>>
>