<div dir="ltr"><div>Hi Valerie,</div><div><br></div><div>Thanks for your review.</div><div><br></div><div>Here it's the new webrev updated to the new repository structure. I've also updated the testcase to use the new @module jtreg feature:</div><div><br></div><div> * <a href="http://cr.openjdk.java.net/~akasko/mbalao/jdk_8029661_tls_12_sunpkcs11/2018_02_02/8029661.webrev.02">http://cr.openjdk.java.net/~akasko/mbalao/jdk_8029661_tls_12_sunpkcs11/2018_02_02/8029661.webrev.02</a> (online)</div><div> * <a href="http://cr.openjdk.java.net/~akasko/mbalao/jdk_8029661_tls_12_sunpkcs11/2018_02_02/8029661.webrev.02.zip">http://cr.openjdk.java.net/~akasko/mbalao/jdk_8029661_tls_12_sunpkcs11/2018_02_02/8029661.webrev.02.zip</a> (download)</div><div><br></div><div>A few comments in regards to creating public APIs from sun.security.internal.spec classes.</div><div><br></div><div>Classes in jdk/src/java.base/share/classes/sun/security/internal/spec:</div><div> * TlsKeyMaterialParameterSpec.java</div><div> * TlsMasterSecretParameterSpec.java</div><div> * TlsRsaPremasterSecretParameterSpec.java</div><div> * TlsKeyMaterialSpec.java</div><div> * TlsPrfParameterSpec.java</div><div><br></div><div>Classes in jdk/src/java.base/share/classes/java/security/spec (which I assume would be the destination):</div><div> * AlgorithmParameterSpec.java</div><div> * ECGenParameterSpec.java</div><div> * InvalidParameterSpecException.java</div><div> * RSAOtherPrimeInfo.java</div><div> * DSAGenParameterSpec.java</div><div> * ECParameterSpec.java</div><div> * KeySpec.java</div><div> * ...</div><div><br></div><div>TlsRsaPremasterSecretParameterSpec class contains information about min and max SSL/TLS version and, optionally, the pre-master encoded key. This information may be useful to any 3rd party class that implements a KeyGenerator to generate RSA pre-master secrets. </div><div><br></div><div>TlsMasterSecretParameterSpec class contains information (client/server random, pre-master secret, hash algorithm, etc.) to generate a master secret from a pre-master secret.</div><div><br></div><div>TlsKeyMaterialParameterSpec class contains information (client/server random, master secret, hash algorithm, etc.) to generate keys for a session from a master secret.</div><div><br></div><div>TlsPrfParameterSpec class contains information (secret key, label, hash algorithm, etc.) to generate handshake authentication codes.</div><div><br></div><div>TlsKeyMaterialSpec class contains information about session keys. This class inherits from SecretKey class.</div><div><br></div><div>So, I agree with you: these parameters/specs may be used by a 3rd party and would be better to have them as public interfaces.</div><div><br></div><div>However, I suggest to address that in a new ticket because:</div><div> * it is not strictly inherent to SunPKCS11 + TLS 1.2 support we are providing in the context of this ticket;</div><div> * it would be more clear both in tickets documentation and repository changeset;</div><div> * this refactoring is going to go through CSR, which SunPKCS11 + TLS 1.2 support does not need; and,</div><div> * we should also evaluate how TLS 1.3 changes going to impact into this.</div><div><br></div><div>Would splitting this into a new ticket work for you?</div><div><br></div><div>Kind regards,</div><div>Martin.-</div><div class="gmail_extra"><br></div></div>