<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p>Hi Martin,</p>
<p>A few comments:<br>
</p>
1) The newly added property is causing 41 regression tests to fail
due to permission problem. You will need to update
src/java.base/share/lib/security/default.policy<br>
<blockquote>--- a/src/java.base/share/lib/security/default.policy<br>
+++ b/src/java.base/share/lib/security/default.policy<br>
@@ -127,6 +127,7 @@<br>
permission java.lang.RuntimePermission
"accessClassInPackage.sun.nio.ch";<br>
permission java.lang.RuntimePermission
"loadLibrary.j2pkcs11";<br>
permission java.util.PropertyPermission
"sun.security.pkcs11.allowSingleThreadedModules", "read";<br>
+ permission java.util.PropertyPermission
"sun.security.pkcs11.enableNativeKeysExtraction", "read";<br>
permission java.util.PropertyPermission "os.name", "read";<br>
permission java.util.PropertyPermission "os.arch", "read";<br>
permission java.util.PropertyPermission
"jdk.crypto.KeyAgreement.legacyKDF", "read";<br>
</blockquote>
2) As for p11_keymgmt.c, I agree that the earlier version maybe too
strict to check for CKR_OK for the first two
C_GetAttributeValue(...) call. Now that we don't check the status,
we can probably remove the TRACE0 code (line 190 and 217). BTW, the
JNI method signature on line 128 and 397 need to be updated to
include the <span class="new">jobject jWrappingMech argument.<br>
<br>
</span>3) For the new system property, it will need a CSR. Here are
some pointers on CSR and its process, faq, etc. <br>
<ul>
<li><a class="moz-txt-link-freetext" href="https://wiki.openjdk.java.net/display/csr/Main">https://wiki.openjdk.java.net/display/csr/Main</a> </li>
<li><a class="moz-txt-link-freetext" href="https://wiki.openjdk.java.net/display/csr/CSR+FAQs">https://wiki.openjdk.java.net/display/csr/CSR+FAQs</a></li>
</ul>
Thanks,<br>
Valerie<br>
<br>
<div class="moz-cite-prefix">On 10/26/2018 10:57 AM, Martin Balao
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CAKZz+gcRCkHFJsRFHSHw_EJbF5MLMj_Tg-omk1ArQcA+22wNsQ@mail.gmail.com">
<meta http-equiv="content-type" content="text/html; charset=utf-8">
<div dir="ltr">
<div dir="ltr">
<div>Hi Valerie,</div>
<div><br>
</div>
<div>I fixed all previously discussed issues in Webrev.13:</div>
<div><br>
</div>
<div> * <a
href="http://cr.openjdk.java.net/%7Embalao/webrevs/6913047/6913047.webrev.13/"
moz-do-not-send="true">http://cr.openjdk.java.net/~mbalao/webrevs/6913047/6913047.webrev.13/</a></div>
<div> * <a
href="http://cr.openjdk.java.net/%7Embalao/webrevs/6913047/6913047.webrev.13.zip"
moz-do-not-send="true">http://cr.openjdk.java.net/~mbalao/webrevs/6913047/6913047.webrev.13.zip</a></div>
<div><br>
</div>
<div>In addition to that, I fixed a couple of bugs introduced
in p11_keymgmt.c.</div>
<div><br>
</div>
<div>In
Java_sun_security_pkcs11_wrapper_PKCS11_getNativeKeyInfo
function, the first call to C_GetAttributeValue (to get
CKA_CLASS, CKA_KEY_TYPE, CKA_SENSITIVE and CKA_NETSCAPE_DB
attributes) may fail because the key may not have a
CKA_NETSCAPE_DB attribute. That is fine. It just means that
we are not going to get that attribute -which does not
exist- and it won't be needed for key unwrapping.</div>
<div><br>
</div>
<div>Later in
Java_sun_security_pkcs11_wrapper_PKCS11_getNativeKeyInfo
function, a similar issue happened. The call to get buffer
lengths may return an error if one of the attributes does
not exist. This is fine because length values are obtained
anyways and based on that we were not going to query for
non-existent attributes later.</div>
<div><br>
</div>
<div>These bugs were silently making all keys not to be
extracted.</div>
<div><br>
</div>
<div>Thanks,</div>
<div>Martin.-</div>
<div><br>
</div>
</div>
</div>
</blockquote>
<br>
</body>
</html>