RFR[11] JDK-8146293 "Add Support for RSA-PSS Signature Algorithm as in PKCS#1 v2.2"

[Valerie Peng]

Hi Brad,

Thanks for the pointer on netbeans, I will use it to catch stylistic 
issues later. I updated the changes with most of your comments except 
those very minor ones (i.e. finish with . or not). Please find comments 
below:

On 4/13/2018 12:25 PM, Bradford Wetmore wrote:
>
> PSSParameterSpec.java
> ---------------------
> Maybe add trailerFieldBC(1)?
Not sure what do u want me to add. A constants for TrailerFieldBC, or else?

> 157/202: We talked about this in person, but I wanted to mention here 
> for a wider audience.  I had concerns about this typo, and any interop 
> problems this might bring.  I looked over the Bouncy Castle impl, and 
> it appears as though they also assumed it to be bytes, not bits.  Can 
> you check with the other vendors who might have their own PSS 
> implementations and verify this is not going to be a problem?  I 
> talked with our CSR lead (Joe Darcy), he doesn't think it should be a 
> problem if other impls are using bytes.
Thanks for checking on BouncyCastle, given the default is stated in the 
class javadoc to be 20 and the norm is to use byte as the unit, I feel 
the chance of other vendors using bits are very low. We can remind other 
vendors about this typo, but we should fix this regardless.

>
> RSAPrivateCrtKeySpec.java
> -------------------------
> 60/88:  Do you want to add v1.2?
>
I think you mean v2.2. Actually, I prefer to only mention the version 
information in class javadoc. Easier to maintain this way. So, I 
actually removed the version info from the method javadoc of other 
classes for consistency.

>
> SunRsaSignEntries.java
> ----------------------
> 145:  Where did you come up with this convention for your aliases?
>
>     SHA1withRSA-PSS
>
> I see Bouncy Castle[1] and Android[2] are both using:
>
>     SHA*withRSA/PSS
>     RSASSA-PSS (name from PKCS#1)
>
> [1] 
> https://github.com/bcgit/bc-java/blob/master/prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/RSA.java
> [2] https://developer.android.com/reference/java/security/Signature.html
>
I removed the <digest>withRSA-PSS aliasses and am considering removing 
the <digest>withRSAandMGF1 impls. The RSA-PSS (or RSASSA-PSS) scheme in 
PKCS#1 v2.2 passes in the digest as part of signature parameters 
(required) at runtime. Also, the oid corresponds to RSA-PSS unlike in 
PKCS#1 v1.5 where oid is defined for each digest with RSA signature 
scheme. So, I am having second thought on supporting the 
<digest>withRSAandMGF1 names. The RSA-PSS signature impl code can use 
less checks if we don't support these "friendly" names.
As for the standard name, I didn't want to use RSA/PSS as the Cipher 
transformation string uses "/" in its syntax. As for RSASSA-PSS, it is 
also a little different from what we normally use. I don't have a strong 
preference on names though. I can change it to whatever the groups' 
preference is.

Thanks,
Valerie
>
>
> On 3/27/2018 6:40 PM, Valerie Peng wrote:
>> Hi Brad,
>>
>> Can you please help review the changes for RSA-PSS support? I also 
>> added some minor enhancement which add 2 more digest algorithms for 
>> OAEP padding.
>> There are quite some changes involved. The main changes are in the 
>> SunRsaSign provider, i.e. sun.security.rsa packages. I reused 
>> existing RSAKeyFactory, RSAKeyPairGenerator, and the RSA KeyImpl 
>> classes as much as possible. However, given that RSA-PSS signatures 
>> requires parameters, I put its implementation in a separate class, 
>> i.e. RSAPSSSignature.java.
>>
>> RFE: https://bugs.openjdk.java.net/browse/JDK-8146293
>> Webrev: http://cr.openjdk.java.net/~valeriep/8146293/webrev.00/
>>
>> Existing and new regression tests have been run and result looks fine.
>> Thanks,
>> Valerie