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/...
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/...
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