RFR 8140422: Add mechanism to allow non default root CAs to be not subject to algorithm restrictions

Sean Mullan sean.mullan at oracle.com
Mon Mar 21 19:44:39 UTC 2016


Looking good, mostly minor stuff:

AlgorithmChecker
----------------

151             if (trustedMatch && debug != null) 
debug.println("trustedMatch = true");

put braces around.

java.security
-------------

553 #     lib/security/cacerts.fingerprints file which is initially 
seeded with the

lib/security/cacerts.properties file with a "restricted=yes" attribute 
which is ...

562 # chain that terminate at a distribution provided trust anchor and 
contain

s/terminate/terminates/
s/contain/contains/

CaCertsHasher
-------------

   40  *   Fingerprint

I recommend specifying a format for the attributes, and then defining a 
specific attribute that tags a cert as being restricted. This will make 
the properties file more generally useful if we want to add more 
attributes later. Ex:

Fingerprint [attribute]{;attribute}

   69         if (inFilename == null) {
   70             System.err.println("Input file not defined.");
   71         }
   72         if (outFilename == null) {
   73             System.err.println("Output file not defined.");
   74         }

You should not continue on error in these cases. I think you should just 
change these to:

   69         if (inFilename == null) {
   70             throw new Exception("Input file not defined.");
   71         }
   72         if (outFilename == null) {
   73             throw new Exception("Output file not defined.");
   74         }


   81         PrintStream out = new PrintStream(outFilename);

should use try-with-resources to avoid having to close the file.

   88             if (verbose) {
   89                 System.out.println("No certificates in " + 
inFilename +
   90                         ".  Hash file not created");
   91             }

But you already created the file on line 81.

  103                 prop.store(out, c.getIssuerX500Principal().getName());

Maybe you should use the keystore alias here instead. DNs can be very long.

AnchorCertificates
------------------

   40  * anchor certificate is in the cacerts.fingerprints file which is
   43  * file but not the cacerts.fingerprints file would not be a match.

s/cacerts.fingerprints/cacerts.properties/

--Sean

On 03/18/2016 01:09 PM, Anthony Scarpino wrote:
> I believe I got everyone's comments. I've updated the webrev.
>
> http://cr.openjdk.java.net/~ascarpino/8140422/webrev.02/
>
> Thanks
>
> Tony
>
>
> On 02/29/2016 08:55 AM, Anthony Scarpino wrote:
>> Currently CertPath algorithm restrictions allow or deny all
>> certificates.  This change adds the ability to reject certificate chains
>> that contain a restricted algorithm and the chain terminates at a root
>> CA; therefore, allowing a self-signed or chain that does not terminate
>> at a root CA.
>>
>> https://bugs.openjdk.java.net/browse/JDK-8140422
>>
>> Thanks
>>
>> Tony
>>
>



More information about the security-dev mailing list