[Pki-devel] Certificate Transparency SCT signature verification?
Christina Fu
cfu at redhat.com
Fri Jun 12 00:08:25 UTC 2020
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
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pki-devel/attachments/20200611/5d93bd46/attachment.htm>
More information about the Pki-devel
mailing list