[Pki-devel] Certificate Transparency SCT signature verification?

Fraser Tweedale ftweedal at redhat.com
Fri Jun 12 00:58:18 UTC 2020


Hi Christina,

I will find a day next week to have a close look.  Probably Tuesday
or Wednesday.  It will help to have test environment setup
documentation, i.e. how to set up a log server to test with, how to
configure Dogtag, etc.  If this stuff is already written then you
just need to tell me where to look :)

Cheers,
Fraser

On Thu, Jun 11, 2020 at 05:08:25PM -0700, Christina Fu wrote:
> HI Fraser,
> verifySCT still fails.  I still think the fact the rfc does not require the
> signed object to accompany the signature presents undue challenge to the
> party that needs to verify the signature.  Although I understand that this
> is v1, and the issue would not be present in v2 since there will not be
> poison extension ;-/.
> I'd appreciate it if you could find time to take a closer look.
> 
> Here is my latest attempt:
> https://github.com/dogtagpki/pki/pull/440
> Since it's a patch against the latest code, for a full view, it would be
> easier if you just apply the patch and read from "(Certificate
> Transparency)" in
> base/ca/src/com/netscape/ca/CAService.java
> This patch would require JSS change at:
> https://github.com/dogtagpki/jss/pull/575
> 
> Code still requires some refinement but I wish to address the verification
> issue before cleaning things up.  Of course I still let verifySCT returns
> success for now just so people could still play with CT.
> Much appreciated!
> Christina
> 
> On Tue, Jun 2, 2020 at 3:05 PM Christina Fu <cfu at redhat.com> wrote:
> 
> > Hi Fraser,
> > Thanks for the response!
> > Regarding the poison extension, yes I was aware that it needed to be
> > removed so the code already had it removed.  It was the order of things
> > left inside tbsCert that I was concerned about since I used the existing
> > delete method provided for the Extension class, which I wasn't sure if it'd
> > preserve the order of the remaining extensions.  Thanks for confirming my
> > suspicion.  I will double-check the order.
> >
> > Also thanks for the input on how to handle failed CT log communication
> > v.s. response verification failure.   I will address them separately as
> > suggested.
> > Finally, nice catch with the missing data length!!  I'll add that and go
> > from there.
> >
> > thanks again!
> > Christina
> >
> > On Mon, Jun 1, 2020 at 7:31 PM Fraser Tweedale <ftweedal at redhat.com>
> > wrote:
> >
> >> Hi Christina,
> >>
> >> Adding pki-devel@ for wider audience.  Comments below.
> >>
> >> On Mon, Jun 01, 2020 at 06:28:42PM -0700, Christina Fu wrote:
> >> > Hi Fraser,
> >> > Do you know how the signature returned in the SCT response could be
> >> > verified by the CA?
> >> > My thought is that the CA should somehow verify the CT response after
> >> > sending the add-pre-chain request and before signing the cert.  Since
> >> log
> >> > inclusion verification would not be feasible right after the request
> >> (the
> >> > SCT response is supposed to be just a "promise, according to the rfc),
> >> I
> >> > ruled that out and intend to stay with just the following two
> >> verifications
> >> > on the response itself:
> >> >
> >> >    - checking if log id (CT log signer public key hash) returned in the
> >> CT
> >> >    response is correct
> >> >    - this I have no problem verifying
> >> >       - Verifying if the signature returned in the CT response is
> >> correct
> >> >       - this I can't seem to get it working.
> >> >
> >> > I put the verification work above in the method "verifySCT".
> >> >
> >> https://github.com/dogtagpki/pki/blob/master/base/ca/src/com/netscape/ca/CAService.java#L1209
> >> > What I am wondering is how this can be done properly.  Since the
> >> tbsCert is
> >> > not to contain the poison extension, the poison extension needs to be
> >> > removed by the CT server before it signs.  What if the order of the
> >> > extensions contained in the tbsCert gets changed in the process?
> >> >
> >> The poison extension must be removed, and care must be taken to keep
> >> everything else in the same order, and reserialise the parts in
> >> exactly the same way.
> >>
> >> > It seems that the response should contain the tbsCert that it signs
> >> (which
> >> > isn't per the rfc) or I am not sure how the CA could verify the
> >> signature.
> >> >
> >> The response does not contain the TBCCertificate.  Both sides (log
> >> and client) remove the poison extension (and change nothing else),
> >> then both sides can sign the same data.
> >>
> >> > So the question now is if I should just leave out the check, unless you
> >> > have other suggestions.
> >> >
> >> I do think we should verify the signature, to ensure the message was
> >> actually received by the log server we wanted and not an impostor.
> >>
> >> > Of course, I also could have missed something in my code.
> >> >
> >> The binary format is complex and it's easy to miss something.  After
> >> you implement removal of the poison extension, if it is still not
> >> working I will go over the code with a fine-tooth comb.
> >>
> >> I copied some of the code and left comments below, too.  Comments
> >> begin with `!!'.  I think I found one bug and a couple of possible
> >> improvements.
> >>
> >> > One last question, currently in the code, if verifySCT fails, I just
> >> > "continue" to process for next CT log.  Should this just bail out all
> >> > together or it's fine to continue? Or could this be a choice by the
> >> admin.
> >> > What do you think and why?
> >> >
> >> https://github.com/dogtagpki/pki/blob/master/base/ca/src/com/netscape/ca/CAService.java#L951
> >> >
> >> My line of thinking is this:
> >>
> >> - we should tolerate communication errors with log (perhaps
> >>   enqueuing the cert for a retry later)
> >>
> >> - but (assuming we implement it correctly) verifySCT failure is
> >>   indicative of something wrong with the log (e.g. key changed); it
> >>   is not a communication error and can be treated differently.
> >>
> >> - I think it's OK to fail hard.  Admins will likely want to know if
> >>   something is wrong with CT logging.
> >>
> >> - But in case we make a mistake, or an org needs issuance to
> >>   continue despite CT log misbehaviour, there should be a config
> >>   knob to allow this condition to be ignored.  "In case of
> >>   emergency..."
> >>
> >>
> >> >
> >> > thanks,
> >> > Christina
> >>
> >>     boolean verifySCT(CTResponse response, byte[] tbsCert, String
> >> logPublicKey)
> >>             throws Exception {
> >>
> >>         /* ... SNIP ... */
> >>
> >>         byte[] extensions = new byte[] {0, 0};
> >> !! although no extensions have been defined I think we should we take
> >>    extensions from the CT response to future-proof this code.  i.e.
> >>    decode the base64 data from the "extensions" field, and prepend the
> >> length.
> >>
> >>         // piece them together
> >>
> >>         int data_len = version.length + signature_type.length +
> >>                  timestamp.length + entry_type.length +
> >>                  issuer_key_hash.length + tbsCert.length +
> >> extensions.length;
> >>
> >>         logger.debug(method + " data_len = "+ data_len);
> >>         ByteArrayOutputStream ostream = new ByteArrayOutputStream();
> >>
> >>         ostream.write(version);
> >>         ostream.write(signature_type);
> >>         ostream.write(timestamp);
> >>
> >>         ostream.write(entry_type);
> >>         ostream.write(issuer_key_hash);
> >>         ostream.write(tbsCert);
> >> !! I believe you need to prepend the length of tbsCert as a
> >>    THREE-byte length field, because its definition is
> >>    `opaque TBSCertificate<1..2^24-1>;'
> >>
> >>         ostream.write(extensions);
> >>
> >>         byte[] data = ostream.toByteArray();
> >>
> >>         // Now, verify the signature
> >>         // Note: this part currently does not work; see method comment
> >> above
> >>
> >>         // cfu ToDo: interpret the alg bytes later; hardcode for now
> >>         Signature signer = Signature.getInstance("SHA256withEC",
> >> "Mozilla-JSS");
> >>         signer.initVerify(log_pubKey);
> >>         signer.update(data);
> >> !! We could call signer.update() multiple times instead of making an
> >>    intermediate ByteArrayOutputStream.  I do not care about the
> >>    performance, just whatever might simplify the routine.
> >>
> >>         if (!signer.verify(signature)) {
> >>             logger.error(method + "failed to verify SCT signature; pass
> >> for now");
> >>             // this method is not yet working;  Let this pass for now
> >>             // return false;
> >>         } else {
> >>             logger.debug(method + "SCT signature verified successfully");
> >>         }
> >>         logger.debug("verifySCT ends");
> >>
> >>         return true;
> >>     }
> >>
> >>
> >>
> >> Cheers,
> >> Fraser
> >>
> >>




More information about the Pki-devel mailing list