Hi Ivan,
I added your comments.. I had thought usePattern() wouldn't reuse the
entry, but apparently the description is not the way I read it.
You comments are incorporated into the webrev.01 that I posted previously
thanks
Tony
On 05/12/2016 04:13 PM, Ivan Gerasimov wrote:
Hi Anthony!
A few minor comments:
*AlgorithmChecker.java*
It would be more consistent to use {@code ...} tags in place of
<code>...</code>
*DisabledAlgorithmConstraints.java*
275 Matcher dmatch = denyAfterPattern.matcher(entry);
296 } else if (dmatch.matches()) {
It might be a bit more efficient to reuse already declared `Matcher
matcher` like this:
} else if (matcher.usePattern(denyAfterPattern).matches()) {
With kind regards,
Ivan
On 13.05.2016 0:34, Anthony Scarpino wrote:
I've updated the webrev to at:
http://cr.openjdk.java.net/~ascarpino/8154005/webrev.01/
Comments addressed below...
On 05/11/2016 04:55 PM, e...@zusammenkunft.net wrote:
Hello,
In AlgorithmChecker the Javadoc seems to not follow "@param name
desc" format (in two places). Also it should most likely describe
something like "time the signature claimed to be made to check time
range limited ciphers after that date or similiar)
* @param PKIXParameter timestamp (or null)
Thanks for seeing that.. I updated them.
DisabledAlgorithmConstrained: The regular expression allows
denyafter20160101 its clear, but \s+ might be clearer? Can optional
iso Idate seperators, be added. "(\d {4})-?(\d {2})-?...."
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.
The lowercase constraint classes are rather strange, but fits into
existing code...
I dont see in the patch how the date param is certified. Is this only
the issued date as certified (by the weak) signature or does it look
at timestamps (especially codesigning) too?
The date is passed as part of PKIX, it's optional that PKIX can have a
date parameter set to specify a time date.
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.
There are a few conditions which could be unit tested:
RSA keySize <= 1024 & disablesAfter 20160101 SHA1 disabledAfter
20160102 // valid RSA disabledAfter 20160101 & disabledAfter 20160101
// not valid Etc
Yes. There are a number of tests that are in the closed test repo
because they use certificates that are not for public use.
thanks
Tony
Gruss
>
Bernd
