<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<p>Hi Anthony!</p>
<p><br>
</p>
<p>A few minor comments:</p>
<p><b>AlgorithmChecker.java</b></p>
<p>It would be more consistent to use {@code ...} tags in place of
<code>...</code></p>
<p><br>
</p>
<p><b>DisabledAlgorithmConstraints.java</b></p>
<pre style="color: rgb(0, 0, 0); font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; widows: 1; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(238, 238, 238);"><span class="new" style="color: blue; font-weight: bold;">275 Matcher dmatch = denyAfterPattern.matcher(entry);</span></pre>
<pre style="color: rgb(0, 0, 0); font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; widows: 1; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(238, 238, 238);"><span class="new" style="color: blue; font-weight: bold;">296 } else if (dmatch.matches()) {</span></pre>
<br>
It might be a bit more efficient to reuse already declared `Matcher
matcher` like this:<br>
<br>
} else if (matcher.usePattern(denyAfterPattern).matches()) {<br>
<br>
<br>
<br>
With kind regards,<br>
Ivan<br>
<br>
<br>
<div class="moz-cite-prefix">On 13.05.2016 0:34, Anthony Scarpino
wrote:<br>
</div>
<blockquote cite="mid:5734F6F9.1090501@oracle.com" type="cite">I've
updated the webrev to at:
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~ascarpino/8154005/webrev.01/">http://cr.openjdk.java.net/~ascarpino/8154005/webrev.01/</a>
<br>
<br>
Comments addressed below...
<br>
<br>
On 05/11/2016 04:55 PM, <a class="moz-txt-link-abbreviated" href="mailto:ecki@zusammenkunft.net">ecki@zusammenkunft.net</a> wrote:
<br>
<blockquote type="cite">Hello,
<br>
<br>
In AlgorithmChecker the Javadoc seems to not follow "@param name
<br>
desc" format (in two places). Also it should most likely
describe
<br>
something like "time the signature claimed to be made to check
time
<br>
range limited ciphers after that date or similiar)
<br>
<br>
* @param PKIXParameter timestamp (or null)
<br>
</blockquote>
<br>
Thanks for seeing that.. I updated them.
<br>
<br>
<blockquote type="cite">
<br>
DisabledAlgorithmConstrained: The regular expression allows
<br>
denyafter20160101 its clear, but \s+ might be clearer? Can
optional
<br>
iso Idate seperators, be added. "(\d {4})-?(\d {2})-?...."
<br>
</blockquote>
<br>
I think I prefer to require '-' as standard syntax and not use
YYYYMMDD. Sometimes YYYYMMDD not as clear to read as YYYY-MM-DD.
I would like to not have two valid formats.
<br>
<br>
<blockquote type="cite">
<br>
The lowercase constraint classes are rather strange, but fits
into
<br>
existing code...
<br>
<br>
I dont see in the patch how the date param is certified. Is this
only
<br>
the issued date as certified (by the weak) signature or does it
look
<br>
at timestamps (especially codesigning) too?
<br>
</blockquote>
<br>
The date is passed as part of PKIX, it's optional that PKIX can
have a date parameter set to specify a time date.
<br>
The date disallows a certificate with the disabled algorithm on
that date. It does not check validity of the certificate. This
is meant to shutoff the algorithm in certificate checking. There
maybe a exception to this to allowing codesigning certificates a
bit longer, but that hasn't been completely decided yet.
<br>
<br>
<blockquote type="cite">
<br>
There are a few conditions which could be unit tested:
<br>
<br>
RSA keySize <= 1024 & disablesAfter 20160101 SHA1
disabledAfter
<br>
20160102 // valid RSA disabledAfter 20160101 & disabledAfter
20160101
<br>
// not valid Etc
<br>
</blockquote>
<br>
Yes. There are a number of tests that are in the closed test repo
because they use certificates that are not for public use.
<br>
<br>
thanks
<br>
<br>
Tony
<br>
<br>
<blockquote type="cite">
<br>
Gruss
<br>
</blockquote>
>
<br>
<blockquote type="cite">Bernd
<br>
<br>
</blockquote>
<br>
<br>
</blockquote>
<br>
</body>
</html>