[Pki-devel] Certificate Transparency SCT signature verification?
Fraser Tweedale
ftweedal at redhat.com
Wed Jun 17 02:45:54 UTC 2020
On Wed, Jun 17, 2020 at 12:59:57AM +1000, Fraser Tweedale wrote:
> Thanks for the testing notes, Christina.
>
> Today I set up a local test CT log server using a container image.
> I plan to document more thoroughly but rough notes at [1].
>
> Now to the issue I found - I have a commit[2] in a private branch.
> Hopefully the commit message and comments explain it well enough.
>
> Is it the last issue? Not sure yet, I finally got it compiled but
> haven't run the code yet. Tomorrow's adventure.
>
> [1] https://github.com/frasertweedale/notes-redhat/blob/master/ct.rst
> [2] https://github.com/frasertweedale/pki/tree/fix/ct-verify
>
> Cheers,
> Fraser
>
I went down the garden path on this one. It turns out that JSS/NSS
*does* use the DER-encoded ECDSA-Sig-Value as the signature format.
(I wrote a small test program to check this). So most of the code I
wrote yesterday is wrong.
Analysis continues.
Thanks,
Fraser
> On Mon, Jun 15, 2020 at 10:45:53AM -0700, Christina Fu wrote:
> > Hi Fraser,
> > That sounds good! I just added the following page to document my "quick
> > test" procedure which I use during development:
> > https://www.dogtagpki.org/wiki/PKI_10.9_Certificate_Transparency
> > btw, the verifySCT is currently enabled, but the failure is ignored.
> > However, you could look in the debug log for "verifySCT" to see relevant
> > debug messages.
> >
> > I'll ask Dinesh to add his more comprehensive testing procedure to the page.
> > thanks!!
> > Christina
> >
> > On Thu, Jun 11, 2020 at 5:58 PM Fraser Tweedale <ftweedal at redhat.com> wrote:
> >
> > > 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