<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p><br>
</p>
I am ok with the idea of putting the public API refactoring under a
separate RFE.<br>
Let me apply your updated patch below and run existing regressions
again.<br>
Thanks,<br>
Valerie<br>
<br>
<div class="moz-cite-prefix">On 2/5/2018 5:51 AM, Martin Balao
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CAKZz+gdR_B_sqoQ+3xUx855BX_BDD81fsCxvd-oXo4O2xdihGQ@mail.gmail.com">
<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/%7Eakasko/mbalao/jdk_8029661_tls_12_sunpkcs11/2018_02_02/8029661.webrev.02"
moz-do-not-send="true">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/%7Eakasko/mbalao/jdk_8029661_tls_12_sunpkcs11/2018_02_02/8029661.webrev.02.zip"
moz-do-not-send="true">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>
</blockquote>
<br>
</body>
</html>