RFR: 8237962: give better error output for invalid OCSP response intervals in CertPathValidator checks

[Baesken, Matthias]

Hi Sean, new webrev :


http://cr.openjdk.java.net/~mbaesken/webrevs/8237962.1/


Best Regards, Matthias


> 
> Message: 2
> Date: Tue, 28 Jan 2020 10:45:59 -0500
> From: Sean Mullan <sean.mullan at oracle.com>
> To: security-dev at openjdk.java.net
> Subject: Re: RFR: 8237962: give better error output for invalid OCSP
> 	response intervals in CertPathValidator checks
> Message-ID: <d0509125-9322-a269-622f-552cd90f8232 at oracle.com>
> Content-Type: text/plain; charset=windows-1252; format=flowed
> 
> Hi Matthias,
> 
> On 1/28/20 8:18 AM, Baesken, Matthias wrote:
> > Hello, please review this small change .
> >
> > While working on
> > https://bugs.openjdk.java.net/browse/JDK-8237869
> > and
> > https://bugs.openjdk.java.net/browse/JDK-8237888
> > I noticed that the error output in case of invalid OCSP response
> > intervals? could be improved a bit.
> >
> > Bug/webrev :
> > > https://bugs.openjdk.java.net/browse/JDK-8237962
> 
> This should be an Enhancement, and not a Bug. You should also add an
> appropriate noreg label (maybe noreg-trivial).
> 
> > http://cr.openjdk.java.net/~mbaesken/webrevs/8237962.0/
> 
> Please try to keep the lines within 80 characters to be consistent with
> the rest of the file.
> 
> It seems you could combine lines 602 and 604. I would also tweak the
> wording a bit, ex:
> 
> debug.println("Checking validity of OCSP response on " +
>                new Date(now) + " with allowed interval between " +
>                nowMinusSkew + " and " + nowPlusSkew);
> 
> Lines 615 through 625 should only be executed if debug is enabled. They
> should be in an "if (debug != null)" block. Also, if we were going to
> add this, I would probably restructure this entire block of code so you
> avoid code duplication.
> 
> But, I actually don't think these extra debugging statements are
> necessary. There is enough information in lines 600-605 to know exactly
> why the validity check failed. My general opinion on debugging logging
> is to give enough information to help diagnose errors, but don't give
> too much extraneous information.
> 
> --Sean
>