google/certificate-transparency-java

Index of of bounds when submitting a pre-certificate issued directly from a root CA

Closed this issue · 14 comments

From RFC 6962

Each submitted certificate MUST be accompanied by all additional certificates required to verify the certificate chain up to an accepted root certificate. The root certificate itself MAY be omitted from the chain submitted to the log server.

This means I can submit a pre-certificate issued directly from an accepted root. This MAY be omitted on submission, causing the following code to throw IndexOutOfBounds.

 boolean isPreCertificate = CertificateInfo.isPreCertificate(certificatesChain.get(0));
 if (isPreCertificate && CertificateInfo.isPreCertificateSigningCert(certificatesChain.get(1))) {
      Preconditions.checkArgument(
          certificatesChain.size() >= 3,
          "When signing a PreCertificate with a PreCertificate Signing Cert,"
              + " the issuer certificate must follow.");
 }

Fixing this part of the code results in another error later on:

Caused by: java.lang.IllegalArgumentException: Chain with PreCertificate must contain issuer.
at com.google.common.base.Preconditions.checkArgument(Preconditions.java:122)
at org.certificatetransparency.ctlog.LogSignatureVerifier.verifySignature(LogSignatureVerifier.java:154)
at org.cesecore.certificates.certificatetransparency.CtSubmission.verifyResponse(CtSubmission.java:547)
at org.cesecore.certificates.certificatetransparency.CtSubmission.fetchSct(CtSubmission.java:514)
at org.cesecore.certificates.certificatetransparency.CtSubmission.access$300(CtSubmission.java:56)
at org.cesecore.certificates.certificatetransparency.CtSubmission$1.call(CtSubmission.java:344)
at org.cesecore.certificates.certificatetransparency.CtSubmission$1.call(CtSubmission.java:338)
at java.util.concurrent.FutureTask.run(FutureTask.java:266)

Ok, I don't know if that's correct or another bug.

Me neither :-). The RFC seems not too exact on this point.

The Java client is doing more checks than the Go client and probably others. I think most of these checks should be removed and rely on the server to enforce the correct requirements for the chain.

It's beneficial if the client makes sanity checks though, if issuance is stopped before the pre-certificate is actually submitted we don't have to issue the certificate and revoke it.

I'm not sure the above check is valid though? Anyone knows?

The Python and Go clients don't have these checks.

The server rules are in cert_checker.cc CheckPreCertChain. It doesn't seem to me that it will accept a single certificate chain.

I suggest you create an example and add it to the cases in cert_checker_test.cc. If it doesn't pass then these client changes won't allow you to submit it to logs running this implementation and further thought will be needed.

@eranmes Do you have an opinion on whether these submissions should be accepted?

Just to confirm, in the original report, is the root CA you're referring to the special-purpose Precertificate Signing Cert mentioned in https://tools.ietf.org/html/rfc6962#section-3.1 ?
Also, are you using those classes in a log server or client context?

RFC6962 defines the ability to sign a precertificate with a special Precertificate Signing Cert (which has a specific CT OID EKU) - if the certificate is signed with such a Precertificate Signing Cert then at least three entries have to be in the chain:
PreCert -> Precert signing cert -> CA cert which issued the Precert Signing Cert
(When serving the chain with the final certificate the Precert Signing Cert should be omitted).

So if the root certificate is such a special precert signing cert then the check would kick in - and (incorrectly) require you to have a chain of at least 3 elements. However logs should not be configured with the Precert Signing Cert as a trust anchor - because then the real issuer would be missing.

If the root certificate is not the special precert signing cert then maybe it's simply the case of missing the root from the chain?

The check in LogSignatureVerifier is definitely correct - if you're validating the signature over a precert you must have the issuer certificate around to extract the SPKI from.

Note that this code was written with a Java log client in mind - if you're using it in a log server or a TLS client then some things may not be as straightforward as could be.

And yes, a pre-certificate issued directly from a root CA is a submission that could be accepted (I don't recall any reason it shouldn't be, and in the Java code that validates the signature the root CA must be included so the issuer key hash could be later extracted from it).

Thanks Eran. So this might be a server issue as well. We'll have to verify it against C++ and Go servers.

Thanks Eran,

Sorry, but I think we forgot about the Precert signing cert as we're not using that. In our case it's pre-certificates signed directly by the issuing CA.

We only use the library as a java client, posting certificates to logs.

I think it's well understood now, but I'll just re-iterate it with examples.

We used to submit like this, which works in the current java client:
Pre certificate->Issuing CA->Root CA
or
Pre certificate->Issuing Root CA

Since you don't have to submit Root CA according to RFC6962 you can save bytes in the submission by removing that:
Pre certificate->Issuing CA
or
Pre certificate

In the current java client the second row doesn't work, i.e to submit a pre-certificate signed directly by a Root CA, without also submitting the Root CA certificate.

I believe the solution to the problem would be to simply append the Root CA certificate to the chain you verify, after submission.
That is, no need to send the Root CA when you submit to the log, but you need it in the chain when you verify the SCT.

Good. Then a requirement is that the CT client API must be changed to be able to submit without root certificate, and provide a separate issuer for verification. Currently it is all done inside the submission code where you only provide the chain to be submitted.

Closing this issue. This repo is no longer being maintained. See the README for info.