bcgit / bc-csharp

BouncyCastle.NET Cryptography Library (Mirror)
https://www.bouncycastle.org/csharp
MIT License
1.6k stars 537 forks source link

Org.BouncyCastle.Pkix.PkixCertPath implements incorrect order #238

Open kyanha opened 4 years ago

kyanha commented 4 years ago

The comment for Org.BouncyCastle.Pkix.PkixCertPath says:

     * By convention, X.509 CertPaths (consisting of X509Certificates), are ordered
     * starting with the target certificate and ending with a certificate issued by
     * the trust anchor. That is, the issuer of one certificate is the subject of
     * the following one. The certificate representing the
     * {@link TrustAnchor TrustAnchor} should not be included in the certification
     * path. Unvalidated X.509 CertPaths may not follow these conventions. PKIX
     * CertPathValidators will detect any departure from these conventions that
     * cause the certification path to be invalid and throw a
     * CertPathValidatorException.<br />

The definition given in RFC6066 for application/pkix-pkipath (which is a SEQUENCE of Certificate structures) has this to say:

        PkiPath ::= SEQUENCE OF Certificate
        PkiPath is used to represent a certification path.  Within the
        sequence, the order of certificates is such that the subject of
        the first certificate is the issuer of the second certificate,
        etc.
      This is identical to the definition published in [X509-4th-TC1];
      note that it is different from that in [X509-4th].

It should be recognized that RFC6066 obsoleted RFC4366, which specified the identical definition of application/pkix-pkipath in 2006. X509-4th was published in 2000, with its TC1 published in 2001. This year will be 19 years since the specification was changed at ITU, and 14 years since it was originally specified at IETF/PKIX. The comments for PkixCertPath only mention a "convention" which is then enforced by CertPathValidators.

kyanha commented 4 years ago

I'd like to address this by reversing the list in the PkixCertPathValidator if it would initially throw and then reprocessing it internally (adding 1 bit of entropy to the process, but allowing for backward compatibility), and adding a method with a flag to the process of PkixCertPathBuilder to specify RFC4366/RFC6066 ordering (which would just generate the CertPath as normal but then .Reverse it before returning it to the caller), which would allow current callers to obtain the same output they're used to, while still allowing for people who actually need the specified behavior to be able to get it, albeit at a cost in performance.

peterdettman commented 4 years ago

I believe the CertPath stuff in bc-csharp is trying to behave like the CertPath API in Java, which has the stated convention (target cert first). This convention is for the API itself and is independent of the ordering used when a certification path is encoded in ASN.1.

PkixCertPath appears to provide separate encodings that treat the order differently ("PkiPath" vs "PKCS7" or "PEM"). If something's not working as desired I expect it can be fixed there. PkixCertPathValidator works with a fixed ordering and shouldn't be changed.

kyanha commented 4 years ago

PkixCertPath uses the same order for PkiPath and PEM. (PKCS7 will not necessarily provide the same ordering, but it also does not provide TC1/RFC-specified ordering for PkiPath.) There is no mechanism, as far as I can see, to create or properly order a spec-conformant (as opposed to pre-spec conventional) PkiPath.

Don't get me wrong, I do appreciate what a reversed PkiPath enables -- short-circuiting of the entire path evaluation (in the case where the local evaluator has preloaded an intermediate instead of only the root), and cross-certification of an intermediate in the path (again, in the case where the local evaluator has been preloaded) are only two examples.

But, I need to work in a standards-conformant manner here. I can't label something "application/pkix-pkipath" if it's not conformant to the specification of that MIME Content-Type.

How should this be approached, given that BouncyCastle has had a prior instance of a reversed output that affected interoperability? (Specifically, BC's original implementation of Serpent was byte-reversed, so it created the Tnepres class to enable interoperability with prior buggy BouncyCastle deployments.)

I don't think that BouncyCastle should turn a blind eye to the specification simply based on convention, or based on the fact that JCA/JCE were created prior to the 2001 specification and never changed their implementation away from the convention. C# is not actually tied to Java's implementation, especially since there's no equivalent to JCA/JCE in C# land and JCA/JCE's assumptions have had to be vaguely hacked in to the C# environment in the first place.

(I also think that BouncyCastle should lend its considerable weight to a bug report against the JCA/JCE specifications, because BC has been very cognizant of and compliant with other specifications that it implements, and the JCA/JCE specifications have been virtually untouched since they were created in the pre-TC1 days. But I also know that I'm not on BouncyCastle's team, and persuading the team to do something because I think it should be done is very unlikely.)

I just really want to know how I can use or modify the library (in a manner that the BC team will accept patches for) to produce PKIX specification-compliant "application/pkix-pkipath" blobs. Would you have any suggestions?