<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">On 1/21/2019 7:06 PM, Weijun Wang
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:4C678867-E241-4887-843D-AA35769E34CE@oracle.com"><br>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">On Jan 22, 2019, at 10:33 AM, Xuelei Fan <a class="moz-txt-link-rfc2396E" href="mailto:xuelei.fan@oracle.com"><xuelei.fan@oracle.com></a> wrote:
On 1/21/2019 4:38 PM, Weijun Wang wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">So what do you think of my original webrev? It only compares KID and subject/issuer, not caring about other extensions (like BC).
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">The original webrev looks right to me except that I'm not sure if a new AuthorityKeyIdentifierExtension was needed. Is it sufficient to use the octet string of the DER value?
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
The struct of AuthorityKeyIdentifier and SubjectKeyIdentifier is a little different. By using the AuthorityKeyIdentifierExtension class, I don't need to extract the field myself.
AuthorityKeyIdentifier ::= SEQUENCE {
keyIdentifier [0] KeyIdentifier OPTIONAL,
authorityCertIssuer [1] GeneralNames OPTIONAL,
authorityCertSerialNumber [2] CertificateSerialNumber OPTIONAL }
SubjectKeyIdentifier ::= KeyIdentifier
and since getExtensionValue() returns the extension value encoded as an OCTET STRING, I will also need to extract the content inside.
I also cannot call the X509CertImpl methods directly because it's only X509Certificate here.
</pre>
</blockquote>
<p>I see. Thanks!<br>
</p>
<blockquote type="cite"
cite="mid:4C678867-E241-4887-843D-AA35769E34CE@oracle.com"><br>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">It may need to selectors to use the X509CertSelector, for issuers w/o AKID. I will leave it to you for the final decision.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
I'll either need to go thru all certs twice or remember the fallback one like what I did in the current loop. It doesn't make much difference.
</pre>
</blockquote>
<p>I meant to use the certs one time.<br>
</p>
<p>for (X509Certificate cert : allCerts) {</p>
<p> if (cert has SKID) {</p>
<p> // use the selector with SKID<br>
</p>
<p> } else {</p>
<p> // use the selector without SKID<br>
</p>
<p> }<br>
</p>
<p>}<br>
</p>
<p>The benefit is limited as you have to construct the AKID.</p>
<p>I'm fine and have no more comments if you want to go with
webrev.00.</p>
<p>Xuelei<br>
</p>
<p><br>
</p>
<blockquote type="cite"
cite="mid:4C678867-E241-4887-843D-AA35769E34CE@oracle.com">
<pre class="moz-quote-pre" wrap="">
Thanks,
Max
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">Xuelei
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">Thanks,
Max
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">On Jan 22, 2019, at 1:39 AM, Xuelei Fan <a class="moz-txt-link-rfc2396E" href="mailto:xuelei.fan@oracle.com"><xuelei.fan@oracle.com></a>
wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">but it seems it cannot deal with the case where a cert has the correct subject but no SKID extension. Or do you think this should never happen?
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">It could happen, especially for self-signed cert. See also, the sun.security.provider.certpath.ForwardBuilder#PKIXCertComparator.
Xuelei
On 1/21/2019 2:05 AM, Weijun Wang wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">;
but it seems it cannot deal with the case where a cert has the correct subject but no SKID extension. Or do you think this should never happen?
Thanks
Max
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">On Jan 17, 2019, at 11:41 AM, Weijun Wang <a class="moz-txt-link-rfc2396E" href="mailto:weijun.wang@oracle.com"><weijun.wang@oracle.com></a>
wrote:
I'll take a look. I thought java.security.cert.X509CertSelector is used by CertPath validators and builders internally and never thought it can be called directly.
Thanks,
Max
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">On Jan 17, 2019, at 1:49 AM, Xuelei Fan <a class="moz-txt-link-rfc2396E" href="mailto:xuelei.fan@oracle.com"><xuelei.fan@oracle.com></a>
wrote:
Hi Max,
I did not look into the detailed implementation of findIssuer() yet. Have you considered to use java.security.cert.X509CertSelector?
Thanks,
Xuelei
On 1/9/2019 6:59 AM, Weijun Wang wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">Please take a review at
<a class="moz-txt-link-freetext" href="https://cr.openjdk.java.net/~weijun/8215776/webrev.00/">https://cr.openjdk.java.net/~weijun/8215776/webrev.00/</a>
PKCS12KeyStore now can find certificate issuers more precisely using SubjectKeyIdentifier and AuthorityKeyIdentifier. I thought about using CertPath builder or checking signatures but those changes are too much.
Thanks,
Max
</pre>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">
</pre>
</blockquote>
</body>
</html>