On Thu, Jan 22, 2015 at 04:15:34PM +0000, Joe Mason wrote: > > From: curl-library [[email protected]] on behalf of > > Alessandro > > Ghedini [[email protected]] > > Sent: Thursday, January 22, 2015 6:38 AM > > To: [email protected] > > Subject: Re: [PATCH v3] OCSP stapling for GnuTLS and NSS > > > > So, thanks to Joe Mason [0], I think I have a 100% working OpenSSL patch > > now, > > it's really ugly though [1]... I'll send it to the mailing list after some > > more > > testing. > > Now that my mail's validated I can send this to the list instead of straight > to you... > > Thanks for finding X509_check_issued - I was looking for a function like that > but missed that one. > > I'm not sure that the call to OCSP_basic_add1_cert is correct if the responder > cert isn't the last one in the chain, though. Isn't the X509 stack supposed to > be strictly following the chain of issuers? So if the OCSP response contains > the responder A, which is issued by B, which is issued by C, the stack should > contain one of: > > (1) A > (2) A -> B > (3) A -> B -> C > > So in case (1) this patch will work, but in the other two it would add a > second copy of B to the end of the chain: > > A -> B -> B > A -> B -> C -> B > > Or are the STACK_OF(X509) structures in openssl more general than that?
The STACK_OF() thing is really just an array of pointers (i.e. not ordered in any way). Even more so that the br->certs field isn't really supposed to have any "structure" (the server can put in it whatever it wants I think). But since OpenSSL blindly uses br->certs in X509_verify_cert() (which I think does expect the chain to be in proper order) we can assume it's ordered or the verification would fail anyway. So yeah, I think you are right in practice, so I removed the whole ocsp_find_responder thing. > If my above interpretation is right, I think we should always check the last > X509 in the stack, like my original patch did, but include the checks for > V_OCSP_RESPID_NAME, etc that you added, and simply do nothing if the last cert > is NOT the responder. I don't think there's any point in checking whether the last cert in br->certs is actually the responder cert, since the verification would probably fail anyway. In practice I think that in 99.999999999% (if not 100%) of the cases, br->certs is either empty or contains only one certificate so we shouldn't worry about the "stupidly ordered case" or even the "multiple certs in br->certs case" much. Cheers
signature.asc
Description: Digital signature
------------------------------------------------------------------- List admin: http://cool.haxx.se/list/listinfo/curl-library Etiquette: http://curl.haxx.se/mail/etiquette.html
