[security-dev 00965]: Re: code review request 6852744: PIT b61: PKI test suite fails because self signed certificates are being rejected

[Sean Mullan]

Hi Andrew,

Here are some comments -

ForwardBuilder:

line 864:

typo: s/abchor/anchor

In this block of code:

  858                 if (principal != null && publicKey != null &&
  859                         principal.equals(cert.getSubjectX500Principal())) {
  860                     if (publicKey.equals(cert.getPublicKey())) {
  861                         this.trustAnchor = anchor;
  862                         return true;
  863                     }
  864                     // else, it is a self-issued certificate of the abchor
  865                 }

you never check if the trust anchor name is equal to the issuer of the cert 
before returning true. That seems to violate RFC 5280.

lines 977-995

RFC 5280 requires (MUST) that the KeyIdentifier be used in the 
AuthorityKeyIdentifier extension of the CRL, so I don't think we need to check 
the serial number/general names as those would be non-compliant. I would remove 
this block. In any case I think the intention is that there only be one form of 
identifier (key id OR issuer/serial) per AKID/SKID extension, so you should only 
have to check one of them.

I'm still looking over the tests but these are some comments so far.

--Sean


Xuelei Fan wrote:
> Hi,
> 
> bug description: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6852744
> webrev: http://cr.openjdk.java.net/~xuelei/6852744/webrev/
> 
> Evaluation of the bug:
> 1. There is a loop of forward builder for self-issused intermediate 
> certificates.
>   The ForwardBuilder looks for the next certificate based on 
> IssuerDN/SubjectDN. However, a self-issued certificate has the same 
> IssuerDN and SubjectDN, the looking will loop on the self-issued 
> certificate untill the loop detected.
> 
> 2. Circular dependences
>   In the PIT tests,  the valid of the intermediate CA certificate 
> (oldCA) depends on the CRL; the valid of CRL depends on its issuer, the 
> self-issued intermediate CA certificate (newWithOldCA); the valid of 
> newWithOldCA depends on its issuer, the oldCA, here comes a dead loop.
> 
> Thanks,
> Xuelei