On Thu, Jan 22, 2015 at 04:15:34PM +0000, Joe Mason wrote:
> > From: curl-library [curl-library-bounces_at_cool.haxx.se] on behalf of Alessandro
> > Ghedini [alessandro_at_ghedini.me]
> > Sent: Thursday, January 22, 2015 6:38 AM
> > To: curl-library_at_cool.haxx.se
> > 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

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html