On Thu, 25 Mar 2021 02:18:06 GMT, Valerie Peng <valer...@openjdk.org> wrote:
>> Ping, anyone has time to review this? > >> >> >> _Mailing list message from [Michael StJohns](mailto:mstjo...@comcast.net) on >> [security-dev](mailto:security-dev@openjdk.java.net):_ >> >> On 3/23/2021 4:15 PM, Greg Rubin wrote: >> >> > > 177: System.out.println("Testing " + ALGO); >> > > 178: c = Cipher.getInstance(ALGO, "SunJCE"); >> > > 179: for (int i = 0; i < MAX_KWP_PAD_LEN; i++) { >> > > I see that here (and earlier) we do test all padding lengths. I'd still >> > > like some KATs generated by a known good implementation to ensure that >> > > we are not just compatible with ourselves. >> >> http://csrc.nist.gov/groups/STM/cavp/documents/mac/kwtestvectors.zip has >> the NIST test vectors.? See >> https://csrc.nist.gov/CSRC/media/Projects/Cryptographic-Algorithm-Validation-Program/documents/mac/KWVS.pdf >> for details. >> >> Mike >> >> -------------- next part -------------- >> An HTML attachment was scrubbed... >> URL: >> <https://mail.openjdk.java.net/pipermail/security-dev/attachments/20210323/e1a400db/attachment.htm> > > Sure, I will add some, thanks Mike for the pointers. Hi Mike, Please find comments in line below. > KeyWrapCipher.java: > > > 212 * Sets the padding mechanism of this cipher. Currently, only > > 213 * "NoPadding" scheme is accepted for this cipher. > > 214 * > > 215 * @param padding the padding mechanism > > 216 * > > 217 * @exception NoSuchPaddingException if the requested padding > > mechanism > > 218 * does not match the supported padding scheme > > 219 */ > > 220 @Override > > 221 protected void engineSetPadding(String padding) > > 222 throws NoSuchPaddingException { > > 223 if ((this.padding == null && > > !"NoPadding".equalsIgnoreCase(padding)) || > > 224 this.padding instanceof PKCS5Padding && > > 225 "PKCS5Padding".equalsIgnoreCase(padding)) { > > 226 throw new NoSuchPaddingException(); > > 227 } > > 228 } > > Shouldn't this be rejecting PKCS5Padding? Right, will fix this and update the javadoc above as well. > Actually, I keep wondering why this isn't? AES/KW/NoPadding, > AES/KW/KWPPadding and AES/KW/AutoPadding where the latter doesn't take a > user provided IV, but figures out which of the two default IV values to > use based on whether the input is a multiple of the blocksize or not? > > Decrypting is similar - do the decryption, and check which IV you get to > figure out padded or not padded. The way I view it, both KW and KWP are modes. For interoperability, I referenced the corresponding mechanism definition from PKCS#11 v3.0 and base the java transformation on the corresponding PKCS#11 mechanisms. As for AES/KW/AutoPadding, it's an interesting idea, is there any other provider or library support it? To encrypt data of any size, we can just use AES/KWP/NoPadding. Is there additional benefit for this AutoPadding scheme? Thanks, Valerie > > Mike > > -------------- next part -------------- > An HTML attachment was scrubbed... > URL: > <https://mail.openjdk.java.net/pipermail/security-dev/attachments/20210323/0372393b/attachment-0001.htm> ------------- PR: https://git.openjdk.java.net/jdk/pull/2404